golangci-lint
golangci-lint copied to clipboard
Automatically detect local prefixes
This PR is an attempt to automatically detect local prefixes from module paths. The rationale behind this feature is making config files less verbose/redundant and a little bit more portable.
It is not perfect, but it's working.
More specifically, I'm not sure if loading module information for every linter is a good idea, so module information loading should be incorporated into the LoadMode, but I'm not sure how since it's a string.
I'm also not sure if the gci -> goimports fallback is necessary.
Last, but not least the config option name feels a bit weird, but I couldn't come up with a better one.
Any feedback would be highly appreciated.
Fixes #931
Hey, thank you for opening your first Pull Request !
@SVilgelm thanks for the feedback!
Is there already an example somewhere that reads the go.mod file or uses the module name? Speaking from experience, it can get tricky.
Here is an example of parsing go.mod file
import (
"golang.org/x/mod/modfile"
)
func getModulePath() string {
mfile, _ := modfile.ParseLax("go.mod", []byte{}, nil)
return mfile.Module.Mod.Path
}
Great idea in general, just please make sure to make it opt-in so we don't break backward compatibility (looks like it is opt in via local-prefix-module: true
which is good)
@SVilgelm I've spent some time trying to figure out how to get the most accurate module information and it's a little bit complicated than just calling that modfile package.
First, we need to find the go.mod file. The easiest (and probably the most accurate) way to do it is running go env GOMOD
(that requires the Go tool to be in the path though). Then we can parse the go.mod file.
It might also make sense to just defer this task to a library that already does this well: https://github.com/gobuffalo/here
Last, but not least: I don't know if it's possible to analyse multiple packages from different modules at the same time, but if it is, then the current solution will provide the most accurate result. (It doesn't necessarily have to live in the context though, can be moved to the linter implementations as well.
What do you think?
you can take a look at what I did here: https://github.com/ldez/gomoddirectives/blob/82673faed925c794e4a179ae238dd0b10f260732/module.go#L22-L24
Personally, I think that autodetecting is not a good idea because it will not work well in some cases (like golangci-lint by file inside an IDE), and I think it's an overcomplicated solution for something that is not a real problem.
Can we get to an agreement for whether this change would be merged or not before I invest any more time in it?
@ldez that's a fair point and I think the current solution would be better from that perspective as well (if no module info is available, no prefix will be applied, whereas manually looking for the go.mod file might be problematic).
Fixed by #4522