Add failing for certain methods in a given package (i.e: fmt.Errorf)

[fatih]

Currently faillint only fails on package level (based on the import path). We should also make it possible to fail for a sub-set of exported functions, types or variables (i.e: ast.TypeSpec and ast.ValueSpec should be enough).

Why is this useful? Let's look at this example:

If a user opted to use github.com/pkg/errors not only want they prevent the usage of errors package, they probably also want to avoid using fmt.Errorf and start using errors.Errorf method from the github.com/pkg/errors package. There are many reasons for that. For example the github.com/pkg/errors package includes stack information in each error value. Hence they want to make sure all errors are created with the github.com/pkg/errors package.

We could still use the paths flags and could define the sub-set with a dot identifier. For example the following command could fail for all usages of errors and the usage of fmt.Errorf:

faillint --paths "errors,fmt.Errorf"

If anyone wants to work on this, please comment on the PR before starting on it. I'll work on that at some point otherwise.

[bwplotka]

Happy to take this.

Our main use case is that we need this to block prometheus.New... and promethues.MustRegister to force the use of new amazing promauto.With(..).New... addition that ensures that we never forget about registering metric again! However https://godoc.org/github.com/prometheus/client_golang/prometheus package has much more than just constructors, so we cannot block the whole package. (:

[bwplotka]

Some additional work would be to make it less painful to configure multiple functions like

faillint -paths "github.com/foo/bar,github.com/foo/bar/foo.{A,B,D,G,Z}

But let's maybe attack this later on - also it would work weirdly with suggestions.

[bwplotka]

This would be super easy to implement, but might be not very clear for users (:

faillint -paths "github.com/foo/bar,github.com/foo/bar/foo.A.B.D.G.Z

[bwplotka]

Nice edge case:

faillint --paths "errors,gopkg.in/yaml.v2"

Should we block v2 function of yaml or just gopkg.in/yaml.v2 import? ;p I guess since you cannot in theory use unexported v2 function, it's the former (:

[bwplotka]

Also package.ABC is invalid as package name but a valid repository name.. 🤦‍♂️

No way really to figure it out, so I guess we either assume no such package will be provided.. or we might need something different than . for function name separator.

[bwplotka]

{} might do...

faillint -paths "github.com/foo/bar,github.com/foo/bar/foo.{A}

[pracucci]

{} might do...

Looks a feasible solution to me.

[bwplotka]

Ack, playing with some nice regexp for this.

[fatih]

Thanks for the comments. There are a lot of edge cases and it's great we capture them here.

So after looking at #2 and reading the comments here, I think we need a proper and scalable syntax for the config. Either we try to put everything inside the --paths flag and try to improve the syntax there OR we split certain things into separate flags.

If we go route 1, I think using a Regex style syntax would solve a lot of stuff. For example #2 would benefit from it:

faillint --paths "github.com/golang.org/x/net/**" 

Or for this issues it could be

faillint --pahts "errors,fmt.(Errorf|Foo)"

But regex is the last thing I want to use anywhere in this tool. faillint should be easy to use without any surprises. regex is the opposite. Also I don't want to use a config file, because storing and managing a file is a chore and burden.

Assuming we use this format (no regex):

faillint -paths "errors,fmt.{Errorf}"

How should we define the suggestion in this case? Currently it's like this:

faillint -paths "errors=github.com/pkg/errors"

So we need to come up with a solution to provide suggestion for methods as well. One idea:

faillint -paths "errors=github.com/pkg/errors,fmt.{Errorf}=github.com/pkg/errors.{Errorf}"

I'm still tinkering about this so if you have more ideas about the syntax or if you think it would make sense to introduce new flags let me know.

[bwplotka]

Suggestions for the function is not really a problem. It's bit tricky to add suggestions for grouped functions with {A,B,C} which I proposed here: #8 (:

I went for a simple solution, so just a single suggestion for a single group, so it's up to the user to make it work. The suggestion is kind of import-like arbitrary string you can put. I think that's easy enough for users.

Anyway, all should be fixed here: #8