From 5d6e9d733336b47f2aef6c33ddc43bb27950be91 Mon Sep 17 00:00:00 2001 From: Sven Anderson Date: Sat, 30 Jul 2022 09:15:42 +0000 Subject: [PATCH 1/3] Add test that gofmt and revive work with cgo Signed-off-by: Sven Anderson --- test/run_test.go | 2 ++ test/testdata/cgo_with_issues/main.go | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/test/run_test.go b/test/run_test.go index 254f659787b9..706cb1317804 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -104,6 +104,8 @@ func TestCgoWithIssues(t *testing.T) { ExpectHasIssue("Printf format %t has arg cs of wrong type") r.Run("--no-config", "--disable-all", "-Estaticcheck", getTestDataDir("cgo_with_issues")). ExpectHasIssue("SA5009: Printf format %t has arg #1 of wrong type") + r.Run("--no-config", "--disable-all", "-Egofmt", getTestDataDir("cgo_with_issues")). + ExpectHasIssue("File is not `gofmt`-ed with `-s` (gofmt)") } func TestUnsafeOk(t *testing.T) { diff --git a/test/testdata/cgo_with_issues/main.go b/test/testdata/cgo_with_issues/main.go index 6eab19a25e18..3e880bf5ce56 100644 --- a/test/testdata/cgo_with_issues/main.go +++ b/test/testdata/cgo_with_issues/main.go @@ -21,3 +21,15 @@ func Example() { fmt.Printf("bad format %t", cs) C.free(unsafe.Pointer(cs)) } + +func notFormattedForGofmt() { +} + +func errorForRevive(p *int) error { + if p == nil { + return nil + } else { + return nil + } +} + From ce4f33f14e104ef099f706b22de966b3a779b4cd Mon Sep 17 00:00:00 2001 From: Sven Anderson Date: Sat, 30 Jul 2022 11:57:39 +0000 Subject: [PATCH 2/3] Fix many linters ignoring Cgo files This change fixes a bug caused that many linters ignored all files that are using Cgo. It was introduced by PR #1065, which assumed that all files referenced by //line directives are non-Go files, which is not true. In the case of Cgo they point to the original Go files which are used by Cgo as templates to generate other Go files. The fix replaces all calls of Pass.Fset.PositionFor with a function getFileNames() that first calls Pass.Fset.PositionFor with adjustment, and only if it results in a non-Go file it calls Pass.Fset.PositionFor again without adjustment. Fixes: #2910 Signed-off-by: Sven Anderson --- pkg/golinters/dupl.go | 6 +----- pkg/golinters/gci.go | 6 +----- pkg/golinters/gofmt.go | 6 +----- pkg/golinters/gofumpt.go | 6 +----- pkg/golinters/goimports.go | 6 +----- pkg/golinters/gomodguard.go | 7 +------ pkg/golinters/lll.go | 6 +----- pkg/golinters/misspell.go | 7 +------ pkg/golinters/revive.go | 6 +----- pkg/golinters/util.go | 17 +++++++++++++++++ pkg/golinters/wsl.go | 6 +----- test/run_test.go | 4 +++- 12 files changed, 30 insertions(+), 53 deletions(-) diff --git a/pkg/golinters/dupl.go b/pkg/golinters/dupl.go index 8c8d8fe4f310..fe7b12773542 100644 --- a/pkg/golinters/dupl.go +++ b/pkg/golinters/dupl.go @@ -55,11 +55,7 @@ func NewDupl(settings *config.DuplSettings) *goanalysis.Linter { } func runDupl(pass *analysis.Pass, settings *config.DuplSettings) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) issues, err := duplAPI.Run(fileNames, settings.Threshold) if err != nil { diff --git a/pkg/golinters/gci.go b/pkg/golinters/gci.go index 82eba2ff0ef6..0f719513d794 100644 --- a/pkg/golinters/gci.go +++ b/pkg/golinters/gci.go @@ -83,11 +83,7 @@ func NewGci(settings *config.GciSettings) *goanalysis.Linter { } func runGci(pass *analysis.Pass, lintCtx *linter.Context, cfg *gcicfg.Config, lock *sync.Mutex) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) var diffs []string err := diffFormattedFilesToArray(fileNames, *cfg, &diffs, lock) diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 1d50bfc55be9..e8c02411c389 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -53,11 +53,7 @@ func NewGofmt(settings *config.GoFmtSettings) *goanalysis.Linter { } func runGofmt(lintCtx *linter.Context, pass *analysis.Pass, settings *config.GoFmtSettings) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) var issues []goanalysis.Issue diff --git a/pkg/golinters/gofumpt.go b/pkg/golinters/gofumpt.go index 60d97b944b08..312dfd6d93ba 100644 --- a/pkg/golinters/gofumpt.go +++ b/pkg/golinters/gofumpt.go @@ -73,11 +73,7 @@ func NewGofumpt(settings *config.GofumptSettings) *goanalysis.Linter { } func runGofumpt(lintCtx *linter.Context, pass *analysis.Pass, diff differ, options format.Options) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) var issues []goanalysis.Issue diff --git a/pkg/golinters/goimports.go b/pkg/golinters/goimports.go index e59ee3dd5bad..42aa69b41819 100644 --- a/pkg/golinters/goimports.go +++ b/pkg/golinters/goimports.go @@ -55,11 +55,7 @@ func NewGoimports(settings *config.GoImportsSettings) *goanalysis.Linter { } func runGoiImports(lintCtx *linter.Context, pass *analysis.Pass) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) var issues []goanalysis.Issue diff --git a/pkg/golinters/gomodguard.go b/pkg/golinters/gomodguard.go index 76dd67012b56..e21658d5d2a3 100644 --- a/pkg/golinters/gomodguard.go +++ b/pkg/golinters/gomodguard.go @@ -73,12 +73,7 @@ func NewGomodguard(settings *config.GoModGuardSettings) *goanalysis.Linter { } analyzer.Run = func(pass *analysis.Pass) (interface{}, error) { - var files []string - for _, file := range pass.Files { - files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) - } - - gomodguardIssues := processor.ProcessFiles(files) + gomodguardIssues := processor.ProcessFiles(getFileNames(pass)) mu.Lock() defer mu.Unlock() diff --git a/pkg/golinters/lll.go b/pkg/golinters/lll.go index 65d4c15a3784..25b767a0b58a 100644 --- a/pkg/golinters/lll.go +++ b/pkg/golinters/lll.go @@ -56,11 +56,7 @@ func NewLLL(settings *config.LllSettings) *goanalysis.Linter { } func runLll(pass *analysis.Pass, settings *config.LllSettings) ([]goanalysis.Issue, error) { - var fileNames []string - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) spaces := strings.Repeat(" ", settings.TabWidth) diff --git a/pkg/golinters/misspell.go b/pkg/golinters/misspell.go index 47eb7a7b924f..b5cc5c8a89d4 100644 --- a/pkg/golinters/misspell.go +++ b/pkg/golinters/misspell.go @@ -61,12 +61,7 @@ func NewMisspell(settings *config.MisspellSettings) *goanalysis.Linter { } func runMisspell(lintCtx *linter.Context, pass *analysis.Pass, replacer *misspell.Replacer) ([]goanalysis.Issue, error) { - var fileNames []string - - for _, f := range pass.Files { - pos := pass.Fset.PositionFor(f.Pos(), false) - fileNames = append(fileNames, pos.Filename) - } + fileNames := getFileNames(pass) var issues []goanalysis.Issue for _, filename := range fileNames { diff --git a/pkg/golinters/revive.go b/pkg/golinters/revive.go index 0e150720c50c..906e1c3ef44d 100644 --- a/pkg/golinters/revive.go +++ b/pkg/golinters/revive.go @@ -75,11 +75,7 @@ func NewRevive(settings *config.ReviveSettings) *goanalysis.Linter { } func runRevive(lintCtx *linter.Context, pass *analysis.Pass, settings *config.ReviveSettings) ([]goanalysis.Issue, error) { - var files []string - for _, file := range pass.Files { - files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) - } - packages := [][]string{files} + packages := [][]string{getFileNames(pass)} conf, err := getReviveConfig(settings) if err != nil { diff --git a/pkg/golinters/util.go b/pkg/golinters/util.go index 1940f30e3ffd..1044567a951b 100644 --- a/pkg/golinters/util.go +++ b/pkg/golinters/util.go @@ -2,8 +2,11 @@ package golinters import ( "fmt" + "path/filepath" "strings" + "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/config" ) @@ -22,3 +25,17 @@ func formatCodeBlock(code string, _ *config.Config) string { return fmt.Sprintf("```\n%s\n```", code) } + +func getFileNames(pass *analysis.Pass) []string { + var fileNames []string + for _, f := range pass.Files { + fileName := pass.Fset.PositionFor(f.Pos(), true).Filename + ext := filepath.Ext(fileName) + if ext != "" && ext != ".go" { + // position has been adjusted to a non-go file, revert to original file + fileName = pass.Fset.PositionFor(f.Pos(), false).Filename + } + fileNames = append(fileNames, fileName) + } + return fileNames +} diff --git a/pkg/golinters/wsl.go b/pkg/golinters/wsl.go index 0b10798eb629..9d7060b0b3d6 100644 --- a/pkg/golinters/wsl.go +++ b/pkg/golinters/wsl.go @@ -67,15 +67,11 @@ func NewWSL(settings *config.WSLSettings) *goanalysis.Linter { } func runWSL(pass *analysis.Pass, conf *wsl.Configuration) []goanalysis.Issue { - var files = make([]string, 0, len(pass.Files)) - for _, file := range pass.Files { - files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) - } - if conf == nil { return nil } + files := getFileNames(pass) wslErrors, _ := wsl.NewProcessorWithConfig(*conf).ProcessFiles(files) if len(wslErrors) == 0 { return nil diff --git a/test/run_test.go b/test/run_test.go index 706cb1317804..75ed787b9c06 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -106,6 +106,8 @@ func TestCgoWithIssues(t *testing.T) { ExpectHasIssue("SA5009: Printf format %t has arg #1 of wrong type") r.Run("--no-config", "--disable-all", "-Egofmt", getTestDataDir("cgo_with_issues")). ExpectHasIssue("File is not `gofmt`-ed with `-s` (gofmt)") + r.Run("--no-config", "--disable-all", "-Erevive", getTestDataDir("cgo_with_issues")). + ExpectHasIssue("indent-error-flow: if block ends with a return statement") } func TestUnsafeOk(t *testing.T) { @@ -129,7 +131,7 @@ func TestLineDirectiveProcessedFilesLiteLoading(t *testing.T) { } func TestSortedResults(t *testing.T) { - var testCases = []struct { + testCases := []struct { opt string want string }{ From ce8014ba30bdd1478c0cf93ef88f593f5ecc5861 Mon Sep 17 00:00:00 2001 From: Sven Anderson Date: Sat, 30 Jul 2022 13:41:54 +0000 Subject: [PATCH 3/3] Disable gci linter in cgo test It seems like the gci linter doesn't know about the special import rules for Cgo. --- test/run_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_test.go b/test/run_test.go index 75ed787b9c06..cf1f55496912 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -95,7 +95,7 @@ func TestTestsAreLintedByDefault(t *testing.T) { } func TestCgoOk(t *testing.T) { - testshared.NewLintRunner(t).Run("--no-config", "--enable-all", "-D", "nosnakecase", getTestDataDir("cgo")).ExpectNoIssues() + testshared.NewLintRunner(t).Run("--no-config", "--enable-all", "-D", "nosnakecase,gci", getTestDataDir("cgo")).ExpectNoIssues() } func TestCgoWithIssues(t *testing.T) {