diff --git a/pkg/lint/lintersdb/enabled_set.go b/pkg/lint/lintersdb/enabled_set.go index 6f7b91b4d139..0cdafb7a6cf1 100644 --- a/pkg/lint/lintersdb/enabled_set.go +++ b/pkg/lint/lintersdb/enabled_set.go @@ -148,11 +148,14 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) 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 } @@ -185,6 +188,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) ml := goanalysis.NewMetaLinter(goanalysisLinters) presets := maps.Keys(goanalysisPresets) + sort.Strings(presets) mlConfig := &linter.Config{ Linter: ml, @@ -197,6 +201,7 @@ func (es EnabledSet) combineGoAnalysisLinters(linters map[string]*linter.Config) mlConfig = mlConfig.WithLoadForGoAnalysis() linters[ml.Name()] = mlConfig + es.debugf("Combined %d go/analysis linters into one metalinter", len(goanalysisLinters)) } diff --git a/pkg/lint/lintersdb/enabled_set_test.go b/pkg/lint/lintersdb/enabled_set_test.go index 1ab75d4d15eb..d4fd75c410a4 100644 --- a/pkg/lint/lintersdb/enabled_set_test.go +++ b/pkg/lint/lintersdb/enabled_set_test.go @@ -4,12 +4,91 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/logutils" ) -func TestGetEnabledLintersSet(t *testing.T) { +type dummyLogger struct{} + +func (d dummyLogger) Fatalf(_ string, _ ...any) {} + +func (d dummyLogger) Panicf(_ string, _ ...any) {} + +func (d dummyLogger) Errorf(_ string, _ ...any) {} + +func (d dummyLogger) Warnf(_ string, _ ...any) {} + +func (d dummyLogger) Infof(_ string, _ ...any) {} + +func (d dummyLogger) Child(_ string) logutils.Log { + return nil +} + +func (d dummyLogger) SetLevel(_ logutils.LogLevel) {} + +func TestEnabledSet_GetEnabledLintersMap(t *testing.T) { + m := NewManager(nil, nil) + + cfg := config.NewDefault() + + cfg.Linters.DisableAll = true + cfg.Linters.Enable = []string{"gofmt"} + + es := NewEnabledSet(m, NewValidator(m), dummyLogger{}, cfg) + + lintersMap, err := es.GetEnabledLintersMap() + require.NoError(t, err) + + gofmtConfigs := m.GetLinterConfigs("gofmt") + typecheckConfigs := m.GetLinterConfigs("typecheck") + + expected := map[string]*linter.Config{ + "gofmt": gofmtConfigs[0], + "typecheck": typecheckConfigs[0], + } + + assert.Equal(t, expected, lintersMap) +} + +func TestEnabledSet_GetOptimizedLinters(t *testing.T) { + m := NewManager(nil, nil) + + cfg := config.NewDefault() + + cfg.Linters.DisableAll = true + cfg.Linters.Enable = []string{"gofmt"} + + es := NewEnabledSet(m, NewValidator(m), dummyLogger{}, cfg) + + optimizedLinters, err := es.GetOptimizedLinters() + require.NoError(t, err) + + gofmtConfigs := m.GetLinterConfigs("gofmt") + typecheckConfigs := m.GetLinterConfigs("typecheck") + + var gaLinters []*goanalysis.Linter + for _, l := range gofmtConfigs { + gaLinters = append(gaLinters, l.Linter.(*goanalysis.Linter)) + } + for _, l := range typecheckConfigs { + gaLinters = append(gaLinters, l.Linter.(*goanalysis.Linter)) + } + + mlConfig := &linter.Config{ + Linter: goanalysis.NewMetaLinter(gaLinters), + InPresets: []string{"bugs", "format"}, + } + + expected := []*linter.Config{mlConfig.WithLoadForGoAnalysis()} + + assert.Equal(t, expected, optimizedLinters) +} + +func TestEnabledSet_build(t *testing.T) { type cs struct { cfg config.Linters name string // test case name @@ -94,7 +173,7 @@ func TestGetEnabledLintersSet(t *testing.T) { } m := NewManager(nil, nil) - es := NewEnabledSet(m, NewValidator(m), nil, nil) + es := NewEnabledSet(m, NewValidator(m), dummyLogger{}, nil) for _, c := range cases { c := c @@ -117,3 +196,68 @@ func TestGetEnabledLintersSet(t *testing.T) { }) } } + +func TestEnabledSet_combineGoAnalysisLinters(t *testing.T) { + m := NewManager(nil, nil) + + es := NewEnabledSet(m, NewValidator(m), dummyLogger{}, config.NewDefault()) + + foo := goanalysis.NewLinter("foo", "example foo", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) + bar := goanalysis.NewLinter("bar", "example bar", nil, nil).WithLoadMode(goanalysis.LoadModeTypesInfo) + + testCases := []struct { + desc string + linters map[string]*linter.Config + expected map[string]*linter.Config + }{ + { + desc: "no combined, one linter", + linters: map[string]*linter.Config{ + "foo": { + Linter: foo, + InPresets: []string{"A"}, + }, + }, + expected: map[string]*linter.Config{ + "foo": { + Linter: foo, + InPresets: []string{"A"}, + }, + }, + }, + { + desc: "combined, several linters", + linters: map[string]*linter.Config{ + "foo": { + Linter: foo, + InPresets: []string{"A"}, + }, + "bar": { + Linter: bar, + InPresets: []string{"B"}, + }, + }, + expected: func() map[string]*linter.Config { + mlConfig := &linter.Config{ + Linter: goanalysis.NewMetaLinter([]*goanalysis.Linter{bar, foo}), + InPresets: []string{"A", "B"}, + } + + return map[string]*linter.Config{ + "goanalysis_metalinter": mlConfig.WithLoadForGoAnalysis(), + } + }(), + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + es.combineGoAnalysisLinters(test.linters) + + assert.Equal(t, test.expected, test.linters) + }) + } +} diff --git a/pkg/lint/lintersdb/validator_test.go b/pkg/lint/lintersdb/validator_test.go new file mode 100644 index 000000000000..944fadd77663 --- /dev/null +++ b/pkg/lint/lintersdb/validator_test.go @@ -0,0 +1,410 @@ +package lintersdb + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/config" +) + +type validateErrorTestCase struct { + desc string + cfg *config.Linters + expected string +} + +var validateLintersNamesErrorTestCases = []validateErrorTestCase{ + { + desc: "unknown enabled linter", + cfg: &config.Linters{ + Enable: []string{"golangci"}, + Disable: nil, + }, + expected: `unknown linters: 'golangci', run 'golangci-lint help linters' to see the list of supported linters`, + }, + { + desc: "unknown disabled linter", + cfg: &config.Linters{ + Enable: nil, + Disable: []string{"golangci"}, + }, + expected: `unknown linters: 'golangci', run 'golangci-lint help linters' to see the list of supported linters`, + }, +} + +var validatePresetsErrorTestCases = []validateErrorTestCase{ + { + desc: "unknown preset", + cfg: &config.Linters{ + EnableAll: false, + Presets: []string{"golangci"}, + }, + expected: "no such preset \"golangci\": only next presets exist: " + + "(bugs|comment|complexity|error|format|import|metalinter|module|performance|sql|style|test|unused)", + }, + { + desc: "presets and enable-all", + cfg: &config.Linters{ + EnableAll: true, + Presets: []string{"bugs"}, + }, + expected: `--presets is incompatible with --enable-all`, + }, +} + +var validateDisabledAndEnabledAtOneMomentErrorTestCases = []validateErrorTestCase{ + { + desc: "disable one linter of the enabled linters", + cfg: &config.Linters{ + Enable: []string{"dupl", "gofmt", "misspell"}, + Disable: []string{"dupl", "gosec", "nolintlint"}, + }, + expected: `linter "dupl" can't be disabled and enabled at one moment`, + }, + { + desc: "disable multiple enabled linters", + cfg: &config.Linters{ + Enable: []string{"dupl", "gofmt", "misspell"}, + Disable: []string{"dupl", "gofmt", "misspell"}, + }, + expected: `linter "dupl" can't be disabled and enabled at one moment`, + }, +} + +var validateAllDisableEnableOptionsErrorTestCases = []validateErrorTestCase{ + { + desc: "enable-all and disable-all", + cfg: &config.Linters{ + Enable: nil, + EnableAll: true, + Disable: nil, + DisableAll: true, + Fast: false, + }, + expected: "--enable-all and --disable-all options must not be combined", + }, + { + desc: "disable-all and disable no enable no preset", + cfg: &config.Linters{ + Enable: nil, + EnableAll: false, + Disable: []string{"dupl", "gofmt", "misspell"}, + DisableAll: true, + Fast: false, + }, + expected: "all linters were disabled, but no one linter was enabled: must enable at least one", + }, + { + desc: "disable-all and disable with enable", + cfg: &config.Linters{ + Enable: []string{"nolintlint"}, + EnableAll: false, + Disable: []string{"dupl", "gofmt", "misspell"}, + DisableAll: true, + Fast: false, + }, + expected: "can't combine options --disable-all and --disable dupl", + }, + { + desc: "enable-all and enable", + cfg: &config.Linters{ + Enable: []string{"dupl", "gofmt", "misspell"}, + EnableAll: true, + Disable: nil, + DisableAll: false, + Fast: false, + }, + expected: "can't combine options --enable-all and --enable dupl", + }, +} + +type validatorTestCase struct { + desc string + cfg *config.Linters +} + +var validateLintersNamesTestCases = []validatorTestCase{ + { + desc: "no enable no disable", + cfg: &config.Linters{ + Enable: nil, + Disable: nil, + }, + }, + { + desc: "existing enabled linter", + cfg: &config.Linters{ + Enable: []string{"gofmt"}, + Disable: nil, + }, + }, + { + desc: "existing disabled linter", + cfg: &config.Linters{ + Enable: nil, + Disable: []string{"gofmt"}, + }, + }, +} + +var validatePresetsTestCases = []validatorTestCase{ + { + desc: "known preset", + cfg: &config.Linters{ + EnableAll: false, + Presets: []string{"bugs"}, + }, + }, + { + desc: "enable-all and no presets", + cfg: &config.Linters{ + EnableAll: true, + Presets: nil, + }, + }, + { + desc: "no presets", + cfg: &config.Linters{ + EnableAll: false, + Presets: nil, + }, + }, +} + +var validateDisabledAndEnabledAtOneMomentTestCases = []validatorTestCase{ + { + desc: "2 different sets", + cfg: &config.Linters{ + Enable: []string{"dupl", "gofmt", "misspell"}, + Disable: []string{"goimports", "gosec", "nolintlint"}, + }, + }, + { + desc: "only enable", + cfg: &config.Linters{ + Enable: []string{"goimports", "gosec", "nolintlint"}, + Disable: nil, + }, + }, + { + desc: "only disable", + cfg: &config.Linters{ + Enable: nil, + Disable: []string{"dupl", "gofmt", "misspell"}, + }, + }, + { + desc: "no sets", + cfg: &config.Linters{ + Enable: nil, + Disable: nil, + }, + }, +} + +var validateAllDisableEnableOptionsTestCases = []validatorTestCase{ + { + desc: "nothing", + cfg: &config.Linters{}, + }, + { + desc: "enable and disable", + cfg: &config.Linters{ + Enable: []string{"goimports", "gosec", "nolintlint"}, + EnableAll: false, + Disable: []string{"dupl", "gofmt", "misspell"}, + DisableAll: false, + }, + }, + { + desc: "disable-all and enable", + cfg: &config.Linters{ + Enable: []string{"goimports", "gosec", "nolintlint"}, + EnableAll: false, + Disable: nil, + DisableAll: true, + }, + }, + { + desc: "enable-all and disable", + cfg: &config.Linters{ + Enable: nil, + EnableAll: true, + Disable: []string{"goimports", "gosec", "nolintlint"}, + DisableAll: false, + }, + }, + { + desc: "enable-all and enable and fast", + cfg: &config.Linters{ + Enable: []string{"dupl", "gofmt", "misspell"}, + EnableAll: true, + Disable: nil, + DisableAll: false, + Fast: true, + }, + }, +} + +func TestValidator_validateEnabledDisabledLintersConfig(t *testing.T) { + v := NewValidator(NewManager(nil, nil)) + + var testCases []validatorTestCase + testCases = append(testCases, validateLintersNamesTestCases...) + testCases = append(testCases, validatePresetsTestCases...) + testCases = append(testCases, validateDisabledAndEnabledAtOneMomentTestCases...) + testCases = append(testCases, validateAllDisableEnableOptionsTestCases...) + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateEnabledDisabledLintersConfig(test.cfg) + require.NoError(t, err) + }) + } +} + +func TestValidator_validateEnabledDisabledLintersConfig_error(t *testing.T) { + v := NewValidator(NewManager(nil, nil)) + + var testCases []validateErrorTestCase + testCases = append(testCases, validateLintersNamesErrorTestCases...) + testCases = append(testCases, validatePresetsErrorTestCases...) + testCases = append(testCases, validateDisabledAndEnabledAtOneMomentErrorTestCases...) + testCases = append(testCases, validateAllDisableEnableOptionsErrorTestCases...) + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateEnabledDisabledLintersConfig(test.cfg) + require.Error(t, err) + + require.EqualError(t, err, test.expected) + }) + } +} + +func TestValidator_validateLintersNames(t *testing.T) { + v := NewValidator(NewManager(nil, nil)) + + for _, test := range validateLintersNamesTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateLintersNames(test.cfg) + require.NoError(t, err) + }) + } +} + +func TestValidator_validateLintersNames_error(t *testing.T) { + v := NewValidator(NewManager(nil, nil)) + + for _, test := range validateLintersNamesErrorTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateLintersNames(test.cfg) + require.Error(t, err) + + require.EqualError(t, err, test.expected) + }) + } +} + +func TestValidator_validatePresets(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validatePresetsTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validatePresets(test.cfg) + require.NoError(t, err) + }) + } +} + +func TestValidator_validatePresets_error(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validatePresetsErrorTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validatePresets(test.cfg) + require.Error(t, err) + + require.EqualError(t, err, test.expected) + }) + } +} + +func TestValidator_validateDisabledAndEnabledAtOneMoment(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validateDisabledAndEnabledAtOneMomentTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateDisabledAndEnabledAtOneMoment(test.cfg) + require.NoError(t, err) + }) + } +} + +func TestValidator_validateDisabledAndEnabledAtOneMoment_error(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validateDisabledAndEnabledAtOneMomentErrorTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateDisabledAndEnabledAtOneMoment(test.cfg) + require.Error(t, err) + + require.EqualError(t, err, test.expected) + }) + } +} + +func TestValidator_validateAllDisableEnableOptions(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validateAllDisableEnableOptionsTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateAllDisableEnableOptions(test.cfg) + require.NoError(t, err) + }) + } +} + +func TestValidator_validateAllDisableEnableOptions_error(t *testing.T) { + v := NewValidator(nil) + + for _, test := range validateAllDisableEnableOptionsErrorTestCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := v.validateAllDisableEnableOptions(test.cfg) + require.Error(t, err) + + require.EqualError(t, err, test.expected) + }) + } +}