dev: rewrite linters Manager (#4419)

Author: ldez

docs/src/docs/contributing/architecture.mdx

@@ -22,11 +22,18 @@ graph LR
 
 </ResponsiveContainer>
 
+## Init
+
+The configuration is loaded from file and flags by `config.Loader` inside `PersistentPreRun` (or `PreRun`) of the commands that require configuration.
+
+The linter database (`linterdb.Manager`) is fill based on the configuration:
+- The linters ("internals" and plugins) are built by `linterdb.LinterBuilder` and `linterdb.PluginBuilder` builders.
+- The configuration is validated by `linterdb.Validator`.
+
 ## Load Packages
 
 Loading packages is listing all packages and their recursive dependencies for analysis.
-Also, depending on the enabled linters set some parsing of the source code can be performed
-at this step.
+Also, depending on the enabled linters set some parsing of the source code can be performed at this step.
 
 Packages loading starts here:
 
@@ -79,10 +86,10 @@ and outputs list of packages and requested information about them: filenames, ty
 
 First, we need to find all enabled linters. All linters are registered here:
 
-```go title=pkg/lint/lintersdb/manager.go
-func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
+```go title=pkg/lint/lintersdb/builder_linter.go
+func (b LinterBuilder) Build(cfg *config.Config) []*linter.Config {
 	// ...
-	linters = append(linters,
+	return []*linter.Config{
 		// ...
 		linter.NewConfig(golinters.NewBodyclose()).
 			WithSince("v1.18.0").
@@ -97,26 +104,25 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetBugs, linter.PresetMetaLinter).
 			WithAlternativeNames("vet", "vetshadow").
 			WithURL("https://pkg.go.dev/cmd/vet"),
+		// ...
 	}
-	// ...
 }
 ```
 
 We filter requested in config and command-line linters in `EnabledSet`:
 
-```go title=pkg/lint/lintersdb/enabled_set.go
-func (es EnabledSet) GetEnabledLintersMap() (map[string]*linter.Config, error)
+```go title=pkg/lint/lintersdb/manager.go
+func (m *Manager) GetEnabledLintersMap() (map[string]*linter.Config, error)
 ```
 
 We merge enabled linters into one `MetaLinter` to improve execution time if we can:
 
-```go title=pkg/lint/lintersdb/enabled_set.go
-// GetOptimizedLinters returns enabled linters after optimization (merging) of multiple linters
-// into a fewer number of linters. E.g. some go/analysis linters can be optimized into
-// one metalinter for data reuse and speed up.
-func (es EnabledSet) GetOptimizedLinters() ([]*linter.Config, error) {
+```go titlepkg/lint/lintersdb/manager.go
+// GetOptimizedLinters returns enabled linters after optimization (merging) of multiple linters into a fewer number of linters.
+// E.g. some go/analysis linters can be optimized into one metalinter for data reuse and speed up.
+func (m *Manager) GetOptimizedLinters() ([]*linter.Config, error) {
 	// ...
-	es.combineGoAnalysisLinters(resultLintersSet)
+	m.combineGoAnalysisLinters(resultLintersSet)
 	// ...
 }
 ```
@@ -133,9 +139,11 @@ type MetaLinter struct {
 Currently, all linters except `unused` can be merged into this meta linter.
 The `unused` isn't merged because it has high memory usage.
 
-Linters execution starts in `runAnalyzers`. It's the most complex part of the `golangci-lint`.
-We use custom [go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) runner there. It runs as much as it can in parallel. It lazy-loads as much as it can
-to reduce memory usage. Also, it sets all heavyweight data to `nil` as becomes unneeded to save memory.
+Linters execution starts in `runAnalyzers`.
+It's the most complex part of the `golangci-lint`.
+We use custom [go/analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) runner there.
+It runs as much as it can in parallel. It lazy-loads as much as it can to reduce memory usage.
+Also, it sets all heavyweight data to `nil` as becomes unneeded to save memory.
 
 We don't use existing [multichecker](https://pkg.go.dev/golang.org/x/tools/go/analysis/multichecker) because
 it doesn't use caching and doesn't have some important performance optimizations.

docs/src/docs/contributing/new-linters.mdx

@@ -28,8 +28,8 @@ After that:
    Look at other linters in this directory.
    Implement linter integration and check that test passes.
 3. Add the new struct for the linter (which you've implemented in `pkg/golinters/{yourlintername}.go`) to the
-   list of all supported linters in [`pkg/lint/lintersdb/manager.go`](https://github.com/golangci/golangci-lint/blob/master/pkg/lint/lintersdb/manager.go)
-   to the function `GetAllSupportedLinterConfigs`.
+   list of all supported linters in [`pkg/lint/lintersdb/builder_linter.go`](https://github.com/golangci/golangci-lint/blob/master/pkg/lint/lintersdb/builder_linter.go)
+   to the method `LinterBuilder.Build`.
     - Add `WithSince("next_version")`, where `next_version` must be replaced by the next minor version. (ex: v1.2.0 if the current version is v1.1.0)
 4. Find out what options do you need to configure for the linter.
    For example, `nakedret` has only 1 option: [`max-func-lines`](https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml).

pkg/commands/help.go

@@ -41,7 +41,7 @@ func newHelpCommand(logger logutils.Log) *helpCommand {
 			Args:              cobra.NoArgs,
 			ValidArgsFunction: cobra.NoFileCompletions,
 			Run:               c.execute,
-			PreRun:            c.preRun,
+			PreRunE:           c.preRunE,
 		},
 	)
 
@@ -50,10 +50,18 @@ func newHelpCommand(logger logutils.Log) *helpCommand {
 	return c
 }
 
-func (c *helpCommand) preRun(_ *cobra.Command, _ []string) {
+func (c *helpCommand) preRunE(_ *cobra.Command, _ []string) error {
 	// The command doesn't depend on the real configuration.
 	// It just needs the list of all plugins and all presets.
-	c.dbManager = lintersdb.NewManager(config.NewDefault(), c.log)
+	dbManager, err := lintersdb.NewManager(c.log.Child(logutils.DebugKeyLintersDB), config.NewDefault(),
+		lintersdb.NewPluginBuilder(c.log), lintersdb.NewLinterBuilder())
+	if err != nil {
+		return err
+	}
+
+	c.dbManager = dbManager
+
+	return nil
 }
 
 func (c *helpCommand) execute(_ *cobra.Command, _ []string) {

pkg/commands/linters.go

@@ -27,8 +27,7 @@ type lintersCommand struct {
 
 	log logutils.Log
 
-	dbManager         *lintersdb.Manager
-	enabledLintersSet *lintersdb.EnabledSet
+	dbManager *lintersdb.Manager
 }
 
 func newLintersCommand(logger logutils.Log, cfg *config.Config) *lintersCommand {
@@ -65,15 +64,19 @@ func (c *lintersCommand) preRunE(cmd *cobra.Command, _ []string) error {
 		return fmt.Errorf("can't load config: %w", err)
 	}
 
-	c.dbManager = lintersdb.NewManager(c.cfg, c.log)
-	c.enabledLintersSet = lintersdb.NewEnabledSet(c.dbManager,
-		lintersdb.NewValidator(c.dbManager), c.log.Child(logutils.DebugKeyLintersDB), c.cfg)
+	dbManager, err := lintersdb.NewManager(c.log.Child(logutils.DebugKeyLintersDB), c.cfg,
+		lintersdb.NewPluginBuilder(c.log), lintersdb.NewLinterBuilder())
+	if err != nil {
+		return err
+	}
+
+	c.dbManager = dbManager
 
 	return nil
 }
 
 func (c *lintersCommand) execute(_ *cobra.Command, _ []string) error {
-	enabledLintersMap, err := c.enabledLintersSet.GetEnabledLintersMap()
+	enabledLintersMap, err := c.dbManager.GetEnabledLintersMap()
 	if err != nil {
 		return fmt.Errorf("can't get enabled linters: %w", err)
 	}

pkg/commands/run.go

@@ -80,8 +80,7 @@ type runCommand struct {
 
 	buildInfo BuildInfo
 
-	dbManager         *lintersdb.Manager
-	enabledLintersSet *lintersdb.EnabledSet
+	dbManager *lintersdb.Manager
 
 	log        logutils.Log
 	debugf     logutils.DebugFunc
@@ -171,9 +170,13 @@ func (c *runCommand) persistentPostRunE(_ *cobra.Command, _ []string) error {
 }
 
 func (c *runCommand) preRunE(_ *cobra.Command, _ []string) error {
-	c.dbManager = lintersdb.NewManager(c.cfg, c.log)
-	c.enabledLintersSet = lintersdb.NewEnabledSet(c.dbManager,
-		lintersdb.NewValidator(c.dbManager), c.log.Child(logutils.DebugKeyLintersDB), c.cfg)
+	dbManager, err := lintersdb.NewManager(c.log.Child(logutils.DebugKeyLintersDB), c.cfg,
+		lintersdb.NewPluginBuilder(c.log), lintersdb.NewLinterBuilder())
+	if err != nil {
+		return err
+	}
+
+	c.dbManager = dbManager
 
 	c.goenv = goutil.NewEnv(c.log.Child(logutils.DebugKeyGoEnv))
 
@@ -340,12 +343,12 @@ func (c *runCommand) runAndPrint(ctx context.Context, args []string) error {
 func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) {
 	c.cfg.Run.Args = args
 
-	lintersToRun, err := c.enabledLintersSet.GetOptimizedLinters()
+	lintersToRun, err := c.dbManager.GetOptimizedLinters()
 	if err != nil {
 		return nil, err
 	}
 
-	enabledLintersMap, err := c.enabledLintersSet.GetEnabledLintersMap()
+	enabledLintersMap, err := c.dbManager.GetEnabledLintersMap()
 	if err != nil {
 		return nil, err
 	}
@@ -361,8 +364,8 @@ func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.I
 	}
 	lintCtx.Log = c.log.Child(logutils.DebugKeyLintersContext)
 
-	runner, err := lint.NewRunner(c.cfg, c.log.Child(logutils.DebugKeyRunner),
-		c.goenv, c.enabledLintersSet, c.lineCache, c.fileCache, c.dbManager, lintCtx.Packages)
+	runner, err := lint.NewRunner(c.log.Child(logutils.DebugKeyRunner),
+		c.cfg, c.goenv, c.lineCache, c.fileCache, c.dbManager, lintCtx.Packages)
 	if err != nil {
 		return nil, err
 	}

pkg/commands/version.go

@@ -25,8 +25,8 @@ type versionInfo struct {
 }
 
 type versionOptions struct {
-	Format string `mapstructure:"format"`
-	Debug  bool   `mapstructure:"debug"`
+	Format string
+	Debug  bool
 }
 
 type versionCommand struct {

pkg/config/config.go

@@ -1,9 +1,8 @@
 package config
 
 import (
-	"errors"
-	"fmt"
 	"os"
+	"regexp"
 	"strings"
 
 	hcversion "github.com/hashicorp/go-version"
@@ -33,18 +32,16 @@ func (c *Config) GetConfigDir() string {
 }
 
 func (c *Config) Validate() error {
-	for i, rule := range c.Issues.ExcludeRules {
-		if err := rule.Validate(); err != nil {
-			return fmt.Errorf("error in exclude rule #%d: %w", i, err)
-		}
+	validators := []func() error{
+		c.Issues.Validate,
+		c.Severity.Validate,
+		c.LintersSettings.Validate,
+		c.Linters.Validate,
 	}
 
-	if len(c.Severity.Rules) > 0 && c.Severity.Default == "" {
-		return errors.New("can't set severity rule option: no default severity defined")
-	}
-	for i, rule := range c.Severity.Rules {
-		if err := rule.Validate(); err != nil {
-			return fmt.Errorf("error in severity rule #%d: %w", i, err)
+	for _, v := range validators {
+		if err := v(); err != nil {
+			return err
 		}
 	}
 
@@ -90,3 +87,22 @@ func detectGoVersion() string {
 
 	return "1.17"
 }
+
+// Trims the Go version to keep only M.m.
+// Since Go 1.21 the version inside the go.mod can be a patched version (ex: 1.21.0).
+// The version can also include information which we want to remove (ex: 1.21alpha1)
+// https://go.dev/doc/toolchain#versions
+// This a problem with staticcheck and gocritic.
+func trimGoVersion(v string) string {
+	if v == "" {
+		return ""
+	}
+
+	exp := regexp.MustCompile(`(\d\.\d+)(?:\.\d+|[a-z]+\d)`)
+
+	if exp.MatchString(v) {
+		return exp.FindStringSubmatch(v)[1]
+	}
+
+	return v
+}

pkg/config/config_test.go

@@ -84,3 +84,52 @@ func TestIsGoGreaterThanOrEqual(t *testing.T) {
 		})
 	}
 }
+
+func Test_trimGoVersion(t *testing.T) {
+	testCases := []struct {
+		desc     string
+		version  string
+		expected string
+	}{
+		{
+			desc:     "patched version",
+			version:  "1.22.0",
+			expected: "1.22",
+		},
+		{
+			desc:     "minor version",
+			version:  "1.22",
+			expected: "1.22",
+		},
+		{
+			desc:     "RC version",
+			version:  "1.22rc1",
+			expected: "1.22",
+		},
+		{
+			desc:     "alpha version",
+			version:  "1.22alpha1",
+			expected: "1.22",
+		},
+		{
+			desc:     "beta version",
+			version:  "1.22beta1",
+			expected: "1.22",
+		},
+		{
+			desc:     "semver RC version",
+			version:  "1.22.0-rc1",
+			expected: "1.22",
+		},
+	}
+
+	for _, test := range testCases {
+		test := test
+		t.Run(test.desc, func(t *testing.T) {
+			t.Parallel()
+
+			version := trimGoVersion(test.version)
+			assert.Equal(t, test.expected, version)
+		})
+	}
+}

pkg/config/issues.go

@@ -122,6 +122,16 @@ type Issues struct {
 	NeedFix bool `mapstructure:"fix"`
 }
 
+func (i *Issues) Validate() error {
+	for i, rule := range i.ExcludeRules {
+		if err := rule.Validate(); err != nil {
+			return fmt.Errorf("error in exclude rule #%d: %w", i, err)
+		}
+	}
+
+	return nil
+}
+
 type ExcludeRule struct {
 	BaseRule `mapstructure:",squash"`
 }