Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignoreCase: factory methods #21

Open
butuzov opened this issue Sep 6, 2021 · 3 comments
Open

ignoreCase: factory methods #21

butuzov opened this issue Sep 6, 2021 · 3 comments

Comments

@butuzov
Copy link
Owner

butuzov commented Sep 6, 2021

add to ignore case a factory functions (ones that can return interface, depends on incoming parameters)

@busser
Copy link

busser commented Apr 12, 2024

Any update on this? We'd love to help :)

Here's a minimal example:

// main.go
package main

type Foo interface {
  DoStuff()
}

type FooFactory interface {
  NewFoo() Foo
}

type MyFooFactory struct {}

func (f *MyFooFactory) NewFoo() Foo {
  return nil
}

Running the linter:

ireturn main.go

Yields this error:

main.go:13:1: NewFoo returns interface (command-line-arguments.Foo)

Factories are a pretty common pattern, so we'd like for ireturn to not complain about them. We'd love for ireturn to have a configuration option that removes such errors.

Currently we can configure ireturn to allow methods returning a specific interface. In our case it would be:

ireturn --allow=Foo main.go 

However this has two drawbacks:

  1. It only allows a factory for a specific type, so we end up needing to list many types in the allowlist.
  2. It also allows methods that return that type even when the method's receiver is not a factory, which we'd like to keep errors for.

We would love for ireturn to have an option that makes it allow methods that return interfaces when the method's receiver's name ends with Factory. The current configuration options don't enable this.

Is there any chance that we could add an allowReceiver option or something like that?

@butuzov
Copy link
Owner Author

butuzov commented Apr 13, 2024

Hi. Thank you for commenting on this issue, its kinda related to the #91.

Our problem is we are tracking tunction declarations, not function calls, so we can't determine what is calling - interface or struct (this comment regarding propose in #91). And looking up for any other type declaration to find out (is it suppose to be interface declaration) isn't going to be free. I will take a look to it, once i have some time (which I mostly doesn't have as I am in army now), but i will try to check what can be done.

@butuzov
Copy link
Owner Author

butuzov commented Apr 13, 2024

This is a comment from @drumsta, on same question asked week ago. Copied here for propose to tracking this issue.


How to deal with the case below?

// Bad.
type Doer interface { Do() Doer }
type IDoer struct{}
func New() Doer { return new(IDoer)}
func (d *IDoer) Do() Doer {/*...*/ return d }

// Good.
type Doer interface { Do() Doer }
type IDoer struct{}
func New() *IDoer { return new(IDoer)}
func (d *IDoer) Do() *IDoer {/*...*/ return d}

// Very Good (Verify Interface Compliance in compile time)
var _ Doer = (*IDoer)(nil)

The above won't compile because of the error:

cannot use (*IDoer)(nil) (value of type *IDoer) as Doer value in variable declaration: *IDoer does not implement Doer (wrong type for method Do)
		have Do() *IDoer
		want Do() Doer

This use case is quite common designing libraries. As an example, standard slog package has this interface, that's Handler methods return the Handler interface.

type Handler interface {
	Enabled(context.Context, Level) bool
	WithAttrs(attrs []Attr) Handler
	WithGroup(name string) Handler
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants