Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Exported func with unexported interface parameter is not linted #475

Closed
voithos opened this issue Dec 3, 2019 · 3 comments
Closed

Exported func with unexported interface parameter is not linted #475

voithos opened this issue Dec 3, 2019 · 3 comments

Comments

@voithos
Copy link

voithos commented Dec 3, 2019

Consider the following pattern (which currently doesn't trigger any warnings):

type unexportedInterface interface {
  ...
}

func ExportedFunc(i unexportedInterface) {
  ...
}

My understanding is that this pattern should be avoided, since it can be troublesome to use (on similar grounds to #210, which golint does catch). Seems like it'd be nice if golint flagged this pattern as well.

@dsymonds
Copy link
Contributor

dsymonds commented Dec 3, 2019

I think the implementation only looked at return values because it was easy to miss (e.g. callers writing x := ... get away with it) but arguments are a bit more of an immediate stumbling block so I don't think people write code like this much.

Seems reasonable to me to have golint flag this if this pattern happens in practice.

@gtrewitt
Copy link

What about code like this?
MyFactory is an interface declared elsewhere. For the sake of this example, let's say that it has 100 methods.

In a package that takes a MyFactory as a parameter, I want to be able to inject a fake factory for testing, so...

// minimumFactory encapsulates the minimum set of methods of pkg.MyFactory
// needed by this package.  It's here so that tests can inject fakes.
type minimumFactory interface {
	MakeThing(name, host string) (*otherpkg.Object, error)
	GetThing(name string) (*otherpkg.Object, error)
}
struct DoesntMatter {
   factory *minimumFactory
}
func New(factory minumumFactory) (*DoeantMatter) {
   return &DoesntMatter { factory: factory }
}

In my tests, a fake can just implement two methods (rather than 100).
This works just fine, but the GoDoc for New() needs to be documented carefully.

I don't like it, but really don't want to export the fake interface.
This seems like a legitimate case.

@mvdan
Copy link
Member

mvdan commented May 8, 2021

Thank you for submitting this issue! As per golang/go#38968, we are freezing and deprecating golint. There's no drop-in replacement to golint per se, but you should find that Staticcheck works well in encouraging good Go code, much like golint did in the past, since it also includes style checks. There's always gofmt and go vet too, of course.

If you would like to contribute further, I'd encourage you to engage Staticcheck's issue tracker or look at vet's open issues, as they are both actively maintained. If you have an idea that doesn't fit into either of those tools, you could look at other Go linters, or write your own - these days it's fairly straightforward with go/analysis.

To help avoid confusion, I'm closing all issues before we freeze the repository. If you have any feedback, you can leave a comment on the proposal thread where it was decided to deprecate golint - though note that the proposal has been accepted for nearly a year. Thanks!

@mvdan mvdan closed this as completed May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants