From 15c57c176e0b0f3bdadb41febd6540c186c116c0 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Thu, 18 Apr 2024 21:45:50 +0200 Subject: [PATCH] fix: speed up "fast" linters (#4653) --- pkg/golinters/typecheck.go | 2 +- pkg/lint/lintersdb/builder_linter.go | 5 +- pkg/lint/lintersdb/manager.go | 34 ++++----- pkg/lint/lintersdb/manager_test.go | 101 ++++++++++++++++++++++++--- test/enabled_linters_test.go | 6 +- 5 files changed, 112 insertions(+), 36 deletions(-) diff --git a/pkg/golinters/typecheck.go b/pkg/golinters/typecheck.go index ed403648..d0eaa00d 100644 --- a/pkg/golinters/typecheck.go +++ b/pkg/golinters/typecheck.go @@ -20,5 +20,5 @@ func NewTypecheck() *goanalysis.Linter { "Like the front-end of a Go compiler, parses and type-checks Go code", []*analysis.Analyzer{analyzer}, nil, - ).WithLoadMode(goanalysis.LoadModeTypesInfo) + ).WithLoadMode(goanalysis.LoadModeNone) } diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index c0670130..349057f2 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -752,10 +752,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { linter.NewConfig(golinters.NewTypecheck()). WithInternal(). WithEnabledByDefault(). - WithSince("v1.3.0"). - WithLoadForGoAnalysis(). - WithPresets(linter.PresetBugs). - WithURL(""), + WithSince("v1.3.0"), linter.NewConfig(unconvert.New(&cfg.LintersSettings.Unconvert)). WithSince("v1.0.0"). diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 0cd6cec2..19995005 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -204,21 +204,30 @@ func (m *Manager) build(enabledByDefaultLinters []*linter.Config) map[string]*li } func (m *Manager) combineGoAnalysisLinters(linters map[string]*linter.Config) { + mlConfig := &linter.Config{} + var goanalysisLinters []*goanalysis.Linter - goanalysisPresets := map[string]bool{} + for _, lc := range linters { lnt, ok := lc.Linter.(*goanalysis.Linter) if !ok { continue } + if lnt.LoadMode() == goanalysis.LoadModeWholeProgram { // It's ineffective by CPU and memory to run whole-program and incremental analyzers at once. continue } - goanalysisLinters = append(goanalysisLinters, lnt) - for _, p := range lc.InPresets { - goanalysisPresets[p] = true + + mlConfig.LoadMode |= lc.LoadMode + + if lc.IsSlowLinter() { + mlConfig.ConsiderSlow() } + + mlConfig.InPresets = append(mlConfig.InPresets, lc.InPresets...) + + goanalysisLinters = append(goanalysisLinters, lnt) } if len(goanalysisLinters) <= 1 { @@ -245,22 +254,13 @@ func (m *Manager) combineGoAnalysisLinters(linters map[string]*linter.Config) { return a.Name() <= b.Name() }) - ml := goanalysis.NewMetaLinter(goanalysisLinters) + mlConfig.Linter = goanalysis.NewMetaLinter(goanalysisLinters) - presets := maps.Keys(goanalysisPresets) - sort.Strings(presets) + sort.Strings(mlConfig.InPresets) + mlConfig.InPresets = slices.Compact(mlConfig.InPresets) - mlConfig := &linter.Config{ - Linter: ml, - EnabledByDefault: false, - InPresets: presets, - AlternativeNames: nil, - OriginalURL: "", - } + linters[mlConfig.Linter.Name()] = mlConfig - mlConfig = mlConfig.WithLoadForGoAnalysis() - - linters[ml.Name()] = mlConfig m.debugf("Combined %d go/analysis linters into one metalinter", len(goanalysisLinters)) } diff --git a/pkg/lint/lintersdb/manager_test.go b/pkg/lint/lintersdb/manager_test.go index 856f4155..227a0ca6 100644 --- a/pkg/lint/lintersdb/manager_test.go +++ b/pkg/lint/lintersdb/manager_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/goanalysis" @@ -55,10 +56,10 @@ func TestManager_GetOptimizedLinters(t *testing.T) { mlConfig := &linter.Config{ Linter: goanalysis.NewMetaLinter(gaLinters), - InPresets: []string{"bugs", "format"}, + InPresets: []string{"format"}, } - expected := []*linter.Config{mlConfig.WithLoadForGoAnalysis()} + expected := []*linter.Config{mlConfig.WithLoadFiles()} assert.Equal(t, expected, optimizedLinters) } @@ -176,8 +177,11 @@ func TestManager_combineGoAnalysisLinters(t *testing.T) { m, err := NewManager(nil, nil) require.NoError(t, err) - foo := goanalysis.NewLinter("foo", "example foo", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) - bar := goanalysis.NewLinter("bar", "example bar", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) + fooTyped := goanalysis.NewLinter("foo", "example foo", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) + barTyped := goanalysis.NewLinter("bar", "example bar", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) + + fooSyntax := goanalysis.NewLinter("foo", "example foo", nil, nil).WithLoadMode(goanalysis.LoadModeSyntax) + barSyntax := goanalysis.NewLinter("bar", "example bar", nil, nil).WithLoadMode(goanalysis.LoadModeSyntax) testCases := []struct { desc string @@ -188,37 +192,112 @@ func TestManager_combineGoAnalysisLinters(t *testing.T) { desc: "no combined, one linter", linters: map[string]*linter.Config{ "foo": { - Linter: foo, + Linter: fooTyped, InPresets: []string{"A"}, }, }, expected: map[string]*linter.Config{ "foo": { - Linter: foo, + Linter: fooTyped, InPresets: []string{"A"}, }, }, }, { - desc: "combined, several linters", + desc: "combined, several linters (typed)", linters: map[string]*linter.Config{ "foo": { - Linter: foo, + Linter: fooTyped, InPresets: []string{"A"}, }, "bar": { - Linter: bar, + Linter: barTyped, InPresets: []string{"B"}, }, }, expected: func() map[string]*linter.Config { mlConfig := &linter.Config{ - Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{bar, foo}), + Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barTyped, fooTyped}), InPresets: []string{"A", "B"}, } return map[string]*linter.Config{ - "goanalysis_metalinter": mlConfig.WithLoadForGoAnalysis(), + "goanalysis_metalinter": mlConfig, + } + }(), + }, + { + desc: "combined, several linters (different LoadMode)", + linters: map[string]*linter.Config{ + "foo": { + Linter: fooTyped, + InPresets: []string{"A"}, + LoadMode: packages.NeedName, + }, + "bar": { + Linter: barTyped, + InPresets: []string{"B"}, + LoadMode: packages.NeedTypesSizes, + }, + }, + expected: func() map[string]*linter.Config { + mlConfig := &linter.Config{ + Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barTyped, fooTyped}), + InPresets: []string{"A", "B"}, + LoadMode: packages.NeedName | packages.NeedTypesSizes, + } + + return map[string]*linter.Config{ + "goanalysis_metalinter": mlConfig, + } + }(), + }, + { + desc: "combined, several linters (same LoadMode)", + linters: map[string]*linter.Config{ + "foo": { + Linter: fooTyped, + InPresets: []string{"A"}, + LoadMode: packages.NeedName, + }, + "bar": { + Linter: barTyped, + InPresets: []string{"B"}, + LoadMode: packages.NeedName, + }, + }, + expected: func() map[string]*linter.Config { + mlConfig := &linter.Config{ + Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barTyped, fooTyped}), + InPresets: []string{"A", "B"}, + LoadMode: packages.NeedName, + } + + return map[string]*linter.Config{ + "goanalysis_metalinter": mlConfig, + } + }(), + }, + { + desc: "combined, several linters (syntax)", + linters: map[string]*linter.Config{ + "foo": { + Linter: fooSyntax, + InPresets: []string{"A"}, + }, + "bar": { + Linter: barSyntax, + InPresets: []string{"B"}, + }, + }, + expected: func() map[string]*linter.Config { + mlConfig := &linter.Config{ + Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barSyntax, fooSyntax}), + InPresets: []string{"A", "B"}, + } + + return map[string]*linter.Config{ + "goanalysis_metalinter": mlConfig, } }(), }, diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index 1c61b996..6044e9c6 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -148,7 +148,7 @@ func getEnabledByDefaultFastLintersExcept(t *testing.T, except ...string) []stri ebdl := m.GetAllEnabledByDefaultLinters() var ret []string for _, lc := range ebdl { - if lc.IsSlowLinter() { + if lc.IsSlowLinter() || lc.Internal { continue } @@ -169,7 +169,7 @@ func getAllFastLintersWith(t *testing.T, with ...string) []string { linters := dbManager.GetAllSupportedLinterConfigs() ret := append([]string{}, with...) for _, lc := range linters { - if lc.IsSlowLinter() { + if lc.IsSlowLinter() || lc.Internal { continue } ret = append(ret, lc.Name()) @@ -206,7 +206,7 @@ func getEnabledByDefaultFastLintersWith(t *testing.T, with ...string) []string { ebdl := dbManager.GetAllEnabledByDefaultLinters() ret := append([]string{}, with...) for _, lc := range ebdl { - if lc.IsSlowLinter() { + if lc.IsSlowLinter() || lc.Internal { continue }