feat: disable copyloopvar and intrange on Go < 1.22 (#4397)

This commit is contained in:
Ludovic Fernandez 2024-02-19 14:58:58 +01:00 committed by GitHub
parent c65868c105
commit 64492b5e59
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 148 additions and 32 deletions

View File

@ -116,7 +116,18 @@ func NewExecutor(buildInfo BuildInfo) *Executor {
e.log.Fatalf("Can't read config: %s", err) 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() e.cfg.Run.Go = config.DetectGoVersion()
} }

View File

@ -41,18 +41,18 @@ type Version struct {
Debug bool `mapstructure:"debug"` Debug bool `mapstructure:"debug"`
} }
func IsGreaterThanOrEqualGo122(v string) bool { func IsGoGreaterThanOrEqual(current, limit string) bool {
v1, err := hcversion.NewVersion(strings.TrimPrefix(v, "go")) v1, err := hcversion.NewVersion(strings.TrimPrefix(current, "go"))
if err != nil { if err != nil {
return false return false
} }
limit, err := hcversion.NewVersion("1.22") l, err := hcversion.NewVersion(limit)
if err != nil { if err != nil {
return false return false
} }
return v1.GreaterThanOrEqual(limit) return v1.GreaterThanOrEqual(l)
} }
func DetectGoVersion() string { func DetectGoVersion() string {

86
pkg/config/config_test.go Normal file
View File

@ -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))
})
}
}

View File

@ -173,7 +173,7 @@ func analyzersFromConfig(settings *config.GovetSettings) []*analysis.Analyzer {
func isAnalyzerEnabled(name string, cfg *config.GovetSettings, defaultAnalyzers []*analysis.Analyzer) bool { func isAnalyzerEnabled(name string, cfg *config.GovetSettings, defaultAnalyzers []*analysis.Analyzer) bool {
// TODO(ldez) remove loopclosure when go1.23 // 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 return false
} }

View File

@ -18,7 +18,7 @@ func NewParallelTest(settings *config.ParallelTestSettings) *goanalysis.Linter {
"ignoremissingsubtests": settings.IgnoreMissingSubtests, "ignoremissingsubtests": settings.IgnoreMissingSubtests,
} }
if config.IsGreaterThanOrEqualGo122(settings.Go) { if config.IsGoGreaterThanOrEqual(settings.Go, "1.22") {
d["ignoreloopVar"] = true d["ignoreloopVar"] = true
} }

View File

@ -1,7 +1,8 @@
package linter package linter
import ( import (
"golang.org/x/tools/go/analysis" "fmt"
"golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages"
"github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/config"
@ -133,23 +134,27 @@ func (lc *Config) Name() string {
return lc.Linter.Name() return lc.Linter.Name()
} }
func (lc *Config) WithNoopFallback(cfg *config.Config) *Config { func (lc *Config) WithNoopFallback(cfg *config.Config, cond func(cfg *config.Config) error) *Config {
if cfg != nil && config.IsGreaterThanOrEqualGo122(cfg.Run.Go) { if err := cond(cfg); err != nil {
lc.Linter = &Noop{ lc.Linter = NewNoop(lc.Linter, err.Error())
name: lc.Linter.Name(),
desc: lc.Linter.Desc(),
run: func(_ *analysis.Pass) (any, error) {
return nil, nil
},
}
lc.LoadMode = 0 lc.LoadMode = 0
return lc.WithLoadFiles() return lc.WithLoadFiles()
} }
return lc return lc
} }
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 fmt.Errorf("this linter is disabled because the Go version (%s) of your project is lower than Go 1.22", cfg.Run.Go)
}
}
func NewConfig(linter Linter) *Config { func NewConfig(linter Linter) *Config {
lc := &Config{ lc := &Config{
Linter: linter, Linter: linter,

View File

@ -3,8 +3,6 @@ package linter
import ( import (
"context" "context"
"golang.org/x/tools/go/analysis"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
@ -15,14 +13,23 @@ type Linter interface {
} }
type Noop struct { type Noop struct {
name string name string
desc string desc string
run func(pass *analysis.Pass) (any, error) reason string
}
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) { func (n Noop) Run(_ context.Context, lintCtx *Context) ([]result.Issue, error) {
lintCtx.Log.Warnf("%s is disabled because of generics."+ if n.reason != "" {
" You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.", n.name) lintCtx.Log.Warnf("%s: %s", n.name, n.reason)
}
return nil, nil return nil, nil
} }

View File

@ -303,7 +303,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
linter.NewConfig(golinters.NewCopyLoopVar()). linter.NewConfig(golinters.NewCopyLoopVar()).
WithSince("v1.57.0"). WithSince("v1.57.0").
WithPresets(linter.PresetStyle). WithPresets(linter.PresetStyle).
WithURL("https://github.com/karamaru-alpha/copyloopvar"), WithURL("https://github.com/karamaru-alpha/copyloopvar").
WithNoopFallback(m.cfg, linter.IsGoLowerThanGo122()),
linter.NewConfig(golinters.NewCyclop(cyclopCfg)). linter.NewConfig(golinters.NewCyclop(cyclopCfg)).
WithSince("v1.37.0"). WithSince("v1.37.0").
@ -617,7 +618,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
linter.NewConfig(golinters.NewIntrange()). linter.NewConfig(golinters.NewIntrange()).
WithSince("v1.57.0"). WithSince("v1.57.0").
WithURL("https://github.com/ckaznocha/intrange"), WithURL("https://github.com/ckaznocha/intrange").
WithNoopFallback(m.cfg, linter.IsGoLowerThanGo122()),
linter.NewConfig(golinters.NewIreturn(ireturnCfg)). linter.NewConfig(golinters.NewIreturn(ireturnCfg)).
WithSince("v1.43.0"). WithSince("v1.43.0").

View File

@ -84,7 +84,7 @@ func testOneSource(t *testing.T, log *logutils.StderrLog, binPath, sourcePath st
} }
args := []string{ 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", "--disable-all",
"--out-format=json", "--out-format=json",
"--max-same-issues=100", "--max-same-issues=100",
@ -99,7 +99,6 @@ func testOneSource(t *testing.T, log *logutils.StderrLog, binPath, sourcePath st
cmd := testshared.NewRunnerBuilder(t). cmd := testshared.NewRunnerBuilder(t).
WithBinPath(binPath). WithBinPath(binPath).
WithNoParallelRunners().
WithArgs(caseArgs...). WithArgs(caseArgs...).
WithRunContext(rc). WithRunContext(rc).
WithTargetPath(sourcePath). WithTargetPath(sourcePath).

View File

@ -133,12 +133,12 @@ func TestTestsAreLintedByDefault(t *testing.T) {
func TestCgoOk(t *testing.T) { func TestCgoOk(t *testing.T) {
testshared.NewRunnerBuilder(t). testshared.NewRunnerBuilder(t).
WithNoConfig(). WithNoConfig().
WithArgs( WithArgs("--timeout=3m",
"--timeout=3m",
"--enable-all", "--enable-all",
"-D", "-D",
"nosnakecase,gci", "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"). WithTargetPath(testdataDir, "cgo").
Runner(). Runner().
Install(). Install().
@ -356,6 +356,7 @@ func TestUnsafeOk(t *testing.T) {
testshared.NewRunnerBuilder(t). testshared.NewRunnerBuilder(t).
WithNoConfig(). WithNoConfig().
WithArgs("--enable-all"). 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"). WithTargetPath(testdataDir, "unsafe").
Runner(). Runner().
Install(). Install().
@ -514,6 +515,7 @@ func TestEnableAllFastAndEnableCanCoexist(t *testing.T) {
testshared.NewRunnerBuilder(t). testshared.NewRunnerBuilder(t).
WithNoConfig(). WithNoConfig().
WithArgs(test.args...). 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). WithTargetPath(testdataDir, minimalPkg).
Runner(). Runner().
Run(). Run().

View File

@ -1,3 +1,5 @@
//go:build go1.22
//golangcitest:args -Ecopyloopvar //golangcitest:args -Ecopyloopvar
package testdata package testdata

View File

@ -1,3 +1,5 @@
//go:build go1.22
//golangcitest:args -Eintrange //golangcitest:args -Eintrange
package testdata package testdata