From 7f3900b6720083f2e15bedd7b1c75876b750d59f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 22 Feb 2024 23:51:33 +0100 Subject: [PATCH 01/42] chore: clean config comments --- pkg/config/run.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/config/run.go b/pkg/config/run.go index 52c6ce262b57..b9e4f74e0d7a 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -35,10 +35,10 @@ type Run struct { MemProfilePath string // Flag only. TracePath string // Flag only. - PrintResourcesUsage bool `mapstructure:"print-resources-usage"` // Flag only. // TODO(ldez) need to be enforced. + PrintResourcesUsage bool `mapstructure:"print-resources-usage"` // Flag only. Config string // Flag only. The path to the golangci config file, as specified with the --config argument. NoConfig bool // Flag only. - Args []string // Flag only. // TODO(ldez) identify the real need and usage. + Args []string // Internal needs. // TODO(ldez) it's only use by flags and for the tests, need to removed in another PR. } From ec25f9ede428bc2923ef512ca591b889cf2430b6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 22 Feb 2024 23:53:34 +0100 Subject: [PATCH 02/42] feat: add cache command --- pkg/cmder/cache.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 pkg/cmder/cache.go diff --git a/pkg/cmder/cache.go b/pkg/cmder/cache.go new file mode 100644 index 000000000000..df7da718b670 --- /dev/null +++ b/pkg/cmder/cache.go @@ -0,0 +1,82 @@ +package cmder + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/spf13/cobra" + + "github.com/golangci/golangci-lint/internal/cache" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +type cacheCommand struct { + cmd *cobra.Command +} + +func newCacheCommand() *cacheCommand { + c := &cacheCommand{} + + cacheCmd := &cobra.Command{ + Use: "cache", + Short: "Cache control and information", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + } + + cacheCmd.AddCommand( + &cobra.Command{ + Use: "clean", + Short: "Clean cache", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: c.executeClean, + }, + &cobra.Command{ + Use: "status", + Short: "Show cache status", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + Run: c.executeStatus, + }, + ) + + c.cmd = cacheCmd + + return c +} + +func (c *cacheCommand) executeClean(_ *cobra.Command, _ []string) error { + cacheDir := cache.DefaultDir() + + if err := os.RemoveAll(cacheDir); err != nil { + return fmt.Errorf("failed to remove dir %s: %w", cacheDir, err) + } + + return nil +} + +func (c *cacheCommand) executeStatus(_ *cobra.Command, _ []string) { + cacheDir := cache.DefaultDir() + _, _ = fmt.Fprintf(logutils.StdOut, "Dir: %s\n", cacheDir) + + cacheSizeBytes, err := dirSizeBytes(cacheDir) + if err == nil { + _, _ = fmt.Fprintf(logutils.StdOut, "Size: %s\n", fsutils.PrettifyBytesCount(cacheSizeBytes)) + } +} + +func dirSizeBytes(path string) (int64, error) { + var size int64 + err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { + if err == nil && !info.IsDir() { + size += info.Size() + } + return err + }) + return size, err +} From 8cf5225b353b6692b00e972a9b6b88d4f101e96d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 22 Feb 2024 23:54:36 +0100 Subject: [PATCH 03/42] feat: add version command --- pkg/cmder/version.go | 97 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 pkg/cmder/version.go diff --git a/pkg/cmder/version.go b/pkg/cmder/version.go new file mode 100644 index 000000000000..18029cc614af --- /dev/null +++ b/pkg/cmder/version.go @@ -0,0 +1,97 @@ +package cmder + +import ( + "encoding/json" + "fmt" + "io" + "os" + "runtime/debug" + "strings" + + "github.com/spf13/cobra" +) + +type BuildInfo struct { + GoVersion string `json:"goVersion"` + Version string `json:"version"` + Commit string `json:"commit"` + Date string `json:"date"` +} + +type versionInfo struct { + Info BuildInfo + BuildInfo *debug.BuildInfo +} + +type versionOptions struct { + Format string `mapstructure:"format"` + Debug bool `mapstructure:"debug"` +} + +type versionCommand struct { + cmd *cobra.Command + opts versionOptions + + info BuildInfo +} + +func newVersionCommand(info BuildInfo) *versionCommand { + c := &versionCommand{info: info} + + versionCmd := &cobra.Command{ + Use: "version", + Short: "Version", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: c.execute, + } + + fs := versionCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + + fs.StringVar(&c.opts.Format, "format", "", wh("The version's format can be: 'short', 'json'")) + fs.BoolVar(&c.opts.Debug, "debug", false, wh("Add build information")) + + c.cmd = versionCmd + + return c +} + +func (c *versionCommand) execute(_ *cobra.Command, _ []string) error { + if c.opts.Debug { + info, ok := debug.ReadBuildInfo() + if !ok { + return nil + } + + switch strings.ToLower(c.opts.Format) { + case "json": + return json.NewEncoder(os.Stdout).Encode(versionInfo{ + Info: c.info, + BuildInfo: info, + }) + + default: + fmt.Println(info.String()) + return printVersion(os.Stdout, c.info) + } + } + + switch strings.ToLower(c.opts.Format) { + case "short": + fmt.Println(c.info.Version) + return nil + + case "json": + return json.NewEncoder(os.Stdout).Encode(c.info) + + default: + return printVersion(os.Stdout, c.info) + } +} + +func printVersion(w io.Writer, info BuildInfo) error { + _, err := fmt.Fprintf(w, "golangci-lint has version %s built with %s from %s on %s\n", + info.Version, info.GoVersion, info.Commit, info.Date) + return err +} From 885878d75a7a22e4665291758ef52a1726cc41e0 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 22 Feb 2024 23:58:53 +0100 Subject: [PATCH 04/42] feat: add help command --- pkg/cmder/help.go | 127 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 pkg/cmder/help.go diff --git a/pkg/cmder/help.go b/pkg/cmder/help.go new file mode 100644 index 000000000000..17b4bacaaf75 --- /dev/null +++ b/pkg/cmder/help.go @@ -0,0 +1,127 @@ +package cmder + +import ( + "fmt" + "sort" + "strings" + + "github.com/fatih/color" + "github.com/spf13/cobra" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +type helpCommand struct { + cmd *cobra.Command + + cfg *config.Config + + dbManager *lintersdb.Manager + + log logutils.Log +} + +func newHelpCommand(logger logutils.Log, cfg *config.Config) *helpCommand { + c := &helpCommand{log: logger, cfg: cfg} + + helpCmd := &cobra.Command{ + Use: "help", + Short: "Help", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + } + + helpCmd.AddCommand( + &cobra.Command{ + Use: "linters", + Short: "Help about linters", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + Run: c.execute, + PreRun: c.preRun, + }, + ) + + c.cmd = helpCmd + + return c +} + +func (c *helpCommand) preRun(_ *cobra.Command, _ []string) { + c.dbManager = lintersdb.NewManager(c.cfg, c.log) +} + +func (c *helpCommand) execute(_ *cobra.Command, _ []string) { + var enabledLCs, disabledLCs []*linter.Config + for _, lc := range c.dbManager.GetAllSupportedLinterConfigs() { + if lc.Internal { + continue + } + + if lc.EnabledByDefault { + enabledLCs = append(enabledLCs, lc) + } else { + disabledLCs = append(disabledLCs, lc) + } + } + + color.Green("Enabled by default linters:\n") + printLinters(enabledLCs) + + color.Red("\nDisabled by default linters:\n") + printLinters(disabledLCs) + + color.Green("\nLinters presets:") + c.printPresets() +} + +func (c *helpCommand) printPresets() { + for _, p := range lintersdb.AllPresets() { + linters := c.dbManager.GetAllLinterConfigsForPreset(p) + + var linterNames []string + for _, lc := range linters { + if lc.Internal { + continue + } + + linterNames = append(linterNames, lc.Name()) + } + sort.Strings(linterNames) + + _, _ = fmt.Fprintf(logutils.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", ")) + } +} + +func printLinters(lcs []*linter.Config) { + sort.Slice(lcs, func(i, j int) bool { + return lcs[i].Name() < lcs[j].Name() + }) + + for _, lc := range lcs { + altNamesStr := "" + if len(lc.AlternativeNames) != 0 { + altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) + } + + // If the linter description spans multiple lines, truncate everything following the first newline + linterDescription := lc.Linter.Desc() + firstNewline := strings.IndexRune(linterDescription, '\n') + if firstNewline > 0 { + linterDescription = linterDescription[:firstNewline] + } + + deprecatedMark := "" + if lc.IsDeprecated() { + deprecatedMark = " [" + color.RedString("deprecated") + "]" + } + + _, _ = fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", + color.YellowString(lc.Name()), altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) + } +} From d2f0a8fc7d7dd814e7d53bc87d718519e7b1cac7 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:08:22 +0100 Subject: [PATCH 05/42] feat: add new configuration loader --- pkg/config/loader.go | 267 +++++++++++++++++++++++++++++++++++++++++++ pkg/config/reader.go | 3 + 2 files changed, 270 insertions(+) create mode 100644 pkg/config/loader.go diff --git a/pkg/config/loader.go b/pkg/config/loader.go new file mode 100644 index 000000000000..595a97ebb584 --- /dev/null +++ b/pkg/config/loader.go @@ -0,0 +1,267 @@ +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/go-viper/mapstructure/v2" + "github.com/mitchellh/go-homedir" + "github.com/spf13/viper" + + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +type LoaderOptions struct { + Config string // Flag only. The path to the golangci config file, as specified with the --config argument. + NoConfig bool // Flag only. +} + +type Loader struct { + opts LoaderOptions + + viper *viper.Viper + + log logutils.Log + + cfg *Config +} + +func NewLoader(log logutils.Log, v *viper.Viper, opts LoaderOptions, cfg *Config) *Loader { + return &Loader{ + opts: opts, + viper: v, + log: log, + cfg: cfg, + } +} + +func (r *Loader) Load() error { + err := r.setConfigFile() + if err != nil { + return err + } + + return r.parseConfig() +} + +func (r *Loader) setConfigFile() error { + configFile, err := r.evaluateOptions() + if err != nil { + if errors.Is(err, errConfigDisabled) { + return nil + } + + return fmt.Errorf("can't parse --config option: %w", err) + } + + if configFile != "" { + r.viper.SetConfigFile(configFile) + + // Assume YAML if the file has no extension. + if filepath.Ext(configFile) == "" { + r.viper.SetConfigType("yaml") + } + } else { + r.setupConfigFileSearch() + } + + return nil +} + +func (r *Loader) evaluateOptions() (string, error) { + if r.opts.NoConfig && r.opts.Config != "" { + return "", errors.New("can't combine option --config and --no-config") + } + + if r.opts.NoConfig { + return "", errConfigDisabled + } + + configFile, err := homedir.Expand(r.opts.Config) + if err != nil { + return "", errors.New("failed to expand configuration path") + } + + return configFile, nil +} + +//nolint:dupl // FIXME just during the dev +func (r *Loader) setupConfigFileSearch() { + firstArg := extractFirstPathArg() + + absStartPath, err := filepath.Abs(firstArg) + if err != nil { + r.log.Warnf("Can't make abs path for %q: %s", firstArg, err) + absStartPath = filepath.Clean(firstArg) + } + + // start from it + var curDir string + if fsutils.IsDir(absStartPath) { + curDir = absStartPath + } else { + curDir = filepath.Dir(absStartPath) + } + + // find all dirs from it up to the root + configSearchPaths := []string{"./"} + + for { + configSearchPaths = append(configSearchPaths, curDir) + + newCurDir := filepath.Dir(curDir) + if curDir == newCurDir || newCurDir == "" { + break + } + + curDir = newCurDir + } + + // find home directory for global config + if home, err := homedir.Dir(); err != nil { + r.log.Warnf("Can't get user's home directory: %s", err.Error()) + } else if !slices.Contains(configSearchPaths, home) { + configSearchPaths = append(configSearchPaths, home) + } + + r.log.Infof("Config search paths: %s", configSearchPaths) + + r.viper.SetConfigName(".golangci") + + for _, p := range configSearchPaths { + r.viper.AddConfigPath(p) + } +} + +func (r *Loader) parseConfig() error { + if err := r.viper.ReadInConfig(); err != nil { + var configFileNotFoundError viper.ConfigFileNotFoundError + if errors.As(err, &configFileNotFoundError) { + // Load configuration from flags only. + err = r.viper.Unmarshal(r.cfg) + if err != nil { + return err + } + + if err = r.validateConfig(); err != nil { + return fmt.Errorf("can't validate config: %w", err) + } + + return nil + } + + return fmt.Errorf("can't read viper config: %w", err) + } + + err := r.setConfigDir() + if err != nil { + return err + } + + // Load configuration from all sources (flags, file). + if err := r.viper.Unmarshal(r.cfg, fileDecoderHook()); err != nil { + return fmt.Errorf("can't unmarshal config by viper: %w", err) + } + + if err := r.validateConfig(); err != nil { + return fmt.Errorf("can't validate config: %w", err) + } + + if r.cfg.InternalTest { // just for testing purposes: to detect config file usage + _, _ = fmt.Fprintln(logutils.StdOut, "test") + os.Exit(exitcodes.Success) + } + + return nil +} + +func (r *Loader) setConfigDir() error { + usedConfigFile := r.viper.ConfigFileUsed() + if usedConfigFile == "" { + return nil + } + + if usedConfigFile == os.Stdin.Name() { + usedConfigFile = "" + r.log.Infof("Reading config file stdin") + } else { + var err error + usedConfigFile, err = fsutils.ShortestRelPath(usedConfigFile, "") + if err != nil { + r.log.Warnf("Can't pretty print config file path: %v", err) + } + + r.log.Infof("Used config file %s", usedConfigFile) + } + + usedConfigDir, err := filepath.Abs(filepath.Dir(usedConfigFile)) + if err != nil { + return errors.New("can't get config directory") + } + + r.cfg.cfgDir = usedConfigDir + + return nil +} + +// FIXME move to Config struct. +func (r *Loader) validateConfig() error { + for i, rule := range r.cfg.Issues.ExcludeRules { + if err := rule.Validate(); err != nil { + return fmt.Errorf("error in exclude rule #%d: %w", i, err) + } + } + + if len(r.cfg.Severity.Rules) > 0 && r.cfg.Severity.Default == "" { + return errors.New("can't set severity rule option: no default severity defined") + } + for i, rule := range r.cfg.Severity.Rules { + if err := rule.Validate(); err != nil { + return fmt.Errorf("error in severity rule #%d: %w", i, err) + } + } + + return nil +} + +func fileDecoderHook() viper.DecoderConfigOption { + return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( + // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). + mapstructure.StringToTimeDurationHookFunc(), + mapstructure.StringToSliceHookFunc(","), + + // Needed for forbidigo. + mapstructure.TextUnmarshallerHookFunc(), + )) +} + +func extractFirstPathArg() string { + args := os.Args + + // skip all args ([golangci-lint, run/linters]) before files/dirs list + for len(args) != 0 { + if args[0] == "run" { + args = args[1:] + break + } + + args = args[1:] + } + + // find first file/dir arg + firstArg := "./..." + for _, arg := range args { + if !strings.HasPrefix(arg, "-") { + firstArg = arg + break + } + } + + return firstArg +} diff --git a/pkg/config/reader.go b/pkg/config/reader.go index 40ff6f0e1c68..944da7ee6be3 100644 --- a/pkg/config/reader.go +++ b/pkg/config/reader.go @@ -180,6 +180,9 @@ func getFirstPathArg() string { return firstArg } +// FIXME just during the dev +// +//nolint:dupl func (r *FileReader) setupConfigFileSearch() { firstArg := getFirstPathArg() absStartPath, err := filepath.Abs(firstArg) From 084ce0b92908ba0e03fe0fdc069a186f587ec3b0 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:08:57 +0100 Subject: [PATCH 06/42] feat: add config command --- pkg/cmder/config.go | 92 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 pkg/cmder/config.go diff --git a/pkg/cmder/config.go b/pkg/cmder/config.go new file mode 100644 index 000000000000..46721e803087 --- /dev/null +++ b/pkg/cmder/config.go @@ -0,0 +1,92 @@ +package cmder + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +type configCommand struct { + viper *viper.Viper + cmd *cobra.Command + + cfg *config.Config + + log logutils.Log +} + +func newConfigCommand(log logutils.Log, cfg *config.Config) *configCommand { + c := &configCommand{ + viper: viper.New(), + log: log, + cfg: cfg, + } + + configCmd := &cobra.Command{ + Use: "config", + Short: "Config file information", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, + } + + configCmd.AddCommand( + &cobra.Command{ + Use: "path", + Short: "Print used config path", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + Run: c.execute, + PreRunE: c.preRunE, + }, + ) + + c.cmd = configCmd + + return c +} + +func (c *configCommand) preRunE(_ *cobra.Command, _ []string) error { + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, config.LoaderOptions{}, c.cfg) + + if err := loader.Load(); err != nil { + return fmt.Errorf("can't load config: %w", err) + } + + return nil +} + +func (c *configCommand) execute(_ *cobra.Command, _ []string) { + usedConfigFile := c.getUsedConfig() + if usedConfigFile == "" { + c.log.Warnf("No config file detected") + os.Exit(exitcodes.NoConfigFileDetected) + } + + fmt.Println(usedConfigFile) +} + +// getUsedConfig returns the resolved path to the golangci config file, +// or the empty string if no configuration could be found. +func (c *configCommand) getUsedConfig() string { + usedConfigFile := c.viper.ConfigFileUsed() + if usedConfigFile == "" { + return "" + } + + prettyUsedConfigFile, err := fsutils.ShortestRelPath(usedConfigFile, "") + if err != nil { + c.log.Warnf("Can't pretty print config file path: %s", err) + return usedConfigFile + } + + return prettyUsedConfigFile +} From 07953cd08054a2736778fadc2396eda497bc33c5 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:10:00 +0100 Subject: [PATCH 07/42] feat: add new configuration flagset bindings --- pkg/cmder/flagsets.go | 100 ++++++++++++++++++++++++++++++++++++ pkg/cmder/internal/vibra.go | 32 ++++++++++++ 2 files changed, 132 insertions(+) create mode 100644 pkg/cmder/flagsets.go create mode 100644 pkg/cmder/internal/vibra.go diff --git a/pkg/cmder/flagsets.go b/pkg/cmder/flagsets.go new file mode 100644 index 000000000000..ff6030859e96 --- /dev/null +++ b/pkg/cmder/flagsets.go @@ -0,0 +1,100 @@ +package cmder + +import ( + "fmt" + "strings" + + "github.com/fatih/color" + "github.com/spf13/pflag" + "github.com/spf13/viper" + + "github.com/golangci/golangci-lint/pkg/cmder/internal" + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" +) + +func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { + internal.VibraP(v, fs, fs.StringSliceP, "disable", "D", "linters.disable", nil, wh("Disable specific linter")) + internal.Vibra(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, wh("Disable all linters")) + + internal.VibraP(v, fs, fs.StringSliceP, "enable", "E", "linters.enable", nil, wh("Enable specific linter")) + internal.Vibra(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, wh("Enable all linters")) + + internal.Vibra(v, fs, fs.Bool, "fast", "linters.fast", false, + wh("Enable only fast linters from enabled linters set (first run won't be fast)")) + + internal.VibraP(v, fs, fs.StringSliceP, "presets", "p", "linters.presets", nil, + wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ + "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) +} + +func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) { + internal.VibraP(v, fs, fs.IntP, "concurrency", "j", "run.concurrency", getDefaultConcurrency(), + wh("Number of CPUs to use (Default: number of logical CPUs)")) + + internal.Vibra(v, fs, fs.String, "modules-download-mode", "run.modules-download-mode", "", + wh("Modules download mode. If not empty, passed as -mod= to go tools")) + internal.Vibra(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound, + wh("Exit code when issues were found")) + internal.Vibra(v, fs, fs.String, "go", "run.go", "", wh("Targeted Go version")) + internal.Vibra(v, fs, fs.StringSlice, "build-tags", "run.build-tags", nil, wh("Build tags")) + + internal.Vibra(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, wh("Timeout for total work")) + + internal.Vibra(v, fs, fs.Bool, "tests", "run.tests", true, wh("Analyze tests (*_test.go)")) + internal.Vibra(v, fs, fs.StringSlice, "skip-dirs", "run.skip-dirs", nil, wh("Regexps of directories to skip")) + internal.Vibra(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) + internal.Vibra(v, fs, fs.StringSlice, "skip-files", "run.skip-files", nil, wh("Regexps of files to skip")) + + const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + + "If false (default) - golangci-lint acquires file lock on start." + internal.Vibra(v, fs, fs.Bool, "allow-parallel-runners", "run.allow-parallel-runners", false, wh(allowParallelDesc)) + const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + + "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." + internal.Vibra(v, fs, fs.Bool, "allow-serial-runners", "run.allow-serial-runners", false, wh(allowSerialDesc)) + internal.Vibra(v, fs, fs.Bool, "show-stats", "run.show-stats", false, wh("Show statistics per linter")) +} + +func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) { + internal.Vibra(v, fs, fs.String, "out-format", "output.format", config.OutFormatColoredLineNumber, + wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) + internal.Vibra(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true, wh("Print lines of code with issue")) + internal.Vibra(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, wh("Print linter name in issue line")) + internal.Vibra(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, wh("Make issues output unique by line")) + internal.Vibra(v, fs, fs.Bool, "sort-results", "output.sort-results", false, wh("Sort linter results")) + internal.Vibra(v, fs, fs.Bool, "print-welcome", "output.print-welcome", false, wh("Print welcome message")) + internal.Vibra(v, fs, fs.String, "path-prefix", "output.path-prefix", "", wh("Path prefix to add to output")) +} + +//nolint:gomnd +func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { + internal.VibraP(v, fs, fs.StringSliceP, "exclude", "e", "issues.exclude", nil, wh("Exclude issue by regexp")) + internal.Vibra(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, getDefaultIssueExcludeHelp()) + internal.Vibra(v, fs, fs.Bool, "exclude-case-sensitive", "issues.exclude-case-sensitive", false, + wh("If set to true exclude and exclude rules regular expressions are case-sensitive")) + + internal.Vibra(v, fs, fs.Int, "max-issues-per-linter", "issues.max-issues-per-linter", 50, + wh("Maximum issues count per one linter. Set to 0 to disable")) + internal.Vibra(v, fs, fs.Int, "max-same-issues", "issues.max-same-issues", 3, + wh("Maximum count of issues with the same text. Set to 0 to disable")) + + const newDesc = "Show only new issues: if there are unstaged changes or untracked files, only those changes " + + "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration " + + "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at " + + "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer " + + "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate " + + "unstaged files before golangci-lint runs." + internal.VibraP(v, fs, fs.BoolP, "new", "n", "issues.new", false, wh(newDesc)) + internal.Vibra(v, fs, fs.String, "new-from-rev", "issues.new-from-rev", "", + wh("Show only new issues created after git revision `REV`")) + internal.Vibra(v, fs, fs.String, "new-from-patch", "issues.new-from-patch", "", + wh("Show only new issues created in git patch with file path `PATH`")) + internal.Vibra(v, fs, fs.Bool, "whole-files", "issues.whole-files", false, + wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) + internal.Vibra(v, fs, fs.Bool, "fix", "issues.fix", false, wh("Fix found issues (if it's supported by the linter)")) +} + +func wh(text string) string { + return color.GreenString(text) +} diff --git a/pkg/cmder/internal/vibra.go b/pkg/cmder/internal/vibra.go new file mode 100644 index 000000000000..6d2fa4a92b0e --- /dev/null +++ b/pkg/cmder/internal/vibra.go @@ -0,0 +1,32 @@ +package internal + +import ( + "fmt" + + "github.com/spf13/pflag" + "github.com/spf13/viper" +) + +type FlagFunc[T any] func(name string, value T, usage string) *T + +type FlagPFunc[T any] func(name, shorthand string, value T, usage string) *T + +// Vibra adds a Cobra flag and binds it with Viper. +func Vibra[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagFunc[T], name, bind string, value T, usage string) { + pfn(name, value, usage) + + err := v.BindPFlag(bind, fs.Lookup(name)) + if err != nil { + panic(fmt.Sprintf("failed to bind flag %s: %v", name, err)) + } +} + +// VibraP adds a Cobra flag and binds it with Viper. +func VibraP[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagPFunc[T], name, shorthand, bind string, value T, usage string) { + pfn(name, shorthand, value, usage) + + err := v.BindPFlag(bind, fs.Lookup(name)) + if err != nil { + panic(fmt.Sprintf("failed to bind flag %s: %v", name, err)) + } +} From 817a775831ebbfc68039050c2a0e749600d75b1a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:10:58 +0100 Subject: [PATCH 08/42] feat: add linters command --- pkg/cmder/linters.go | 102 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 pkg/cmder/linters.go diff --git a/pkg/cmder/linters.go b/pkg/cmder/linters.go new file mode 100644 index 000000000000..2c38b37c9539 --- /dev/null +++ b/pkg/cmder/linters.go @@ -0,0 +1,102 @@ +package cmder + +import ( + "fmt" + + "github.com/fatih/color" + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" + "github.com/golangci/golangci-lint/pkg/logutils" +) + +type lintersOptions struct { + config.LoaderOptions +} + +type lintersCommand struct { + viper *viper.Viper + cmd *cobra.Command + + opts lintersOptions + + cfg *config.Config + + log logutils.Log + + dbManager *lintersdb.Manager + enabledLintersSet *lintersdb.EnabledSet +} + +func newLintersCommand(logger logutils.Log, cfg *config.Config) *lintersCommand { + c := &lintersCommand{ + viper: viper.New(), + cfg: cfg, + log: logger, + } + + lintersCmd := &cobra.Command{ + Use: "linters", + Short: "List current linters configuration", + Args: cobra.NoArgs, + ValidArgsFunction: cobra.NoFileCompletions, + RunE: c.execute, + PreRunE: c.preRunE, + } + + fs := lintersCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + + setupConfigFileFlagSet(fs, &c.opts.LoaderOptions) + setupLintersFlagSet(c.viper, fs) + + c.cmd = lintersCmd + + return c +} + +func (c *lintersCommand) preRunE(_ *cobra.Command, _ []string) error { + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, c.opts.LoaderOptions, c.cfg) + + if err := loader.Load(); err != nil { + 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) + + return nil +} + +func (c *lintersCommand) execute(_ *cobra.Command, _ []string) error { + enabledLintersMap, err := c.enabledLintersSet.GetEnabledLintersMap() + if err != nil { + return fmt.Errorf("can't get enabled linters: %w", err) + } + + var enabledLinters []*linter.Config + var disabledLCs []*linter.Config + + for _, lc := range c.dbManager.GetAllSupportedLinterConfigs() { + if lc.Internal { + continue + } + + if enabledLintersMap[lc.Name()] == nil { + disabledLCs = append(disabledLCs, lc) + } else { + enabledLinters = append(enabledLinters, lc) + } + } + + color.Green("Enabled by your configuration linters:\n") + printLinters(enabledLinters) + color.Red("\nDisabled by your configuration linters:\n") + printLinters(disabledLCs) + + return nil +} From 806eeafd0f058d6de2b20a7f95bf1cb02c4feb42 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:11:22 +0100 Subject: [PATCH 09/42] feat: add run command --- pkg/cmder/run.go | 748 +++++++++++++++++++++++++++++++++++++++ pkg/logutils/logutils.go | 7 + 2 files changed, 755 insertions(+) create mode 100644 pkg/cmder/run.go diff --git a/pkg/cmder/run.go b/pkg/cmder/run.go new file mode 100644 index 000000000000..347ed660b11b --- /dev/null +++ b/pkg/cmder/run.go @@ -0,0 +1,748 @@ +package cmder + +import ( + "bytes" + "context" + "crypto/sha256" + "errors" + "fmt" + "io" + "log" + "os" + "path/filepath" + "runtime" + "runtime/pprof" + "runtime/trace" + "sort" + "strconv" + "strings" + "time" + + "github.com/fatih/color" + "github.com/gofrs/flock" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/spf13/viper" + "golang.org/x/exp/maps" + "gopkg.in/yaml.v3" + + "github.com/golangci/golangci-lint/internal/cache" + "github.com/golangci/golangci-lint/internal/pkgcache" + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/fsutils" + "github.com/golangci/golangci-lint/pkg/golinters/goanalysis/load" + "github.com/golangci/golangci-lint/pkg/goutil" + "github.com/golangci/golangci-lint/pkg/lint" + "github.com/golangci/golangci-lint/pkg/lint/lintersdb" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/packages" + "github.com/golangci/golangci-lint/pkg/printers" + "github.com/golangci/golangci-lint/pkg/report" + "github.com/golangci/golangci-lint/pkg/result" + "github.com/golangci/golangci-lint/pkg/timeutils" +) + +const defaultFileMode = 0644 + +const defaultTimeout = time.Minute + +const ( + // envFailOnWarnings value: "1" + envFailOnWarnings = "FAIL_ON_WARNINGS" + // envMemLogEvery value: "1" + envMemLogEvery = "GL_MEM_LOG_EVERY" +) + +const ( + // envHelpRun value: "1". + envHelpRun = "HELP_RUN" + envMemProfileRate = "GL_MEM_PROFILE_RATE" +) + +type runOptions struct { + config.LoaderOptions + + CPUProfilePath string // Flag only. + MemProfilePath string // Flag only. + TracePath string // Flag only. + + PrintResourcesUsage bool // Flag only. +} + +type runCommand struct { + viper *viper.Viper + cmd *cobra.Command + + opts runOptions + + cfg *config.Config + + buildInfo BuildInfo + + dbManager *lintersdb.Manager + enabledLintersSet *lintersdb.EnabledSet + + log logutils.Log + debugf logutils.DebugFunc + reportData *report.Data + + contextLoader *lint.ContextLoader + goenv *goutil.Env + + fileCache *fsutils.FileCache + lineCache *fsutils.LineCache + + flock *flock.Flock + + exitCode int +} + +func newRunCommand(logger logutils.Log, cfg *config.Config, reportData *report.Data, info BuildInfo) *runCommand { + c := &runCommand{ + viper: viper.New(), + log: logger, + debugf: logutils.Debug(logutils.DebugKeyExec), + cfg: cfg, + reportData: reportData, + buildInfo: info, + } + + runCmd := &cobra.Command{ + Use: "run", + Short: "Run the linters", + Run: c.execute, + PreRunE: c.preRunE, + PostRun: c.postRun, + PersistentPreRunE: c.persistentPreRunE, + PersistentPostRunE: c.persistentPostRunE, + } + + runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals + runCmd.SetErr(logutils.StdErr) + + fs := runCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + + // Only for testing purpose. + // Don't add other flags here. + fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) + _ = fs.MarkHidden("internal-cmd-test") + + setupConfigFileFlagSet(fs, &c.opts.LoaderOptions) + + setupLintersFlagSet(c.viper, fs) + setupRunFlagSet(c.viper, fs) + setupOutputFlagSet(c.viper, fs) + setupIssuesFlagSet(c.viper, fs) + + setupRunPersistentFlags(runCmd.PersistentFlags(), &c.opts) + + c.cmd = runCmd + + return c +} + +func (c *runCommand) persistentPreRunE(_ *cobra.Command, _ []string) error { + if err := c.startTracing(); err != nil { + return err + } + + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, c.opts.LoaderOptions, c.cfg) + + if err := loader.Load(); err != nil { + return fmt.Errorf("can't load config: %w", err) + } + + if c.cfg.Run.Go == "" { + c.cfg.Run.Go = config.DetectGoVersion() + } + + runtime.GOMAXPROCS(c.cfg.Run.Concurrency) + + return c.startTracing() +} + +func (c *runCommand) persistentPostRunE(_ *cobra.Command, _ []string) error { + if err := c.stopTracing(); err != nil { + return err + } + + os.Exit(c.exitCode) + + return nil +} + +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) + + c.goenv = goutil.NewEnv(c.log.Child(logutils.DebugKeyGoEnv)) + + c.fileCache = fsutils.NewFileCache() + c.lineCache = fsutils.NewLineCache(c.fileCache) + + sw := timeutils.NewStopwatch("pkgcache", c.log.Child(logutils.DebugKeyStopwatch)) + + pkgCache, err := pkgcache.NewCache(sw, c.log.Child(logutils.DebugKeyPkgCache)) + if err != nil { + return fmt.Errorf("failed to build packages cache: %w", err) + } + + c.contextLoader = lint.NewContextLoader(c.cfg, c.log.Child(logutils.DebugKeyLoader), c.goenv, + c.lineCache, c.fileCache, pkgCache, load.NewGuard()) + + if err = initHashSalt(c.buildInfo.Version, c.cfg); err != nil { + return fmt.Errorf("failed to init hash salt: %w", err) + } + + if ok := c.acquireFileLock(); !ok { + return errors.New("parallel golangci-lint is running") + } + + return nil +} + +func (c *runCommand) postRun(_ *cobra.Command, _ []string) { + c.releaseFileLock() +} + +func (c *runCommand) execute(_ *cobra.Command, args []string) { + needTrackResources := logutils.IsVerbose() || c.opts.PrintResourcesUsage + + trackResourcesEndCh := make(chan struct{}) + defer func() { // XXX: this defer must be before ctx.cancel defer + if needTrackResources { // wait until resource tracking finished to print properly + <-trackResourcesEndCh + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), c.cfg.Run.Timeout) + defer cancel() + + if needTrackResources { + go watchResources(ctx, trackResourcesEndCh, c.log, c.debugf) + } + + if err := c.runAndPrint(ctx, args); err != nil { + c.log.Errorf("Running error: %s", err) + if c.exitCode == exitcodes.Success { + var exitErr *exitcodes.ExitError + if errors.As(err, &exitErr) { + c.exitCode = exitErr.Code + } else { + c.exitCode = exitcodes.Failure + } + } + } + + c.setupExitCode(ctx) +} + +func (c *runCommand) startTracing() error { + if c.opts.CPUProfilePath != "" { + f, err := os.Create(c.opts.CPUProfilePath) + if err != nil { + return fmt.Errorf("can't create file %s: %w", c.opts.CPUProfilePath, err) + } + if err := pprof.StartCPUProfile(f); err != nil { + return fmt.Errorf("can't start CPU profiling: %w", err) + } + } + + if c.opts.MemProfilePath != "" { + if rate := os.Getenv(envMemProfileRate); rate != "" { + runtime.MemProfileRate, _ = strconv.Atoi(rate) + } + } + + if c.opts.TracePath != "" { + f, err := os.Create(c.opts.TracePath) + if err != nil { + return fmt.Errorf("can't create file %s: %w", c.opts.TracePath, err) + } + if err = trace.Start(f); err != nil { + return fmt.Errorf("can't start tracing: %w", err) + } + } + + return nil +} + +func (c *runCommand) stopTracing() error { + if c.opts.CPUProfilePath != "" { + pprof.StopCPUProfile() + } + + if c.opts.MemProfilePath != "" { + f, err := os.Create(c.opts.MemProfilePath) + if err != nil { + return fmt.Errorf("can't create file %s: %w", c.opts.MemProfilePath, err) + } + + var ms runtime.MemStats + runtime.ReadMemStats(&ms) + printMemStats(&ms, c.log) + + if err := pprof.WriteHeapProfile(f); err != nil { + return fmt.Errorf("can't write heap profile: %w", err) + } + _ = f.Close() + } + + if c.opts.TracePath != "" { + trace.Stop() + } + + return nil +} + +func (c *runCommand) runAndPrint(ctx context.Context, args []string) error { + if err := c.goenv.Discover(ctx); err != nil { + c.log.Warnf("Failed to discover go env: %s", err) + } + + if !logutils.HaveDebugTag(logutils.DebugKeyLintersOutput) { + // Don't allow linters and loader to print anything + log.SetOutput(io.Discard) + savedStdout, savedStderr := c.setOutputToDevNull() + defer func() { + os.Stdout, os.Stderr = savedStdout, savedStderr + }() + } + + issues, err := c.runAnalysis(ctx, args) + if err != nil { + return err // XXX: don't loose type + } + + formats := strings.Split(c.cfg.Output.Format, ",") + for _, format := range formats { + out := strings.SplitN(format, ":", 2) + if len(out) < 2 { + out = append(out, "") + } + + err := c.printReports(issues, out[1], out[0]) + if err != nil { + return err + } + } + + c.printStats(issues) + + c.setExitCodeIfIssuesFound(issues) + + c.fileCache.PrintStats(c.log) + + return nil +} + +// runAnalysis executes the linters that have been enabled in the configuration. +func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { + c.cfg.Run.Args = args + + lintersToRun, err := c.enabledLintersSet.GetOptimizedLinters() + if err != nil { + return nil, err + } + + enabledLintersMap, err := c.enabledLintersSet.GetEnabledLintersMap() + if err != nil { + return nil, err + } + + for _, lc := range c.dbManager.GetAllSupportedLinterConfigs() { + isEnabled := enabledLintersMap[lc.Name()] != nil + c.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault) + } + + lintCtx, err := c.contextLoader.Load(ctx, lintersToRun) + if err != nil { + return nil, fmt.Errorf("context loading failed: %w", err) + } + 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) + if err != nil { + return nil, err + } + + return runner.Run(ctx, lintersToRun, lintCtx) +} + +func (c *runCommand) setOutputToDevNull() (savedStdout, savedStderr *os.File) { + savedStdout, savedStderr = os.Stdout, os.Stderr + devNull, err := os.Open(os.DevNull) + if err != nil { + c.log.Warnf("Can't open null device %q: %s", os.DevNull, err) + return + } + + os.Stdout, os.Stderr = devNull, devNull + return +} + +func (c *runCommand) setExitCodeIfIssuesFound(issues []result.Issue) { + if len(issues) != 0 { + c.exitCode = c.cfg.Run.ExitCodeIfIssuesFound + } +} + +func (c *runCommand) printReports(issues []result.Issue, path, format string) error { + w, shouldClose, err := c.createWriter(path) + if err != nil { + return fmt.Errorf("can't create output for %s: %w", path, err) + } + + p, err := c.createPrinter(format, w) + if err != nil { + if file, ok := w.(io.Closer); shouldClose && ok { + _ = file.Close() + } + return err + } + + if err = p.Print(issues); err != nil { + if file, ok := w.(io.Closer); shouldClose && ok { + _ = file.Close() + } + return fmt.Errorf("can't print %d issues: %w", len(issues), err) + } + + if file, ok := w.(io.Closer); shouldClose && ok { + _ = file.Close() + } + + return nil +} + +func (c *runCommand) createWriter(path string) (io.Writer, bool, error) { + if path == "" || path == "stdout" { + return logutils.StdOut, false, nil + } + if path == "stderr" { + return logutils.StdErr, false, nil + } + f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, defaultFileMode) + if err != nil { + return nil, false, err + } + return f, true, nil +} + +func (c *runCommand) createPrinter(format string, w io.Writer) (printers.Printer, error) { + var p printers.Printer + switch format { + case config.OutFormatJSON: + p = printers.NewJSON(c.reportData, w) + case config.OutFormatColoredLineNumber, config.OutFormatLineNumber: + p = printers.NewText(c.cfg.Output.PrintIssuedLine, + format == config.OutFormatColoredLineNumber, c.cfg.Output.PrintLinterName, + c.log.Child(logutils.DebugKeyTextPrinter), w) + case config.OutFormatTab, config.OutFormatColoredTab: + p = printers.NewTab(c.cfg.Output.PrintLinterName, + format == config.OutFormatColoredTab, + c.log.Child(logutils.DebugKeyTabPrinter), w) + case config.OutFormatCheckstyle: + p = printers.NewCheckstyle(w) + case config.OutFormatCodeClimate: + p = printers.NewCodeClimate(w) + case config.OutFormatHTML: + p = printers.NewHTML(w) + case config.OutFormatJunitXML: + p = printers.NewJunitXML(w) + case config.OutFormatGithubActions: + p = printers.NewGithub(w) + case config.OutFormatTeamCity: + p = printers.NewTeamCity(w) + default: + return nil, fmt.Errorf("unknown output format %s", format) + } + + return p, nil +} + +func (c *runCommand) printStats(issues []result.Issue) { + if !c.cfg.Run.ShowStats { + return + } + + if len(issues) == 0 { + c.cmd.Println("0 issues.") + return + } + + stats := map[string]int{} + for idx := range issues { + stats[issues[idx].FromLinter]++ + } + + c.cmd.Printf("%d issues:\n", len(issues)) + + keys := maps.Keys(stats) + sort.Strings(keys) + + for _, key := range keys { + c.cmd.Printf("* %s: %d\n", key, stats[key]) + } +} + +func (c *runCommand) setupExitCode(ctx context.Context) { + if ctx.Err() != nil { + c.exitCode = exitcodes.Timeout + c.log.Errorf("Timeout exceeded: try increasing it by passing --timeout option") + return + } + + if c.exitCode != exitcodes.Success { + return + } + + needFailOnWarnings := os.Getenv(lintersdb.EnvTestRun) == "1" || os.Getenv(envFailOnWarnings) == "1" + if needFailOnWarnings && len(c.reportData.Warnings) != 0 { + c.exitCode = exitcodes.WarningInTest + return + } + + if c.reportData.Error != "" { + // it's a case e.g. when typecheck linter couldn't parse and error and just logged it + c.exitCode = exitcodes.ErrorWasLogged + return + } +} + +func (c *runCommand) acquireFileLock() bool { + if c.cfg.Run.AllowParallelRunners { + c.debugf("Parallel runners are allowed, no locking") + return true + } + + lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock") + c.debugf("Locking on file %s...", lockFile) + f := flock.New(lockFile) + const retryDelay = time.Second + + ctx := context.Background() + if !c.cfg.Run.AllowSerialRunners { + const totalTimeout = 5 * time.Second + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, totalTimeout) + defer cancel() + } + if ok, _ := f.TryLockContext(ctx, retryDelay); !ok { + return false + } + + c.flock = f + return true +} + +func (c *runCommand) releaseFileLock() { + if c.cfg.Run.AllowParallelRunners { + return + } + + if err := c.flock.Unlock(); err != nil { + c.debugf("Failed to unlock on file: %s", err) + } + if err := os.Remove(c.flock.Path()); err != nil { + c.debugf("Failed to remove lock file: %s", err) + } +} + +func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log, debugf logutils.DebugFunc) { + startedAt := time.Now() + debugf("Started tracking time") + + var maxRSSMB, totalRSSMB float64 + var iterationsCount int + + const intervalMS = 100 + ticker := time.NewTicker(intervalMS * time.Millisecond) + defer ticker.Stop() + + logEveryRecord := os.Getenv(envMemLogEvery) == "1" + const MB = 1024 * 1024 + + track := func() { + var m runtime.MemStats + runtime.ReadMemStats(&m) + + if logEveryRecord { + debugf("Stopping memory tracing iteration, printing ...") + printMemStats(&m, logger) + } + + rssMB := float64(m.Sys) / MB + if rssMB > maxRSSMB { + maxRSSMB = rssMB + } + totalRSSMB += rssMB + iterationsCount++ + } + + for { + track() + + stop := false + select { + case <-ctx.Done(): + stop = true + debugf("Stopped resources tracking") + case <-ticker.C: + } + + if stop { + break + } + } + track() + + avgRSSMB := totalRSSMB / float64(iterationsCount) + + logger.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", + iterationsCount, avgRSSMB, maxRSSMB) + logger.Infof("Execution took %s", time.Since(startedAt)) + close(done) +} + +func setupConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.LoaderOptions) { + fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) + fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) +} + +func getDefaultIssueExcludeHelp() string { + parts := []string{color.GreenString("Use or not use default excludes:")} + for _, ep := range config.DefaultExcludePatterns { + parts = append(parts, + fmt.Sprintf(" # %s %s: %s", ep.ID, ep.Linter, ep.Why), + fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), + "", + ) + } + return strings.Join(parts, "\n") +} + +func getDefaultDirectoryExcludeHelp() string { + parts := []string{color.GreenString("Use or not use default excluded directories:")} + for _, dir := range packages.StdExcludeDirRegexps { + parts = append(parts, fmt.Sprintf(" - %s", color.YellowString(dir))) + } + parts = append(parts, "") + return strings.Join(parts, "\n") +} + +func setupRunPersistentFlags(fs *pflag.FlagSet, opts *runOptions) { + fs.BoolVar(&opts.PrintResourcesUsage, "print-resources-usage", false, + wh("Print avg and max memory usage of golangci-lint and total time")) + + fs.StringVar(&opts.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) + fs.StringVar(&opts.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) + fs.StringVar(&opts.TracePath, "trace-path", "", wh("Path to trace output file")) +} + +func getDefaultConcurrency() int { + if os.Getenv(envHelpRun) == "1" { + // Make stable concurrency for generating help documentation. + const prettyConcurrency = 8 + return prettyConcurrency + } + + return runtime.NumCPU() +} + +func printMemStats(ms *runtime.MemStats, logger logutils.Log) { + logger.Infof("Mem stats: alloc=%s total_alloc=%s sys=%s "+ + "heap_alloc=%s heap_sys=%s heap_idle=%s heap_released=%s heap_in_use=%s "+ + "stack_in_use=%s stack_sys=%s "+ + "mspan_sys=%s mcache_sys=%s buck_hash_sys=%s gc_sys=%s other_sys=%s "+ + "mallocs_n=%d frees_n=%d heap_objects_n=%d gc_cpu_fraction=%.2f", + formatMemory(ms.Alloc), formatMemory(ms.TotalAlloc), formatMemory(ms.Sys), + formatMemory(ms.HeapAlloc), formatMemory(ms.HeapSys), + formatMemory(ms.HeapIdle), formatMemory(ms.HeapReleased), formatMemory(ms.HeapInuse), + formatMemory(ms.StackInuse), formatMemory(ms.StackSys), + formatMemory(ms.MSpanSys), formatMemory(ms.MCacheSys), formatMemory(ms.BuckHashSys), + formatMemory(ms.GCSys), formatMemory(ms.OtherSys), + ms.Mallocs, ms.Frees, ms.HeapObjects, ms.GCCPUFraction) +} + +func formatMemory(memBytes uint64) string { + const Kb = 1024 + const Mb = Kb * 1024 + + if memBytes < Kb { + return fmt.Sprintf("%db", memBytes) + } + if memBytes < Mb { + return fmt.Sprintf("%dkb", memBytes/Kb) + } + return fmt.Sprintf("%dmb", memBytes/Mb) +} + +// --- Related to cache. + +func initHashSalt(version string, cfg *config.Config) error { + binSalt, err := computeBinarySalt(version) + if err != nil { + return fmt.Errorf("failed to calculate binary salt: %w", err) + } + + configSalt, err := computeConfigSalt(cfg) + if err != nil { + return fmt.Errorf("failed to calculate config salt: %w", err) + } + + b := bytes.NewBuffer(binSalt) + b.Write(configSalt) + cache.SetSalt(b.Bytes()) + return nil +} + +func computeBinarySalt(version string) ([]byte, error) { + if version != "" && version != "(devel)" { + return []byte(version), nil + } + + if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { + return []byte("debug"), nil + } + + p, err := os.Executable() + if err != nil { + return nil, err + } + f, err := os.Open(p) + if err != nil { + return nil, err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} + +// computeConfigSalt computes configuration hash. +// We don't hash all config fields to reduce meaningless cache invalidations. +// At least, it has a huge impact on tests speed. +// Fields: `LintersSettings` and `Run.BuildTags`. +func computeConfigSalt(cfg *config.Config) ([]byte, error) { + lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) + if err != nil { + return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) + } + + configData := bytes.NewBufferString("linters-settings=") + configData.Write(lintersSettingsBytes) + configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) + + h := sha256.New() + if _, err := h.Write(configData.Bytes()); err != nil { + return nil, err + } + return h.Sum(nil), nil +} diff --git a/pkg/logutils/logutils.go b/pkg/logutils/logutils.go index 94479bc7b9a8..2b7ac6e287e6 100644 --- a/pkg/logutils/logutils.go +++ b/pkg/logutils/logutils.go @@ -99,8 +99,15 @@ func HaveDebugTag(tag string) bool { return enabledDebugs[tag] } +var verbose bool + func SetupVerboseLog(log Log, isVerbose bool) { if isVerbose { + verbose = isVerbose log.SetLevel(LogLevelInfo) } } + +func IsVerbose() bool { + return verbose +} From c306ef45a432299b8592e1abec5af32412ff0161 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:11:50 +0100 Subject: [PATCH 10/42] feat: add root command --- pkg/cmder/root.go | 167 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 pkg/cmder/root.go diff --git a/pkg/cmder/root.go b/pkg/cmder/root.go new file mode 100644 index 000000000000..998ff7604857 --- /dev/null +++ b/pkg/cmder/root.go @@ -0,0 +1,167 @@ +package cmder + +import ( + "errors" + "fmt" + "os" + "slices" + + "github.com/fatih/color" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/golangci/golangci-lint/pkg/config" + "github.com/golangci/golangci-lint/pkg/logutils" + "github.com/golangci/golangci-lint/pkg/report" +) + +func Execute(info BuildInfo) error { + return newRootCommand(info).Execute() +} + +type rootOptions struct { + PrintVersion bool + + Verbose bool // Persistent flag. + Color string // Persistent flag. +} + +type rootCommand struct { + cmd *cobra.Command + opts rootOptions + + log logutils.Log +} + +func newRootCommand(info BuildInfo) *rootCommand { + c := &rootCommand{} + + rootCmd := &cobra.Command{ + Use: "golangci-lint", + Short: "golangci-lint is a smart linters runner.", + Long: `Smart, fast linters runner.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + if c.opts.PrintVersion { + _ = printVersion(logutils.StdOut, info) + return nil + } + + return cmd.Help() + }, + } + + fs := rootCmd.Flags() + fs.BoolVar(&c.opts.PrintVersion, "version", false, wh("Print version")) + + setupRootPersistentFlags(rootCmd.PersistentFlags(), &c.opts) + + reportData := &report.Data{} + log := report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), reportData) + + // Dedicated configuration for each command to avoid side effects of bindings. + rootCmd.AddCommand( + newLintersCommand(log, config.NewDefault()).cmd, + newRunCommand(log, config.NewDefault(), reportData, info).cmd, + newCacheCommand().cmd, + newConfigCommand(log, config.NewDefault()).cmd, + newVersionCommand(info).cmd, + newHelpCommand(log, config.NewDefault()).cmd, + ) + + c.log = log + c.cmd = rootCmd + + return c +} + +func (c *rootCommand) Execute() error { + err := setupLogger(c.log) + if err != nil { + return err + } + + return c.cmd.Execute() +} + +func setupRootPersistentFlags(fs *pflag.FlagSet, opts *rootOptions) { + fs.BoolVarP(&opts.Verbose, "verbose", "v", false, wh("Verbose output")) + fs.StringVar(&opts.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) +} + +func setupLogger(logger logutils.Log) error { + opts, err := forceRootParsePersistentFlags() + if err != nil && !errors.Is(err, pflag.ErrHelp) { + return err + } + + if opts == nil { + return nil + } + + logutils.SetupVerboseLog(logger, opts.Verbose) + + switch opts.Color { + case "always": + color.NoColor = false + case "never": + color.NoColor = true + case "auto": + // nothing + default: + logger.Fatalf("invalid value %q for --color; must be 'always', 'auto', or 'never'", opts.Color) + } + + return nil +} + +func forceRootParsePersistentFlags() (*rootOptions, error) { + // We use another pflag.FlagSet here to not set `changed` flag on cmd.Flags() options. + // Otherwise, string slice options will be duplicated. + fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) + + // Ignore unknown flags because we will parse the command flags later. + fs.ParseErrorsWhitelist = pflag.ParseErrorsWhitelist{UnknownFlags: true} + + opts := &rootOptions{} + + // Don't do `fs.AddFlagSet(cmd.Flags())` because it shares flags representations: + // `changed` variable inside string slice vars will be shared. + // Use another config variable here, + // to not affect main parsing by this parsing of only config option. + setupRootPersistentFlags(fs, opts) + + fs.Usage = func() {} // otherwise, help text will be printed twice + + if err := fs.Parse(safeArgs(fs, os.Args)); err != nil { + if errors.Is(err, pflag.ErrHelp) { + return nil, err + } + + return nil, fmt.Errorf("can't parse args: %w", err) + } + + return opts, nil +} + +// Shorthands are a problem because pflag, with UnknownFlags, will try to parse all the letters as options. +// A shorthand can aggregate several letters (ex `ps -aux`) +// The function replaces non-supported shorthands by a dumb flag. +func safeArgs(fs *pflag.FlagSet, args []string) []string { + var shorthands []string + fs.VisitAll(func(flag *pflag.Flag) { + shorthands = append(shorthands, flag.Shorthand) + }) + + var cleanArgs []string + for _, arg := range args { + if len(arg) > 1 && arg[0] == '-' && arg[1] != '-' && !slices.Contains(shorthands, string(arg[1])) { + cleanArgs = append(cleanArgs, "--potato") + continue + } + + cleanArgs = append(cleanArgs, arg) + } + + return cleanArgs +} From 871c1c6c004d5d53904acfe2e1f4614eb34dfb4b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:12:22 +0100 Subject: [PATCH 11/42] feat: use the new commands --- cmd/golangci-lint/main.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/golangci-lint/main.go b/cmd/golangci-lint/main.go index 9d1daa81df92..c735c778426a 100644 --- a/cmd/golangci-lint/main.go +++ b/cmd/golangci-lint/main.go @@ -5,7 +5,7 @@ import ( "os" "runtime/debug" - "github.com/golangci/golangci-lint/pkg/commands" + "github.com/golangci/golangci-lint/pkg/cmder" "github.com/golangci/golangci-lint/pkg/exitcodes" ) @@ -29,17 +29,15 @@ func main() { } } - info := commands.BuildInfo{ + info := cmder.BuildInfo{ GoVersion: goVersion, Version: version, Commit: commit, Date: date, } - e := commands.NewExecutor(info) - - if err := e.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "failed executing command with error %v\n", err) + if err := cmder.Execute(info); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "failed executing command with error %v\n", err) os.Exit(exitcodes.Failure) } } From 639b90b0e6e6786cee357e62dabfd3ad79cefdb8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:14:26 +0100 Subject: [PATCH 12/42] chore: remove old cache command --- pkg/commands/cache.go | 144 --------------------------------------- pkg/commands/executor.go | 71 ++++++++++++++++++- 2 files changed, 70 insertions(+), 145 deletions(-) delete mode 100644 pkg/commands/cache.go diff --git a/pkg/commands/cache.go b/pkg/commands/cache.go deleted file mode 100644 index 3dbe2427f578..000000000000 --- a/pkg/commands/cache.go +++ /dev/null @@ -1,144 +0,0 @@ -package commands - -import ( - "bytes" - "crypto/sha256" - "fmt" - "io" - "os" - "path/filepath" - "strings" - - "github.com/spf13/cobra" - "gopkg.in/yaml.v3" - - "github.com/golangci/golangci-lint/internal/cache" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/logutils" -) - -func (e *Executor) initCache() { - cacheCmd := &cobra.Command{ - Use: "cache", - Short: "Cache control and information", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return cmd.Help() - }, - } - - cacheCmd.AddCommand(&cobra.Command{ - Use: "clean", - Short: "Clean cache", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - RunE: e.executeCacheClean, - }) - cacheCmd.AddCommand(&cobra.Command{ - Use: "status", - Short: "Show cache status", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - Run: e.executeCacheStatus, - }) - - // TODO: add trim command? - - e.rootCmd.AddCommand(cacheCmd) -} - -func (e *Executor) executeCacheClean(_ *cobra.Command, _ []string) error { - cacheDir := cache.DefaultDir() - if err := os.RemoveAll(cacheDir); err != nil { - return fmt.Errorf("failed to remove dir %s: %w", cacheDir, err) - } - - return nil -} - -func (e *Executor) executeCacheStatus(_ *cobra.Command, _ []string) { - cacheDir := cache.DefaultDir() - fmt.Fprintf(logutils.StdOut, "Dir: %s\n", cacheDir) - - cacheSizeBytes, err := dirSizeBytes(cacheDir) - if err == nil { - fmt.Fprintf(logutils.StdOut, "Size: %s\n", fsutils.PrettifyBytesCount(cacheSizeBytes)) - } -} - -func dirSizeBytes(path string) (int64, error) { - var size int64 - err := filepath.Walk(path, func(_ string, info os.FileInfo, err error) error { - if err == nil && !info.IsDir() { - size += info.Size() - } - return err - }) - return size, err -} - -// --- Related to cache but not used directly by the cache command. - -func initHashSalt(version string, cfg *config.Config) error { - binSalt, err := computeBinarySalt(version) - if err != nil { - return fmt.Errorf("failed to calculate binary salt: %w", err) - } - - configSalt, err := computeConfigSalt(cfg) - if err != nil { - return fmt.Errorf("failed to calculate config salt: %w", err) - } - - b := bytes.NewBuffer(binSalt) - b.Write(configSalt) - cache.SetSalt(b.Bytes()) - return nil -} - -func computeBinarySalt(version string) ([]byte, error) { - if version != "" && version != "(devel)" { - return []byte(version), nil - } - - if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { - return []byte("debug"), nil - } - - p, err := os.Executable() - if err != nil { - return nil, err - } - f, err := os.Open(p) - if err != nil { - return nil, err - } - defer f.Close() - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} - -// computeConfigSalt computes configuration hash. -// We don't hash all config fields to reduce meaningless cache invalidations. -// At least, it has a huge impact on tests speed. -// Fields: `LintersSettings` and `Run.BuildTags`. -func computeConfigSalt(cfg *config.Config) ([]byte, error) { - lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) - if err != nil { - return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) - } - - configData := bytes.NewBufferString("linters-settings=") - configData.Write(lintersSettingsBytes) - configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) - - h := sha256.New() - if _, err := h.Write(configData.Bytes()); err != nil { - return nil, err - } - return h.Sum(nil), nil -} diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 211466cfb645..6456c9ecd87b 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -1,8 +1,11 @@ package commands import ( + "bytes" + "crypto/sha256" "errors" "fmt" + "io" "os" "strings" "time" @@ -11,7 +14,9 @@ import ( "github.com/gofrs/flock" "github.com/spf13/cobra" "github.com/spf13/pflag" + "gopkg.in/yaml.v3" + "github.com/golangci/golangci-lint/internal/cache" "github.com/golangci/golangci-lint/internal/pkgcache" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/fsutils" @@ -83,7 +88,6 @@ func (e *Executor) initCommands() { e.initLinters() e.initConfig() e.initVersion() - e.initCache() } func (e *Executor) initConfiguration() { @@ -228,3 +232,68 @@ func fixSlicesFlags(fs *pflag.FlagSet) { func wh(text string) string { return color.GreenString(text) } + +// --- Related to cache but not used directly by the cache command. + +func initHashSalt(version string, cfg *config.Config) error { + binSalt, err := computeBinarySalt(version) + if err != nil { + return fmt.Errorf("failed to calculate binary salt: %w", err) + } + + configSalt, err := computeConfigSalt(cfg) + if err != nil { + return fmt.Errorf("failed to calculate config salt: %w", err) + } + + b := bytes.NewBuffer(binSalt) + b.Write(configSalt) + cache.SetSalt(b.Bytes()) + return nil +} + +func computeBinarySalt(version string) ([]byte, error) { + if version != "" && version != "(devel)" { + return []byte(version), nil + } + + if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { + return []byte("debug"), nil + } + + p, err := os.Executable() + if err != nil { + return nil, err + } + f, err := os.Open(p) + if err != nil { + return nil, err + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} + +// computeConfigSalt computes configuration hash. +// We don't hash all config fields to reduce meaningless cache invalidations. +// At least, it has a huge impact on tests speed. +// Fields: `LintersSettings` and `Run.BuildTags`. +func computeConfigSalt(cfg *config.Config) ([]byte, error) { + lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) + if err != nil { + return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) + } + + configData := bytes.NewBufferString("linters-settings=") + configData.Write(lintersSettingsBytes) + configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) + + h := sha256.New() + if _, err := h.Write(configData.Bytes()); err != nil { + return nil, err + } + return h.Sum(nil), nil +} From 3870c56cdcfac4569f000f1442690915494b62a4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:17:15 +0100 Subject: [PATCH 13/42] chore: remove old version command --- pkg/commands/executor.go | 11 ++++- pkg/commands/root.go | 9 ++++ pkg/commands/version.go | 90 ---------------------------------------- 3 files changed, 18 insertions(+), 92 deletions(-) delete mode 100644 pkg/commands/version.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 6456c9ecd87b..96e1f495d5a8 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -87,7 +87,6 @@ func (e *Executor) initCommands() { e.initHelp() e.initLinters() e.initConfig() - e.initVersion() } func (e *Executor) initConfiguration() { @@ -180,7 +179,6 @@ func getConfigForCommandLine() (*config.Config, error) { // Use another config variable here, not e.cfg, to not // affect main parsing by this parsing of only config option. initRunFlagSet(fs, &cfg) - initVersionFlagSet(fs, &cfg) // Parse max options, even force version option: don't want // to get access to Executor here: it's error-prone to use @@ -297,3 +295,12 @@ func computeConfigSalt(cfg *config.Config) ([]byte, error) { } return h.Sum(nil), nil } + +// --- Related to version but use here. + +type BuildInfo struct { + GoVersion string `json:"goVersion"` + Version string `json:"version"` + Commit string `json:"commit"` + Date string `json:"date"` +} diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 0ae05480fa73..f0d70b784d9c 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "io" "os" "runtime" "runtime/pprof" @@ -158,3 +159,11 @@ func getDefaultConcurrency() int { return runtime.NumCPU() } + +// --- Related to version but use here. + +func printVersion(w io.Writer, buildInfo BuildInfo) error { + _, err := fmt.Fprintf(w, "golangci-lint has version %s built with %s from %s on %s\n", + buildInfo.Version, buildInfo.GoVersion, buildInfo.Commit, buildInfo.Date) + return err +} diff --git a/pkg/commands/version.go b/pkg/commands/version.go deleted file mode 100644 index 18e5716b7e90..000000000000 --- a/pkg/commands/version.go +++ /dev/null @@ -1,90 +0,0 @@ -package commands - -import ( - "encoding/json" - "fmt" - "io" - "os" - "runtime/debug" - "strings" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - - "github.com/golangci/golangci-lint/pkg/config" -) - -type BuildInfo struct { - GoVersion string `json:"goVersion"` - Version string `json:"version"` - Commit string `json:"commit"` - Date string `json:"date"` -} - -type versionInfo struct { - Info BuildInfo - BuildInfo *debug.BuildInfo -} - -func (e *Executor) initVersion() { - versionCmd := &cobra.Command{ - Use: "version", - Short: "Version", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - RunE: e.executeVersion, - } - - fs := versionCmd.Flags() - fs.SortFlags = false // sort them as they are defined here - - initVersionFlagSet(fs, e.cfg) - - e.rootCmd.AddCommand(versionCmd) -} - -func (e *Executor) executeVersion(_ *cobra.Command, _ []string) error { - if e.cfg.Version.Debug { - info, ok := debug.ReadBuildInfo() - if !ok { - return nil - } - - switch strings.ToLower(e.cfg.Version.Format) { - case "json": - return json.NewEncoder(os.Stdout).Encode(versionInfo{ - Info: e.buildInfo, - BuildInfo: info, - }) - - default: - fmt.Println(info.String()) - return printVersion(os.Stdout, e.buildInfo) - } - } - - switch strings.ToLower(e.cfg.Version.Format) { - case "short": - fmt.Println(e.buildInfo.Version) - return nil - - case "json": - return json.NewEncoder(os.Stdout).Encode(e.buildInfo) - - default: - return printVersion(os.Stdout, e.buildInfo) - } -} - -func initVersionFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - // Version config - vc := &cfg.Version - fs.StringVar(&vc.Format, "format", "", wh("The version's format can be: 'short', 'json'")) - fs.BoolVar(&vc.Debug, "debug", false, wh("Add build information")) -} - -func printVersion(w io.Writer, buildInfo BuildInfo) error { - _, err := fmt.Fprintf(w, "golangci-lint has version %s built with %s from %s on %s\n", - buildInfo.Version, buildInfo.GoVersion, buildInfo.Commit, buildInfo.Date) - return err -} From ce6b24cd81ac4e4484f1bad48073da230412f8ce Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:18:45 +0100 Subject: [PATCH 14/42] chore: remove old help command --- pkg/commands/executor.go | 1 - pkg/commands/help.go | 97 ---------------------------------------- pkg/commands/linters.go | 31 +++++++++++++ 3 files changed, 31 insertions(+), 98 deletions(-) delete mode 100644 pkg/commands/help.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 96e1f495d5a8..db562acb26e1 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -84,7 +84,6 @@ func NewExecutor(buildInfo BuildInfo) *Executor { func (e *Executor) initCommands() { e.initRoot() e.initRun() - e.initHelp() e.initLinters() e.initConfig() } diff --git a/pkg/commands/help.go b/pkg/commands/help.go deleted file mode 100644 index 016df2c13a56..000000000000 --- a/pkg/commands/help.go +++ /dev/null @@ -1,97 +0,0 @@ -package commands - -import ( - "fmt" - "sort" - "strings" - - "github.com/fatih/color" - "github.com/spf13/cobra" - - "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" - "github.com/golangci/golangci-lint/pkg/logutils" -) - -func (e *Executor) initHelp() { - helpCmd := &cobra.Command{ - Use: "help", - Short: "Help", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return cmd.Help() - }, - } - - helpCmd.AddCommand(&cobra.Command{ - Use: "linters", - Short: "Help about linters", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - Run: e.executeHelp, - }) - - e.rootCmd.SetHelpCommand(helpCmd) -} - -func (e *Executor) executeHelp(_ *cobra.Command, _ []string) { - var enabledLCs, disabledLCs []*linter.Config - for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { - if lc.Internal { - continue - } - - if lc.EnabledByDefault { - enabledLCs = append(enabledLCs, lc) - } else { - disabledLCs = append(disabledLCs, lc) - } - } - - color.Green("Enabled by default linters:\n") - printLinterConfigs(enabledLCs) - color.Red("\nDisabled by default linters:\n") - printLinterConfigs(disabledLCs) - - color.Green("\nLinters presets:") - for _, p := range lintersdb.AllPresets() { - linters := e.dbManager.GetAllLinterConfigsForPreset(p) - var linterNames []string - for _, lc := range linters { - if lc.Internal { - continue - } - - linterNames = append(linterNames, lc.Name()) - } - sort.Strings(linterNames) - fmt.Fprintf(logutils.StdOut, "%s: %s\n", color.YellowString(p), strings.Join(linterNames, ", ")) - } -} - -func printLinterConfigs(lcs []*linter.Config) { - sort.Slice(lcs, func(i, j int) bool { - return lcs[i].Name() < lcs[j].Name() - }) - for _, lc := range lcs { - altNamesStr := "" - if len(lc.AlternativeNames) != 0 { - altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) - } - - // If the linter description spans multiple lines, truncate everything following the first newline - linterDescription := lc.Linter.Desc() - firstNewline := strings.IndexRune(linterDescription, '\n') - if firstNewline > 0 { - linterDescription = linterDescription[:firstNewline] - } - - deprecatedMark := "" - if lc.IsDeprecated() { - deprecatedMark = " [" + color.RedString("deprecated") + "]" - } - - fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()), - altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) - } -} diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 5ed2353fd546..98c8dde5f6e1 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -2,9 +2,11 @@ package commands import ( "fmt" + "sort" "strings" "github.com/fatih/color" + "github.com/golangci/golangci-lint/pkg/logutils" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -73,3 +75,32 @@ func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) } + +// --- Related to help but use here. + +func printLinterConfigs(lcs []*linter.Config) { + sort.Slice(lcs, func(i, j int) bool { + return lcs[i].Name() < lcs[j].Name() + }) + for _, lc := range lcs { + altNamesStr := "" + if len(lc.AlternativeNames) != 0 { + altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) + } + + // If the linter description spans multiple lines, truncate everything following the first newline + linterDescription := lc.Linter.Desc() + firstNewline := strings.IndexRune(linterDescription, '\n') + if firstNewline > 0 { + linterDescription = linterDescription[:firstNewline] + } + + deprecatedMark := "" + if lc.IsDeprecated() { + deprecatedMark = " [" + color.RedString("deprecated") + "]" + } + + fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()), + altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) + } +} From bad8c148acb0b5a7dbf44a9123b7cb44da5f4a65 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:22:49 +0100 Subject: [PATCH 15/42] chore: remove old config command --- pkg/commands/config.go | 73 ---------------------------------------- pkg/commands/executor.go | 1 - pkg/commands/run.go | 7 ++++ 3 files changed, 7 insertions(+), 74 deletions(-) delete mode 100644 pkg/commands/config.go diff --git a/pkg/commands/config.go b/pkg/commands/config.go deleted file mode 100644 index f269d881bd75..000000000000 --- a/pkg/commands/config.go +++ /dev/null @@ -1,73 +0,0 @@ -package commands - -import ( - "fmt" - "os" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "github.com/spf13/viper" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/fsutils" -) - -func (e *Executor) initConfig() { - configCmd := &cobra.Command{ - Use: "config", - Short: "Config file information", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return cmd.Help() - }, - } - - pathCmd := &cobra.Command{ - Use: "path", - Short: "Print used config path", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - Run: e.executePath, - } - - fs := pathCmd.Flags() - fs.SortFlags = false // sort them as they are defined here - - configCmd.AddCommand(pathCmd) - e.rootCmd.AddCommand(configCmd) -} - -func (e *Executor) executePath(_ *cobra.Command, _ []string) { - usedConfigFile := e.getUsedConfig() - if usedConfigFile == "" { - e.log.Warnf("No config file detected") - os.Exit(exitcodes.NoConfigFileDetected) - } - - fmt.Println(usedConfigFile) -} - -// getUsedConfig returns the resolved path to the golangci config file, -// or the empty string if no configuration could be found. -func (e *Executor) getUsedConfig() string { - usedConfigFile := viper.ConfigFileUsed() - if usedConfigFile == "" { - return "" - } - - prettyUsedConfigFile, err := fsutils.ShortestRelPath(usedConfigFile, "") - if err != nil { - e.log.Warnf("Can't pretty print config file path: %s", err) - return usedConfigFile - } - - return prettyUsedConfigFile -} - -// --- Related to config but not used directly by the config command. - -func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { - fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) - fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) -} diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index db562acb26e1..7fa032d516ef 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -85,7 +85,6 @@ func (e *Executor) initCommands() { e.initRoot() e.initRun() e.initLinters() - e.initConfig() } func (e *Executor) initConfiguration() { diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 6af4e2863837..216c39308441 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -517,3 +517,10 @@ func getDefaultDirectoryExcludeHelp() string { parts = append(parts, "") return strings.Join(parts, "\n") } + +// --- Related to config but use here. + +func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { + fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) + fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) +} From e24e2949e8a3bfc6fb4b57f243ad8e6c11618bdd Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:25:41 +0100 Subject: [PATCH 16/42] chore: remove old linters command --- pkg/commands/executor.go | 1 - pkg/commands/linters.go | 106 --------------------------------------- pkg/commands/run.go | 13 +++++ 3 files changed, 13 insertions(+), 107 deletions(-) delete mode 100644 pkg/commands/linters.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 7fa032d516ef..855ada6b5160 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -84,7 +84,6 @@ func NewExecutor(buildInfo BuildInfo) *Executor { func (e *Executor) initCommands() { e.initRoot() e.initRun() - e.initLinters() } func (e *Executor) initConfiguration() { diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go deleted file mode 100644 index 98c8dde5f6e1..000000000000 --- a/pkg/commands/linters.go +++ /dev/null @@ -1,106 +0,0 @@ -package commands - -import ( - "fmt" - "sort" - "strings" - - "github.com/fatih/color" - "github.com/golangci/golangci-lint/pkg/logutils" - "github.com/spf13/cobra" - "github.com/spf13/pflag" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/lint/linter" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" -) - -func (e *Executor) initLinters() { - lintersCmd := &cobra.Command{ - Use: "linters", - Short: "List current linters configuration", - Args: cobra.NoArgs, - ValidArgsFunction: cobra.NoFileCompletions, - RunE: e.executeLinters, - } - - fs := lintersCmd.Flags() - fs.SortFlags = false // sort them as they are defined here - - initConfigFileFlagSet(fs, &e.cfg.Run) - initLintersFlagSet(fs, &e.cfg.Linters) - - e.rootCmd.AddCommand(lintersCmd) - - e.lintersCmd = lintersCmd -} - -// executeLinters runs the 'linters' CLI command, which displays the supported linters. -func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { - enabledLintersMap, err := e.enabledLintersSet.GetEnabledLintersMap() - if err != nil { - return fmt.Errorf("can't get enabled linters: %w", err) - } - - var enabledLinters []*linter.Config - var disabledLCs []*linter.Config - - for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { - if lc.Internal { - continue - } - - if enabledLintersMap[lc.Name()] == nil { - disabledLCs = append(disabledLCs, lc) - } else { - enabledLinters = append(enabledLinters, lc) - } - } - - color.Green("Enabled by your configuration linters:\n") - printLinterConfigs(enabledLinters) - color.Red("\nDisabled by your configuration linters:\n") - printLinterConfigs(disabledLCs) - - return nil -} - -func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { - fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) - fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) - fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) - fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) - fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) - fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, - wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ - "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) -} - -// --- Related to help but use here. - -func printLinterConfigs(lcs []*linter.Config) { - sort.Slice(lcs, func(i, j int) bool { - return lcs[i].Name() < lcs[j].Name() - }) - for _, lc := range lcs { - altNamesStr := "" - if len(lc.AlternativeNames) != 0 { - altNamesStr = fmt.Sprintf(" (%s)", strings.Join(lc.AlternativeNames, ", ")) - } - - // If the linter description spans multiple lines, truncate everything following the first newline - linterDescription := lc.Linter.Desc() - firstNewline := strings.IndexRune(linterDescription, '\n') - if firstNewline > 0 { - linterDescription = linterDescription[:firstNewline] - } - - deprecatedMark := "" - if lc.IsDeprecated() { - deprecatedMark = " [" + color.RedString("deprecated") + "]" - } - - fmt.Fprintf(logutils.StdOut, "%s%s%s: %s [fast: %t, auto-fix: %t]\n", color.YellowString(lc.Name()), - altNamesStr, deprecatedMark, linterDescription, !lc.IsSlowLinter(), lc.CanAutoFix) - } -} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 216c39308441..e59c6364d004 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -524,3 +524,16 @@ func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) } + +// --- Related to linters but use here. + +func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { + fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) + fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) + fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) + fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) + fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) + fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, + wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ + "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) +} From 86aae641750c09f87e19447b5f030b661e9bd31b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:31:12 +0100 Subject: [PATCH 17/42] chore: remove old run command --- pkg/commands/executor.go | 135 +++++++++- pkg/commands/run.go | 539 --------------------------------------- 2 files changed, 134 insertions(+), 540 deletions(-) delete mode 100644 pkg/commands/run.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 855ada6b5160..5f398cdcd5a8 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -12,6 +12,8 @@ import ( "github.com/fatih/color" "github.com/gofrs/flock" + "github.com/golangci/golangci-lint/pkg/exitcodes" + "github.com/golangci/golangci-lint/pkg/packages" "github.com/spf13/cobra" "github.com/spf13/pflag" "gopkg.in/yaml.v3" @@ -83,7 +85,6 @@ func NewExecutor(buildInfo BuildInfo) *Executor { func (e *Executor) initCommands() { e.initRoot() - e.initRun() } func (e *Executor) initConfiguration() { @@ -301,3 +302,135 @@ type BuildInfo struct { Commit string `json:"commit"` Date string `json:"date"` } + +// --- Related to run but use here. + +//nolint:gomnd +func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { + fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) + if err := fs.MarkHidden("internal-cmd-test"); err != nil { + panic(err) + } + + // --- Output config + + oc := &cfg.Output + fs.StringVar(&oc.Format, "out-format", + config.OutFormatColoredLineNumber, + wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) + fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue")) + fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line")) + fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line")) + fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) + fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) + fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) + + // --- Run config + + rc := &cfg.Run + + // Config file config + initConfigFileFlagSet(fs, rc) + + fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", + wh("Modules download mode. If not empty, passed as -mod= to go tools")) + fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", + exitcodes.IssuesFound, wh("Exit code when issues were found")) + fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) + fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) + + fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) + + fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) + fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, + wh("Print avg and max memory usage of golangci-lint and total time")) + fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) + fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) + fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) + + const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + + "If false (default) - golangci-lint acquires file lock on start." + fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc)) + const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + + "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." + fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) + fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) + + // --- Linters config + + lc := &cfg.Linters + initLintersFlagSet(fs, lc) + + // --- Issues config + + ic := &cfg.Issues + fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) + fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) + fs.BoolVar(&ic.ExcludeCaseSensitive, "exclude-case-sensitive", false, wh("If set to true exclude "+ + "and exclude rules regular expressions are case sensitive")) + + fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, + wh("Maximum issues count per one linter. Set to 0 to disable")) + fs.IntVar(&ic.MaxSameIssues, "max-same-issues", 3, + wh("Maximum count of issues with the same text. Set to 0 to disable")) + + fs.BoolVarP(&ic.Diff, "new", "n", false, + wh("Show only new issues: if there are unstaged changes or untracked files, only those changes "+ + "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration "+ + "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at "+ + "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer "+ + "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate "+ + "unstaged files before golangci-lint runs.")) + fs.StringVar(&ic.DiffFromRevision, "new-from-rev", "", + wh("Show only new issues created after git revision `REV`")) + fs.StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", + wh("Show only new issues created in git patch with file path `PATH`")) + fs.BoolVar(&ic.WholeFiles, "whole-files", false, + wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) + fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) +} + +// --- Related to config but use here. + +func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { + fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) + fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) +} + +// --- Related to linters but use here. + +func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { + fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) + fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) + fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) + fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) + fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) + fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, + wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ + "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) +} + +// --- Related to run but use here. + +const defaultTimeout = time.Minute + +func getDefaultIssueExcludeHelp() string { + parts := []string{color.GreenString("Use or not use default excludes:")} + for _, ep := range config.DefaultExcludePatterns { + parts = append(parts, + fmt.Sprintf(" # %s %s: %s", ep.ID, ep.Linter, ep.Why), + fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), + "", + ) + } + return strings.Join(parts, "\n") +} + +func getDefaultDirectoryExcludeHelp() string { + parts := []string{color.GreenString("Use or not use default excluded directories:")} + for _, dir := range packages.StdExcludeDirRegexps { + parts = append(parts, fmt.Sprintf(" - %s", color.YellowString(dir))) + } + parts = append(parts, "") + return strings.Join(parts, "\n") +} diff --git a/pkg/commands/run.go b/pkg/commands/run.go deleted file mode 100644 index e59c6364d004..000000000000 --- a/pkg/commands/run.go +++ /dev/null @@ -1,539 +0,0 @@ -package commands - -import ( - "context" - "errors" - "fmt" - "io" - "log" - "os" - "path/filepath" - "runtime" - "sort" - "strings" - "time" - - "github.com/fatih/color" - "github.com/gofrs/flock" - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "golang.org/x/exp/maps" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/lint" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" - "github.com/golangci/golangci-lint/pkg/logutils" - "github.com/golangci/golangci-lint/pkg/packages" - "github.com/golangci/golangci-lint/pkg/printers" - "github.com/golangci/golangci-lint/pkg/result" -) - -const defaultFileMode = 0644 - -const defaultTimeout = time.Minute - -const ( - // envFailOnWarnings value: "1" - envFailOnWarnings = "FAIL_ON_WARNINGS" - // envMemLogEvery value: "1" - envMemLogEvery = "GL_MEM_LOG_EVERY" -) - -func (e *Executor) initRun() { - runCmd := &cobra.Command{ - Use: "run", - Short: "Run the linters", - Run: e.executeRun, - PreRunE: func(_ *cobra.Command, _ []string) error { - if ok := e.acquireFileLock(); !ok { - return errors.New("parallel golangci-lint is running") - } - return nil - }, - PostRun: func(_ *cobra.Command, _ []string) { - e.releaseFileLock() - }, - } - - runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals - runCmd.SetErr(logutils.StdErr) - - fs := runCmd.Flags() - fs.SortFlags = false // sort them as they are defined here - - initRunFlagSet(fs, e.cfg) - - e.rootCmd.AddCommand(runCmd) - - e.runCmd = runCmd -} - -// executeRun executes the 'run' CLI command, which runs the linters. -func (e *Executor) executeRun(_ *cobra.Command, args []string) { - needTrackResources := e.cfg.Run.IsVerbose || e.cfg.Run.PrintResourcesUsage - trackResourcesEndCh := make(chan struct{}) - defer func() { // XXX: this defer must be before ctx.cancel defer - if needTrackResources { // wait until resource tracking finished to print properly - <-trackResourcesEndCh - } - }() - - ctx, cancel := context.WithTimeout(context.Background(), e.cfg.Run.Timeout) - defer cancel() - - if needTrackResources { - go watchResources(ctx, trackResourcesEndCh, e.log, e.debugf) - } - - if err := e.runAndPrint(ctx, args); err != nil { - e.log.Errorf("Running error: %s", err) - if e.exitCode == exitcodes.Success { - var exitErr *exitcodes.ExitError - if errors.As(err, &exitErr) { - e.exitCode = exitErr.Code - } else { - e.exitCode = exitcodes.Failure - } - } - } - - e.setupExitCode(ctx) -} - -func (e *Executor) runAndPrint(ctx context.Context, args []string) error { - if err := e.goenv.Discover(ctx); err != nil { - e.log.Warnf("Failed to discover go env: %s", err) - } - - if !logutils.HaveDebugTag(logutils.DebugKeyLintersOutput) { - // Don't allow linters and loader to print anything - log.SetOutput(io.Discard) - savedStdout, savedStderr := e.setOutputToDevNull() - defer func() { - os.Stdout, os.Stderr = savedStdout, savedStderr - }() - } - - issues, err := e.runAnalysis(ctx, args) - if err != nil { - return err // XXX: don't loose type - } - - formats := strings.Split(e.cfg.Output.Format, ",") - for _, format := range formats { - out := strings.SplitN(format, ":", 2) - if len(out) < 2 { - out = append(out, "") - } - - err := e.printReports(issues, out[1], out[0]) - if err != nil { - return err - } - } - - e.printStats(issues) - - e.setExitCodeIfIssuesFound(issues) - - e.fileCache.PrintStats(e.log) - - return nil -} - -// runAnalysis executes the linters that have been enabled in the configuration. -func (e *Executor) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { - e.cfg.Run.Args = args - - lintersToRun, err := e.enabledLintersSet.GetOptimizedLinters() - if err != nil { - return nil, err - } - - enabledLintersMap, err := e.enabledLintersSet.GetEnabledLintersMap() - if err != nil { - return nil, err - } - - for _, lc := range e.dbManager.GetAllSupportedLinterConfigs() { - isEnabled := enabledLintersMap[lc.Name()] != nil - e.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault) - } - - lintCtx, err := e.contextLoader.Load(ctx, lintersToRun) - if err != nil { - return nil, fmt.Errorf("context loading failed: %w", err) - } - lintCtx.Log = e.log.Child(logutils.DebugKeyLintersContext) - - runner, err := lint.NewRunner(e.cfg, e.log.Child(logutils.DebugKeyRunner), - e.goenv, e.enabledLintersSet, e.lineCache, e.fileCache, e.dbManager, lintCtx.Packages) - if err != nil { - return nil, err - } - - return runner.Run(ctx, lintersToRun, lintCtx) -} - -func (e *Executor) setOutputToDevNull() (savedStdout, savedStderr *os.File) { - savedStdout, savedStderr = os.Stdout, os.Stderr - devNull, err := os.Open(os.DevNull) - if err != nil { - e.log.Warnf("Can't open null device %q: %s", os.DevNull, err) - return - } - - os.Stdout, os.Stderr = devNull, devNull - return -} - -func (e *Executor) setExitCodeIfIssuesFound(issues []result.Issue) { - if len(issues) != 0 { - e.exitCode = e.cfg.Run.ExitCodeIfIssuesFound - } -} - -func (e *Executor) printReports(issues []result.Issue, path, format string) error { - w, shouldClose, err := e.createWriter(path) - if err != nil { - return fmt.Errorf("can't create output for %s: %w", path, err) - } - - p, err := e.createPrinter(format, w) - if err != nil { - if file, ok := w.(io.Closer); shouldClose && ok { - _ = file.Close() - } - return err - } - - if err = p.Print(issues); err != nil { - if file, ok := w.(io.Closer); shouldClose && ok { - _ = file.Close() - } - return fmt.Errorf("can't print %d issues: %w", len(issues), err) - } - - if file, ok := w.(io.Closer); shouldClose && ok { - _ = file.Close() - } - - return nil -} - -func (e *Executor) createWriter(path string) (io.Writer, bool, error) { - if path == "" || path == "stdout" { - return logutils.StdOut, false, nil - } - if path == "stderr" { - return logutils.StdErr, false, nil - } - f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, defaultFileMode) - if err != nil { - return nil, false, err - } - return f, true, nil -} - -func (e *Executor) createPrinter(format string, w io.Writer) (printers.Printer, error) { - var p printers.Printer - switch format { - case config.OutFormatJSON: - p = printers.NewJSON(&e.reportData, w) - case config.OutFormatColoredLineNumber, config.OutFormatLineNumber: - p = printers.NewText(e.cfg.Output.PrintIssuedLine, - format == config.OutFormatColoredLineNumber, e.cfg.Output.PrintLinterName, - e.log.Child(logutils.DebugKeyTextPrinter), w) - case config.OutFormatTab, config.OutFormatColoredTab: - p = printers.NewTab(e.cfg.Output.PrintLinterName, - format == config.OutFormatColoredTab, - e.log.Child(logutils.DebugKeyTabPrinter), w) - case config.OutFormatCheckstyle: - p = printers.NewCheckstyle(w) - case config.OutFormatCodeClimate: - p = printers.NewCodeClimate(w) - case config.OutFormatHTML: - p = printers.NewHTML(w) - case config.OutFormatJunitXML: - p = printers.NewJunitXML(w) - case config.OutFormatGithubActions: - p = printers.NewGithub(w) - case config.OutFormatTeamCity: - p = printers.NewTeamCity(w) - default: - return nil, fmt.Errorf("unknown output format %s", format) - } - - return p, nil -} - -func (e *Executor) printStats(issues []result.Issue) { - if !e.cfg.Run.ShowStats { - return - } - - if len(issues) == 0 { - e.runCmd.Println("0 issues.") - return - } - - stats := map[string]int{} - for idx := range issues { - stats[issues[idx].FromLinter]++ - } - - e.runCmd.Printf("%d issues:\n", len(issues)) - - keys := maps.Keys(stats) - sort.Strings(keys) - - for _, key := range keys { - e.runCmd.Printf("* %s: %d\n", key, stats[key]) - } -} - -func (e *Executor) setupExitCode(ctx context.Context) { - if ctx.Err() != nil { - e.exitCode = exitcodes.Timeout - e.log.Errorf("Timeout exceeded: try increasing it by passing --timeout option") - return - } - - if e.exitCode != exitcodes.Success { - return - } - - needFailOnWarnings := os.Getenv(lintersdb.EnvTestRun) == "1" || os.Getenv(envFailOnWarnings) == "1" - if needFailOnWarnings && len(e.reportData.Warnings) != 0 { - e.exitCode = exitcodes.WarningInTest - return - } - - if e.reportData.Error != "" { - // it's a case e.g. when typecheck linter couldn't parse and error and just logged it - e.exitCode = exitcodes.ErrorWasLogged - return - } -} - -func (e *Executor) acquireFileLock() bool { - if e.cfg.Run.AllowParallelRunners { - e.debugf("Parallel runners are allowed, no locking") - return true - } - - lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock") - e.debugf("Locking on file %s...", lockFile) - f := flock.New(lockFile) - const retryDelay = time.Second - - ctx := context.Background() - if !e.cfg.Run.AllowSerialRunners { - const totalTimeout = 5 * time.Second - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, totalTimeout) - defer cancel() - } - if ok, _ := f.TryLockContext(ctx, retryDelay); !ok { - return false - } - - e.flock = f - return true -} - -func (e *Executor) releaseFileLock() { - if e.cfg.Run.AllowParallelRunners { - return - } - - if err := e.flock.Unlock(); err != nil { - e.debugf("Failed to unlock on file: %s", err) - } - if err := os.Remove(e.flock.Path()); err != nil { - e.debugf("Failed to remove lock file: %s", err) - } -} - -//nolint:gomnd -func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) - if err := fs.MarkHidden("internal-cmd-test"); err != nil { - panic(err) - } - - // --- Output config - - oc := &cfg.Output - fs.StringVar(&oc.Format, "out-format", - config.OutFormatColoredLineNumber, - wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) - fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue")) - fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line")) - fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line")) - fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) - fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) - fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) - - // --- Run config - - rc := &cfg.Run - - // Config file config - initConfigFileFlagSet(fs, rc) - - fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", - wh("Modules download mode. If not empty, passed as -mod= to go tools")) - fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", - exitcodes.IssuesFound, wh("Exit code when issues were found")) - fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) - fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) - - fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) - - fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) - fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, - wh("Print avg and max memory usage of golangci-lint and total time")) - fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) - fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) - fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) - - const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + - "If false (default) - golangci-lint acquires file lock on start." - fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc)) - const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + - "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." - fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) - fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) - - // --- Linters config - - lc := &cfg.Linters - initLintersFlagSet(fs, lc) - - // --- Issues config - - ic := &cfg.Issues - fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) - fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) - fs.BoolVar(&ic.ExcludeCaseSensitive, "exclude-case-sensitive", false, wh("If set to true exclude "+ - "and exclude rules regular expressions are case sensitive")) - - fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, - wh("Maximum issues count per one linter. Set to 0 to disable")) - fs.IntVar(&ic.MaxSameIssues, "max-same-issues", 3, - wh("Maximum count of issues with the same text. Set to 0 to disable")) - - fs.BoolVarP(&ic.Diff, "new", "n", false, - wh("Show only new issues: if there are unstaged changes or untracked files, only those changes "+ - "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration "+ - "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at "+ - "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer "+ - "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate "+ - "unstaged files before golangci-lint runs.")) - fs.StringVar(&ic.DiffFromRevision, "new-from-rev", "", - wh("Show only new issues created after git revision `REV`")) - fs.StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", - wh("Show only new issues created in git patch with file path `PATH`")) - fs.BoolVar(&ic.WholeFiles, "whole-files", false, - wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) - fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) -} - -func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log, debugf logutils.DebugFunc) { - startedAt := time.Now() - debugf("Started tracking time") - - var maxRSSMB, totalRSSMB float64 - var iterationsCount int - - const intervalMS = 100 - ticker := time.NewTicker(intervalMS * time.Millisecond) - defer ticker.Stop() - - logEveryRecord := os.Getenv(envMemLogEvery) == "1" - const MB = 1024 * 1024 - - track := func() { - var m runtime.MemStats - runtime.ReadMemStats(&m) - - if logEveryRecord { - debugf("Stopping memory tracing iteration, printing ...") - printMemStats(&m, logger) - } - - rssMB := float64(m.Sys) / MB - if rssMB > maxRSSMB { - maxRSSMB = rssMB - } - totalRSSMB += rssMB - iterationsCount++ - } - - for { - track() - - stop := false - select { - case <-ctx.Done(): - stop = true - debugf("Stopped resources tracking") - case <-ticker.C: - } - - if stop { - break - } - } - track() - - avgRSSMB := totalRSSMB / float64(iterationsCount) - - logger.Infof("Memory: %d samples, avg is %.1fMB, max is %.1fMB", - iterationsCount, avgRSSMB, maxRSSMB) - logger.Infof("Execution took %s", time.Since(startedAt)) - close(done) -} - -func getDefaultIssueExcludeHelp() string { - parts := []string{color.GreenString("Use or not use default excludes:")} - for _, ep := range config.DefaultExcludePatterns { - parts = append(parts, - fmt.Sprintf(" # %s %s: %s", ep.ID, ep.Linter, ep.Why), - fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), - "", - ) - } - return strings.Join(parts, "\n") -} - -func getDefaultDirectoryExcludeHelp() string { - parts := []string{color.GreenString("Use or not use default excluded directories:")} - for _, dir := range packages.StdExcludeDirRegexps { - parts = append(parts, fmt.Sprintf(" - %s", color.YellowString(dir))) - } - parts = append(parts, "") - return strings.Join(parts, "\n") -} - -// --- Related to config but use here. - -func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { - fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) - fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) -} - -// --- Related to linters but use here. - -func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { - fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) - fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) - fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) - fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) - fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) - fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, - wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ - "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) -} From d3945332641fec61db61cbe49bbebc5243fca549 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:33:53 +0100 Subject: [PATCH 18/42] chore: remove old root command --- pkg/commands/executor.go | 34 +++++++- pkg/commands/root.go | 169 --------------------------------------- 2 files changed, 31 insertions(+), 172 deletions(-) delete mode 100644 pkg/commands/root.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go index 5f398cdcd5a8..4050e2ded4b6 100644 --- a/pkg/commands/executor.go +++ b/pkg/commands/executor.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "runtime" "strings" "time" @@ -83,9 +84,7 @@ func NewExecutor(buildInfo BuildInfo) *Executor { return e } -func (e *Executor) initCommands() { - e.initRoot() -} +func (e *Executor) initCommands() {} func (e *Executor) initConfiguration() { // to set up log level early we need to parse config from command line extra time to find `-v` option. @@ -434,3 +433,32 @@ func getDefaultDirectoryExcludeHelp() string { parts = append(parts, "") return strings.Join(parts, "\n") } + +// --- Related to root but use here. + +// envHelpRun value: "1". +const envHelpRun = "HELP_RUN" + +func getDefaultConcurrency() int { + if os.Getenv(envHelpRun) == "1" { + // Make stable concurrency for generating help documentation. + const prettyConcurrency = 8 + return prettyConcurrency + } + + return runtime.NumCPU() +} + +func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config) { + fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) + fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) + + fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) + fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) + fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) + + fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), + wh("Number of CPUs to use (Default: number of logical CPUs)")) + + fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) +} diff --git a/pkg/commands/root.go b/pkg/commands/root.go deleted file mode 100644 index f0d70b784d9c..000000000000 --- a/pkg/commands/root.go +++ /dev/null @@ -1,169 +0,0 @@ -package commands - -import ( - "fmt" - "io" - "os" - "runtime" - "runtime/pprof" - "runtime/trace" - "strconv" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/logutils" -) - -const ( - // envHelpRun value: "1". - envHelpRun = "HELP_RUN" - envMemProfileRate = "GL_MEM_PROFILE_RATE" -) - -func (e *Executor) initRoot() { - rootCmd := &cobra.Command{ - Use: "golangci-lint", - Short: "golangci-lint is a smart linters runner.", - Long: `Smart, fast linters runner.`, - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, _ []string) error { - return cmd.Help() - }, - PersistentPreRunE: e.persistentPreRun, - PersistentPostRunE: e.persistentPostRun, - } - - initRootFlagSet(rootCmd.PersistentFlags(), e.cfg) - - e.rootCmd = rootCmd -} - -func (e *Executor) persistentPreRun(_ *cobra.Command, _ []string) error { - if e.cfg.Run.PrintVersion { - _ = printVersion(logutils.StdOut, e.buildInfo) - os.Exit(exitcodes.Success) // a return nil is not enough to stop the process because we are inside the `preRun`. - } - - runtime.GOMAXPROCS(e.cfg.Run.Concurrency) - - if e.cfg.Run.CPUProfilePath != "" { - f, err := os.Create(e.cfg.Run.CPUProfilePath) - if err != nil { - return fmt.Errorf("can't create file %s: %w", e.cfg.Run.CPUProfilePath, err) - } - if err := pprof.StartCPUProfile(f); err != nil { - return fmt.Errorf("can't start CPU profiling: %w", err) - } - } - - if e.cfg.Run.MemProfilePath != "" { - if rate := os.Getenv(envMemProfileRate); rate != "" { - runtime.MemProfileRate, _ = strconv.Atoi(rate) - } - } - - if e.cfg.Run.TracePath != "" { - f, err := os.Create(e.cfg.Run.TracePath) - if err != nil { - return fmt.Errorf("can't create file %s: %w", e.cfg.Run.TracePath, err) - } - if err = trace.Start(f); err != nil { - return fmt.Errorf("can't start tracing: %w", err) - } - } - - return nil -} - -func (e *Executor) persistentPostRun(_ *cobra.Command, _ []string) error { - if e.cfg.Run.CPUProfilePath != "" { - pprof.StopCPUProfile() - } - - if e.cfg.Run.MemProfilePath != "" { - f, err := os.Create(e.cfg.Run.MemProfilePath) - if err != nil { - return fmt.Errorf("can't create file %s: %w", e.cfg.Run.MemProfilePath, err) - } - - var ms runtime.MemStats - runtime.ReadMemStats(&ms) - printMemStats(&ms, e.log) - - if err := pprof.WriteHeapProfile(f); err != nil { - return fmt.Errorf("can't write heap profile: %w", err) - } - _ = f.Close() - } - - if e.cfg.Run.TracePath != "" { - trace.Stop() - } - - os.Exit(e.exitCode) - - return nil -} - -func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) - fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) - - fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) - fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) - fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) - - fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), - wh("Number of CPUs to use (Default: number of logical CPUs)")) - - fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) -} - -func printMemStats(ms *runtime.MemStats, logger logutils.Log) { - logger.Infof("Mem stats: alloc=%s total_alloc=%s sys=%s "+ - "heap_alloc=%s heap_sys=%s heap_idle=%s heap_released=%s heap_in_use=%s "+ - "stack_in_use=%s stack_sys=%s "+ - "mspan_sys=%s mcache_sys=%s buck_hash_sys=%s gc_sys=%s other_sys=%s "+ - "mallocs_n=%d frees_n=%d heap_objects_n=%d gc_cpu_fraction=%.2f", - formatMemory(ms.Alloc), formatMemory(ms.TotalAlloc), formatMemory(ms.Sys), - formatMemory(ms.HeapAlloc), formatMemory(ms.HeapSys), - formatMemory(ms.HeapIdle), formatMemory(ms.HeapReleased), formatMemory(ms.HeapInuse), - formatMemory(ms.StackInuse), formatMemory(ms.StackSys), - formatMemory(ms.MSpanSys), formatMemory(ms.MCacheSys), formatMemory(ms.BuckHashSys), - formatMemory(ms.GCSys), formatMemory(ms.OtherSys), - ms.Mallocs, ms.Frees, ms.HeapObjects, ms.GCCPUFraction) -} - -func formatMemory(memBytes uint64) string { - const Kb = 1024 - const Mb = Kb * 1024 - - if memBytes < Kb { - return fmt.Sprintf("%db", memBytes) - } - if memBytes < Mb { - return fmt.Sprintf("%dkb", memBytes/Kb) - } - return fmt.Sprintf("%dmb", memBytes/Mb) -} - -func getDefaultConcurrency() int { - if os.Getenv(envHelpRun) == "1" { - // Make stable concurrency for generating help documentation. - const prettyConcurrency = 8 - return prettyConcurrency - } - - return runtime.NumCPU() -} - -// --- Related to version but use here. - -func printVersion(w io.Writer, buildInfo BuildInfo) error { - _, err := fmt.Fprintf(w, "golangci-lint has version %s built with %s from %s on %s\n", - buildInfo.Version, buildInfo.GoVersion, buildInfo.Commit, buildInfo.Date) - return err -} From 22057e43b5c159fc027829182d27c1e3518c0846 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:34:41 +0100 Subject: [PATCH 19/42] chore: death of the Executor --- pkg/commands/executor.go | 464 --------------------------------------- 1 file changed, 464 deletions(-) delete mode 100644 pkg/commands/executor.go diff --git a/pkg/commands/executor.go b/pkg/commands/executor.go deleted file mode 100644 index 4050e2ded4b6..000000000000 --- a/pkg/commands/executor.go +++ /dev/null @@ -1,464 +0,0 @@ -package commands - -import ( - "bytes" - "crypto/sha256" - "errors" - "fmt" - "io" - "os" - "runtime" - "strings" - "time" - - "github.com/fatih/color" - "github.com/gofrs/flock" - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/packages" - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "gopkg.in/yaml.v3" - - "github.com/golangci/golangci-lint/internal/cache" - "github.com/golangci/golangci-lint/internal/pkgcache" - "github.com/golangci/golangci-lint/pkg/config" - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/golinters/goanalysis/load" - "github.com/golangci/golangci-lint/pkg/goutil" - "github.com/golangci/golangci-lint/pkg/lint" - "github.com/golangci/golangci-lint/pkg/lint/lintersdb" - "github.com/golangci/golangci-lint/pkg/logutils" - "github.com/golangci/golangci-lint/pkg/report" - "github.com/golangci/golangci-lint/pkg/timeutils" -) - -type Executor struct { - rootCmd *cobra.Command - - runCmd *cobra.Command // used by fixSlicesFlags, printStats - lintersCmd *cobra.Command // used by fixSlicesFlags - - exitCode int - - buildInfo BuildInfo - - cfg *config.Config // cfg is the unmarshaled data from the golangci config file. - - log logutils.Log - debugf logutils.DebugFunc - reportData report.Data - - dbManager *lintersdb.Manager - enabledLintersSet *lintersdb.EnabledSet - - contextLoader *lint.ContextLoader - goenv *goutil.Env - - fileCache *fsutils.FileCache - lineCache *fsutils.LineCache - - flock *flock.Flock -} - -// NewExecutor creates and initializes a new command executor. -func NewExecutor(buildInfo BuildInfo) *Executor { - e := &Executor{ - cfg: config.NewDefault(), - buildInfo: buildInfo, - debugf: logutils.Debug(logutils.DebugKeyExec), - } - - e.log = report.NewLogWrapper(logutils.NewStderrLog(logutils.DebugKeyEmpty), &e.reportData) - - // init of commands must be done before config file reading because init sets config with the default values of flags. - e.initCommands() - - startedAt := time.Now() - e.debugf("Starting execution...") - - e.initConfiguration() - e.initExecutor() - - e.debugf("Initialized executor in %s", time.Since(startedAt)) - - return e -} - -func (e *Executor) initCommands() {} - -func (e *Executor) initConfiguration() { - // to set up log level early we need to parse config from command line extra time to find `-v` option. - commandLineCfg, err := getConfigForCommandLine() - if err != nil && !errors.Is(err, pflag.ErrHelp) { - e.log.Fatalf("Can't get config for command line: %s", err) - } - if commandLineCfg != nil { - logutils.SetupVerboseLog(e.log, commandLineCfg.Run.IsVerbose) - - switch commandLineCfg.Output.Color { - case "always": - color.NoColor = false - case "never": - color.NoColor = true - case "auto": - // nothing - default: - e.log.Fatalf("invalid value %q for --color; must be 'always', 'auto', or 'never'", commandLineCfg.Output.Color) - } - } - - // init e.cfg by values from config: flags parse will see these values like the default ones. - // It will overwrite them only if the same option is found in command-line: it's ok, command-line has higher priority. - - r := config.NewFileReader(e.cfg, commandLineCfg, e.log.Child(logutils.DebugKeyConfigReader)) - if err = r.Read(); err != nil { - e.log.Fatalf("Can't read config: %s", err) - } - - 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() - } - - // Slice options must be explicitly set for proper merging of config and command-line options. - fixSlicesFlags(e.runCmd.Flags()) - fixSlicesFlags(e.lintersCmd.Flags()) -} - -func (e *Executor) initExecutor() { - e.dbManager = lintersdb.NewManager(e.cfg, e.log) - - e.enabledLintersSet = lintersdb.NewEnabledSet(e.dbManager, - lintersdb.NewValidator(e.dbManager), e.log.Child(logutils.DebugKeyLintersDB), e.cfg) - - e.goenv = goutil.NewEnv(e.log.Child(logutils.DebugKeyGoEnv)) - - e.fileCache = fsutils.NewFileCache() - e.lineCache = fsutils.NewLineCache(e.fileCache) - - sw := timeutils.NewStopwatch("pkgcache", e.log.Child(logutils.DebugKeyStopwatch)) - - pkgCache, err := pkgcache.NewCache(sw, e.log.Child(logutils.DebugKeyPkgCache)) - if err != nil { - e.log.Fatalf("Failed to build packages cache: %s", err) - } - - e.contextLoader = lint.NewContextLoader(e.cfg, e.log.Child(logutils.DebugKeyLoader), e.goenv, - e.lineCache, e.fileCache, pkgCache, load.NewGuard()) - - if err = initHashSalt(e.buildInfo.Version, e.cfg); err != nil { - e.log.Fatalf("Failed to init hash salt: %s", err) - } -} - -func (e *Executor) Execute() error { - return e.rootCmd.Execute() -} - -func getConfigForCommandLine() (*config.Config, error) { - // We use another pflag.FlagSet here to not set `changed` flag - // on cmd.Flags() options. Otherwise, string slice options will be duplicated. - fs := pflag.NewFlagSet("config flag set", pflag.ContinueOnError) - - var cfg config.Config - // Don't do `fs.AddFlagSet(cmd.Flags())` because it shares flags representations: - // `changed` variable inside string slice vars will be shared. - // Use another config variable here, not e.cfg, to not - // affect main parsing by this parsing of only config option. - initRunFlagSet(fs, &cfg) - - // Parse max options, even force version option: don't want - // to get access to Executor here: it's error-prone to use - // cfg vs e.cfg. - initRootFlagSet(fs, &cfg) - - fs.Usage = func() {} // otherwise, help text will be printed twice - if err := fs.Parse(os.Args); err != nil { - if errors.Is(err, pflag.ErrHelp) { - return nil, err - } - - return nil, fmt.Errorf("can't parse args: %w", err) - } - - return &cfg, nil -} - -func fixSlicesFlags(fs *pflag.FlagSet) { - // It's a dirty hack to set flag.Changed to true for every string slice flag. - // It's necessary to merge config and command-line slices: otherwise command-line - // flags will always overwrite ones from the config. - fs.VisitAll(func(f *pflag.Flag) { - if f.Value.Type() != "stringSlice" { - return - } - - s, err := fs.GetStringSlice(f.Name) - if err != nil { - return - } - - if s == nil { // assume that every string slice flag has nil as the default - return - } - - var safe []string - for _, v := range s { - // add quotes to escape comma because spf13/pflag use a CSV parser: - // https://github.com/spf13/pflag/blob/85dd5c8bc61cfa382fecd072378089d4e856579d/string_slice.go#L43 - safe = append(safe, `"`+v+`"`) - } - - // calling Set sets Changed to true: next Set calls will append, not overwrite - _ = f.Value.Set(strings.Join(safe, ",")) - }) -} - -func wh(text string) string { - return color.GreenString(text) -} - -// --- Related to cache but not used directly by the cache command. - -func initHashSalt(version string, cfg *config.Config) error { - binSalt, err := computeBinarySalt(version) - if err != nil { - return fmt.Errorf("failed to calculate binary salt: %w", err) - } - - configSalt, err := computeConfigSalt(cfg) - if err != nil { - return fmt.Errorf("failed to calculate config salt: %w", err) - } - - b := bytes.NewBuffer(binSalt) - b.Write(configSalt) - cache.SetSalt(b.Bytes()) - return nil -} - -func computeBinarySalt(version string) ([]byte, error) { - if version != "" && version != "(devel)" { - return []byte(version), nil - } - - if logutils.HaveDebugTag(logutils.DebugKeyBinSalt) { - return []byte("debug"), nil - } - - p, err := os.Executable() - if err != nil { - return nil, err - } - f, err := os.Open(p) - if err != nil { - return nil, err - } - defer f.Close() - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} - -// computeConfigSalt computes configuration hash. -// We don't hash all config fields to reduce meaningless cache invalidations. -// At least, it has a huge impact on tests speed. -// Fields: `LintersSettings` and `Run.BuildTags`. -func computeConfigSalt(cfg *config.Config) ([]byte, error) { - lintersSettingsBytes, err := yaml.Marshal(cfg.LintersSettings) - if err != nil { - return nil, fmt.Errorf("failed to json marshal config linter settings: %w", err) - } - - configData := bytes.NewBufferString("linters-settings=") - configData.Write(lintersSettingsBytes) - configData.WriteString("\nbuild-tags=%s" + strings.Join(cfg.Run.BuildTags, ",")) - - h := sha256.New() - if _, err := h.Write(configData.Bytes()); err != nil { - return nil, err - } - return h.Sum(nil), nil -} - -// --- Related to version but use here. - -type BuildInfo struct { - GoVersion string `json:"goVersion"` - Version string `json:"version"` - Commit string `json:"commit"` - Date string `json:"date"` -} - -// --- Related to run but use here. - -//nolint:gomnd -func initRunFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) - if err := fs.MarkHidden("internal-cmd-test"); err != nil { - panic(err) - } - - // --- Output config - - oc := &cfg.Output - fs.StringVar(&oc.Format, "out-format", - config.OutFormatColoredLineNumber, - wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) - fs.BoolVar(&oc.PrintIssuedLine, "print-issued-lines", true, wh("Print lines of code with issue")) - fs.BoolVar(&oc.PrintLinterName, "print-linter-name", true, wh("Print linter name in issue line")) - fs.BoolVar(&oc.UniqByLine, "uniq-by-line", true, wh("Make issues output unique by line")) - fs.BoolVar(&oc.SortResults, "sort-results", false, wh("Sort linter results")) - fs.BoolVar(&oc.PrintWelcomeMessage, "print-welcome", false, wh("Print welcome message")) - fs.StringVar(&oc.PathPrefix, "path-prefix", "", wh("Path prefix to add to output")) - - // --- Run config - - rc := &cfg.Run - - // Config file config - initConfigFileFlagSet(fs, rc) - - fs.StringVar(&rc.ModulesDownloadMode, "modules-download-mode", "", - wh("Modules download mode. If not empty, passed as -mod= to go tools")) - fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code", - exitcodes.IssuesFound, wh("Exit code when issues were found")) - fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version")) - fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags")) - - fs.DurationVar(&rc.Timeout, "timeout", defaultTimeout, wh("Timeout for total work")) - - fs.BoolVar(&rc.AnalyzeTests, "tests", true, wh("Analyze tests (*_test.go)")) - fs.BoolVar(&rc.PrintResourcesUsage, "print-resources-usage", false, - wh("Print avg and max memory usage of golangci-lint and total time")) - fs.StringSliceVar(&rc.SkipDirs, "skip-dirs", nil, wh("Regexps of directories to skip")) - fs.BoolVar(&rc.UseDefaultSkipDirs, "skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) - fs.StringSliceVar(&rc.SkipFiles, "skip-files", nil, wh("Regexps of files to skip")) - - const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + - "If false (default) - golangci-lint acquires file lock on start." - fs.BoolVar(&rc.AllowParallelRunners, "allow-parallel-runners", false, wh(allowParallelDesc)) - const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + - "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." - fs.BoolVar(&rc.AllowSerialRunners, "allow-serial-runners", false, wh(allowSerialDesc)) - fs.BoolVar(&rc.ShowStats, "show-stats", false, wh("Show statistics per linter")) - - // --- Linters config - - lc := &cfg.Linters - initLintersFlagSet(fs, lc) - - // --- Issues config - - ic := &cfg.Issues - fs.StringSliceVarP(&ic.ExcludePatterns, "exclude", "e", nil, wh("Exclude issue by regexp")) - fs.BoolVar(&ic.UseDefaultExcludes, "exclude-use-default", true, getDefaultIssueExcludeHelp()) - fs.BoolVar(&ic.ExcludeCaseSensitive, "exclude-case-sensitive", false, wh("If set to true exclude "+ - "and exclude rules regular expressions are case sensitive")) - - fs.IntVar(&ic.MaxIssuesPerLinter, "max-issues-per-linter", 50, - wh("Maximum issues count per one linter. Set to 0 to disable")) - fs.IntVar(&ic.MaxSameIssues, "max-same-issues", 3, - wh("Maximum count of issues with the same text. Set to 0 to disable")) - - fs.BoolVarP(&ic.Diff, "new", "n", false, - wh("Show only new issues: if there are unstaged changes or untracked files, only those changes "+ - "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration "+ - "of golangci-lint into existing large codebase.\nIt's not practical to fix all existing issues at "+ - "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer "+ - "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate "+ - "unstaged files before golangci-lint runs.")) - fs.StringVar(&ic.DiffFromRevision, "new-from-rev", "", - wh("Show only new issues created after git revision `REV`")) - fs.StringVar(&ic.DiffPatchFilePath, "new-from-patch", "", - wh("Show only new issues created in git patch with file path `PATH`")) - fs.BoolVar(&ic.WholeFiles, "whole-files", false, - wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) - fs.BoolVar(&ic.NeedFix, "fix", false, wh("Fix found issues (if it's supported by the linter)")) -} - -// --- Related to config but use here. - -func initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { - fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) - fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) -} - -// --- Related to linters but use here. - -func initLintersFlagSet(fs *pflag.FlagSet, cfg *config.Linters) { - fs.StringSliceVarP(&cfg.Disable, "disable", "D", nil, wh("Disable specific linter")) - fs.BoolVar(&cfg.DisableAll, "disable-all", false, wh("Disable all linters")) - fs.StringSliceVarP(&cfg.Enable, "enable", "E", nil, wh("Enable specific linter")) - fs.BoolVar(&cfg.EnableAll, "enable-all", false, wh("Enable all linters")) - fs.BoolVar(&cfg.Fast, "fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) - fs.StringSliceVarP(&cfg.Presets, "presets", "p", nil, - wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ - "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) -} - -// --- Related to run but use here. - -const defaultTimeout = time.Minute - -func getDefaultIssueExcludeHelp() string { - parts := []string{color.GreenString("Use or not use default excludes:")} - for _, ep := range config.DefaultExcludePatterns { - parts = append(parts, - fmt.Sprintf(" # %s %s: %s", ep.ID, ep.Linter, ep.Why), - fmt.Sprintf(" - %s", color.YellowString(ep.Pattern)), - "", - ) - } - return strings.Join(parts, "\n") -} - -func getDefaultDirectoryExcludeHelp() string { - parts := []string{color.GreenString("Use or not use default excluded directories:")} - for _, dir := range packages.StdExcludeDirRegexps { - parts = append(parts, fmt.Sprintf(" - %s", color.YellowString(dir))) - } - parts = append(parts, "") - return strings.Join(parts, "\n") -} - -// --- Related to root but use here. - -// envHelpRun value: "1". -const envHelpRun = "HELP_RUN" - -func getDefaultConcurrency() int { - if os.Getenv(envHelpRun) == "1" { - // Make stable concurrency for generating help documentation. - const prettyConcurrency = 8 - return prettyConcurrency - } - - return runtime.NumCPU() -} - -func initRootFlagSet(fs *pflag.FlagSet, cfg *config.Config) { - fs.BoolVarP(&cfg.Run.IsVerbose, "verbose", "v", false, wh("Verbose output")) - fs.StringVar(&cfg.Output.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) - - fs.StringVar(&cfg.Run.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) - fs.StringVar(&cfg.Run.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) - fs.StringVar(&cfg.Run.TracePath, "trace-path", "", wh("Path to trace output file")) - - fs.IntVarP(&cfg.Run.Concurrency, "concurrency", "j", getDefaultConcurrency(), - wh("Number of CPUs to use (Default: number of logical CPUs)")) - - fs.BoolVar(&cfg.Run.PrintVersion, "version", false, wh("Print version")) -} From a4cbde74f06eddd851d2236181e991f6b661a3c1 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:36:15 +0100 Subject: [PATCH 20/42] chore: remove config.Reader --- pkg/config/loader.go | 3 +- pkg/config/reader.go | 251 ------------------------------------------- 2 files changed, 2 insertions(+), 252 deletions(-) delete mode 100644 pkg/config/reader.go diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 595a97ebb584..1990ca5380e3 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -17,6 +17,8 @@ import ( "github.com/golangci/golangci-lint/pkg/logutils" ) +var errConfigDisabled = errors.New("config is disabled by --no-config") + type LoaderOptions struct { Config string // Flag only. The path to the golangci config file, as specified with the --config argument. NoConfig bool // Flag only. @@ -91,7 +93,6 @@ func (r *Loader) evaluateOptions() (string, error) { return configFile, nil } -//nolint:dupl // FIXME just during the dev func (r *Loader) setupConfigFileSearch() { firstArg := extractFirstPathArg() diff --git a/pkg/config/reader.go b/pkg/config/reader.go deleted file mode 100644 index 944da7ee6be3..000000000000 --- a/pkg/config/reader.go +++ /dev/null @@ -1,251 +0,0 @@ -package config - -import ( - "errors" - "fmt" - "os" - "path/filepath" - "slices" - "strings" - - "github.com/go-viper/mapstructure/v2" - "github.com/mitchellh/go-homedir" - "github.com/spf13/viper" - - "github.com/golangci/golangci-lint/pkg/exitcodes" - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/logutils" -) - -type FileReader struct { - log logutils.Log - cfg *Config - commandLineCfg *Config -} - -func NewFileReader(toCfg, commandLineCfg *Config, log logutils.Log) *FileReader { - return &FileReader{ - log: log, - cfg: toCfg, - commandLineCfg: commandLineCfg, - } -} - -func (r *FileReader) Read() error { - // XXX: hack with double parsing for 2 purposes: - // 1. to access "config" option here. - // 2. to give config less priority than command line. - - configFile, err := r.parseConfigOption() - if err != nil { - if errors.Is(err, errConfigDisabled) { - return nil - } - - return fmt.Errorf("can't parse --config option: %w", err) - } - - if configFile != "" { - viper.SetConfigFile(configFile) - - // Assume YAML if the file has no extension. - if filepath.Ext(configFile) == "" { - viper.SetConfigType("yaml") - } - } else { - r.setupConfigFileSearch() - } - - return r.parseConfig() -} - -func (r *FileReader) parseConfig() error { - if err := viper.ReadInConfig(); err != nil { - var configFileNotFoundError viper.ConfigFileNotFoundError - if errors.As(err, &configFileNotFoundError) { - return nil - } - - return fmt.Errorf("can't read viper config: %w", err) - } - - usedConfigFile := viper.ConfigFileUsed() - if usedConfigFile == "" { - return nil - } - - if usedConfigFile == os.Stdin.Name() { - usedConfigFile = "" - r.log.Infof("Reading config file stdin") - } else { - var err error - usedConfigFile, err = fsutils.ShortestRelPath(usedConfigFile, "") - if err != nil { - r.log.Warnf("Can't pretty print config file path: %v", err) - } - - r.log.Infof("Used config file %s", usedConfigFile) - } - - usedConfigDir, err := filepath.Abs(filepath.Dir(usedConfigFile)) - if err != nil { - return errors.New("can't get config directory") - } - r.cfg.cfgDir = usedConfigDir - - if err := viper.Unmarshal(r.cfg, viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( - // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). - mapstructure.StringToTimeDurationHookFunc(), - mapstructure.StringToSliceHookFunc(","), - - // Needed for forbidigo. - mapstructure.TextUnmarshallerHookFunc(), - ))); err != nil { - return fmt.Errorf("can't unmarshal config by viper: %w", err) - } - - if err := r.validateConfig(); err != nil { - return fmt.Errorf("can't validate config: %w", err) - } - - if r.cfg.InternalTest { // just for testing purposes: to detect config file usage - fmt.Fprintln(logutils.StdOut, "test") - os.Exit(exitcodes.Success) - } - - return nil -} - -func (r *FileReader) validateConfig() error { - if len(r.cfg.Run.Args) != 0 { - return errors.New("option run.args in config isn't supported now") - } - - if r.cfg.Run.CPUProfilePath != "" { - return errors.New("option run.cpuprofilepath in config isn't allowed") - } - - if r.cfg.Run.MemProfilePath != "" { - return errors.New("option run.memprofilepath in config isn't allowed") - } - - if r.cfg.Run.TracePath != "" { - return errors.New("option run.tracepath in config isn't allowed") - } - - if r.cfg.Run.IsVerbose { - return errors.New("can't set run.verbose option with config: only on command-line") - } - - for i, rule := range r.cfg.Issues.ExcludeRules { - if err := rule.Validate(); err != nil { - return fmt.Errorf("error in exclude rule #%d: %w", i, err) - } - } - - if len(r.cfg.Severity.Rules) > 0 && r.cfg.Severity.Default == "" { - return errors.New("can't set severity rule option: no default severity defined") - } - for i, rule := range r.cfg.Severity.Rules { - if err := rule.Validate(); err != nil { - return fmt.Errorf("error in severity rule #%d: %w", i, err) - } - } - - return nil -} - -func getFirstPathArg() string { - args := os.Args - - // skip all args ([golangci-lint, run/linters]) before files/dirs list - for len(args) != 0 { - if args[0] == "run" { - args = args[1:] - break - } - - args = args[1:] - } - - // find first file/dir arg - firstArg := "./..." - for _, arg := range args { - if !strings.HasPrefix(arg, "-") { - firstArg = arg - break - } - } - - return firstArg -} - -// FIXME just during the dev -// -//nolint:dupl -func (r *FileReader) setupConfigFileSearch() { - firstArg := getFirstPathArg() - absStartPath, err := filepath.Abs(firstArg) - if err != nil { - r.log.Warnf("Can't make abs path for %q: %s", firstArg, err) - absStartPath = filepath.Clean(firstArg) - } - - // start from it - var curDir string - if fsutils.IsDir(absStartPath) { - curDir = absStartPath - } else { - curDir = filepath.Dir(absStartPath) - } - - // find all dirs from it up to the root - configSearchPaths := []string{"./"} - - for { - configSearchPaths = append(configSearchPaths, curDir) - newCurDir := filepath.Dir(curDir) - if curDir == newCurDir || newCurDir == "" { - break - } - curDir = newCurDir - } - - // find home directory for global config - if home, err := homedir.Dir(); err != nil { - r.log.Warnf("Can't get user's home directory: %s", err.Error()) - } else if !slices.Contains(configSearchPaths, home) { - configSearchPaths = append(configSearchPaths, home) - } - - r.log.Infof("Config search paths: %s", configSearchPaths) - viper.SetConfigName(".golangci") - for _, p := range configSearchPaths { - viper.AddConfigPath(p) - } -} - -var errConfigDisabled = errors.New("config is disabled by --no-config") - -func (r *FileReader) parseConfigOption() (string, error) { - cfg := r.commandLineCfg - if cfg == nil { - return "", nil - } - - configFile := cfg.Run.Config - if cfg.Run.NoConfig && configFile != "" { - return "", errors.New("can't combine option --config and --no-config") - } - - if cfg.Run.NoConfig { - return "", errConfigDisabled - } - - configFile, err := homedir.Expand(configFile) - if err != nil { - return "", errors.New("failed to expand configuration path") - } - - return configFile, nil -} From 45f310990a046fc7dada188efd8f0d7af69fd29f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:45:23 +0100 Subject: [PATCH 21/42] chore: move configuration validation to config --- pkg/config/config.go | 21 +++++++++++++++++++++ pkg/config/loader.go | 24 ++---------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index f73563a2a4ef..7b6a0f71f728 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,6 +1,8 @@ package config import ( + "errors" + "fmt" "os" "strings" @@ -32,6 +34,25 @@ func (c *Config) GetConfigDir() string { return c.cfgDir } +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) + } + } + + 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) + } + } + + return nil +} + func NewDefault() *Config { return &Config{ LintersSettings: defaultLintersSettings, diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 1990ca5380e3..6bf6d5fdc19d 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -150,7 +150,7 @@ func (r *Loader) parseConfig() error { return err } - if err = r.validateConfig(); err != nil { + if err = r.cfg.Validate(); err != nil { return fmt.Errorf("can't validate config: %w", err) } @@ -170,7 +170,7 @@ func (r *Loader) parseConfig() error { return fmt.Errorf("can't unmarshal config by viper: %w", err) } - if err := r.validateConfig(); err != nil { + if err := r.cfg.Validate(); err != nil { return fmt.Errorf("can't validate config: %w", err) } @@ -211,26 +211,6 @@ func (r *Loader) setConfigDir() error { return nil } -// FIXME move to Config struct. -func (r *Loader) validateConfig() error { - for i, rule := range r.cfg.Issues.ExcludeRules { - if err := rule.Validate(); err != nil { - return fmt.Errorf("error in exclude rule #%d: %w", i, err) - } - } - - if len(r.cfg.Severity.Rules) > 0 && r.cfg.Severity.Default == "" { - return errors.New("can't set severity rule option: no default severity defined") - } - for i, rule := range r.cfg.Severity.Rules { - if err := rule.Validate(); err != nil { - return fmt.Errorf("error in severity rule #%d: %w", i, err) - } - } - - return nil -} - func fileDecoderHook() viper.DecoderConfigOption { return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). From f529899df536f6ed233828fa11538b24d272f061 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:55:05 +0100 Subject: [PATCH 22/42] chore: clean Run structure --- pkg/config/run.go | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/pkg/config/run.go b/pkg/config/run.go index b9e4f74e0d7a..4c6289de8d42 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -25,20 +25,6 @@ type Run struct { ShowStats bool `mapstructure:"show-stats"` - // --- Flags only section. - - IsVerbose bool `mapstructure:"verbose"` // Flag only - - PrintVersion bool // Flag only. (used by the root command) - - CPUProfilePath string // Flag only. - MemProfilePath string // Flag only. - TracePath string // Flag only. - - PrintResourcesUsage bool `mapstructure:"print-resources-usage"` // Flag only. - - Config string // Flag only. The path to the golangci config file, as specified with the --config argument. - NoConfig bool // Flag only. - - Args []string // Internal needs. // TODO(ldez) it's only use by flags and for the tests, need to removed in another PR. + // TODO(ldez) it's only use by flags and for the tests, need to removed in another PR. + Args []string // Internal needs. } From b82dddfbd89cae9a330827e23974b62073f05e26 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 00:56:01 +0100 Subject: [PATCH 23/42] chore: clean Output structure --- pkg/config/output.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/config/output.go b/pkg/config/output.go index 28e4f29b3414..768999285dfd 100644 --- a/pkg/config/output.go +++ b/pkg/config/output.go @@ -35,7 +35,4 @@ type Output struct { SortResults bool `mapstructure:"sort-results"` PrintWelcomeMessage bool `mapstructure:"print-welcome"` PathPrefix string `mapstructure:"path-prefix"` - - // only work with CLI flags because the setup of logs is done before the config file parsing. - Color string // Flag only. } From 397e80d9bb6a328fe0bc288f0aa37d2d303c9527 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 01:04:13 +0100 Subject: [PATCH 24/42] docs: flags override file configuration even for slices --- docs/src/docs/usage/configuration.mdx | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/docs/usage/configuration.mdx b/docs/src/docs/usage/configuration.mdx index f354643968e0..b9cc3e6ba8e1 100644 --- a/docs/src/docs/usage/configuration.mdx +++ b/docs/src/docs/usage/configuration.mdx @@ -4,7 +4,6 @@ title: Configuration The config file has lower priority than command-line options. If the same bool/string/int option is provided on the command-line and in the config file, the option from command-line will be used. -Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and config file. To see a list of linters enabled by your configuration use: From 0a885977367262fb946727dd12cc5c36f2ac5237 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 01:07:22 +0100 Subject: [PATCH 25/42] docs: remove old Init section --- docs/src/docs/contributing/architecture.mdx | 84 --------------------- 1 file changed, 84 deletions(-) diff --git a/docs/src/docs/contributing/architecture.mdx b/docs/src/docs/contributing/architecture.mdx index 3f1aa17da1c9..a449156673a7 100644 --- a/docs/src/docs/contributing/architecture.mdx +++ b/docs/src/docs/contributing/architecture.mdx @@ -22,90 +22,6 @@ graph LR -## Init - -The execution starts here: - -```go title=cmd/golangci-lint/main.go -func main() { - e := commands.NewExecutor(info) - - if err := e.Execute(); err != nil { - fmt.Fprintf(os.Stderr, "failed executing command with error %v\n", err) - os.Exit(exitcodes.Failure) - } -} -``` - -The **executer** is our abstraction: - -```go title=pkg/commands/executor.go -type Executor struct { - rootCmd *cobra.Command - runCmd *cobra.Command - lintersCmd *cobra.Command - - exitCode int - buildInfo BuildInfo - - cfg *config.Config - log logutils.Log - reportData report.Data - DBManager *lintersdb.Manager - EnabledLintersSet *lintersdb.EnabledSet - contextLoader *lint.ContextLoader - goenv *goutil.Env - fileCache *fsutils.FileCache - lineCache *fsutils.LineCache - pkgCache *pkgcache.Cache - debugf logutils.DebugFunc - sw *timeutils.Stopwatch - - loadGuard *load.Guard - flock *flock.Flock -} -``` - -We use dependency injection and all root dependencies are stored in this executor. - -In the function `NewExecutor` we do the following: - -1. Initialize dependencies. -2. Initialize [cobra](https://github.com/spf13/cobra) commands. -3. Parse the config file using [viper](https://github.com/spf13/viper) and merge it with command line arguments. - -The following execution is controlled by `cobra`. If a user executes `golangci-lint run` -then `cobra` executes `e.runCmd`. - -Different `cobra` commands have different runners, e.g. a `run` command is configured in the following way: - -```go title=pkg/commands/run.go -func (e *Executor) initRun() { - e.runCmd = &cobra.Command{ - Use: "run", - Short: "Run the linters", - Run: e.executeRun, - PreRunE: func(_ *cobra.Command, _ []string) error { - if ok := e.acquireFileLock(); !ok { - return errors.New("parallel golangci-lint is running") - } - return nil - }, - PostRun: func(_ *cobra.Command, _ []string) { - e.releaseFileLock() - }, - } - e.rootCmd.AddCommand(e.runCmd) - - e.runCmd.SetOut(logutils.StdOut) // use custom output to properly color it in Windows terminals - e.runCmd.SetErr(logutils.StdErr) - - e.initRunConfiguration(e.runCmd) -} -``` - -The primary execution function of the `run` command is `executeRun`. - ## Load Packages Loading packages is listing all packages and their recursive dependencies for analysis. From 5e89135100b6b53c34bbff4e8d17fc319a70df79 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 20:51:23 +0100 Subject: [PATCH 26/42] fix: remove tests on unsupported options in config --- test/run_test.go | 80 ------------------------------------------------ 1 file changed, 80 deletions(-) diff --git a/test/run_test.go b/test/run_test.go index 3b4870c485fc..c5e24fcb04a2 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -536,86 +536,6 @@ func TestAbsPathFileAnalysis(t *testing.T) { ExpectHasIssue("indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)") } -func TestDisallowedOptionsInConfig(t *testing.T) { - cases := []struct { - cfg string - option string - }{ - { - cfg: ` - ruN: - Args: - - 1 - `, - }, - { - cfg: ` - run: - CPUProfilePath: path - `, - option: "--cpu-profile-path=path", - }, - { - cfg: ` - run: - MemProfilePath: path - `, - option: "--mem-profile-path=path", - }, - { - cfg: ` - run: - TracePath: path - `, - option: "--trace-path=path", - }, - { - cfg: ` - run: - Verbose: true - `, - option: "-v", - }, - } - - testshared.InstallGolangciLint(t) - - for _, c := range cases { - // Run with disallowed option set only in config - testshared.NewRunnerBuilder(t). - WithConfig(c.cfg). - WithTargetPath(testdataDir, minimalPkg). - Runner(). - Run(). - ExpectExitCode(exitcodes.Failure) - - if c.option == "" { - continue - } - - args := []string{c.option, "--fast"} - - // Run with disallowed option set only in command-line - testshared.NewRunnerBuilder(t). - WithNoConfig(). - WithArgs(args...). - WithTargetPath(testdataDir, minimalPkg). - Runner(). - Run(). - ExpectExitCode(exitcodes.Success) - - // Run with disallowed option set both in command-line and in config - - testshared.NewRunnerBuilder(t). - WithConfig(c.cfg). - WithArgs(args...). - WithTargetPath(testdataDir, minimalPkg). - Runner(). - Run(). - ExpectExitCode(exitcodes.Failure) - } -} - func TestPathPrefix(t *testing.T) { testCases := []struct { desc string From e9104249ca19fc653807bd7cb665764909dbeac1 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 20:51:45 +0100 Subject: [PATCH 27/42] fix: tests with slice option merge --- test/enabled_linters_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index e2589ef894c2..8c40f741160b 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -53,7 +53,7 @@ func TestEnabledLinters(t *testing.T) { enable: - revive `, - enabledLinters: getEnabledByDefaultFastLintersWith("revive", "gofmt"), + enabledLinters: getEnabledByDefaultFastLintersWith("gofmt"), }, { name: "fast option in config", From 0c34280a461212b5d05586a5528a2b7c42bbf875 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 21:07:48 +0100 Subject: [PATCH 28/42] chore: rename new package to commands --- cmd/golangci-lint/main.go | 6 +++--- pkg/{cmder => commands}/cache.go | 2 +- pkg/{cmder => commands}/config.go | 2 +- pkg/{cmder => commands}/flagsets.go | 4 ++-- pkg/{cmder => commands}/help.go | 2 +- pkg/{cmder => commands}/internal/vibra.go | 0 pkg/{cmder => commands}/linters.go | 2 +- pkg/{cmder => commands}/root.go | 2 +- pkg/{cmder => commands}/run.go | 2 +- pkg/{cmder => commands}/version.go | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) rename pkg/{cmder => commands}/cache.go (99%) rename pkg/{cmder => commands}/config.go (99%) rename pkg/{cmder => commands}/flagsets.go (98%) rename pkg/{cmder => commands}/help.go (99%) rename pkg/{cmder => commands}/internal/vibra.go (100%) rename pkg/{cmder => commands}/linters.go (99%) rename pkg/{cmder => commands}/root.go (99%) rename pkg/{cmder => commands}/run.go (99%) rename pkg/{cmder => commands}/version.go (99%) diff --git a/cmd/golangci-lint/main.go b/cmd/golangci-lint/main.go index c735c778426a..84d979562182 100644 --- a/cmd/golangci-lint/main.go +++ b/cmd/golangci-lint/main.go @@ -5,7 +5,7 @@ import ( "os" "runtime/debug" - "github.com/golangci/golangci-lint/pkg/cmder" + "github.com/golangci/golangci-lint/pkg/commands" "github.com/golangci/golangci-lint/pkg/exitcodes" ) @@ -29,14 +29,14 @@ func main() { } } - info := cmder.BuildInfo{ + info := commands.BuildInfo{ GoVersion: goVersion, Version: version, Commit: commit, Date: date, } - if err := cmder.Execute(info); err != nil { + if err := commands.Execute(info); err != nil { _, _ = fmt.Fprintf(os.Stderr, "failed executing command with error %v\n", err) os.Exit(exitcodes.Failure) } diff --git a/pkg/cmder/cache.go b/pkg/commands/cache.go similarity index 99% rename from pkg/cmder/cache.go rename to pkg/commands/cache.go index df7da718b670..4aa8130518b0 100644 --- a/pkg/cmder/cache.go +++ b/pkg/commands/cache.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "fmt" diff --git a/pkg/cmder/config.go b/pkg/commands/config.go similarity index 99% rename from pkg/cmder/config.go rename to pkg/commands/config.go index 46721e803087..064a2010ad24 100644 --- a/pkg/cmder/config.go +++ b/pkg/commands/config.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "fmt" diff --git a/pkg/cmder/flagsets.go b/pkg/commands/flagsets.go similarity index 98% rename from pkg/cmder/flagsets.go rename to pkg/commands/flagsets.go index ff6030859e96..b058a0420c13 100644 --- a/pkg/cmder/flagsets.go +++ b/pkg/commands/flagsets.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "fmt" @@ -8,7 +8,7 @@ import ( "github.com/spf13/pflag" "github.com/spf13/viper" - "github.com/golangci/golangci-lint/pkg/cmder/internal" + "github.com/golangci/golangci-lint/pkg/commands/internal" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/exitcodes" "github.com/golangci/golangci-lint/pkg/lint/lintersdb" diff --git a/pkg/cmder/help.go b/pkg/commands/help.go similarity index 99% rename from pkg/cmder/help.go rename to pkg/commands/help.go index 17b4bacaaf75..01060a156f72 100644 --- a/pkg/cmder/help.go +++ b/pkg/commands/help.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "fmt" diff --git a/pkg/cmder/internal/vibra.go b/pkg/commands/internal/vibra.go similarity index 100% rename from pkg/cmder/internal/vibra.go rename to pkg/commands/internal/vibra.go diff --git a/pkg/cmder/linters.go b/pkg/commands/linters.go similarity index 99% rename from pkg/cmder/linters.go rename to pkg/commands/linters.go index 2c38b37c9539..779deeda29c1 100644 --- a/pkg/cmder/linters.go +++ b/pkg/commands/linters.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "fmt" diff --git a/pkg/cmder/root.go b/pkg/commands/root.go similarity index 99% rename from pkg/cmder/root.go rename to pkg/commands/root.go index 998ff7604857..cafe56bd0176 100644 --- a/pkg/cmder/root.go +++ b/pkg/commands/root.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "errors" diff --git a/pkg/cmder/run.go b/pkg/commands/run.go similarity index 99% rename from pkg/cmder/run.go rename to pkg/commands/run.go index 347ed660b11b..e01a29f33220 100644 --- a/pkg/cmder/run.go +++ b/pkg/commands/run.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "bytes" diff --git a/pkg/cmder/version.go b/pkg/commands/version.go similarity index 99% rename from pkg/cmder/version.go rename to pkg/commands/version.go index 18029cc614af..b73d8dd63a57 100644 --- a/pkg/cmder/version.go +++ b/pkg/commands/version.go @@ -1,4 +1,4 @@ -package cmder +package commands import ( "encoding/json" From f3bcb5cdf67e94774b283967b287e96c9e4d5b69 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 21:45:33 +0100 Subject: [PATCH 29/42] chore: move the Go detection inside the configuration loader --- pkg/commands/run.go | 4 ---- pkg/config/config.go | 2 +- pkg/config/loader.go | 11 ++++++++++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index e01a29f33220..f670de1f40bc 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -154,10 +154,6 @@ func (c *runCommand) persistentPreRunE(_ *cobra.Command, _ []string) error { return fmt.Errorf("can't load config: %w", err) } - if c.cfg.Run.Go == "" { - c.cfg.Run.Go = config.DetectGoVersion() - } - runtime.GOMAXPROCS(c.cfg.Run.Concurrency) return c.startTracing() diff --git a/pkg/config/config.go b/pkg/config/config.go index 7b6a0f71f728..7c1509f97c30 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -78,7 +78,7 @@ func IsGoGreaterThanOrEqual(current, limit string) bool { return v1.GreaterThanOrEqual(l) } -func DetectGoVersion() string { +func detectGoVersion() string { file, _ := gomoddirectives.GetModuleFile() if file != nil && file.Go != nil && file.Go.Version != "" { diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 6bf6d5fdc19d..ee2474984568 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -49,7 +49,16 @@ func (r *Loader) Load() error { return err } - return r.parseConfig() + err = r.parseConfig() + if err != nil { + return err + } + + if r.cfg.Run.Go == "" { + r.cfg.Run.Go = detectGoVersion() + } + + return nil } func (r *Loader) setConfigFile() error { From 8497bc72c2e58d7d03ad2c1cfd822325e213118c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 23 Feb 2024 23:30:05 +0100 Subject: [PATCH 30/42] chore: clean Config structure --- pkg/config/config.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7c1509f97c30..9a4cc97f03f8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -23,8 +23,6 @@ type Config struct { Issues Issues `mapstructure:"issues"` Severity Severity `mapstructure:"severity"` - Version Version // Flag only. // TODO(ldez) only used by the version command. - InternalCmdTest bool // Option is used only for testing golangci-lint command, don't use it InternalTest bool // Option is used only for testing golangci-lint code, don't use it } From fcee63952c1d52e51010d1711160a585c67fa517 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 24 Feb 2024 13:52:45 +0100 Subject: [PATCH 31/42] fix: override help command of the root command --- pkg/commands/root.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/commands/root.go b/pkg/commands/root.go index cafe56bd0176..f995aa98547d 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -66,9 +66,10 @@ func newRootCommand(info BuildInfo) *rootCommand { newCacheCommand().cmd, newConfigCommand(log, config.NewDefault()).cmd, newVersionCommand(info).cmd, - newHelpCommand(log, config.NewDefault()).cmd, ) + rootCmd.SetHelpCommand(newHelpCommand(log, config.NewDefault()).cmd) + c.log = log c.cmd = rootCmd From fb8d76b47f173b798b0098eaa4cece68ffdcb5d8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 24 Feb 2024 23:42:45 +0100 Subject: [PATCH 32/42] chore(plugins): replace viper.ConfigFileUsed() by cfg.GetConfigDir() --- pkg/lint/lintersdb/custom_linters.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/lint/lintersdb/custom_linters.go b/pkg/lint/lintersdb/custom_linters.go index 188c14d9138d..76e8fc58922b 100644 --- a/pkg/lint/lintersdb/custom_linters.go +++ b/pkg/lint/lintersdb/custom_linters.go @@ -6,7 +6,6 @@ import ( "path/filepath" "plugin" - "github.com/spf13/viper" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -67,12 +66,7 @@ func (m *Manager) loadCustomLinterConfig(name string, settings config.CustomLint func (m *Manager) getAnalyzerPlugin(path string, settings any) ([]*analysis.Analyzer, error) { if !filepath.IsAbs(path) { // resolve non-absolute paths relative to config file's directory - configFilePath := viper.ConfigFileUsed() - absConfigFilePath, err := filepath.Abs(configFilePath) - if err != nil { - return nil, fmt.Errorf("could not get absolute representation of config file path %q: %w", configFilePath, err) - } - path = filepath.Join(filepath.Dir(absConfigFilePath), path) + path = filepath.Join(m.cfg.GetConfigDir(), path) } plug, err := plugin.Open(path) From 8e7beebad8d8e8f12b1d074e2a3e2eff9938019e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 25 Feb 2024 00:30:31 +0100 Subject: [PATCH 33/42] chore: remove configuration fields for commands when a command doesn't depend on the real configuration --- pkg/commands/config.go | 9 ++++----- pkg/commands/help.go | 10 +++++----- pkg/commands/root.go | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 064a2010ad24..e57c5a4b4111 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -17,16 +17,13 @@ type configCommand struct { viper *viper.Viper cmd *cobra.Command - cfg *config.Config - log logutils.Log } -func newConfigCommand(log logutils.Log, cfg *config.Config) *configCommand { +func newConfigCommand(log logutils.Log) *configCommand { c := &configCommand{ viper: viper.New(), log: log, - cfg: cfg, } configCmd := &cobra.Command{ @@ -55,7 +52,9 @@ func newConfigCommand(log logutils.Log, cfg *config.Config) *configCommand { } func (c *configCommand) preRunE(_ *cobra.Command, _ []string) error { - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, config.LoaderOptions{}, c.cfg) + // The command doesn't depend on the real configuration. + // It only needs to know the path of the configuration file. + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, config.LoaderOptions{}, config.NewDefault()) if err := loader.Load(); err != nil { return fmt.Errorf("can't load config: %w", err) diff --git a/pkg/commands/help.go b/pkg/commands/help.go index 01060a156f72..d6f0ce11ce83 100644 --- a/pkg/commands/help.go +++ b/pkg/commands/help.go @@ -17,15 +17,13 @@ import ( type helpCommand struct { cmd *cobra.Command - cfg *config.Config - dbManager *lintersdb.Manager log logutils.Log } -func newHelpCommand(logger logutils.Log, cfg *config.Config) *helpCommand { - c := &helpCommand{log: logger, cfg: cfg} +func newHelpCommand(logger logutils.Log) *helpCommand { + c := &helpCommand{log: logger} helpCmd := &cobra.Command{ Use: "help", @@ -53,7 +51,9 @@ func newHelpCommand(logger logutils.Log, cfg *config.Config) *helpCommand { } func (c *helpCommand) preRun(_ *cobra.Command, _ []string) { - c.dbManager = lintersdb.NewManager(c.cfg, c.log) + // 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) } func (c *helpCommand) execute(_ *cobra.Command, _ []string) { diff --git a/pkg/commands/root.go b/pkg/commands/root.go index f995aa98547d..7a1551110c38 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -64,11 +64,11 @@ func newRootCommand(info BuildInfo) *rootCommand { newLintersCommand(log, config.NewDefault()).cmd, newRunCommand(log, config.NewDefault(), reportData, info).cmd, newCacheCommand().cmd, - newConfigCommand(log, config.NewDefault()).cmd, + newConfigCommand(log).cmd, newVersionCommand(info).cmd, ) - rootCmd.SetHelpCommand(newHelpCommand(log, config.NewDefault()).cmd) + rootCmd.SetHelpCommand(newHelpCommand(log).cmd) c.log = log c.cmd = rootCmd From aba8d5bfb41bd04470a91bc92a5b59cd3b02d22a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 25 Feb 2024 03:33:31 +0100 Subject: [PATCH 34/42] chore: hack to append values on StringSlice --- docs/src/docs/usage/configuration.mdx | 1 + pkg/commands/config.go | 4 ++-- pkg/commands/flagsets.go | 15 +++++++------ pkg/commands/linters.go | 4 ++-- pkg/commands/run.go | 4 ++-- pkg/config/loader.go | 32 ++++++++++++++++++++++++++- test/enabled_linters_test.go | 2 +- 7 files changed, 47 insertions(+), 15 deletions(-) diff --git a/docs/src/docs/usage/configuration.mdx b/docs/src/docs/usage/configuration.mdx index b9cc3e6ba8e1..f354643968e0 100644 --- a/docs/src/docs/usage/configuration.mdx +++ b/docs/src/docs/usage/configuration.mdx @@ -4,6 +4,7 @@ title: Configuration The config file has lower priority than command-line options. If the same bool/string/int option is provided on the command-line and in the config file, the option from command-line will be used. +Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and config file. To see a list of linters enabled by your configuration use: diff --git a/pkg/commands/config.go b/pkg/commands/config.go index e57c5a4b4111..3d74c89d33f1 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -51,10 +51,10 @@ func newConfigCommand(log logutils.Log) *configCommand { return c } -func (c *configCommand) preRunE(_ *cobra.Command, _ []string) error { +func (c *configCommand) preRunE(cmd *cobra.Command, _ []string) error { // The command doesn't depend on the real configuration. // It only needs to know the path of the configuration file. - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, config.LoaderOptions{}, config.NewDefault()) + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), config.LoaderOptions{}, config.NewDefault()) if err := loader.Load(); err != nil { return fmt.Errorf("can't load config: %w", err) diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index b058a0420c13..9cc78acdb61f 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -15,16 +15,17 @@ import ( ) func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - internal.VibraP(v, fs, fs.StringSliceP, "disable", "D", "linters.disable", nil, wh("Disable specific linter")) + fs.StringSliceP("disable", "D", nil, wh("Disable specific linter")) // Hack see Loader.applyStringSliceHack internal.Vibra(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, wh("Disable all linters")) - internal.VibraP(v, fs, fs.StringSliceP, "enable", "E", "linters.enable", nil, wh("Enable specific linter")) + fs.StringSliceP("enable", "E", nil, wh("Enable specific linter")) // Hack see Loader.applyStringSliceHack internal.Vibra(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, wh("Enable all linters")) internal.Vibra(v, fs, fs.Bool, "fast", "linters.fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) - internal.VibraP(v, fs, fs.StringSliceP, "presets", "p", "linters.presets", nil, + // Hack see Loader.applyStringSliceHack + fs.StringSliceP("presets", "p", nil, wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) } @@ -38,14 +39,14 @@ func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.Vibra(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound, wh("Exit code when issues were found")) internal.Vibra(v, fs, fs.String, "go", "run.go", "", wh("Targeted Go version")) - internal.Vibra(v, fs, fs.StringSlice, "build-tags", "run.build-tags", nil, wh("Build tags")) + fs.StringSlice("build-tags", nil, wh("Build tags")) // Hack see Loader.applyStringSliceHack internal.Vibra(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, wh("Timeout for total work")) internal.Vibra(v, fs, fs.Bool, "tests", "run.tests", true, wh("Analyze tests (*_test.go)")) - internal.Vibra(v, fs, fs.StringSlice, "skip-dirs", "run.skip-dirs", nil, wh("Regexps of directories to skip")) + fs.StringSlice("skip-dirs", nil, wh("Regexps of directories to skip")) // Hack see Loader.applyStringSliceHack internal.Vibra(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) - internal.Vibra(v, fs, fs.StringSlice, "skip-files", "run.skip-files", nil, wh("Regexps of files to skip")) + fs.StringSlice("skip-files", nil, wh("Regexps of files to skip")) // Hack see Loader.applyStringSliceHack const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + "If false (default) - golangci-lint acquires file lock on start." @@ -69,7 +70,7 @@ func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) { //nolint:gomnd func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - internal.VibraP(v, fs, fs.StringSliceP, "exclude", "e", "issues.exclude", nil, wh("Exclude issue by regexp")) + fs.StringSliceP("exclude", "e", nil, wh("Exclude issue by regexp")) // Hack see Loader.applyStringSliceHack internal.Vibra(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, getDefaultIssueExcludeHelp()) internal.Vibra(v, fs, fs.Bool, "exclude-case-sensitive", "issues.exclude-case-sensitive", false, wh("If set to true exclude and exclude rules regular expressions are case-sensitive")) diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 779deeda29c1..02e9447d4ef3 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -58,8 +58,8 @@ func newLintersCommand(logger logutils.Log, cfg *config.Config) *lintersCommand return c } -func (c *lintersCommand) preRunE(_ *cobra.Command, _ []string) error { - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, c.opts.LoaderOptions, c.cfg) +func (c *lintersCommand) preRunE(cmd *cobra.Command, _ []string) error { + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg) if err := loader.Load(); err != nil { return fmt.Errorf("can't load config: %w", err) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index f670de1f40bc..644ccd227c43 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -143,12 +143,12 @@ func newRunCommand(logger logutils.Log, cfg *config.Config, reportData *report.D return c } -func (c *runCommand) persistentPreRunE(_ *cobra.Command, _ []string) error { +func (c *runCommand) persistentPreRunE(cmd *cobra.Command, _ []string) error { if err := c.startTracing(); err != nil { return err } - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, c.opts.LoaderOptions, c.cfg) + loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg) if err := loader.Load(); err != nil { return fmt.Errorf("can't load config: %w", err) diff --git a/pkg/config/loader.go b/pkg/config/loader.go index ee2474984568..be682443c7db 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -10,6 +10,7 @@ import ( "github.com/go-viper/mapstructure/v2" "github.com/mitchellh/go-homedir" + "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/golangci/golangci-lint/pkg/exitcodes" @@ -28,16 +29,18 @@ type Loader struct { opts LoaderOptions viper *viper.Viper + fs *pflag.FlagSet log logutils.Log cfg *Config } -func NewLoader(log logutils.Log, v *viper.Viper, opts LoaderOptions, cfg *Config) *Loader { +func NewLoader(log logutils.Log, v *viper.Viper, fs *pflag.FlagSet, opts LoaderOptions, cfg *Config) *Loader { return &Loader{ opts: opts, viper: v, + fs: fs, log: log, cfg: cfg, } @@ -54,6 +57,8 @@ func (r *Loader) Load() error { return err } + r.applyStringSliceHack() + if r.cfg.Run.Go == "" { r.cfg.Run.Go = detectGoVersion() } @@ -220,6 +225,31 @@ func (r *Loader) setConfigDir() error { return nil } +// Hack to append values from StringSlice flags. +// Viper always overrides StringSlice values. +// https://github.com/spf13/viper/issues/1448 +// So StringSlice flags are not bind to Viper like that their values are obtain via Cobra Flags. +func (r *Loader) applyStringSliceHack() { + if r.fs == nil { + return + } + + r.appendStringSlice("enable", &r.cfg.Linters.Enable) + r.appendStringSlice("disable", &r.cfg.Linters.Disable) + r.appendStringSlice("presets", &r.cfg.Linters.Presets) + r.appendStringSlice("build-tags", &r.cfg.Run.BuildTags) + r.appendStringSlice("skip-dirs", &r.cfg.Run.SkipDirs) + r.appendStringSlice("skip-files", &r.cfg.Run.SkipFiles) + r.appendStringSlice("exclude", &r.cfg.Issues.ExcludePatterns) +} + +func (r *Loader) appendStringSlice(name string, current *[]string) { + if r.fs.Changed(name) { + val, _ := r.fs.GetStringSlice(name) + *current = append(*current, val...) + } +} + func fileDecoderHook() viper.DecoderConfigOption { return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). diff --git a/test/enabled_linters_test.go b/test/enabled_linters_test.go index 8c40f741160b..e2589ef894c2 100644 --- a/test/enabled_linters_test.go +++ b/test/enabled_linters_test.go @@ -53,7 +53,7 @@ func TestEnabledLinters(t *testing.T) { enable: - revive `, - enabledLinters: getEnabledByDefaultFastLintersWith("gofmt"), + enabledLinters: getEnabledByDefaultFastLintersWith("revive", "gofmt"), }, { name: "fast option in config", From 3fa551e31bb58fe51898f700f7dda7df4def0c37 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 25 Feb 2024 04:39:16 +0100 Subject: [PATCH 35/42] chore: improve comment about Run.Args field --- pkg/config/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/run.go b/pkg/config/run.go index 4c6289de8d42..bd81e0d8c21b 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -25,6 +25,6 @@ type Run struct { ShowStats bool `mapstructure:"show-stats"` - // TODO(ldez) it's only use by flags and for the tests, need to removed in another PR. + // It's obtain by flags and use for the tests and the context loader. Args []string // Internal needs. } From b39613ed8e9524b012df00960939d035a8edfa91 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sun, 25 Feb 2024 05:18:22 +0100 Subject: [PATCH 36/42] chore: remove missing unused configuration field --- pkg/commands/flagsets.go | 1 - pkg/config/output.go | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index 9cc78acdb61f..65fca2cbd320 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -64,7 +64,6 @@ func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.Vibra(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, wh("Print linter name in issue line")) internal.Vibra(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, wh("Make issues output unique by line")) internal.Vibra(v, fs, fs.Bool, "sort-results", "output.sort-results", false, wh("Sort linter results")) - internal.Vibra(v, fs, fs.Bool, "print-welcome", "output.print-welcome", false, wh("Print welcome message")) internal.Vibra(v, fs, fs.String, "path-prefix", "output.path-prefix", "", wh("Path prefix to add to output")) } diff --git a/pkg/config/output.go b/pkg/config/output.go index 768999285dfd..95af38885a67 100644 --- a/pkg/config/output.go +++ b/pkg/config/output.go @@ -28,11 +28,10 @@ var OutFormats = []string{ } type Output struct { - Format string `mapstructure:"format"` - PrintIssuedLine bool `mapstructure:"print-issued-lines"` - PrintLinterName bool `mapstructure:"print-linter-name"` - UniqByLine bool `mapstructure:"uniq-by-line"` - SortResults bool `mapstructure:"sort-results"` - PrintWelcomeMessage bool `mapstructure:"print-welcome"` - PathPrefix string `mapstructure:"path-prefix"` + Format string `mapstructure:"format"` + PrintIssuedLine bool `mapstructure:"print-issued-lines"` + PrintLinterName bool `mapstructure:"print-linter-name"` + UniqByLine bool `mapstructure:"uniq-by-line"` + SortResults bool `mapstructure:"sort-results"` + PathPrefix string `mapstructure:"path-prefix"` } From 8aa1b0457e4a9eecad84c6a91abbf2957b37d764 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 00:18:56 +0100 Subject: [PATCH 37/42] chore: clean configuration reference --- .golangci.reference.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 7766f2909782..8f99a2195356 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -68,17 +68,13 @@ run: # Allow multiple parallel golangci-lint instances running. # If false, golangci-lint acquires file lock on start. # Default: false - allow-parallel-runners: false + allow-parallel-runners: true # Allow multiple golangci-lint instances running, but serialize them around a lock. # If false, golangci-lint exits with an error if it fails to acquire file lock on start. # Default: false allow-serial-runners: true - # Print avg and max memory usage of golangci-lint and total time. - # Default: false - print-resources-usage: true - # Define the Go version limit. # Mainly related to generics support since go1.18. # Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`, fallback on 1.17 @@ -2491,6 +2487,11 @@ linters-settings: # Intended to point to the repo location of the linter. # Optional. original-url: github.com/golangci/example-linter + # Plugins settings/configuration. + # Only work with plugin based on `linterdb.PluginConstructor`. + # Optional. + settings: + foo: bar linters: From 0e6009caddff8cad81a216fdf2d3e33c9919a240 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 22:12:30 +0100 Subject: [PATCH 38/42] review: rename Vibra to AddFlagAndBind --- pkg/commands/flagsets.go | 56 +++++++++++++++++----------------- pkg/commands/internal/vibra.go | 8 ++--- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index 65fca2cbd320..beeba5967cce 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -16,12 +16,12 @@ import ( func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { fs.StringSliceP("disable", "D", nil, wh("Disable specific linter")) // Hack see Loader.applyStringSliceHack - internal.Vibra(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, wh("Disable all linters")) + internal.AddFlagAndBind(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, wh("Disable all linters")) fs.StringSliceP("enable", "E", nil, wh("Enable specific linter")) // Hack see Loader.applyStringSliceHack - internal.Vibra(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, wh("Enable all linters")) + internal.AddFlagAndBind(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, wh("Enable all linters")) - internal.Vibra(v, fs, fs.Bool, "fast", "linters.fast", false, + internal.AddFlagAndBind(v, fs, fs.Bool, "fast", "linters.fast", false, wh("Enable only fast linters from enabled linters set (first run won't be fast)")) // Hack see Loader.applyStringSliceHack @@ -31,52 +31,52 @@ func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { } func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - internal.VibraP(v, fs, fs.IntP, "concurrency", "j", "run.concurrency", getDefaultConcurrency(), + internal.AddFlagAndBindP(v, fs, fs.IntP, "concurrency", "j", "run.concurrency", getDefaultConcurrency(), wh("Number of CPUs to use (Default: number of logical CPUs)")) - internal.Vibra(v, fs, fs.String, "modules-download-mode", "run.modules-download-mode", "", + internal.AddFlagAndBind(v, fs, fs.String, "modules-download-mode", "run.modules-download-mode", "", wh("Modules download mode. If not empty, passed as -mod= to go tools")) - internal.Vibra(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound, + internal.AddFlagAndBind(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound, wh("Exit code when issues were found")) - internal.Vibra(v, fs, fs.String, "go", "run.go", "", wh("Targeted Go version")) + internal.AddFlagAndBind(v, fs, fs.String, "go", "run.go", "", wh("Targeted Go version")) fs.StringSlice("build-tags", nil, wh("Build tags")) // Hack see Loader.applyStringSliceHack - internal.Vibra(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, wh("Timeout for total work")) + internal.AddFlagAndBind(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, wh("Timeout for total work")) - internal.Vibra(v, fs, fs.Bool, "tests", "run.tests", true, wh("Analyze tests (*_test.go)")) + internal.AddFlagAndBind(v, fs, fs.Bool, "tests", "run.tests", true, wh("Analyze tests (*_test.go)")) fs.StringSlice("skip-dirs", nil, wh("Regexps of directories to skip")) // Hack see Loader.applyStringSliceHack - internal.Vibra(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) + internal.AddFlagAndBind(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) fs.StringSlice("skip-files", nil, wh("Regexps of files to skip")) // Hack see Loader.applyStringSliceHack const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + "If false (default) - golangci-lint acquires file lock on start." - internal.Vibra(v, fs, fs.Bool, "allow-parallel-runners", "run.allow-parallel-runners", false, wh(allowParallelDesc)) + internal.AddFlagAndBind(v, fs, fs.Bool, "allow-parallel-runners", "run.allow-parallel-runners", false, wh(allowParallelDesc)) const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." - internal.Vibra(v, fs, fs.Bool, "allow-serial-runners", "run.allow-serial-runners", false, wh(allowSerialDesc)) - internal.Vibra(v, fs, fs.Bool, "show-stats", "run.show-stats", false, wh("Show statistics per linter")) + internal.AddFlagAndBind(v, fs, fs.Bool, "allow-serial-runners", "run.allow-serial-runners", false, wh(allowSerialDesc)) + internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "run.show-stats", false, wh("Show statistics per linter")) } func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - internal.Vibra(v, fs, fs.String, "out-format", "output.format", config.OutFormatColoredLineNumber, + internal.AddFlagAndBind(v, fs, fs.String, "out-format", "output.format", config.OutFormatColoredLineNumber, wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) - internal.Vibra(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true, wh("Print lines of code with issue")) - internal.Vibra(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, wh("Print linter name in issue line")) - internal.Vibra(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, wh("Make issues output unique by line")) - internal.Vibra(v, fs, fs.Bool, "sort-results", "output.sort-results", false, wh("Sort linter results")) - internal.Vibra(v, fs, fs.String, "path-prefix", "output.path-prefix", "", wh("Path prefix to add to output")) + internal.AddFlagAndBind(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true, wh("Print lines of code with issue")) + internal.AddFlagAndBind(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, wh("Print linter name in issue line")) + internal.AddFlagAndBind(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, wh("Make issues output unique by line")) + internal.AddFlagAndBind(v, fs, fs.Bool, "sort-results", "output.sort-results", false, wh("Sort linter results")) + internal.AddFlagAndBind(v, fs, fs.String, "path-prefix", "output.path-prefix", "", wh("Path prefix to add to output")) } //nolint:gomnd func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { fs.StringSliceP("exclude", "e", nil, wh("Exclude issue by regexp")) // Hack see Loader.applyStringSliceHack - internal.Vibra(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, getDefaultIssueExcludeHelp()) - internal.Vibra(v, fs, fs.Bool, "exclude-case-sensitive", "issues.exclude-case-sensitive", false, + internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, getDefaultIssueExcludeHelp()) + internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-case-sensitive", "issues.exclude-case-sensitive", false, wh("If set to true exclude and exclude rules regular expressions are case-sensitive")) - internal.Vibra(v, fs, fs.Int, "max-issues-per-linter", "issues.max-issues-per-linter", 50, + internal.AddFlagAndBind(v, fs, fs.Int, "max-issues-per-linter", "issues.max-issues-per-linter", 50, wh("Maximum issues count per one linter. Set to 0 to disable")) - internal.Vibra(v, fs, fs.Int, "max-same-issues", "issues.max-same-issues", 3, + internal.AddFlagAndBind(v, fs, fs.Int, "max-same-issues", "issues.max-same-issues", 3, wh("Maximum count of issues with the same text. Set to 0 to disable")) const newDesc = "Show only new issues: if there are unstaged changes or untracked files, only those changes " + @@ -85,14 +85,14 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer " + "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate " + "unstaged files before golangci-lint runs." - internal.VibraP(v, fs, fs.BoolP, "new", "n", "issues.new", false, wh(newDesc)) - internal.Vibra(v, fs, fs.String, "new-from-rev", "issues.new-from-rev", "", + internal.AddFlagAndBindP(v, fs, fs.BoolP, "new", "n", "issues.new", false, wh(newDesc)) + internal.AddFlagAndBind(v, fs, fs.String, "new-from-rev", "issues.new-from-rev", "", wh("Show only new issues created after git revision `REV`")) - internal.Vibra(v, fs, fs.String, "new-from-patch", "issues.new-from-patch", "", + internal.AddFlagAndBind(v, fs, fs.String, "new-from-patch", "issues.new-from-patch", "", wh("Show only new issues created in git patch with file path `PATH`")) - internal.Vibra(v, fs, fs.Bool, "whole-files", "issues.whole-files", false, + internal.AddFlagAndBind(v, fs, fs.Bool, "whole-files", "issues.whole-files", false, wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) - internal.Vibra(v, fs, fs.Bool, "fix", "issues.fix", false, wh("Fix found issues (if it's supported by the linter)")) + internal.AddFlagAndBind(v, fs, fs.Bool, "fix", "issues.fix", false, wh("Fix found issues (if it's supported by the linter)")) } func wh(text string) string { diff --git a/pkg/commands/internal/vibra.go b/pkg/commands/internal/vibra.go index 6d2fa4a92b0e..5a5306fdb55e 100644 --- a/pkg/commands/internal/vibra.go +++ b/pkg/commands/internal/vibra.go @@ -11,8 +11,8 @@ type FlagFunc[T any] func(name string, value T, usage string) *T type FlagPFunc[T any] func(name, shorthand string, value T, usage string) *T -// Vibra adds a Cobra flag and binds it with Viper. -func Vibra[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagFunc[T], name, bind string, value T, usage string) { +// AddFlagAndBind adds a Cobra/pflag flag and binds it with Viper. +func AddFlagAndBind[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagFunc[T], name, bind string, value T, usage string) { pfn(name, value, usage) err := v.BindPFlag(bind, fs.Lookup(name)) @@ -21,8 +21,8 @@ func Vibra[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagFunc[T], name, bind } } -// VibraP adds a Cobra flag and binds it with Viper. -func VibraP[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagPFunc[T], name, shorthand, bind string, value T, usage string) { +// AddFlagAndBindP adds a Cobra/pflag flag and binds it with Viper. +func AddFlagAndBindP[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagPFunc[T], name, shorthand, bind string, value T, usage string) { pfn(name, shorthand, value, usage) err := v.BindPFlag(bind, fs.Lookup(name)) From 0ada04e7c7395217a806fd2210d18ad555fdf7b7 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 22:14:42 +0100 Subject: [PATCH 39/42] review: improve rootOptions comments --- pkg/commands/root.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 7a1551110c38..9d54e2f2e126 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -20,10 +20,10 @@ func Execute(info BuildInfo) error { } type rootOptions struct { - PrintVersion bool + PrintVersion bool // Flag only. - Verbose bool // Persistent flag. - Color string // Persistent flag. + Verbose bool // Flag only. + Color string // Flag only. } type rootCommand struct { From c84d5cd0edc26fafee99c7af02682f90d239a548 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 22:17:16 +0100 Subject: [PATCH 40/42] review: remove Loader receiver --- pkg/config/loader.go | 94 ++++++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/pkg/config/loader.go b/pkg/config/loader.go index be682443c7db..789796d8f6ea 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -46,28 +46,28 @@ func NewLoader(log logutils.Log, v *viper.Viper, fs *pflag.FlagSet, opts LoaderO } } -func (r *Loader) Load() error { - err := r.setConfigFile() +func (l *Loader) Load() error { + err := l.setConfigFile() if err != nil { return err } - err = r.parseConfig() + err = l.parseConfig() if err != nil { return err } - r.applyStringSliceHack() + l.applyStringSliceHack() - if r.cfg.Run.Go == "" { - r.cfg.Run.Go = detectGoVersion() + if l.cfg.Run.Go == "" { + l.cfg.Run.Go = detectGoVersion() } return nil } -func (r *Loader) setConfigFile() error { - configFile, err := r.evaluateOptions() +func (l *Loader) setConfigFile() error { + configFile, err := l.evaluateOptions() if err != nil { if errors.Is(err, errConfigDisabled) { return nil @@ -77,29 +77,29 @@ func (r *Loader) setConfigFile() error { } if configFile != "" { - r.viper.SetConfigFile(configFile) + l.viper.SetConfigFile(configFile) // Assume YAML if the file has no extension. if filepath.Ext(configFile) == "" { - r.viper.SetConfigType("yaml") + l.viper.SetConfigType("yaml") } } else { - r.setupConfigFileSearch() + l.setupConfigFileSearch() } return nil } -func (r *Loader) evaluateOptions() (string, error) { - if r.opts.NoConfig && r.opts.Config != "" { +func (l *Loader) evaluateOptions() (string, error) { + if l.opts.NoConfig && l.opts.Config != "" { return "", errors.New("can't combine option --config and --no-config") } - if r.opts.NoConfig { + if l.opts.NoConfig { return "", errConfigDisabled } - configFile, err := homedir.Expand(r.opts.Config) + configFile, err := homedir.Expand(l.opts.Config) if err != nil { return "", errors.New("failed to expand configuration path") } @@ -107,12 +107,12 @@ func (r *Loader) evaluateOptions() (string, error) { return configFile, nil } -func (r *Loader) setupConfigFileSearch() { +func (l *Loader) setupConfigFileSearch() { firstArg := extractFirstPathArg() absStartPath, err := filepath.Abs(firstArg) if err != nil { - r.log.Warnf("Can't make abs path for %q: %s", firstArg, err) + l.log.Warnf("Can't make abs path for %q: %s", firstArg, err) absStartPath = filepath.Clean(firstArg) } @@ -140,31 +140,31 @@ func (r *Loader) setupConfigFileSearch() { // find home directory for global config if home, err := homedir.Dir(); err != nil { - r.log.Warnf("Can't get user's home directory: %s", err.Error()) + l.log.Warnf("Can't get user's home directory: %s", err.Error()) } else if !slices.Contains(configSearchPaths, home) { configSearchPaths = append(configSearchPaths, home) } - r.log.Infof("Config search paths: %s", configSearchPaths) + l.log.Infof("Config search paths: %s", configSearchPaths) - r.viper.SetConfigName(".golangci") + l.viper.SetConfigName(".golangci") for _, p := range configSearchPaths { - r.viper.AddConfigPath(p) + l.viper.AddConfigPath(p) } } -func (r *Loader) parseConfig() error { - if err := r.viper.ReadInConfig(); err != nil { +func (l *Loader) parseConfig() error { + if err := l.viper.ReadInConfig(); err != nil { var configFileNotFoundError viper.ConfigFileNotFoundError if errors.As(err, &configFileNotFoundError) { // Load configuration from flags only. - err = r.viper.Unmarshal(r.cfg) + err = l.viper.Unmarshal(l.cfg) if err != nil { return err } - if err = r.cfg.Validate(); err != nil { + if err = l.cfg.Validate(); err != nil { return fmt.Errorf("can't validate config: %w", err) } @@ -174,21 +174,21 @@ func (r *Loader) parseConfig() error { return fmt.Errorf("can't read viper config: %w", err) } - err := r.setConfigDir() + err := l.setConfigDir() if err != nil { return err } // Load configuration from all sources (flags, file). - if err := r.viper.Unmarshal(r.cfg, fileDecoderHook()); err != nil { + if err := l.viper.Unmarshal(l.cfg, fileDecoderHook()); err != nil { return fmt.Errorf("can't unmarshal config by viper: %w", err) } - if err := r.cfg.Validate(); err != nil { + if err := l.cfg.Validate(); err != nil { return fmt.Errorf("can't validate config: %w", err) } - if r.cfg.InternalTest { // just for testing purposes: to detect config file usage + if l.cfg.InternalTest { // just for testing purposes: to detect config file usage _, _ = fmt.Fprintln(logutils.StdOut, "test") os.Exit(exitcodes.Success) } @@ -196,23 +196,23 @@ func (r *Loader) parseConfig() error { return nil } -func (r *Loader) setConfigDir() error { - usedConfigFile := r.viper.ConfigFileUsed() +func (l *Loader) setConfigDir() error { + usedConfigFile := l.viper.ConfigFileUsed() if usedConfigFile == "" { return nil } if usedConfigFile == os.Stdin.Name() { usedConfigFile = "" - r.log.Infof("Reading config file stdin") + l.log.Infof("Reading config file stdin") } else { var err error usedConfigFile, err = fsutils.ShortestRelPath(usedConfigFile, "") if err != nil { - r.log.Warnf("Can't pretty print config file path: %v", err) + l.log.Warnf("Can't pretty print config file path: %v", err) } - r.log.Infof("Used config file %s", usedConfigFile) + l.log.Infof("Used config file %s", usedConfigFile) } usedConfigDir, err := filepath.Abs(filepath.Dir(usedConfigFile)) @@ -220,7 +220,7 @@ func (r *Loader) setConfigDir() error { return errors.New("can't get config directory") } - r.cfg.cfgDir = usedConfigDir + l.cfg.cfgDir = usedConfigDir return nil } @@ -229,23 +229,23 @@ func (r *Loader) setConfigDir() error { // Viper always overrides StringSlice values. // https://github.com/spf13/viper/issues/1448 // So StringSlice flags are not bind to Viper like that their values are obtain via Cobra Flags. -func (r *Loader) applyStringSliceHack() { - if r.fs == nil { +func (l *Loader) applyStringSliceHack() { + if l.fs == nil { return } - r.appendStringSlice("enable", &r.cfg.Linters.Enable) - r.appendStringSlice("disable", &r.cfg.Linters.Disable) - r.appendStringSlice("presets", &r.cfg.Linters.Presets) - r.appendStringSlice("build-tags", &r.cfg.Run.BuildTags) - r.appendStringSlice("skip-dirs", &r.cfg.Run.SkipDirs) - r.appendStringSlice("skip-files", &r.cfg.Run.SkipFiles) - r.appendStringSlice("exclude", &r.cfg.Issues.ExcludePatterns) + l.appendStringSlice("enable", &l.cfg.Linters.Enable) + l.appendStringSlice("disable", &l.cfg.Linters.Disable) + l.appendStringSlice("presets", &l.cfg.Linters.Presets) + l.appendStringSlice("build-tags", &l.cfg.Run.BuildTags) + l.appendStringSlice("skip-dirs", &l.cfg.Run.SkipDirs) + l.appendStringSlice("skip-files", &l.cfg.Run.SkipFiles) + l.appendStringSlice("exclude", &l.cfg.Issues.ExcludePatterns) } -func (r *Loader) appendStringSlice(name string, current *[]string) { - if r.fs.Changed(name) { - val, _ := r.fs.GetStringSlice(name) +func (l *Loader) appendStringSlice(name string, current *[]string) { + if l.fs.Changed(name) { + val, _ := l.fs.GetStringSlice(name) *current = append(*current, val...) } } From c8a133a692799ce760b1a4a5642a9367994859b0 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 23:26:57 +0100 Subject: [PATCH 41/42] review: remove wh function and lll is stil not my friend --- pkg/commands/flagsets.go | 80 +++++++++++++++++++++------------------- pkg/commands/root.go | 6 +-- pkg/commands/run.go | 15 ++++---- pkg/commands/version.go | 5 ++- 4 files changed, 56 insertions(+), 50 deletions(-) diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index beeba5967cce..0928b41e3a0f 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -15,69 +15,76 @@ import ( ) func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - fs.StringSliceP("disable", "D", nil, wh("Disable specific linter")) // Hack see Loader.applyStringSliceHack - internal.AddFlagAndBind(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, wh("Disable all linters")) + fs.StringSliceP("disable", "D", nil, color.GreenString("Disable specific linter")) // Hack see Loader.applyStringSliceHack + internal.AddFlagAndBind(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, color.GreenString("Disable all linters")) - fs.StringSliceP("enable", "E", nil, wh("Enable specific linter")) // Hack see Loader.applyStringSliceHack - internal.AddFlagAndBind(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, wh("Enable all linters")) + fs.StringSliceP("enable", "E", nil, color.GreenString("Enable specific linter")) // Hack see Loader.applyStringSliceHack + internal.AddFlagAndBind(v, fs, fs.Bool, "enable-all", "linters.enable-all", false, color.GreenString("Enable all linters")) internal.AddFlagAndBind(v, fs, fs.Bool, "fast", "linters.fast", false, - wh("Enable only fast linters from enabled linters set (first run won't be fast)")) + color.GreenString("Enable only fast linters from enabled linters set (first run won't be fast)")) // Hack see Loader.applyStringSliceHack fs.StringSliceP("presets", "p", nil, - wh(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ + color.GreenString(fmt.Sprintf("Enable presets (%s) of linters. Run 'golangci-lint help linters' to see "+ "them. This option implies option --disable-all", strings.Join(lintersdb.AllPresets(), "|")))) } func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBindP(v, fs, fs.IntP, "concurrency", "j", "run.concurrency", getDefaultConcurrency(), - wh("Number of CPUs to use (Default: number of logical CPUs)")) + color.GreenString("Number of CPUs to use (Default: number of logical CPUs)")) internal.AddFlagAndBind(v, fs, fs.String, "modules-download-mode", "run.modules-download-mode", "", - wh("Modules download mode. If not empty, passed as -mod= to go tools")) + color.GreenString("Modules download mode. If not empty, passed as -mod= to go tools")) internal.AddFlagAndBind(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound, - wh("Exit code when issues were found")) - internal.AddFlagAndBind(v, fs, fs.String, "go", "run.go", "", wh("Targeted Go version")) - fs.StringSlice("build-tags", nil, wh("Build tags")) // Hack see Loader.applyStringSliceHack + color.GreenString("Exit code when issues were found")) + internal.AddFlagAndBind(v, fs, fs.String, "go", "run.go", "", color.GreenString("Targeted Go version")) + fs.StringSlice("build-tags", nil, color.GreenString("Build tags")) // Hack see Loader.applyStringSliceHack - internal.AddFlagAndBind(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, wh("Timeout for total work")) + internal.AddFlagAndBind(v, fs, fs.Duration, "timeout", "run.timeout", defaultTimeout, color.GreenString("Timeout for total work")) - internal.AddFlagAndBind(v, fs, fs.Bool, "tests", "run.tests", true, wh("Analyze tests (*_test.go)")) - fs.StringSlice("skip-dirs", nil, wh("Regexps of directories to skip")) // Hack see Loader.applyStringSliceHack + internal.AddFlagAndBind(v, fs, fs.Bool, "tests", "run.tests", true, color.GreenString("Analyze tests (*_test.go)")) + fs.StringSlice("skip-dirs", nil, color.GreenString("Regexps of directories to skip")) // Hack see Loader.applyStringSliceHack internal.AddFlagAndBind(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) - fs.StringSlice("skip-files", nil, wh("Regexps of files to skip")) // Hack see Loader.applyStringSliceHack + fs.StringSlice("skip-files", nil, color.GreenString("Regexps of files to skip")) // Hack see Loader.applyStringSliceHack const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " + "If false (default) - golangci-lint acquires file lock on start." - internal.AddFlagAndBind(v, fs, fs.Bool, "allow-parallel-runners", "run.allow-parallel-runners", false, wh(allowParallelDesc)) + internal.AddFlagAndBind(v, fs, fs.Bool, "allow-parallel-runners", "run.allow-parallel-runners", false, + color.GreenString(allowParallelDesc)) const allowSerialDesc = "Allow multiple golangci-lint instances running, but serialize them around a lock. " + "If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start." - internal.AddFlagAndBind(v, fs, fs.Bool, "allow-serial-runners", "run.allow-serial-runners", false, wh(allowSerialDesc)) - internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "run.show-stats", false, wh("Show statistics per linter")) + internal.AddFlagAndBind(v, fs, fs.Bool, "allow-serial-runners", "run.allow-serial-runners", false, color.GreenString(allowSerialDesc)) + internal.AddFlagAndBind(v, fs, fs.Bool, "show-stats", "run.show-stats", false, color.GreenString("Show statistics per linter")) } func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBind(v, fs, fs.String, "out-format", "output.format", config.OutFormatColoredLineNumber, - wh(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) - internal.AddFlagAndBind(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true, wh("Print lines of code with issue")) - internal.AddFlagAndBind(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, wh("Print linter name in issue line")) - internal.AddFlagAndBind(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, wh("Make issues output unique by line")) - internal.AddFlagAndBind(v, fs, fs.Bool, "sort-results", "output.sort-results", false, wh("Sort linter results")) - internal.AddFlagAndBind(v, fs, fs.String, "path-prefix", "output.path-prefix", "", wh("Path prefix to add to output")) + color.GreenString(fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|")))) + internal.AddFlagAndBind(v, fs, fs.Bool, "print-issued-lines", "output.print-issued-lines", true, + color.GreenString("Print lines of code with issue")) + internal.AddFlagAndBind(v, fs, fs.Bool, "print-linter-name", "output.print-linter-name", true, + color.GreenString("Print linter name in issue line")) + internal.AddFlagAndBind(v, fs, fs.Bool, "uniq-by-line", "output.uniq-by-line", true, + color.GreenString("Make issues output unique by line")) + internal.AddFlagAndBind(v, fs, fs.Bool, "sort-results", "output.sort-results", false, + color.GreenString("Sort linter results")) + internal.AddFlagAndBind(v, fs, fs.String, "path-prefix", "output.path-prefix", "", + color.GreenString("Path prefix to add to output")) } //nolint:gomnd func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - fs.StringSliceP("exclude", "e", nil, wh("Exclude issue by regexp")) // Hack see Loader.applyStringSliceHack - internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, getDefaultIssueExcludeHelp()) + fs.StringSliceP("exclude", "e", nil, color.GreenString("Exclude issue by regexp")) // Hack see Loader.applyStringSliceHack + internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-use-default", "issues.exclude-use-default", true, + getDefaultIssueExcludeHelp()) internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-case-sensitive", "issues.exclude-case-sensitive", false, - wh("If set to true exclude and exclude rules regular expressions are case-sensitive")) + color.GreenString("If set to true exclude and exclude rules regular expressions are case-sensitive")) internal.AddFlagAndBind(v, fs, fs.Int, "max-issues-per-linter", "issues.max-issues-per-linter", 50, - wh("Maximum issues count per one linter. Set to 0 to disable")) + color.GreenString("Maximum issues count per one linter. Set to 0 to disable")) internal.AddFlagAndBind(v, fs, fs.Int, "max-same-issues", "issues.max-same-issues", 3, - wh("Maximum count of issues with the same text. Set to 0 to disable")) + color.GreenString("Maximum count of issues with the same text. Set to 0 to disable")) const newDesc = "Show only new issues: if there are unstaged changes or untracked files, only those changes " + "are analyzed, else only changes in HEAD~ are analyzed.\nIt's a super-useful option for integration " + @@ -85,16 +92,13 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { "the moment of integration: much better to not allow issues in new code.\nFor CI setups, prefer " + "--new-from-rev=HEAD~, as --new can skip linting the current patch if any scripts generate " + "unstaged files before golangci-lint runs." - internal.AddFlagAndBindP(v, fs, fs.BoolP, "new", "n", "issues.new", false, wh(newDesc)) + internal.AddFlagAndBindP(v, fs, fs.BoolP, "new", "n", "issues.new", false, color.GreenString(newDesc)) internal.AddFlagAndBind(v, fs, fs.String, "new-from-rev", "issues.new-from-rev", "", - wh("Show only new issues created after git revision `REV`")) + color.GreenString("Show only new issues created after git revision `REV`")) internal.AddFlagAndBind(v, fs, fs.String, "new-from-patch", "issues.new-from-patch", "", - wh("Show only new issues created in git patch with file path `PATH`")) + color.GreenString("Show only new issues created in git patch with file path `PATH`")) internal.AddFlagAndBind(v, fs, fs.Bool, "whole-files", "issues.whole-files", false, - wh("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) - internal.AddFlagAndBind(v, fs, fs.Bool, "fix", "issues.fix", false, wh("Fix found issues (if it's supported by the linter)")) -} - -func wh(text string) string { - return color.GreenString(text) + color.GreenString("Show issues in any part of update files (requires new-from-rev or new-from-patch)")) + internal.AddFlagAndBind(v, fs, fs.Bool, "fix", "issues.fix", false, + color.GreenString("Fix found issues (if it's supported by the linter)")) } diff --git a/pkg/commands/root.go b/pkg/commands/root.go index 9d54e2f2e126..2a5e7cf1768e 100644 --- a/pkg/commands/root.go +++ b/pkg/commands/root.go @@ -52,7 +52,7 @@ func newRootCommand(info BuildInfo) *rootCommand { } fs := rootCmd.Flags() - fs.BoolVar(&c.opts.PrintVersion, "version", false, wh("Print version")) + fs.BoolVar(&c.opts.PrintVersion, "version", false, color.GreenString("Print version")) setupRootPersistentFlags(rootCmd.PersistentFlags(), &c.opts) @@ -86,8 +86,8 @@ func (c *rootCommand) Execute() error { } func setupRootPersistentFlags(fs *pflag.FlagSet, opts *rootOptions) { - fs.BoolVarP(&opts.Verbose, "verbose", "v", false, wh("Verbose output")) - fs.StringVar(&opts.Color, "color", "auto", wh("Use color when printing; can be 'always', 'auto', or 'never'")) + fs.BoolVarP(&opts.Verbose, "verbose", "v", false, color.GreenString("Verbose output")) + fs.StringVar(&opts.Color, "color", "auto", color.GreenString("Use color when printing; can be 'always', 'auto', or 'never'")) } func setupLogger(logger logutils.Log) error { diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 644ccd227c43..a4ba89058714 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -126,7 +126,8 @@ func newRunCommand(logger logutils.Log, cfg *config.Config, reportData *report.D // Only for testing purpose. // Don't add other flags here. - fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, wh("Option is used only for testing golangci-lint command, don't use it")) + fs.BoolVar(&cfg.InternalCmdTest, "internal-cmd-test", false, + color.GreenString("Option is used only for testing golangci-lint command, don't use it")) _ = fs.MarkHidden("internal-cmd-test") setupConfigFileFlagSet(fs, &c.opts.LoaderOptions) @@ -606,8 +607,8 @@ func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log } func setupConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.LoaderOptions) { - fs.StringVarP(&cfg.Config, "config", "c", "", wh("Read config from file path `PATH`")) - fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) + fs.StringVarP(&cfg.Config, "config", "c", "", color.GreenString("Read config from file path `PATH`")) + fs.BoolVar(&cfg.NoConfig, "no-config", false, color.GreenString("Don't read config file")) } func getDefaultIssueExcludeHelp() string { @@ -633,11 +634,11 @@ func getDefaultDirectoryExcludeHelp() string { func setupRunPersistentFlags(fs *pflag.FlagSet, opts *runOptions) { fs.BoolVar(&opts.PrintResourcesUsage, "print-resources-usage", false, - wh("Print avg and max memory usage of golangci-lint and total time")) + color.GreenString("Print avg and max memory usage of golangci-lint and total time")) - fs.StringVar(&opts.CPUProfilePath, "cpu-profile-path", "", wh("Path to CPU profile output file")) - fs.StringVar(&opts.MemProfilePath, "mem-profile-path", "", wh("Path to memory profile output file")) - fs.StringVar(&opts.TracePath, "trace-path", "", wh("Path to trace output file")) + fs.StringVar(&opts.CPUProfilePath, "cpu-profile-path", "", color.GreenString("Path to CPU profile output file")) + fs.StringVar(&opts.MemProfilePath, "mem-profile-path", "", color.GreenString("Path to memory profile output file")) + fs.StringVar(&opts.TracePath, "trace-path", "", color.GreenString("Path to trace output file")) } func getDefaultConcurrency() int { diff --git a/pkg/commands/version.go b/pkg/commands/version.go index b73d8dd63a57..124ad7f1a339 100644 --- a/pkg/commands/version.go +++ b/pkg/commands/version.go @@ -8,6 +8,7 @@ import ( "runtime/debug" "strings" + "github.com/fatih/color" "github.com/spf13/cobra" ) @@ -49,8 +50,8 @@ func newVersionCommand(info BuildInfo) *versionCommand { fs := versionCmd.Flags() fs.SortFlags = false // sort them as they are defined here - fs.StringVar(&c.opts.Format, "format", "", wh("The version's format can be: 'short', 'json'")) - fs.BoolVar(&c.opts.Debug, "debug", false, wh("Add build information")) + fs.StringVar(&c.opts.Format, "format", "", color.GreenString("The version's format can be: 'short', 'json'")) + fs.BoolVar(&c.opts.Debug, "debug", false, color.GreenString("Add build information")) c.cmd = versionCmd From 740c5ca7039515dd4c6793c7c8c335447a3810a2 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Mon, 26 Feb 2024 23:28:58 +0100 Subject: [PATCH 42/42] Revert "chore: clean configuration reference" This reverts commit 8aa1b0457e4a9eecad84c6a91abbf2957b37d764. --- .golangci.reference.yml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 8f99a2195356..7766f2909782 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -68,13 +68,17 @@ run: # Allow multiple parallel golangci-lint instances running. # If false, golangci-lint acquires file lock on start. # Default: false - allow-parallel-runners: true + allow-parallel-runners: false # Allow multiple golangci-lint instances running, but serialize them around a lock. # If false, golangci-lint exits with an error if it fails to acquire file lock on start. # Default: false allow-serial-runners: true + # Print avg and max memory usage of golangci-lint and total time. + # Default: false + print-resources-usage: true + # Define the Go version limit. # Mainly related to generics support since go1.18. # Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`, fallback on 1.17 @@ -2487,11 +2491,6 @@ linters-settings: # Intended to point to the repo location of the linter. # Optional. original-url: github.com/golangci/example-linter - # Plugins settings/configuration. - # Only work with plugin based on `linterdb.PluginConstructor`. - # Optional. - settings: - foo: bar linters: