Skip to content

Commit

Permalink
fix: speed up "fast" linters (#4653)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Apr 18, 2024
1 parent a660182 commit 15c57c1
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 36 deletions.
2 changes: 1 addition & 1 deletion pkg/golinters/typecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
5 changes: 1 addition & 4 deletions pkg/lint/lintersdb/builder_linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down
34 changes: 17 additions & 17 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)

mlConfig := &linter.Config{
Linter: ml,
EnabledByDefault: false,
InPresets: presets,
AlternativeNames: nil,
OriginalURL: "",
}
sort.Strings(mlConfig.InPresets)
mlConfig.InPresets = slices.Compact(mlConfig.InPresets)

mlConfig = mlConfig.WithLoadForGoAnalysis()
linters[mlConfig.Linter.Name()] = mlConfig

linters[ml.Name()] = mlConfig
m.debugf("Combined %d go/analysis linters into one metalinter", len(goanalysisLinters))
}

Expand Down
101 changes: 90 additions & 11 deletions pkg/lint/lintersdb/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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 (typed)",
linters: map[string]*linter.Config{
"foo": {
Linter: fooTyped,
InPresets: []string{"A"},
},
"bar": {
Linter: barTyped,
InPresets: []string{"B"},
},
},
expected: func() map[string]*linter.Config {
mlConfig := &linter.Config{
Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barTyped, fooTyped}),
InPresets: []string{"A", "B"},
}

return map[string]*linter.Config{
"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",
desc: "combined, several linters (syntax)",
linters: map[string]*linter.Config{
"foo": {
Linter: foo,
Linter: fooSyntax,
InPresets: []string{"A"},
},
"bar": {
Linter: bar,
Linter: barSyntax,
InPresets: []string{"B"},
},
},
expected: func() map[string]*linter.Config {
mlConfig := &linter.Config{
Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{bar, foo}),
Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{barSyntax, fooSyntax}),
InPresets: []string{"A", "B"},
}

return map[string]*linter.Config{
"goanalysis_metalinter": mlConfig.WithLoadForGoAnalysis(),
"goanalysis_metalinter": mlConfig,
}
}(),
},
Expand Down
6 changes: 3 additions & 3 deletions test/enabled_linters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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())
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 15c57c1

Please sign in to comment.