From 5da270b12394f36f1bec969718e747f1de6a6644 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 19:09:10 +0100 Subject: [PATCH 01/10] feat: disable copyloopvar and intrange on Go 1.22 --- pkg/config/config.go | 8 ++++---- pkg/golinters/govet.go | 2 +- pkg/golinters/paralleltest.go | 2 +- pkg/lint/linter/config.go | 15 +++++++++++---- pkg/lint/linter/linter.go | 12 +++++++----- pkg/lint/lintersdb/manager.go | 6 ++++-- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index a5483a20e3c6..2eb82938dc69 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -41,18 +41,18 @@ type Version struct { Debug bool `mapstructure:"debug"` } -func IsGreaterThanOrEqualGo122(v string) bool { - v1, err := hcversion.NewVersion(strings.TrimPrefix(v, "go")) +func IsGoGreaterThanOrEqual(current, limit string) bool { + v1, err := hcversion.NewVersion(strings.TrimPrefix(current, "go")) if err != nil { return false } - limit, err := hcversion.NewVersion("1.22") + l, err := hcversion.NewVersion(limit) if err != nil { return false } - return v1.GreaterThanOrEqual(limit) + return v1.GreaterThanOrEqual(l) } func DetectGoVersion() string { diff --git a/pkg/golinters/govet.go b/pkg/golinters/govet.go index e2c7d2df50d9..066f7e682a3d 100644 --- a/pkg/golinters/govet.go +++ b/pkg/golinters/govet.go @@ -173,7 +173,7 @@ func analyzersFromConfig(settings *config.GovetSettings) []*analysis.Analyzer { func isAnalyzerEnabled(name string, cfg *config.GovetSettings, defaultAnalyzers []*analysis.Analyzer) bool { // TODO(ldez) remove loopclosure when go1.23 - if name == loopclosure.Analyzer.Name && config.IsGreaterThanOrEqualGo122(cfg.Go) { + if name == loopclosure.Analyzer.Name && config.IsGoGreaterThanOrEqual(cfg.Go, "1.22") { return false } diff --git a/pkg/golinters/paralleltest.go b/pkg/golinters/paralleltest.go index 5a99830b5636..8215619c730c 100644 --- a/pkg/golinters/paralleltest.go +++ b/pkg/golinters/paralleltest.go @@ -18,7 +18,7 @@ func NewParallelTest(settings *config.ParallelTestSettings) *goanalysis.Linter { "ignoremissingsubtests": settings.IgnoreMissingSubtests, } - if config.IsGreaterThanOrEqualGo122(settings.Go) { + if config.IsGoGreaterThanOrEqual(settings.Go, "1.22") { d["ignoreloopVar"] = true } diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index ed5e5508ceab..8f2ff483c734 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -133,11 +133,12 @@ func (lc *Config) Name() string { return lc.Linter.Name() } -func (lc *Config) WithNoopFallback(cfg *config.Config) *Config { - if cfg != nil && config.IsGreaterThanOrEqualGo122(cfg.Run.Go) { +func (lc *Config) WithNoopFallback(cfg *config.Config, cond func(cfg *config.Config) bool) *Config { + if cfg != nil && cond(cfg) { lc.Linter = &Noop{ - name: lc.Linter.Name(), - desc: lc.Linter.Desc(), + name: lc.Linter.Name(), + desc: lc.Linter.Desc(), + reason: "This linter is disabled because the Go version of your project is lower than Go 1.22.", run: func(_ *analysis.Pass) (any, error) { return nil, nil }, @@ -150,6 +151,12 @@ func (lc *Config) WithNoopFallback(cfg *config.Config) *Config { return lc } +func IsGoLowerThan(limit string) func(cfg *config.Config) bool { + return func(cfg *config.Config) bool { + return cfg != nil && !config.IsGoGreaterThanOrEqual(cfg.Run.Go, limit) + } +} + func NewConfig(linter Linter) *Config { lc := &Config{ Linter: linter, diff --git a/pkg/lint/linter/linter.go b/pkg/lint/linter/linter.go index a65d6b927852..bdb419514b9e 100644 --- a/pkg/lint/linter/linter.go +++ b/pkg/lint/linter/linter.go @@ -15,14 +15,16 @@ type Linter interface { } type Noop struct { - name string - desc string - run func(pass *analysis.Pass) (any, error) + name string + desc string + reason string + run func(pass *analysis.Pass) (any, error) } func (n Noop) Run(_ context.Context, lintCtx *Context) ([]result.Issue, error) { - lintCtx.Log.Warnf("%s is disabled because of generics."+ - " You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.", n.name) + if n.reason != "" { + lintCtx.Log.Warnf("%s: %s", n.name, n.reason) + } return nil, nil } diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 977ef0d1c61d..a67db1d5b390 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -303,7 +303,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewCopyLoopVar()). WithSince("v1.57.0"). WithPresets(linter.PresetStyle). - WithURL("https://github.com/karamaru-alpha/copyloopvar"), + WithURL("https://github.com/karamaru-alpha/copyloopvar"). + WithNoopFallback(m.cfg, linter.IsGoLowerThan("1.22")), linter.NewConfig(golinters.NewCyclop(cyclopCfg)). WithSince("v1.37.0"). @@ -617,7 +618,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewIntrange()). WithSince("v1.57.0"). - WithURL("https://github.com/ckaznocha/intrange"), + WithURL("https://github.com/ckaznocha/intrange"). + WithNoopFallback(m.cfg, linter.IsGoLowerThan("1.22")), linter.NewConfig(golinters.NewIreturn(ireturnCfg)). WithSince("v1.43.0"). From 3f89d32233d3e4af8f2b36e7c6a8d7c1b092a78a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 20:10:53 +0100 Subject: [PATCH 02/10] fix: tests --- test/testdata/copyloopvar.go | 2 ++ test/testdata/intrange.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/test/testdata/copyloopvar.go b/test/testdata/copyloopvar.go index a1fb44164d3f..7fdd2b04b579 100644 --- a/test/testdata/copyloopvar.go +++ b/test/testdata/copyloopvar.go @@ -1,3 +1,5 @@ +//go:build go1.22 + //golangcitest:args -Ecopyloopvar package testdata diff --git a/test/testdata/intrange.go b/test/testdata/intrange.go index 3d9b711d38bc..2220d9618fdf 100644 --- a/test/testdata/intrange.go +++ b/test/testdata/intrange.go @@ -1,3 +1,5 @@ +//go:build go1.22 + //golangcitest:args -Eintrange package testdata From e26a9edeabec55056c7f59461e5ca42dc9ed5b2f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 20:55:18 +0100 Subject: [PATCH 03/10] fix: improve some tests for Go 1.21 --- test/run_test.go | 49 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/test/run_test.go b/test/run_test.go index 2ec4b64280a0..b13499bdc022 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -2,8 +2,11 @@ package test import ( "path/filepath" + "runtime" + "strings" "testing" + hcversion "github.com/hashicorp/go-version" "github.com/stretchr/testify/require" _ "github.com/valyala/quicktemplate" @@ -131,14 +134,20 @@ func TestTestsAreLintedByDefault(t *testing.T) { } func TestCgoOk(t *testing.T) { + args := []string{"--timeout=3m", + "--enable-all", + "-D", + "nosnakecase", // try to analyze the generated Go. + } + + // TODO(ldez) remove when we will run go1.23 on the CI. + if isGoVersion("1.21") { + args = append(args, "-D", "intrange,copyloopvar") + } + testshared.NewRunnerBuilder(t). WithNoConfig(). - WithArgs( - "--timeout=3m", - "--enable-all", - "-D", - "nosnakecase,gci", - ). + WithArgs(args...). WithTargetPath(testdataDir, "cgo"). Runner(). Install(). @@ -353,9 +362,16 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { } func TestUnsafeOk(t *testing.T) { + args := []string{"--enable-all"} + + // TODO(ldez) remove when we will run go1.23 on the CI. + if isGoVersion("1.21") { + args = append(args, "-D", "intrange,copyloopvar") + } + testshared.NewRunnerBuilder(t). WithNoConfig(). - WithArgs("--enable-all"). + WithArgs(args...). WithTargetPath(testdataDir, "unsafe"). Runner(). Install(). @@ -511,6 +527,11 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() + // TODO(ldez) remove when we will run go1.23 on the CI. + if isGoVersion("1.21") { + test.args = append(test.args, "-D", "intrange,copyloopvar") + } + testshared.NewRunnerBuilder(t). WithNoConfig(). WithArgs(test.args...). @@ -681,3 +702,17 @@ func TestPathPrefix(t *testing.T) { }) } } + +func isGoVersion(tag string) bool { + vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) + if err != nil { + return false + } + + vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) + if err != nil { + return false + } + + return vRuntime.GreaterThanOrEqual(vTag) +} From 808dbf88a5f42c3caaa88bf19743e16ad7833270 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 22:21:18 +0100 Subject: [PATCH 04/10] feat: add ForceDisableUnsupportedLinters option --- test/run_test.go | 51 +++++-------------------- test/testshared/runner.go | 69 ++++++++++++++++++++++++++++++++++ test/testshared/runner_test.go | 47 +++++++++++++++++++++++ 3 files changed, 125 insertions(+), 42 deletions(-) diff --git a/test/run_test.go b/test/run_test.go index b13499bdc022..792ce0b17339 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -2,11 +2,8 @@ package test import ( "path/filepath" - "runtime" - "strings" "testing" - hcversion "github.com/hashicorp/go-version" "github.com/stretchr/testify/require" _ "github.com/valyala/quicktemplate" @@ -134,21 +131,15 @@ func TestTestsAreLintedByDefault(t *testing.T) { } func TestCgoOk(t *testing.T) { - args := []string{"--timeout=3m", - "--enable-all", - "-D", - "nosnakecase", // try to analyze the generated Go. - } - - // TODO(ldez) remove when we will run go1.23 on the CI. - if isGoVersion("1.21") { - args = append(args, "-D", "intrange,copyloopvar") - } - testshared.NewRunnerBuilder(t). WithNoConfig(). - WithArgs(args...). + WithArgs("--timeout=3m", + "--enable-all", + "-D", + "nosnakecase", + ). WithTargetPath(testdataDir, "cgo"). + ForceDisableUnsupportedLinters(). Runner(). Install(). Run(). @@ -362,17 +353,11 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { } func TestUnsafeOk(t *testing.T) { - args := []string{"--enable-all"} - - // TODO(ldez) remove when we will run go1.23 on the CI. - if isGoVersion("1.21") { - args = append(args, "-D", "intrange,copyloopvar") - } - testshared.NewRunnerBuilder(t). WithNoConfig(). - WithArgs(args...). + WithArgs("--enable-all"). WithTargetPath(testdataDir, "unsafe"). + ForceDisableUnsupportedLinters(). Runner(). Install(). Run(). @@ -527,15 +512,11 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - // TODO(ldez) remove when we will run go1.23 on the CI. - if isGoVersion("1.21") { - test.args = append(test.args, "-D", "intrange,copyloopvar") - } - testshared.NewRunnerBuilder(t). WithNoConfig(). WithArgs(test.args...). WithTargetPath(testdataDir, minimalPkg). + ForceDisableUnsupportedLinters(). Runner(). Run(). ExpectExitCode(test.expected...) @@ -702,17 +683,3 @@ func TestPathPrefix(t *testing.T) { }) } } - -func isGoVersion(tag string) bool { - vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) - if err != nil { - return false - } - - vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) - if err != nil { - return false - } - - return vRuntime.GreaterThanOrEqual(vTag) -} diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 0be6d7404930..ead05d24c8e0 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -4,12 +4,15 @@ import ( "os" "os/exec" "path/filepath" + "runtime" + "slices" "strings" "sync" "syscall" "testing" "time" + hcversion "github.com/hashicorp/go-version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -150,6 +153,14 @@ func (b *RunnerBuilder) WithArgs(args ...string) *RunnerBuilder { return b } +// ForceDisableUnsupportedLinters temporary method to disable some linters. +// TODO(ldez) remove when we will run go1.23 on the CI. +func (b *RunnerBuilder) ForceDisableUnsupportedLinters() *RunnerBuilder { + b.args = forceDisableUnsupportedLinters(b.args) + + return b +} + func (b *RunnerBuilder) WithTargetPath(targets ...string) *RunnerBuilder { b.target = filepath.Join(targets...) @@ -358,3 +369,61 @@ func InstallGolangciLint(tb testing.TB) string { return abs } + +// TODO(ldez) remove when we will run go1.23 on the CI. +func forceDisableUnsupportedLinters(args []string) []string { + if !isGoVersion("1.21") { + return args + } + + result := slices.Clone(args) + + if len(result) == 0 { + return append(result, "-D", "intrange,copyloopvar") + } + + if slices.ContainsFunc(args, func(arg string) bool { return strings.HasSuffix(arg, "-disable-all") }) { + return args + } + + var appended bool + + for i, arg := range args { + if !strings.HasSuffix(arg, "-D") && !strings.HasSuffix(arg, "-disable") { + continue + } + + if len(args) <= i+1 || strings.HasPrefix(args[i+1], "-") { + continue + } + + d := strings.Split(args[i+1], ",") + d = append(d, "intrange", "copyloopvar") + + result[i+1] = strings.Join(slices.Compact(d), ",") + + appended = true + break + } + + if !appended { + result = append(result, "-D", "intrange,copyloopvar") + } + + return result +} + +// TODO(ldez) remove when we will run go1.23 on the CI. +func isGoVersion(tag string) bool { + vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) + if err != nil { + return false + } + + vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) + if err != nil { + return false + } + + return vRuntime.GreaterThanOrEqual(vTag) +} diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index 5d2b3634387d..c1d7c7894396 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -217,3 +217,50 @@ func TestRunnerResult_ExpectOutputRegexp(t *testing.T) { r.ExpectOutputRegexp(`an.+`) r.ExpectOutputRegexp("an") } + +// TODO(ldez) remove when we will run go1.23 on the CI. +func Test_forceDisableUnsupportedLinters(t *testing.T) { + t.Skip("only for illustration purpose because works only with go1.21") + + testCases := []struct { + desc string + args []string + expected []string + }{ + { + desc: "no args", + expected: []string{"-D", "intrange,copyloopvar"}, + }, + { + desc: "simple", + args: []string{"-A", "B", "-E"}, + expected: []string{"-A", "B", "-E", "-D", "intrange,copyloopvar"}, + }, + { + desc: "with existing disable linters", + args: []string{"-D", "a,b"}, + expected: []string{"-D", "a,b,intrange,copyloopvar"}, + }, + { + desc: "complex", + args: []string{"-A", "B", "-D", "a,b", "C", "-E", "F"}, + expected: []string{"-A", "B", "-D", "a,b,intrange,copyloopvar", "C", "-E", "F"}, + }, + { + desc: "disable-all", + args: []string{"-disable-all", "-D", "a,b"}, + expected: []string{"-disable-all", "-D", "a,b"}, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + result := forceDisableUnsupportedLinters(test.args) + + assert.Equal(t, test.expected, result) + }) + } +} From c4abd9451bfef01196a12ff0d34f264edd9606f8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 22:47:39 +0100 Subject: [PATCH 05/10] feat: rewrite fallback condition --- pkg/lint/linter/config.go | 28 +++++++++++++--------------- pkg/lint/linter/linter.go | 11 ++++++++--- pkg/lint/lintersdb/manager.go | 4 ++-- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 8f2ff483c734..3c98d39de0c8 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -1,7 +1,8 @@ package linter import ( - "golang.org/x/tools/go/analysis" + "errors" + "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/pkg/config" @@ -133,27 +134,24 @@ func (lc *Config) Name() string { return lc.Linter.Name() } -func (lc *Config) WithNoopFallback(cfg *config.Config, cond func(cfg *config.Config) bool) *Config { - if cfg != nil && cond(cfg) { - lc.Linter = &Noop{ - name: lc.Linter.Name(), - desc: lc.Linter.Desc(), - reason: "This linter is disabled because the Go version of your project is lower than Go 1.22.", - run: func(_ *analysis.Pass) (any, error) { - return nil, nil - }, - } - +func (lc *Config) WithNoopFallback(cfg *config.Config, cond func(cfg *config.Config) error) *Config { + if err := cond(cfg); err != nil { + lc.Linter = NewNoop(lc.Linter, err.Error()) lc.LoadMode = 0 + return lc.WithLoadFiles() } return lc } -func IsGoLowerThan(limit string) func(cfg *config.Config) bool { - return func(cfg *config.Config) bool { - return cfg != nil && !config.IsGoGreaterThanOrEqual(cfg.Run.Go, limit) +func IsGoLowerThanGo122() func(cfg *config.Config) error { + return func(cfg *config.Config) error { + if cfg == nil || config.IsGoGreaterThanOrEqual(cfg.Run.Go, "1.22") { + return nil + } + + return errors.New("this linter is disabled because the Go version of your project is lower than Go 1.22") } } diff --git a/pkg/lint/linter/linter.go b/pkg/lint/linter/linter.go index bdb419514b9e..bb8dcc248afa 100644 --- a/pkg/lint/linter/linter.go +++ b/pkg/lint/linter/linter.go @@ -3,8 +3,6 @@ package linter import ( "context" - "golang.org/x/tools/go/analysis" - "github.com/golangci/golangci-lint/pkg/result" ) @@ -18,7 +16,14 @@ type Noop struct { name string desc string reason string - run func(pass *analysis.Pass) (any, error) +} + +func NewNoop(l Linter, reason string) *Noop { + return &Noop{ + name: l.Name(), + desc: l.Desc(), + reason: reason, + } } func (n Noop) Run(_ context.Context, lintCtx *Context) ([]result.Issue, error) { diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index a67db1d5b390..e6a171f1ff5d 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -304,7 +304,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithSince("v1.57.0"). WithPresets(linter.PresetStyle). WithURL("https://github.com/karamaru-alpha/copyloopvar"). - WithNoopFallback(m.cfg, linter.IsGoLowerThan("1.22")), + WithNoopFallback(m.cfg, linter.IsGoLowerThanGo122()), linter.NewConfig(golinters.NewCyclop(cyclopCfg)). WithSince("v1.37.0"). @@ -619,7 +619,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewIntrange()). WithSince("v1.57.0"). WithURL("https://github.com/ckaznocha/intrange"). - WithNoopFallback(m.cfg, linter.IsGoLowerThan("1.22")), + WithNoopFallback(m.cfg, linter.IsGoLowerThanGo122()), linter.NewConfig(golinters.NewIreturn(ireturnCfg)). WithSince("v1.43.0"). From 3f01e3495e15d6e7f72fbb14c5c115859eb2d6d8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 22:57:36 +0100 Subject: [PATCH 06/10] fix: tests runner --- test/testshared/runner.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/testshared/runner.go b/test/testshared/runner.go index ead05d24c8e0..8f911279ef23 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -372,7 +372,7 @@ func InstallGolangciLint(tb testing.TB) string { // TODO(ldez) remove when we will run go1.23 on the CI. func forceDisableUnsupportedLinters(args []string) []string { - if !isGoVersion("1.21") { + if !isGoLessThan("1.22") { return args } @@ -414,7 +414,7 @@ func forceDisableUnsupportedLinters(args []string) []string { } // TODO(ldez) remove when we will run go1.23 on the CI. -func isGoVersion(tag string) bool { +func isGoLessThan(tag string) bool { vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) if err != nil { return false @@ -425,5 +425,7 @@ func isGoVersion(tag string) bool { return false } - return vRuntime.GreaterThanOrEqual(vTag) + println(vRuntime.LessThanOrEqual(vTag)) + + return vRuntime.LessThan(vTag) } From 79abb4bbb030b0ddfc7f0930a79c82e02e158a43 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 23:12:46 +0100 Subject: [PATCH 07/10] fix: test runner --- test/testshared/runner.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 8f911279ef23..0a375e457fe1 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -372,12 +372,12 @@ func InstallGolangciLint(tb testing.TB) string { // TODO(ldez) remove when we will run go1.23 on the CI. func forceDisableUnsupportedLinters(args []string) []string { + result := slices.Clone(args) + if !isGoLessThan("1.22") { - return args + return append(result, "--go=1.22") } - result := slices.Clone(args) - if len(result) == 0 { return append(result, "-D", "intrange,copyloopvar") } From 92bd617f38cb4521a45903a2137db5b3be0b885c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 17 Feb 2024 23:17:53 +0100 Subject: [PATCH 08/10] Clear is better than clever. https://go-proverbs.github.io/ --- test/run_test.go | 6 +-- test/testshared/runner.go | 71 ---------------------------------- test/testshared/runner_test.go | 47 ---------------------- 3 files changed, 3 insertions(+), 121 deletions(-) diff --git a/test/run_test.go b/test/run_test.go index 792ce0b17339..9fad30e13dc2 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -138,8 +138,8 @@ func TestCgoOk(t *testing.T) { "-D", "nosnakecase", ). + WithArgs("--go=1.22"). // TODO(ldez) remove this line when we will run go1.23 on the CI. (related to intrange, copyloopvar) WithTargetPath(testdataDir, "cgo"). - ForceDisableUnsupportedLinters(). Runner(). Install(). Run(). @@ -356,8 +356,8 @@ func TestUnsafeOk(t *testing.T) { testshared.NewRunnerBuilder(t). WithNoConfig(). WithArgs("--enable-all"). + WithArgs("--go=1.22"). // TODO(ldez) remove this line when we will run go1.23 on the CI. (related to intrange, copyloopvar) WithTargetPath(testdataDir, "unsafe"). - ForceDisableUnsupportedLinters(). Runner(). Install(). Run(). @@ -515,8 +515,8 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) { testshared.NewRunnerBuilder(t). WithNoConfig(). WithArgs(test.args...). + WithArgs("--go=1.22"). // TODO(ldez) remove this line when we will run go1.23 on the CI. (related to intrange, copyloopvar) WithTargetPath(testdataDir, minimalPkg). - ForceDisableUnsupportedLinters(). Runner(). Run(). ExpectExitCode(test.expected...) diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 0a375e457fe1..0be6d7404930 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -4,15 +4,12 @@ import ( "os" "os/exec" "path/filepath" - "runtime" - "slices" "strings" "sync" "syscall" "testing" "time" - hcversion "github.com/hashicorp/go-version" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -153,14 +150,6 @@ func (b *RunnerBuilder) WithArgs(args ...string) *RunnerBuilder { return b } -// ForceDisableUnsupportedLinters temporary method to disable some linters. -// TODO(ldez) remove when we will run go1.23 on the CI. -func (b *RunnerBuilder) ForceDisableUnsupportedLinters() *RunnerBuilder { - b.args = forceDisableUnsupportedLinters(b.args) - - return b -} - func (b *RunnerBuilder) WithTargetPath(targets ...string) *RunnerBuilder { b.target = filepath.Join(targets...) @@ -369,63 +358,3 @@ func InstallGolangciLint(tb testing.TB) string { return abs } - -// TODO(ldez) remove when we will run go1.23 on the CI. -func forceDisableUnsupportedLinters(args []string) []string { - result := slices.Clone(args) - - if !isGoLessThan("1.22") { - return append(result, "--go=1.22") - } - - if len(result) == 0 { - return append(result, "-D", "intrange,copyloopvar") - } - - if slices.ContainsFunc(args, func(arg string) bool { return strings.HasSuffix(arg, "-disable-all") }) { - return args - } - - var appended bool - - for i, arg := range args { - if !strings.HasSuffix(arg, "-D") && !strings.HasSuffix(arg, "-disable") { - continue - } - - if len(args) <= i+1 || strings.HasPrefix(args[i+1], "-") { - continue - } - - d := strings.Split(args[i+1], ",") - d = append(d, "intrange", "copyloopvar") - - result[i+1] = strings.Join(slices.Compact(d), ",") - - appended = true - break - } - - if !appended { - result = append(result, "-D", "intrange,copyloopvar") - } - - return result -} - -// TODO(ldez) remove when we will run go1.23 on the CI. -func isGoLessThan(tag string) bool { - vRuntime, err := hcversion.NewVersion(strings.TrimPrefix(runtime.Version(), "go")) - if err != nil { - return false - } - - vTag, err := hcversion.NewVersion(strings.TrimPrefix(tag, "go")) - if err != nil { - return false - } - - println(vRuntime.LessThanOrEqual(vTag)) - - return vRuntime.LessThan(vTag) -} diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index c1d7c7894396..5d2b3634387d 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -217,50 +217,3 @@ func TestRunnerResult_ExpectOutputRegexp(t *testing.T) { r.ExpectOutputRegexp(`an.+`) r.ExpectOutputRegexp("an") } - -// TODO(ldez) remove when we will run go1.23 on the CI. -func Test_forceDisableUnsupportedLinters(t *testing.T) { - t.Skip("only for illustration purpose because works only with go1.21") - - testCases := []struct { - desc string - args []string - expected []string - }{ - { - desc: "no args", - expected: []string{"-D", "intrange,copyloopvar"}, - }, - { - desc: "simple", - args: []string{"-A", "B", "-E"}, - expected: []string{"-A", "B", "-E", "-D", "intrange,copyloopvar"}, - }, - { - desc: "with existing disable linters", - args: []string{"-D", "a,b"}, - expected: []string{"-D", "a,b,intrange,copyloopvar"}, - }, - { - desc: "complex", - args: []string{"-A", "B", "-D", "a,b", "C", "-E", "F"}, - expected: []string{"-A", "B", "-D", "a,b,intrange,copyloopvar", "C", "-E", "F"}, - }, - { - desc: "disable-all", - args: []string{"-disable-all", "-D", "a,b"}, - expected: []string{"-disable-all", "-D", "a,b"}, - }, - } - - for _, test := range testCases { - test := test - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - result := forceDisableUnsupportedLinters(test.args) - - assert.Equal(t, test.expected, result) - }) - } -} From eb10ee35cac8fdec6c6a65f4f6cce699f46eb854 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 18 Feb 2024 04:24:57 +0100 Subject: [PATCH 09/10] fix: yes it's a hack... --- pkg/commands/executor.go | 13 ++++++++++++- pkg/lint/linter/config.go | 4 ++-- test/linters_test.go | 3 +-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index d241f5656a15..a6e08a48b00d 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -116,7 +116,18 @@ func NewExecutor(buildInfo BuildInfo) *Executor { e.log.Fatalf("Can't read config: %s", err) } - if (commandLineCfg == nil || commandLineCfg.Run.Go == "") && e.cfg != nil && e.cfg.Run.Go == "" { + if commandLineCfg != nil && commandLineCfg.Run.Go != "" { + // This hack allow to have the right Run information at least for the Go version (because the default value of the "go" flag is empty). + // If you put a log for `m.cfg.Run.Go` inside `GetAllSupportedLinterConfigs`, + // you will observe that at end (without this hack) the value will have the right value but too late, + // the linters are already running with the previous uncompleted configuration. + // TODO(ldez) there is a major problem with the executor: + // the parsing of the configuration and the timing to load the configuration and linters are creating unmanageable situations. + // There is no simple solution because it's spaghetti code. + // I need to completely rewrite the command line system and the executor because it's extremely time consuming to debug, + // so it's unmaintainable. + e.cfg.Run.Go = commandLineCfg.Run.Go + } else if e.cfg.Run.Go == "" { e.cfg.Run.Go = config.DetectGoVersion() } diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 3c98d39de0c8..8e57d6bdf6d2 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -1,7 +1,7 @@ package linter import ( - "errors" + "fmt" "golang.org/x/tools/go/packages" @@ -151,7 +151,7 @@ func IsGoLowerThanGo122() func(cfg *config.Config) error { return nil } - return errors.New("this linter is disabled because the Go version of your project is lower than Go 1.22") + return fmt.Errorf("this linter is disabled because the Go version (%s) of your project is lower than Go 1.22", cfg.Run.Go) } } diff --git a/test/linters_test.go b/test/linters_test.go index 1019a1f09c42..ada3dbeb9e45 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -84,7 +84,7 @@ func testOneSource(t *testing.T, log *logutils.StderrLog, binPath, sourcePath st } args := []string{ - "--allow-parallel-runners", + "--go=1.22", // TODO(ldez) remove this line when we will run go1.23 on the CI. (related to intrange, copyloopvar) "--disable-all", "--out-format=json", "--max-same-issues=100", @@ -99,7 +99,6 @@ func testOneSource(t *testing.T, log *logutils.StderrLog, binPath, sourcePath st cmd := testshared.NewRunnerBuilder(t). WithBinPath(binPath). - WithNoParallelRunners(). WithArgs(caseArgs...). WithRunContext(rc). WithTargetPath(sourcePath). From 05caa4d4f0c6bd95f3ad0db9c4dd8231484911e6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 19 Feb 2024 14:24:21 +0100 Subject: [PATCH 10/10] review: unit tests --- pkg/config/config_test.go | 86 +++++++++++++++++++++++++++++++++++++++ pkg/lint/linter/linter.go | 4 +- 2 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 pkg/config/config_test.go diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 000000000000..ac101b95fb53 --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,86 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsGoGreaterThanOrEqual(t *testing.T) { + testCases := []struct { + desc string + current string + limit string + assert assert.BoolAssertionFunc + }{ + { + desc: "current (with minor.major) lower than limit", + current: "go1.21", + limit: "1.22", + assert: assert.False, + }, + { + desc: "current (with 0 patch) lower than limit", + current: "go1.21.0", + limit: "1.22", + assert: assert.False, + }, + { + desc: "current (current with multiple patches) lower than limit", + current: "go1.21.6", + limit: "1.22", + assert: assert.False, + }, + { + desc: "current lower than limit (with minor.major)", + current: "go1.22", + limit: "1.22", + assert: assert.True, + }, + { + desc: "current lower than limit (with 0 patch)", + current: "go1.22.0", + limit: "1.22", + assert: assert.True, + }, + { + desc: "current lower than limit (current with multiple patches)", + current: "go1.22.6", + limit: "1.22", + assert: assert.True, + }, + { + desc: "current greater than limit", + current: "go1.23.0", + limit: "1.22", + assert: assert.True, + }, + { + desc: "current with no prefix", + current: "1.22", + limit: "1.22", + assert: assert.True, + }, + { + desc: "invalid current value", + current: "go", + limit: "1.22", + assert: assert.False, + }, + { + desc: "invalid limit value", + current: "go1.22", + limit: "go", + assert: assert.False, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + test.assert(t, IsGoGreaterThanOrEqual(test.current, test.limit)) + }) + } +} diff --git a/pkg/lint/linter/linter.go b/pkg/lint/linter/linter.go index bb8dcc248afa..e086bbe5fcc7 100644 --- a/pkg/lint/linter/linter.go +++ b/pkg/lint/linter/linter.go @@ -18,8 +18,8 @@ type Noop struct { reason string } -func NewNoop(l Linter, reason string) *Noop { - return &Noop{ +func NewNoop(l Linter, reason string) Noop { + return Noop{ name: l.Name(), desc: l.Desc(), reason: reason,