From 7f48cc88b8309a055a1d78d397ad660c07b7733f Mon Sep 17 00:00:00 2001 From: Soichiro Kashima Date: Wed, 6 May 2020 00:49:34 +0900 Subject: [PATCH] Fix lint errors on files with //line directive (#1065) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the target files contains `//line` directive and it indicates a non-go file, the linter is going to handle it as a go file, which results in failure. The cause of this issue is that the linters (`Analyzer`s) are using `pass.Fset.Position()`. This func returns the adjusted position using `//line` directive. The example project reported in #998 has `//line` directive that indicates other non-go file. According to the description of "Compiler Directives” (https://golang.org/cmd/compile/#hdr-Compiler_Directives), line directives is mainly used for reporting original positions to the generators or something. On linters of golangci-lint, `pass.Fset.Position()` is used just to aggregate file names; we don't have to adjust positions. This changes `Analyzer`s that use `pass.Fset.Position()` to aggregate file names to use `pass.Fset.PositionFor()` with `adjusted == false`. Relates: #998 --- pkg/golinters/dupl.go | 2 +- pkg/golinters/gofmt.go | 2 +- pkg/golinters/goimports.go | 2 +- pkg/golinters/gomodguard.go | 2 +- pkg/golinters/ineffassign.go | 2 +- pkg/golinters/lll.go | 2 +- pkg/golinters/misspell.go | 2 +- pkg/golinters/wsl.go | 2 +- test/run_test.go | 21 ++++++++++++ test/testdata/linedirective/dupl.yml | 3 ++ test/testdata/linedirective/gomodguard.yml | 5 +++ test/testdata/linedirective/hello.go | 37 ++++++++++++++++++++++ test/testdata/linedirective/hello.tmpl | 1 + test/testdata/linedirective/lll.yml | 3 ++ 14 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 test/testdata/linedirective/dupl.yml create mode 100644 test/testdata/linedirective/gomodguard.yml create mode 100644 test/testdata/linedirective/hello.go create mode 100644 test/testdata/linedirective/hello.tmpl create mode 100644 test/testdata/linedirective/lll.yml diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index eaac928e..ed1c4fcb 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -34,7 +34,7 @@ func NewDupl() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index e71f27bb..a7bd88a7 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -31,7 +31,7 @@ func NewGofmt() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/goimports.go b/pkg/golinters/goimports.go index 90e19b0e..97767db8 100644 --- a/pkg/golinters/goimports.go +++ b/pkg/golinters/goimports.go @@ -32,7 +32,7 @@ func NewGoimports() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/gomodguard.go b/pkg/golinters/gomodguard.go index 29cc3c30..5b140631 100644 --- a/pkg/golinters/gomodguard.go +++ b/pkg/golinters/gomodguard.go @@ -54,7 +54,7 @@ func NewGomodguard() *goanalysis.Linter { } for _, file := range pass.Files { - files = append(files, pass.Fset.Position(file.Pos()).Filename) + files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) } processor, err := gomodguard.NewProcessor(processorCfg, log.New(os.Stderr, "", 0)) diff --git a/pkg/golinters/ineffassign.go b/pkg/golinters/ineffassign.go index 601252f7..93c1fb11 100644 --- a/pkg/golinters/ineffassign.go +++ b/pkg/golinters/ineffassign.go @@ -31,7 +31,7 @@ func NewIneffassign() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/lll.go b/pkg/golinters/lll.go index c24b4d14..5f26e91d 100644 --- a/pkg/golinters/lll.go +++ b/pkg/golinters/lll.go @@ -92,7 +92,7 @@ func NewLLL() *goanalysis.Linter { analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/misspell.go b/pkg/golinters/misspell.go index 6cd421e5..80ecf9bb 100644 --- a/pkg/golinters/misspell.go +++ b/pkg/golinters/misspell.go @@ -102,7 +102,7 @@ func NewMisspell() *goanalysis.Linter { var fileNames []string for _, f := range pass.Files { - pos := pass.Fset.Position(f.Pos()) + pos := pass.Fset.PositionFor(f.Pos(), false) fileNames = append(fileNames, pos.Filename) } diff --git a/pkg/golinters/wsl.go b/pkg/golinters/wsl.go index f44d09f2..ca659ac4 100644 --- a/pkg/golinters/wsl.go +++ b/pkg/golinters/wsl.go @@ -52,7 +52,7 @@ func NewWSL() *goanalysis.Linter { ) for _, file := range pass.Files { - files = append(files, pass.Fset.Position(file.Pos()).Filename) + files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) } wslErrors, _ := wsl.NewProcessorWithConfig(processorCfg). diff --git a/test/run_test.go b/test/run_test.go index d1e82ecd..249100b5 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -137,6 +137,27 @@ func TestLineDirectiveProcessedFilesFullLoading(t *testing.T) { r.ExpectExitCode(exitcodes.IssuesFound).ExpectOutputEq(output + "\n") } +func TestLintFilesWithLineDirective(t *testing.T) { + testshared.NewLintRunner(t).Run("-Edupl", "--disable-all", "--config=testdata/linedirective/dupl.yml", getTestDataDir("linedirective")). + ExpectHasIssue("21-23 lines are duplicate of `testdata/linedirective/hello.go:25-27` (dupl)") + testshared.NewLintRunner(t).Run("-Egofmt", "--disable-all", "--no-config", getTestDataDir("linedirective")). + ExpectHasIssue("File is not `gofmt`-ed with `-s` (gofmt)") + testshared.NewLintRunner(t).Run("-Egoimports", "--disable-all", "--no-config", getTestDataDir("linedirective")). + ExpectHasIssue("File is not `goimports`-ed (goimports)") + testshared.NewLintRunner(t). + Run("-Egomodguard", "--disable-all", "--config=testdata/linedirective/gomodguard.yml", getTestDataDir("linedirective")). + ExpectHasIssue("import of package `github.com/ryancurrah/gomodguard` is blocked because the module is not " + + "in the allowed modules list. (gomodguard)") + testshared.NewLintRunner(t).Run("-Eineffassign", "--disable-all", "--no-config", getTestDataDir("linedirective")). + ExpectHasIssue("ineffectual assignment to `x` (ineffassign)") + testshared.NewLintRunner(t).Run("-Elll", "--disable-all", "--config=testdata/linedirective/lll.yml", getTestDataDir("linedirective")). + ExpectHasIssue("line is 57 characters (lll)") + testshared.NewLintRunner(t).Run("-Emisspell", "--disable-all", "--no-config", getTestDataDir("linedirective")). + ExpectHasIssue("is a misspelling of `language` (misspell)") + testshared.NewLintRunner(t).Run("-Ewsl", "--disable-all", "--no-config", getTestDataDir("linedirective")). + ExpectHasIssue("block should not start with a whitespace (wsl)") +} + func TestSkippedDirsNoMatchArg(t *testing.T) { dir := getTestDataDir("skipdirs", "skip_me", "nested") r := testshared.NewLintRunner(t).Run("--print-issued-lines=false", "--no-config", "--skip-dirs", dir, "-Egolint", dir) diff --git a/test/testdata/linedirective/dupl.yml b/test/testdata/linedirective/dupl.yml new file mode 100644 index 00000000..a4211c11 --- /dev/null +++ b/test/testdata/linedirective/dupl.yml @@ -0,0 +1,3 @@ +linters-settings: + dupl: + threshold: 10 diff --git a/test/testdata/linedirective/gomodguard.yml b/test/testdata/linedirective/gomodguard.yml new file mode 100644 index 00000000..6e1b0ef8 --- /dev/null +++ b/test/testdata/linedirective/gomodguard.yml @@ -0,0 +1,5 @@ +linters-settings: + gomodguard: + allowed: + domains: + - golang.org diff --git a/test/testdata/linedirective/hello.go b/test/testdata/linedirective/hello.go new file mode 100644 index 00000000..c511f8b5 --- /dev/null +++ b/test/testdata/linedirective/hello.go @@ -0,0 +1,37 @@ +// Refers a existent, but non-go file with line directive +//line hello.tmpl:1 +package main + +import ( + "github.com/ryancurrah/gomodguard" +) + +func _() { + var x int + _ = x + x = 0 //x +} + +func main() { + a() + b() + wsl() +} + +func a() { + fmt.Println("foo") +} + +func b() { + fmt.Println("foo") +} + +func wsl() bool { + + return true +} + +func notFormatted() { +} + +// langauge diff --git a/test/testdata/linedirective/hello.tmpl b/test/testdata/linedirective/hello.tmpl new file mode 100644 index 00000000..082dc6f6 --- /dev/null +++ b/test/testdata/linedirective/hello.tmpl @@ -0,0 +1 @@ +# This is a template file for some code generator, and is not a valid go source file. diff --git a/test/testdata/linedirective/lll.yml b/test/testdata/linedirective/lll.yml new file mode 100644 index 00000000..0080469b --- /dev/null +++ b/test/testdata/linedirective/lll.yml @@ -0,0 +1,3 @@ +linters-settings: + lll: + line-length: 50