fix: filter files by ldez · Pull Request #5272 · golangci/golangci-lint (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure I fully understand how this is intended to work and when and why you'd want to analyze non go files. I know we talked about the generated files for cgo and all but this feels like a more fundamental thing. If we can't even run gofmt on these files, are we ever interested in the generated files for analysis? Even if we need them for the package to compile don't we always want to skip the non go files themselves for analysis?

To fix what you address here, what do you think about returning a bool from GetFilePosition instead so we know when we get the position if we got it from a .go file or a generated file? That would make it always available and less copying or forgetting to copy in any of the linters?

diff --git a/pkg/goanalysis/position.go b/pkg/goanalysis/position.go index f90a7098..28afe6cb 100644 --- a/pkg/goanalysis/position.go +++ b/pkg/goanalysis/position.go @@ -8,20 +8,20 @@ import ( "golang.org/x/tools/go/analysis" )

-func GetFilePosition(pass *analysis.Pass, f *ast.File) token.Position { +func GetFilePosition(pass *analysis.Pass, f *ast.File) (token.Position, bool) { return GetFilePositionFor(pass.Fset, f.Pos()) }

-func GetFilePositionFor(fset *token.FileSet, p token.Pos) token.Position { +func GetFilePositionFor(fset *token.FileSet, p token.Pos) (token.Position, bool) { pos := fset.PositionFor(p, true)

 ext := filepath.Ext(pos.Filename)
 if ext != ".go" {
     // position has been adjusted to a non-go file, revert to original file

Then we'd just do

position, hasGoSuffix := goanalysis.GetFilePosition(pass, file) if !hasGoSuffix { continue }