From ed58cef510ea70b577771d4dd9cebb9b270037db Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 01:15:30 +0200 Subject: [PATCH 01/14] Makefile --- Makefile | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 9561191f9990..92f3580e74a4 100644 --- a/Makefile +++ b/Makefile @@ -4,17 +4,22 @@ # enable consistent Go 1.12/1.13 GOPROXY behavior. export GOPROXY = https://proxy.golang.org +BINARY = golangci-lint +ifeq ($(OS),Windows_NT) + BINARY := $(BINARY).exe +endif + # Build -build: golangci-lint +build: $(BINARY) .PHONY: build build_race: - go build -race -o golangci-lint ./cmd/golangci-lint + go build -race -o $(BINARY) ./cmd/golangci-lint .PHONY: build_race clean: - rm -f golangci-lint + rm -f $(BINARY) rm -f test/path rm -f tools/Dracula.itermcolors rm -f tools/goreleaser @@ -25,7 +30,7 @@ clean: # Test test: export GOLANGCI_LINT_INSTALLED = true test: build - GL_TEST_RUN=1 ./golangci-lint run -v + GL_TEST_RUN=1 ./$(BINARY) run -v GL_TEST_RUN=1 go test -v -parallel 2 ./... .PHONY: test @@ -36,7 +41,7 @@ test_fix: build .PHONY: test_fix test_race: build_race - GL_TEST_RUN=1 ./golangci-lint run -v --timeout=5m + GL_TEST_RUN=1 ./$(BINARY) run -v --timeout=5m .PHONY: test_race test_linters: @@ -67,7 +72,7 @@ snapshot: .goreleaser.yml tools/goreleaser # Non-PHONY targets (real files) -golangci-lint: FORCE +$(BINARY): FORCE go build -o $@ ./cmd/golangci-lint tools/goreleaser: export GOFLAGS = -mod=readonly @@ -87,7 +92,7 @@ tools/Dracula.itermcolors: assets/demo.svg: tools/svg-term tools/Dracula.itermcolors ./tools/svg-term --cast=183662 --out assets/demo.svg --window --width 110 --height 30 --from 2000 --to 20000 --profile ./tools/Dracula.itermcolors --term iterm2 -assets/github-action-config.json: FORCE golangci-lint +assets/github-action-config.json: FORCE $(BINARY) # go run ./scripts/gen_github_action_config/main.go $@ cd ./scripts/gen_github_action_config/; go run ./main.go ../../$@ From c5ca4f92612c7680cc4ce172f897ff43f33f3833 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 01:18:53 +0200 Subject: [PATCH 02/14] test runner --- test/testshared/runner.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 0c54af7db5e2..521e07e260df 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "sync" "syscall" @@ -17,7 +18,13 @@ import ( "github.com/golangci/golangci-lint/pkg/logutils" ) -const defaultBinPath = "../golangci-lint" +func defaultBinaryName() string { + name := filepath.Join("..", "golangci-lint") + if runtime.GOOS == "windows" { + name += ".exe" + } + return name +} type RunnerBuilder struct { tb testing.TB @@ -43,7 +50,7 @@ func NewRunnerBuilder(tb testing.TB) *RunnerBuilder { return &RunnerBuilder{ tb: tb, log: log, - binPath: defaultBinPath, + binPath: defaultBinaryName(), command: "run", allowParallelRunners: true, } @@ -68,7 +75,10 @@ func (b *RunnerBuilder) WithNoConfig() *RunnerBuilder { } func (b *RunnerBuilder) WithConfigFile(cfgPath string) *RunnerBuilder { - b.configPath = cfgPath + if cfgPath != "" { + b.configPath = filepath.FromSlash(cfgPath) + } + b.noConfig = cfgPath == "" return b @@ -338,10 +348,10 @@ func InstallGolangciLint(tb testing.TB) string { tb.Log(string(output)) } - require.NoError(tb, err, "Can't go install golangci-lint") + assert.NoError(tb, err, "Can't go install golangci-lint %s", string(output)) } - abs, err := filepath.Abs(defaultBinPath) + abs, err := filepath.Abs(defaultBinaryName()) require.NoError(tb, err) return abs From 590c25f573189337c2cf462534906232c12821fe Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 01:27:02 +0200 Subject: [PATCH 03/14] ci: remove limitation --- .github/workflows/pr.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index b067e7644034..565cd6308a57 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -9,7 +9,7 @@ env: GO_VERSION: 1.19 jobs: - # Check if there any dirty change for go mod tidy + # Check if there is any dirty change for go mod tidy go-mod: runs-on: ubuntu-latest steps: @@ -41,8 +41,7 @@ jobs: # ex: # - 1.18beta1 -> 1.18.0-beta.1 # - 1.18rc1 -> 1.18.0-rc.1 -# go-version: ${{ env.GO_VERSION }} # todo(ldez) uncomment after the next release v1.48.0 - go-version: 1.18 + go-version: ${{ env.GO_VERSION }} - name: lint uses: golangci/golangci-lint-action@v3.2.0 with: @@ -66,7 +65,6 @@ jobs: go-version: ${{ env.GO_VERSION }} # test only the latest go version to speed up CI - name: Run tests run: make.exe test - continue-on-error: true tests-on-macos: needs: golangci-lint # run after golangci-lint action to not produce duplicated errors From 469b417d1db868be107bde1ef6ecfbf98f2092dc Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 01:39:28 +0200 Subject: [PATCH 04/14] rules processor --- pkg/result/processors/severity_rules.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/result/processors/severity_rules.go b/pkg/result/processors/severity_rules.go index 7c9a4c1d63a5..4077b34057d1 100644 --- a/pkg/result/processors/severity_rules.go +++ b/pkg/result/processors/severity_rules.go @@ -49,7 +49,8 @@ func createSeverityRules(rules []SeverityRule, prefix string) []severityRule { parsedRule.source = regexp.MustCompile(prefix + rule.Source) } if rule.Path != "" { - parsedRule.path = regexp.MustCompile(rule.Path) + path := normalizePathInRegex(rule.Path) + parsedRule.path = regexp.MustCompile(path) } parsedRules = append(parsedRules, parsedRule) } From aa23cf3986c13f2d2a7143c7f24373cb13d0ee8f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 01:55:07 +0200 Subject: [PATCH 05/14] git attributes --- .gitattributes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitattributes b/.gitattributes index 32f1001be0a5..55f26a1624b2 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,3 @@ go.sum linguist-generated +* text=auto eol=lf +*.ps1 text eol=crlf From b67179cdea21f69bb3d0c898feb508108f2296aa Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 02:10:25 +0200 Subject: [PATCH 06/14] test: processor skip --- pkg/result/processors/skip_files_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/result/processors/skip_files_test.go b/pkg/result/processors/skip_files_test.go index 92c50ca7d0ce..7f16b58158ef 100644 --- a/pkg/result/processors/skip_files_test.go +++ b/pkg/result/processors/skip_files_test.go @@ -2,6 +2,8 @@ package processors import ( "go/token" + "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -33,14 +35,15 @@ func TestSkipFiles(t *testing.T) { processAssertEmpty(t, newTestSkipFiles(t, ".*"), newFileIssue("any.go")) - processAssertEmpty(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/c.go")) - processAssertSame(t, newTestSkipFiles(t, "a/b/c.go"), newFileIssue("a/b/d.go")) + cleanPath := strings.ReplaceAll(filepath.FromSlash("a/b/c.go"), `\`, `\\`) + processAssertEmpty(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/c.go"))) + processAssertSame(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/d.go"))) - processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.pb.go")) - processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue("a/b.go")) + processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.pb.go"))) + processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.go"))) - processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.pb.go")) - processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue("a/b.go")) + processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.pb.go"))) + processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.go"))) } func TestSkipFilesInvalidPattern(t *testing.T) { From f5531d9dfd2c2d565ef0d06648354d7645ba0ff0 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 02:31:45 +0200 Subject: [PATCH 07/14] test: fix TestPathPrefix --- test/run_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/run_test.go b/test/run_test.go index ea5013c14b33..a27c1e7047db 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -2,6 +2,7 @@ package test import ( "path/filepath" + "regexp" "strings" "testing" @@ -680,7 +681,18 @@ func TestPathPrefix(t *testing.T) { WithTargetPath(testdataDir, "withtests"). Runner(). Run(). - ExpectOutputRegexp(test.pattern) + ExpectOutputRegexp(normalizePathInRegex(test.pattern)) }) } } + +// FIXME(ldez) see utils.normalizePathInRegex(...) and maybe move into RunnerResult.ExpectOutputRegexp(...) +func normalizePathInRegex(path string) string { + if filepath.Separator == '/' { + return path + } + + // This replacing should be safe because "/" are disallowed in Windows + // https://docs.microsoft.com/windows/win32/fileio/naming-a-file + return strings.ReplaceAll(path, "/", regexp.QuoteMeta(string(filepath.Separator))) +} From 7eeb74ab3ded65316a4f260fce4103cdf90f2beb Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 02:34:08 +0200 Subject: [PATCH 08/14] test: fix TestRunnerBuilder_Runner --- test/testshared/runner_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index af5fe7603e40..62e016dabdb3 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -1,6 +1,7 @@ package testshared import ( + "path/filepath" "regexp" "testing" @@ -67,7 +68,7 @@ func TestRunnerBuilder_Runner(t *testing.T) { "--internal-cmd-test", "--allow-parallel-runners", "-c", - "./testdata/example.yml", + filepath.FromSlash("./testdata/example.yml"), }, }, }, @@ -82,7 +83,7 @@ func TestRunnerBuilder_Runner(t *testing.T) { "--internal-cmd-test", "--allow-parallel-runners", "-c", - "testdata/example.yml", + filepath.FromSlash("testdata/example.yml"), "-Efoo", "--simple", "--hello=world", @@ -140,7 +141,7 @@ func TestRunnerBuilder_Runner(t *testing.T) { "--go=1.17", "--internal-cmd-test", "--allow-parallel-runners", - "testdata/all.go", + filepath.FromSlash("testdata/all.go"), }, }, }, @@ -149,7 +150,7 @@ func TestRunnerBuilder_Runner(t *testing.T) { builder: NewRunnerBuilder(t). WithRunContext(&RunContext{ Args: []string{"-Efoo", "--simple", "--hello=world"}, - ConfigPath: "testdata/example.yml", + ConfigPath: filepath.FromSlash("testdata/example.yml"), ExpectedLinter: "test", }), expected: &Runner{ @@ -160,7 +161,7 @@ func TestRunnerBuilder_Runner(t *testing.T) { "--internal-cmd-test", "--allow-parallel-runners", "-c", - "testdata/example.yml", + filepath.FromSlash("testdata/example.yml"), "-Efoo", "--simple", "--hello=world", From c031437a9d73070e5d91ef523ae1c3c61a141d70 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 02:57:28 +0200 Subject: [PATCH 09/14] tests: skip on Windows --- test/linters_test.go | 2 ++ test/testshared/analysis.go | 2 +- test/testshared/runner.go | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/linters_test.go b/test/linters_test.go index 0293a74f4c5f..466e983e50e0 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -22,6 +22,8 @@ func TestSourcesFromTestdata(t *testing.T) { } func TestTypecheck(t *testing.T) { + testshared.SkipOnWindows(t) + testSourcesFromDir(t, filepath.Join(testdataDir, "notcompiles")) } diff --git a/test/testshared/analysis.go b/test/testshared/analysis.go index be1ed731275a..9ef29fd81e02 100644 --- a/test/testshared/analysis.go +++ b/test/testshared/analysis.go @@ -46,7 +46,7 @@ func Analyze(t *testing.T, sourcePath string, rawData []byte) { var reportData jsonResult err = json.Unmarshal(rawData, &reportData) - require.NoError(t, err) + require.NoError(t, err, string(rawData)) for _, issue := range reportData.Issues { checkMessage(t, want, issue.Pos, "diagnostic", issue.FromLinter, issue.Text) diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 521e07e260df..ad7c07829b35 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -356,3 +356,9 @@ func InstallGolangciLint(tb testing.TB) string { return abs } + +func SkipOnWindows(tb testing.TB) { + if runtime.GOOS == "windows" { + tb.Skip("not supported on Windows") + } +} From e69fc78f3f925d89ee8cf2e0d8132aa5b1772c5e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 04:00:47 +0200 Subject: [PATCH 10/14] feat: build tags on test files --- test/testdata/tenv_go118.go | 1 - test/testdata/thelper_go118.go | 1 - test/testshared/directives.go | 27 +++++++++++--- test/testshared/directives_test.go | 59 ++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/test/testdata/tenv_go118.go b/test/testdata/tenv_go118.go index bcdf9dcabaf7..97a3489319bd 100644 --- a/test/testdata/tenv_go118.go +++ b/test/testdata/tenv_go118.go @@ -1,5 +1,4 @@ //go:build go1.18 -// +build go1.18 //golangcitest:args -Etenv package testdata diff --git a/test/testdata/thelper_go118.go b/test/testdata/thelper_go118.go index bea2adf9bdaf..e83a387a5b0c 100644 --- a/test/testdata/thelper_go118.go +++ b/test/testdata/thelper_go118.go @@ -1,5 +1,4 @@ //go:build go1.18 -// +build go1.18 //golangcitest:args -Ethelper package testdata diff --git a/test/testshared/directives.go b/test/testshared/directives.go index b29f305f97e6..5e34f006c9e3 100644 --- a/test/testshared/directives.go +++ b/test/testshared/directives.go @@ -51,11 +51,9 @@ func ParseTestDirectives(tb testing.TB, sourcePath string) *RunContext { break } - if strings.HasPrefix(line, "//go:build") || strings.HasPrefix(line, "// +build") { - parse, err := constraint.Parse(line) - require.NoError(tb, err) - - if !parse.Eval(buildTagGoVersion) { + // FIXME(ldez) remove IsPlusBuild? + if constraint.IsGoBuild(line) || constraint.IsPlusBuild(line) { + if !evaluateBuildTags(tb, line) { return nil } @@ -120,6 +118,25 @@ func skipMultilineComment(scanner *bufio.Scanner) { } } +// evaluateBuildTags Naive implementation of the evaluation of the build tags. +// Inspired by https://github.com/golang/go/blob/1dcef7b3bdcea4a829ea22c821e6a9484c325d61/src/cmd/go/internal/modindex/build.go#L914-L972 +func evaluateBuildTags(tb testing.TB, line string) bool { + parse, err := constraint.Parse(line) + require.NoError(tb, err) + + return parse.Eval(func(tag string) bool { + if tag == runtime.GOOS { + return true + } + + if buildTagGoVersion(tag) { + return true + } + + return false + }) +} + func buildTagGoVersion(tag string) bool { vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) if err != nil { diff --git a/test/testshared/directives_test.go b/test/testshared/directives_test.go index bee856cd3083..ffe15c909bf0 100644 --- a/test/testshared/directives_test.go +++ b/test/testshared/directives_test.go @@ -1,6 +1,7 @@ package testshared import ( + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -21,3 +22,61 @@ func TestParseTestDirectives(t *testing.T) { } assert.Equal(t, expected, rc) } + +func Test_evaluateBuildTags(t *testing.T) { + testCases := []struct { + desc string + tag string + assert assert.BoolAssertionFunc + }{ + { + desc: "", + tag: "// +build go1.18", + assert: assert.True, + }, + { + desc: "", + tag: "// +build go1.42", + assert: assert.False, + }, + { + desc: "", + tag: "//go:build go1.18", + assert: assert.True, + }, + { + desc: "", + tag: "//go:build go1.42", + assert: assert.False, + }, + { + desc: "", + tag: "//go:build " + runtime.GOOS, + assert: assert.True, + }, + { + desc: "", + tag: "//go:build !wondiws", + assert: assert.True, + }, + { + desc: "", + tag: "//go:build wondiws", + assert: assert.False, + }, + { + desc: "", + tag: "//go:build go1.18 && " + runtime.GOOS, + assert: assert.True, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + test.assert(t, evaluateBuildTags(t, test.tag)) + }) + } +} From 48deaeb85b364d023933920f6dba8b0137b7a5ab Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 04:36:32 +0200 Subject: [PATCH 11/14] tests: apply skip on linter tests --- test/fix_test.go | 2 ++ test/testdata/depguard_ignore_file_rules.go | 2 ++ test/testdata/ifshort.go | 2 ++ 3 files changed, 6 insertions(+) diff --git a/test/fix_test.go b/test/fix_test.go index 85653ddf40d4..1eeb1fef6481 100644 --- a/test/fix_test.go +++ b/test/fix_test.go @@ -12,6 +12,8 @@ import ( ) func TestFix(t *testing.T) { + testshared.SkipOnWindows(t) + tmpDir := filepath.Join(testdataDir, "fix.tmp") _ = os.RemoveAll(tmpDir) // cleanup previous runs diff --git a/test/testdata/depguard_ignore_file_rules.go b/test/testdata/depguard_ignore_file_rules.go index c1073767a1d8..0fcc766c2dc4 100644 --- a/test/testdata/depguard_ignore_file_rules.go +++ b/test/testdata/depguard_ignore_file_rules.go @@ -1,3 +1,5 @@ +//go:build !windows + //golangcitest:args -Edepguard //golangcitest:config_path testdata/configs/depguard_ignore_file_rules.yml //golangcitest:expected_exitcode 0 diff --git a/test/testdata/ifshort.go b/test/testdata/ifshort.go index 20d79759c001..71906fa850b8 100644 --- a/test/testdata/ifshort.go +++ b/test/testdata/ifshort.go @@ -1,3 +1,5 @@ +//go:build !windows + //golangcitest:args -Eifshort --internal-cmd-test package testdata From 7f342cd69849e45a125d050196f71a34ab72965e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 13:41:50 +0200 Subject: [PATCH 12/14] normalize path --- pkg/result/processors/{utils.go => issues.go} | 16 ------- pkg/result/processors/path_prefixer.go | 4 +- pkg/result/processors/path_prefixer_test.go | 32 +++++++++---- pkg/result/processors/path_unix.go | 7 +++ pkg/result/processors/path_windows.go | 21 ++++++++ test/output_test.go | 9 ++-- test/run_test.go | 16 +------ test/testshared/runner.go | 23 ++------- test/testshared/runner_test.go | 2 - test/testshared/runner_unix.go | 31 ++++++++++++ test/testshared/runner_windows.go | 48 +++++++++++++++++++ 11 files changed, 143 insertions(+), 66 deletions(-) rename pkg/result/processors/{utils.go => issues.go} (72%) create mode 100644 pkg/result/processors/path_unix.go create mode 100644 pkg/result/processors/path_windows.go create mode 100644 test/testshared/runner_unix.go create mode 100644 test/testshared/runner_windows.go diff --git a/pkg/result/processors/utils.go b/pkg/result/processors/issues.go similarity index 72% rename from pkg/result/processors/utils.go rename to pkg/result/processors/issues.go index 7108fd3b3c26..8bc3d847d65a 100644 --- a/pkg/result/processors/utils.go +++ b/pkg/result/processors/issues.go @@ -1,10 +1,6 @@ package processors import ( - "path/filepath" - "regexp" - "strings" - "github.com/pkg/errors" "github.com/golangci/golangci-lint/pkg/result" @@ -48,15 +44,3 @@ func transformIssues(issues []result.Issue, transform func(i *result.Issue) *res return retIssues } - -var separatorToReplace = regexp.QuoteMeta(string(filepath.Separator)) - -func normalizePathInRegex(path string) string { - if filepath.Separator == '/' { - return path - } - - // This replacing should be safe because "/" are disallowed in Windows - // https://docs.microsoft.com/ru-ru/windows/win32/fileio/naming-a-file - return strings.ReplaceAll(path, "/", separatorToReplace) -} diff --git a/pkg/result/processors/path_prefixer.go b/pkg/result/processors/path_prefixer.go index 5ce940b39bf0..04ed83126621 100644 --- a/pkg/result/processors/path_prefixer.go +++ b/pkg/result/processors/path_prefixer.go @@ -1,7 +1,7 @@ package processors import ( - "path" + "path/filepath" "github.com/golangci/golangci-lint/pkg/result" ) @@ -27,7 +27,7 @@ func (*PathPrefixer) Name() string { func (p *PathPrefixer) Process(issues []result.Issue) ([]result.Issue, error) { if p.prefix != "" { for i := range issues { - issues[i].Pos.Filename = path.Join(p.prefix, issues[i].Pos.Filename) + issues[i].Pos.Filename = filepath.Join(p.prefix, issues[i].Pos.Filename) } } return issues, nil diff --git a/pkg/result/processors/path_prefixer_test.go b/pkg/result/processors/path_prefixer_test.go index 55fdbea9264c..9c349d501ae2 100644 --- a/pkg/result/processors/path_prefixer_test.go +++ b/pkg/result/processors/path_prefixer_test.go @@ -2,8 +2,10 @@ package processors import ( "go/token" + "path/filepath" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/pkg/result" @@ -12,26 +14,40 @@ import ( func TestPathPrefixer_Process(t *testing.T) { paths := func(ps ...string) (issues []result.Issue) { for _, p := range ps { - issues = append(issues, result.Issue{Pos: token.Position{Filename: p}}) + issues = append(issues, result.Issue{Pos: token.Position{Filename: filepath.FromSlash(p)}}) } return } + for _, tt := range []struct { name, prefix string issues, want []result.Issue }{ - {"empty prefix", "", paths("some/path", "cool"), paths("some/path", "cool")}, - {"prefix", "ok", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")}, - {"prefix slashed", "ok/", paths("some/path", "cool"), paths("ok/some/path", "ok/cool")}, + { + name: "empty prefix", + issues: paths("some/path", "cool"), + want: paths("some/path", "cool"), + }, + { + name: "prefix", + prefix: "ok", + issues: paths("some/path", "cool"), + want: paths("ok/some/path", "ok/cool"), + }, + { + name: "prefix slashed", + prefix: "ok/", + issues: paths("some/path", "cool"), + want: paths("ok/some/path", "ok/cool"), + }, } { t.Run(tt.name, func(t *testing.T) { - r := require.New(t) - p := NewPathPrefixer(tt.prefix) + got, err := p.Process(tt.issues) - r.NoError(err, "prefixer should never error") + require.NoError(t, err) - r.Equal(got, tt.want) + assert.Equal(t, got, tt.want) }) } } diff --git a/pkg/result/processors/path_unix.go b/pkg/result/processors/path_unix.go new file mode 100644 index 000000000000..d61bbf67a812 --- /dev/null +++ b/pkg/result/processors/path_unix.go @@ -0,0 +1,7 @@ +//go:build !windows + +package processors + +func normalizePathInRegex(path string) string { + return path +} diff --git a/pkg/result/processors/path_windows.go b/pkg/result/processors/path_windows.go new file mode 100644 index 000000000000..edd081c875e0 --- /dev/null +++ b/pkg/result/processors/path_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package processors + +import ( + "path/filepath" + "regexp" + "strings" +) + +var separatorToReplace = regexp.QuoteMeta(string(filepath.Separator)) + +func normalizePathInRegex(path string) string { + if filepath.Separator == '/' { + return path + } + + // This replacing should be safe because "/" are disallowed in Windows + // https://docs.microsoft.com/ru-ru/windows/win32/fileio/naming-a-file + return strings.ReplaceAll(path, "/", separatorToReplace) +} diff --git a/test/output_test.go b/test/output_test.go index e1127b12cedf..fba1bededaa6 100644 --- a/test/output_test.go +++ b/test/output_test.go @@ -3,7 +3,6 @@ package test import ( "fmt" "os" - "path" "path/filepath" "testing" @@ -50,11 +49,11 @@ func TestOutput_Stderr(t *testing.T) { Runner(). Install(). Run(). - ExpectHasIssue(expectedJSONOutput) + ExpectHasIssue(testshared.NormalizeFilePathInJSON(expectedJSONOutput)) } func TestOutput_File(t *testing.T) { - resultPath := path.Join(t.TempDir(), "golangci_lint_test_result") + resultPath := filepath.Join(t.TempDir(), "golangci_lint_test_result") sourcePath := filepath.Join(testdataDir, "misspell.go") @@ -74,7 +73,7 @@ func TestOutput_File(t *testing.T) { b, err := os.ReadFile(resultPath) require.NoError(t, err) - require.Contains(t, string(b), expectedJSONOutput) + require.Contains(t, string(b), testshared.NormalizeFilePathInJSON(expectedJSONOutput)) } func TestOutput_Multiple(t *testing.T) { @@ -94,5 +93,5 @@ func TestOutput_Multiple(t *testing.T) { Run(). //nolint:misspell ExpectHasIssue("testdata/misspell.go:6:38: `occured` is a misspelling of `occurred`"). - ExpectOutputContains(expectedJSONOutput) + ExpectOutputContains(testshared.NormalizeFilePathInJSON(expectedJSONOutput)) } diff --git a/test/run_test.go b/test/run_test.go index a27c1e7047db..d780debe917d 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -2,7 +2,6 @@ package test import ( "path/filepath" - "regexp" "strings" "testing" @@ -46,7 +45,7 @@ func TestNotExistingDirRun(t *testing.T) { Run(). ExpectExitCode(exitcodes.Failure). ExpectOutputContains("cannot find package"). - ExpectOutputContains("/testdata/no_such_dir") + ExpectOutputContains(testshared.NormalizeFileInString("/testdata/no_such_dir")) } func TestSymlinkLoop(t *testing.T) { @@ -681,18 +680,7 @@ func TestPathPrefix(t *testing.T) { WithTargetPath(testdataDir, "withtests"). Runner(). Run(). - ExpectOutputRegexp(normalizePathInRegex(test.pattern)) + ExpectOutputRegexp(test.pattern) }) } } - -// FIXME(ldez) see utils.normalizePathInRegex(...) and maybe move into RunnerResult.ExpectOutputRegexp(...) -func normalizePathInRegex(path string) string { - if filepath.Separator == '/' { - return path - } - - // This replacing should be safe because "/" are disallowed in Windows - // https://docs.microsoft.com/windows/win32/fileio/naming-a-file - return strings.ReplaceAll(path, "/", regexp.QuoteMeta(string(filepath.Separator))) -} diff --git a/test/testshared/runner.go b/test/testshared/runner.go index ad7c07829b35..fa03bab34012 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -4,7 +4,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "strings" "sync" "syscall" @@ -18,14 +17,6 @@ import ( "github.com/golangci/golangci-lint/pkg/logutils" ) -func defaultBinaryName() string { - name := filepath.Join("..", "golangci-lint") - if runtime.GOOS == "windows" { - name += ".exe" - } - return name -} - type RunnerBuilder struct { tb testing.TB log logutils.Log @@ -303,17 +294,17 @@ func (r *RunnerResult) ExpectExitCode(possibleCodes ...int) *RunnerResult { } // ExpectOutputRegexp can be called with either a string or compiled regexp -func (r *RunnerResult) ExpectOutputRegexp(s interface{}) *RunnerResult { +func (r *RunnerResult) ExpectOutputRegexp(s string) *RunnerResult { r.tb.Helper() - assert.Regexp(r.tb, s, r.output, "exit code is %d", r.exitCode) + assert.Regexp(r.tb, normalizeFilePathInRegex(s), r.output, "exit code is %d", r.exitCode) return r } func (r *RunnerResult) ExpectOutputContains(s string) *RunnerResult { r.tb.Helper() - assert.Contains(r.tb, r.output, s, "exit code is %d", r.exitCode) + assert.Contains(r.tb, r.output, normalizeFilePath(s), "exit code is %d", r.exitCode) return r } @@ -327,7 +318,7 @@ func (r *RunnerResult) ExpectOutputNotContains(s string) *RunnerResult { func (r *RunnerResult) ExpectOutputEq(s string) *RunnerResult { r.tb.Helper() - assert.Equal(r.tb, s, r.output, "exit code is %d", r.exitCode) + assert.Equal(r.tb, normalizeFilePath(s), r.output, "exit code is %d", r.exitCode) return r } @@ -356,9 +347,3 @@ func InstallGolangciLint(tb testing.TB) string { return abs } - -func SkipOnWindows(tb testing.TB) { - if runtime.GOOS == "windows" { - tb.Skip("not supported on Windows") - } -} diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index 62e016dabdb3..c647e8c13d98 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -2,7 +2,6 @@ package testshared import ( "path/filepath" - "regexp" "testing" "github.com/stretchr/testify/assert" @@ -220,7 +219,6 @@ func TestRunnerResult_ExpectOutputNotContains(t *testing.T) { func TestRunnerResult_ExpectOutputRegexp(t *testing.T) { r := &RunnerResult{tb: t, output: "this is an output"} - r.ExpectOutputRegexp(regexp.MustCompile(`an.+`)) r.ExpectOutputRegexp(`an.+`) r.ExpectOutputRegexp("an") } diff --git a/test/testshared/runner_unix.go b/test/testshared/runner_unix.go new file mode 100644 index 000000000000..f76b82d6ac0b --- /dev/null +++ b/test/testshared/runner_unix.go @@ -0,0 +1,31 @@ +//go:build !windows + +package testshared + +import ( + "path/filepath" + "testing" +) + +func SkipOnWindows(_ testing.TB) {} + +func NormalizeFilePathInJSON(in string) string { + return in +} + +// NormalizeFileInString normalizes in quoted string. +func NormalizeFileInString(in string) string { + return in +} + +func defaultBinaryName() string { + return filepath.Join("..", "golangci-lint") +} + +func normalizeFilePath(in string) string { + return in +} + +func normalizeFilePathInRegex(path string) string { + return path +} diff --git a/test/testshared/runner_windows.go b/test/testshared/runner_windows.go new file mode 100644 index 000000000000..724590abb3f4 --- /dev/null +++ b/test/testshared/runner_windows.go @@ -0,0 +1,48 @@ +//go:build windows + +package testshared + +import ( + "path/filepath" + "regexp" + "strings" + "testing" +) + +func SkipOnWindows(tb testing.TB) { + tb.Skip("not supported on Windows") +} + +func NormalizeFilePathInJSON(in string) string { + exp := regexp.MustCompile(`(?:^|\b)[\w-/.]+\.go`) + + return exp.ReplaceAllStringFunc(in, func(s string) string { + return strings.ReplaceAll(s, "/", "\\\\") + }) +} + +func defaultBinaryName() string { + return filepath.Join("..", "golangci-lint.exe") +} + +// NormalizeFileInString normalizes in quoted string, ie. `\\\\`. +func NormalizeFileInString(in string) string { + return strings.ReplaceAll(filepath.FromSlash(in), "\\", "\\\\") +} + +func normalizeFilePath(in string) string { + exp := regexp.MustCompile(`(?:^|\b)[\w-/.]+\.go`) + + return exp.ReplaceAllStringFunc(in, func(s string) string { + return strings.ReplaceAll(s, "/", "\\") + }) +} + +// normalizeFilePathInRegex normalizes path in regular expressions. +// FIXME(ldez) see utils.normalizeFilePathInRegex(...) and maybe move into RunnerResult.ExpectOutputRegexp(...) +// FIXME(ldez) use the same approach as normalizeFilePath and NormalizeFilePathInJSON. +func normalizeFilePathInRegex(path string) string { + // This replacing should be safe because "/" are disallowed in Windows + // https://docs.microsoft.com/windows/win32/fileio/naming-a-file + return strings.ReplaceAll(path, "/", regexp.QuoteMeta(string(filepath.Separator))) +} From 9efdab0ec37d683986978d773d8adccd6075597e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 20:51:50 +0200 Subject: [PATCH 13/14] chore: clean FIXME --- test/testshared/directives.go | 5 ++--- test/testshared/runner_windows.go | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/testshared/directives.go b/test/testshared/directives.go index 5e34f006c9e3..01c011bb436f 100644 --- a/test/testshared/directives.go +++ b/test/testshared/directives.go @@ -15,7 +15,7 @@ import ( "github.com/golangci/golangci-lint/pkg/exitcodes" ) -// RunContext FIXME rename? +// RunContext the information extracted from directives. type RunContext struct { Args []string ConfigPath string @@ -51,8 +51,7 @@ func ParseTestDirectives(tb testing.TB, sourcePath string) *RunContext { break } - // FIXME(ldez) remove IsPlusBuild? - if constraint.IsGoBuild(line) || constraint.IsPlusBuild(line) { + if constraint.IsGoBuild(line) { if !evaluateBuildTags(tb, line) { return nil } diff --git a/test/testshared/runner_windows.go b/test/testshared/runner_windows.go index 724590abb3f4..867810e7ac03 100644 --- a/test/testshared/runner_windows.go +++ b/test/testshared/runner_windows.go @@ -39,8 +39,6 @@ func normalizeFilePath(in string) string { } // normalizeFilePathInRegex normalizes path in regular expressions. -// FIXME(ldez) see utils.normalizeFilePathInRegex(...) and maybe move into RunnerResult.ExpectOutputRegexp(...) -// FIXME(ldez) use the same approach as normalizeFilePath and NormalizeFilePathInJSON. func normalizeFilePathInRegex(path string) string { // This replacing should be safe because "/" are disallowed in Windows // https://docs.microsoft.com/windows/win32/fileio/naming-a-file From 61c900a5f72924a49accd9f2023ad7b9ef62c9bc Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 24 Aug 2022 21:17:38 +0200 Subject: [PATCH 14/14] docs: add comments --- pkg/result/processors/path_unix.go | 1 + pkg/result/processors/path_windows.go | 10 ++++------ test/testshared/runner.go | 2 +- test/testshared/runner_unix.go | 9 +++++++-- test/testshared/runner_windows.go | 23 ++++++++++++++--------- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/pkg/result/processors/path_unix.go b/pkg/result/processors/path_unix.go index d61bbf67a812..b0c7c33826cb 100644 --- a/pkg/result/processors/path_unix.go +++ b/pkg/result/processors/path_unix.go @@ -2,6 +2,7 @@ package processors +// normalizePathInRegex it's a noop function on Unix. func normalizePathInRegex(path string) string { return path } diff --git a/pkg/result/processors/path_windows.go b/pkg/result/processors/path_windows.go index edd081c875e0..7f3e3622bb75 100644 --- a/pkg/result/processors/path_windows.go +++ b/pkg/result/processors/path_windows.go @@ -10,12 +10,10 @@ import ( var separatorToReplace = regexp.QuoteMeta(string(filepath.Separator)) +// normalizePathInRegex normalizes path in regular expressions. +// noop on Unix. +// This replacing should be safe because "/" are disallowed in Windows +// https://docs.microsoft.com/windows/win32/fileio/naming-a-file func normalizePathInRegex(path string) string { - if filepath.Separator == '/' { - return path - } - - // This replacing should be safe because "/" are disallowed in Windows - // https://docs.microsoft.com/ru-ru/windows/win32/fileio/naming-a-file return strings.ReplaceAll(path, "/", separatorToReplace) } diff --git a/test/testshared/runner.go b/test/testshared/runner.go index fa03bab34012..9ce8363f460c 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -297,7 +297,7 @@ func (r *RunnerResult) ExpectExitCode(possibleCodes ...int) *RunnerResult { func (r *RunnerResult) ExpectOutputRegexp(s string) *RunnerResult { r.tb.Helper() - assert.Regexp(r.tb, normalizeFilePathInRegex(s), r.output, "exit code is %d", r.exitCode) + assert.Regexp(r.tb, normalizePathInRegex(s), r.output, "exit code is %d", r.exitCode) return r } diff --git a/test/testshared/runner_unix.go b/test/testshared/runner_unix.go index f76b82d6ac0b..c36669b1c6b6 100644 --- a/test/testshared/runner_unix.go +++ b/test/testshared/runner_unix.go @@ -7,25 +7,30 @@ import ( "testing" ) +// SkipOnWindows it's a noop function on Unix. func SkipOnWindows(_ testing.TB) {} +// NormalizeFilePathInJSON it's a noop function on Unix. func NormalizeFilePathInJSON(in string) string { return in } -// NormalizeFileInString normalizes in quoted string. +// NormalizeFileInString it's a noop function on Unix. func NormalizeFileInString(in string) string { return in } +// defaultBinaryName returns the path to the default binary. func defaultBinaryName() string { return filepath.Join("..", "golangci-lint") } +// normalizeFilePath it's a noop function on Unix. func normalizeFilePath(in string) string { return in } -func normalizeFilePathInRegex(path string) string { +// normalizePathInRegex it's a noop function on Unix. +func normalizePathInRegex(path string) string { return path } diff --git a/test/testshared/runner_windows.go b/test/testshared/runner_windows.go index 867810e7ac03..69afec9c5bca 100644 --- a/test/testshared/runner_windows.go +++ b/test/testshared/runner_windows.go @@ -9,10 +9,12 @@ import ( "testing" ) +// SkipOnWindows skip test on Windows. func SkipOnWindows(tb testing.TB) { tb.Skip("not supported on Windows") } +// NormalizeFilePathInJSON find Go file path and replace `/` with `\\\\`. func NormalizeFilePathInJSON(in string) string { exp := regexp.MustCompile(`(?:^|\b)[\w-/.]+\.go`) @@ -21,15 +23,17 @@ func NormalizeFilePathInJSON(in string) string { }) } -func defaultBinaryName() string { - return filepath.Join("..", "golangci-lint.exe") -} - -// NormalizeFileInString normalizes in quoted string, ie. `\\\\`. +// NormalizeFileInString normalizes in quoted string, ie. replace `\\` with `\\\\`. func NormalizeFileInString(in string) string { return strings.ReplaceAll(filepath.FromSlash(in), "\\", "\\\\") } +// defaultBinaryName returns the path to the default binary. +func defaultBinaryName() string { + return filepath.Join("..", "golangci-lint.exe") +} + +// normalizeFilePath find Go file path and replace `/` with `\\`. func normalizeFilePath(in string) string { exp := regexp.MustCompile(`(?:^|\b)[\w-/.]+\.go`) @@ -38,9 +42,10 @@ func normalizeFilePath(in string) string { }) } -// normalizeFilePathInRegex normalizes path in regular expressions. -func normalizeFilePathInRegex(path string) string { - // This replacing should be safe because "/" are disallowed in Windows - // https://docs.microsoft.com/windows/win32/fileio/naming-a-file +// normalizePathInRegex normalizes path in regular expressions. +// Replace all `/` with `\\`. +// This replacing should be safe because "/" are disallowed in Windows +// https://docs.microsoft.com/windows/win32/fileio/naming-a-file +func normalizePathInRegex(path string) string { return strings.ReplaceAll(path, "/", regexp.QuoteMeta(string(filepath.Separator))) }