fix: filter files #5272
fix: filter files #5272
Conversation
ldez
added
bug
There was a problem hiding this comment.
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
- return fset.PositionFor(p, false)
+ return fset.PositionFor(p, false), false
}
- return pos
+ return pos, true
}
func EndOfLinePos(f *token.File, line int) token.Pos {Then we'd just do
position, hasGoSuffix := goanalysis.GetFilePosition(pass, file)
if !hasGoSuffix {
continue
}
|
Your suggestion doesn't work because a non-adjusted file can have |
|
Do you mean that if Wouldn't non-adjusted files with a |
There was a problem hiding this comment.
Approving because I don't want to block you on this bug just to explain to me how it works, but it seems like if we have a .go extension in GetFilePositionFor we'd always return true which I think is correct? So I guess the issue is that we can't blindly return false from the other branch because the adjusted position could potentially also be a .go file?
|
I think an output example will help you to understand: In this example, you have cgo file, go file, test file, and file with line directives. |
Fixes #5271