From 61e744c428241b9a539114e9d1737208cc189a30 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Sat, 23 Dec 2023 12:09:14 -0300 Subject: [PATCH 1/3] fix: Remove unrelated flags from config and linters command Signed-off-by: Mateus Oliveira --- pkg/commands/config.go | 6 ++++-- pkg/commands/linters.go | 26 ++++++++++++++------------ pkg/commands/run.go | 20 +++++++++++++------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index a16ef63106c1..67843f3a68fe 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -14,7 +14,7 @@ import ( func (e *Executor) initConfig() { cmd := &cobra.Command{ Use: "config", - Short: "Config", + Short: "Config file information", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { return cmd.Help() @@ -29,7 +29,9 @@ func (e *Executor) initConfig() { ValidArgsFunction: cobra.NoFileCompletions, Run: e.executePathCmd, } - e.initRunConfiguration(pathCmd) // allow --config + fs := pathCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + e.initConfigFileFlagSet(fs, &e.cfg.Run) cmd.AddCommand(pathCmd) } diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 292713ec9002..58cec261e400 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -17,8 +17,10 @@ func (e *Executor) initLinters() { ValidArgsFunction: cobra.NoFileCompletions, RunE: e.executeLinters, } + fs := e.lintersCmd.Flags() + fs.SortFlags = false // sort them as they are defined here + e.initConfigFileFlagSet(fs, &e.cfg.Run) e.rootCmd.AddCommand(e.lintersCmd) - e.initRunConfiguration(e.lintersCmd) } // executeLinters runs the 'linters' CLI command, which displays the supported linters. @@ -28,18 +30,9 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { return fmt.Errorf("can't get enabled linters: %w", err) } - color.Green("Enabled by your configuration linters:\n") var enabledLinters []*linter.Config - for _, lc := range enabledLintersMap { - if lc.Internal { - continue - } - - enabledLinters = append(enabledLinters, lc) - } - printLinterConfigs(enabledLinters) - var disabledLCs []*linter.Config + for _, lc := range e.DBManager.GetAllSupportedLinterConfigs() { if lc.Internal { continue @@ -47,10 +40,19 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { if enabledLintersMap[lc.Name()] == nil { disabledLCs = append(disabledLCs, lc) + } else { + enabledLinters = append(enabledLinters, lc) } } - color.Red("\nDisabled by your configuration linters:\n") + enabledBy := "your configuration" + if e.cfg.Run.NoConfig { + enabledBy = "default" + } + + color.Green("Enabled by %v linters:\n", enabledBy) + printLinterConfigs(enabledLinters) + color.Red("\nDisabled by %v linters:\n", enabledBy) printLinterConfigs(disabledLCs) return nil diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 933096ada434..8d9b8c160ddc 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -63,8 +63,13 @@ func wh(text string) string { const defaultTimeout = time.Minute +func (e *Executor) 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")) +} + //nolint:funlen,gomnd -func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, isFinalInit bool) { +func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { hideFlag := func(name string) { if err := fs.MarkHidden(name); err != nil { panic(err) @@ -79,6 +84,10 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is } } + // Config file config + rc := &cfg.Run + e.initConfigFileFlagSet(fs, rc) + // Output config oc := &cfg.Output fs.StringVar(&oc.Format, "out-format", @@ -98,7 +107,6 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is } // Run config - rc := &cfg.Run 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", @@ -115,8 +123,6 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is 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.StringVarP(&rc.Config, "config", "c", "", wh("Read config from file path `PATH`")) - fs.BoolVar(&rc.NoConfig, "no-config", false, wh("Don't read config")) 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")) @@ -207,7 +213,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is fs.BoolVar(&lc.DisableAll, "disable-all", false, wh("Disable all linters")) fs.StringSliceVarP(&lc.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(m.AllPresets(), "|")))) + "them. This option implies option --disable-all", strings.Join(e.DBManager.AllPresets(), "|")))) fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set (first run won't be fast)")) // Issues config @@ -241,7 +247,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is func (e *Executor) initRunConfiguration(cmd *cobra.Command) { fs := cmd.Flags() fs.SortFlags = false // sort them as they are defined here - initFlagSet(fs, e.cfg, e.DBManager, true) + e.initFlagSet(fs, e.cfg, true) } func (e *Executor) getConfigForCommandLine() (*config.Config, error) { @@ -254,7 +260,7 @@ func (e *Executor) getConfigForCommandLine() (*config.Config, error) { // `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. - initFlagSet(fs, &cfg, e.DBManager, false) + e.initFlagSet(fs, &cfg, false) initVersionFlagSet(fs, &cfg) // Parse max options, even force version option: don't want From f35f48c774b5e3d520319601fabff402f9f46029 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Sat, 23 Dec 2023 15:30:25 -0300 Subject: [PATCH 2/3] fixup! fix: Remove unrelated flags from config and linters command Signed-off-by: Mateus Oliveira --- .golangci.reference.yml | 2 +- pkg/commands/linters.go | 1 + pkg/commands/run.go | 21 ++++++++++++--------- test/testshared/runner.go | 11 ++++++----- test/testshared/runner_test.go | 11 ++++++++++- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index f9a8d0feec32..383e5633f4e8 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2649,7 +2649,7 @@ linters: - test - unused - # Run only fast linters from enabled linters set (first run won't be fast) + # Enable only fast linters from enabled linters set (first run won't be fast) # Default: false fast: true diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 58cec261e400..9ea8e286a37b 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -20,6 +20,7 @@ func (e *Executor) initLinters() { fs := e.lintersCmd.Flags() fs.SortFlags = false // sort them as they are defined here e.initConfigFileFlagSet(fs, &e.cfg.Run) + e.initLintersFlagSet(fs, &e.cfg.Linters) e.rootCmd.AddCommand(e.lintersCmd) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 8d9b8c160ddc..314afa7d2fb3 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -68,6 +68,17 @@ func (e *Executor) initConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.Run) { fs.BoolVar(&cfg.NoConfig, "no-config", false, wh("Don't read config file")) } +func (e *Executor) 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(e.DBManager.AllPresets(), "|")))) +} + //nolint:funlen,gomnd func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { hideFlag := func(name string) { @@ -206,15 +217,7 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni // Linters config lc := &cfg.Linters - fs.StringSliceVarP(&lc.Enable, "enable", "E", nil, wh("Enable specific linter")) - fs.StringSliceVarP(&lc.Disable, "disable", "D", nil, wh("Disable specific linter")) - fs.BoolVar(&lc.EnableAll, "enable-all", false, wh("Enable all linters")) - - fs.BoolVar(&lc.DisableAll, "disable-all", false, wh("Disable all linters")) - fs.StringSliceVarP(&lc.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(e.DBManager.AllPresets(), "|")))) - fs.BoolVar(&lc.Fast, "fast", false, wh("Run only fast linters from enabled linters set (first run won't be fast)")) + e.initLintersFlagSet(fs, lc) // Issues config ic := &cfg.Issues diff --git a/test/testshared/runner.go b/test/testshared/runner.go index 7d7fb9d4b4de..f6bff20c5d57 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -163,12 +163,13 @@ func (b *RunnerBuilder) Runner() *Runner { b.tb.Fatal("--no-config and -c cannot be used at the same time") } - arguments := []string{ - "--internal-cmd-test", - } + arguments := []string{} - if b.allowParallelRunners { - arguments = append(arguments, "--allow-parallel-runners") + if b.command == "run" { + arguments = append(arguments, "--internal-cmd-test") + if b.allowParallelRunners { + arguments = append(arguments, "--allow-parallel-runners") + } } if b.noConfig { diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index d7e0d32f0baa..dc0d92421d66 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -28,11 +28,20 @@ func TestRunnerBuilder_Runner(t *testing.T) { }, }, { - desc: "with command", + desc: "with non run command", builder: NewRunnerBuilder(t).WithCommand("example"), expected: &Runner{ env: []string(nil), command: "example", + args: []string{}, + }, + }, + { + desc: "with run command", + builder: NewRunnerBuilder(t).WithCommand("run"), + expected: &Runner{ + env: []string(nil), + command: "run", args: []string{ "--internal-cmd-test", "--allow-parallel-runners", From 276a74675733a64834346ed153c6093414b97884 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Fri, 2 Feb 2024 22:06:00 +0100 Subject: [PATCH 3/3] review --- pkg/commands/config.go | 12 +++++- pkg/commands/linters.go | 28 +++++++++---- pkg/commands/run.go | 72 +++++++++++++--------------------- test/testshared/runner.go | 2 +- test/testshared/runner_test.go | 1 - 5 files changed, 60 insertions(+), 55 deletions(-) diff --git a/pkg/commands/config.go b/pkg/commands/config.go index 67843f3a68fe..45c4fcd77cfd 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -5,8 +5,10 @@ import ( "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" ) @@ -29,9 +31,12 @@ func (e *Executor) initConfig() { ValidArgsFunction: cobra.NoFileCompletions, Run: e.executePathCmd, } + fs := pathCmd.Flags() fs.SortFlags = false // sort them as they are defined here - e.initConfigFileFlagSet(fs, &e.cfg.Run) + + initConfigFileFlagSet(fs, &e.cfg.Run) + cmd.AddCommand(pathCmd) } @@ -61,3 +66,8 @@ func (e *Executor) executePathCmd(_ *cobra.Command, _ []string) { fmt.Println(usedConfigFile) } + +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/linters.go b/pkg/commands/linters.go index 9ea8e286a37b..69df211542be 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -2,10 +2,13 @@ package commands import ( "fmt" + "strings" "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/lint/linter" ) @@ -17,13 +20,27 @@ func (e *Executor) initLinters() { ValidArgsFunction: cobra.NoFileCompletions, RunE: e.executeLinters, } + fs := e.lintersCmd.Flags() fs.SortFlags = false // sort them as they are defined here - e.initConfigFileFlagSet(fs, &e.cfg.Run) + + initConfigFileFlagSet(fs, &e.cfg.Run) e.initLintersFlagSet(fs, &e.cfg.Linters) + e.rootCmd.AddCommand(e.lintersCmd) } +func (e *Executor) 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(e.DBManager.AllPresets(), "|")))) +} + // executeLinters runs the 'linters' CLI command, which displays the supported linters. func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { enabledLintersMap, err := e.EnabledLintersSet.GetEnabledLintersMap() @@ -46,14 +63,9 @@ func (e *Executor) executeLinters(_ *cobra.Command, _ []string) error { } } - enabledBy := "your configuration" - if e.cfg.Run.NoConfig { - enabledBy = "default" - } - - color.Green("Enabled by %v linters:\n", enabledBy) + color.Green("Enabled by your configuration linters:\n") printLinterConfigs(enabledLinters) - color.Red("\nDisabled by %v linters:\n", enabledBy) + color.Red("\nDisabled by your configuration linters:\n") printLinterConfigs(disabledLCs) return nil diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 314afa7d2fb3..5c7083c307a5 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -29,6 +29,8 @@ import ( const defaultFileMode = 0644 +const defaultTimeout = time.Minute + const ( // envFailOnWarnings value: "1" envFailOnWarnings = "FAIL_ON_WARNINGS" @@ -36,49 +38,6 @@ const ( envMemLogEvery = "GL_MEM_LOG_EVERY" ) -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 wh(text string) string { - return color.GreenString(text) -} - -const defaultTimeout = time.Minute - -func (e *Executor) 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")) -} - -func (e *Executor) 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(e.DBManager.AllPresets(), "|")))) -} - //nolint:funlen,gomnd func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalInit bool) { hideFlag := func(name string) { @@ -97,7 +56,7 @@ func (e *Executor) initFlagSet(fs *pflag.FlagSet, cfg *config.Config, isFinalIni // Config file config rc := &cfg.Run - e.initConfigFileFlagSet(fs, rc) + initConfigFileFlagSet(fs, rc) // Output config oc := &cfg.Output @@ -648,3 +607,28 @@ func watchResources(ctx context.Context, done chan struct{}, logger logutils.Log 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") +} + +func wh(text string) string { + return color.GreenString(text) +} diff --git a/test/testshared/runner.go b/test/testshared/runner.go index f6bff20c5d57..0be6d7404930 100644 --- a/test/testshared/runner.go +++ b/test/testshared/runner.go @@ -163,7 +163,7 @@ func (b *RunnerBuilder) Runner() *Runner { b.tb.Fatal("--no-config and -c cannot be used at the same time") } - arguments := []string{} + var arguments []string if b.command == "run" { arguments = append(arguments, "--internal-cmd-test") diff --git a/test/testshared/runner_test.go b/test/testshared/runner_test.go index dc0d92421d66..5d2b3634387d 100644 --- a/test/testshared/runner_test.go +++ b/test/testshared/runner_test.go @@ -33,7 +33,6 @@ func TestRunnerBuilder_Runner(t *testing.T) { expected: &Runner{ env: []string(nil), command: "example", - args: []string{}, }, }, {