From 50fb95e6f764081436be2ab7b66665e3e6adb546 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 28 Mar 2024 21:39:41 +0100 Subject: [PATCH 1/2] dev: option to not check deprecation --- .golangci.yml | 9 ------- pkg/commands/config.go | 10 ++------ pkg/commands/linters.go | 10 ++------ pkg/commands/run.go | 3 ++- pkg/config/loader.go | 54 +++++++++++++++++++++-------------------- 5 files changed, 34 insertions(+), 52 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cb937ca8f92e..c093452a27de 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -161,20 +161,11 @@ issues: - path: pkg/commands/run.go linters: [staticcheck] text: "SA1019: c.cfg.Run.ShowStats is deprecated: use Output.ShowStats instead." - - path: pkg/commands/config.go - linters: [staticcheck] - text: "SA1019: cfg.Run.UseDefaultSkipDirs is deprecated: use Issues.UseDefaultExcludeDirs instead." - - path: pkg/commands/linters.go - linters: [staticcheck] - text: "SA1019: c.cfg.Run.UseDefaultSkipDirs is deprecated: use Issues.UseDefaultExcludeDirs instead." # Deprecated linter options. - path: pkg/golinters/errcheck.go linters: [staticcheck] text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead" - - path: pkg/commands/run.go - linters: [staticcheck] - text: "SA1019: lsc.Errcheck.Exclude is deprecated: use ExcludeFunctions instead" - path: pkg/golinters/govet.go linters: [staticcheck] text: "SA1019: cfg.CheckShadowing is deprecated: the linter should be enabled inside Enable." diff --git a/pkg/commands/config.go b/pkg/commands/config.go index defb5fb22a2b..935ec5e8643c 100644 --- a/pkg/commands/config.go +++ b/pkg/commands/config.go @@ -84,16 +84,10 @@ func (c *configCommand) preRunE(cmd *cobra.Command, args []string) error { // It only needs to know the path of the configuration file. cfg := config.NewDefault() - // Hack to hide deprecation messages related to `--skip-dirs-use-default`: - // Flags are not bound then the default values, defined only through flags, are not applied. - // In this command, file path and file information are the only requirements, i.e. it don't need flag values. - // - // TODO(ldez) add an option (check deprecation) to `Loader.Load()` but this require a dedicated PR. - cfg.Run.UseDefaultSkipDirs = true - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts, cfg, args) - if err := loader.Load(); err != nil { + err := loader.Load(config.LoadOptions{}) + if err != nil { return fmt.Errorf("can't load config: %w", err) } diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 72e8a985295a..926c29480af4 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -59,16 +59,10 @@ func newLintersCommand(logger logutils.Log) *lintersCommand { } func (c *lintersCommand) preRunE(cmd *cobra.Command, args []string) error { - // Hack to hide deprecation messages related to `--skip-dirs-use-default`: - // Flags are not bound then the default values, defined only through flags, are not applied. - // In this command, linters information are the only requirements, i.e. it don't need flag values. - // - // TODO(ldez) add an option (check deprecation) to `Loader.Load()` but this require a dedicated PR. - c.cfg.Run.UseDefaultSkipDirs = true - loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg, args) - if err := loader.Load(); err != nil { + err := loader.Load(config.LoadOptions{}) + if err != nil { return fmt.Errorf("can't load config: %w", err) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 655852bde688..2477f023cb37 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -154,7 +154,8 @@ func (c *runCommand) persistentPreRunE(cmd *cobra.Command, args []string) error loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg, args) - if err := loader.Load(); err != nil { + err := loader.Load(config.LoadOptions{CheckDeprecation: true}) + if err != nil { return fmt.Errorf("can't load config: %w", err) } diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 41fda911ae91..c383a135d1a6 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -24,6 +24,10 @@ type LoaderOptions struct { NoConfig bool // Flag only. } +type LoadOptions struct { + CheckDeprecation bool +} + type Loader struct { opts LoaderOptions @@ -47,7 +51,7 @@ func NewLoader(log logutils.Log, v *viper.Viper, fs *pflag.FlagSet, opts LoaderO } } -func (l *Loader) Load() error { +func (l *Loader) Load(opts LoadOptions) error { err := l.setConfigFile() if err != nil { return err @@ -60,9 +64,11 @@ func (l *Loader) Load() error { l.applyStringSliceHack() - err = l.handleDeprecation() - if err != nil { - return err + if opts.CheckDeprecation { + err = l.handleDeprecation() + if err != nil { + return err + } } l.handleGoVersion() @@ -293,35 +299,39 @@ func (l *Loader) handleGoVersion() { } func (l *Loader) handleDeprecation() error { + if l.cfg.InternalTest || l.cfg.InternalCmdTest || os.Getenv(logutils.EnvTestRun) == "1" { + return nil + } + // Deprecated since v1.57.0 if len(l.cfg.Run.SkipFiles) > 0 { - l.warn("The configuration option `run.skip-files` is deprecated, please use `issues.exclude-files`.") + l.log.Warnf("The configuration option `run.skip-files` is deprecated, please use `issues.exclude-files`.") l.cfg.Issues.ExcludeFiles = l.cfg.Run.SkipFiles } // Deprecated since v1.57.0 if len(l.cfg.Run.SkipDirs) > 0 { - l.warn("The configuration option `run.skip-dirs` is deprecated, please use `issues.exclude-dirs`.") + l.log.Warnf("The configuration option `run.skip-dirs` is deprecated, please use `issues.exclude-dirs`.") l.cfg.Issues.ExcludeDirs = l.cfg.Run.SkipDirs } // The 2 options are true by default. // Deprecated since v1.57.0 if !l.cfg.Run.UseDefaultSkipDirs { - l.warn("The configuration option `run.skip-dirs-use-default` is deprecated, please use `issues.exclude-dirs-use-default`.") + l.log.Warnf("The configuration option `run.skip-dirs-use-default` is deprecated, please use `issues.exclude-dirs-use-default`.") } l.cfg.Issues.UseDefaultExcludeDirs = l.cfg.Run.UseDefaultSkipDirs && l.cfg.Issues.UseDefaultExcludeDirs // The 2 options are false by default. // Deprecated since v1.57.0 if l.cfg.Run.ShowStats { - l.warn("The configuration option `run.show-stats` is deprecated, please use `output.show-stats`") + l.log.Warnf("The configuration option `run.show-stats` is deprecated, please use `output.show-stats`") } l.cfg.Output.ShowStats = l.cfg.Run.ShowStats || l.cfg.Output.ShowStats // Deprecated since v1.57.0 if l.cfg.Output.Format != "" { - l.warn("The configuration option `output.format` is deprecated, please use `output.formats`") + l.log.Warnf("The configuration option `output.format` is deprecated, please use `output.formats`") var f OutputFormats err := f.UnmarshalText([]byte(l.cfg.Output.Format)) @@ -341,49 +351,49 @@ func (l *Loader) handleLinterOptionDeprecations() { // Deprecated since v1.57.0, // but it was unofficially deprecated since v1.19 (2019) (https://github.com/golangci/golangci-lint/pull/697). if l.cfg.LintersSettings.Govet.CheckShadowing { - l.warn("The configuration option `linters.govet.check-shadowing` is deprecated. " + + l.log.Warnf("The configuration option `linters.govet.check-shadowing` is deprecated. " + "Please enable `shadow` instead, if you are not using `enable-all`.") } // Deprecated since v1.42.0. if l.cfg.LintersSettings.Errcheck.Exclude != "" { - l.warn("The configuration option `linters.errcheck.exclude` is deprecated, please use `linters.errcheck.exclude-functions`.") + l.log.Warnf("The configuration option `linters.errcheck.exclude` is deprecated, please use `linters.errcheck.exclude-functions`.") } // Deprecated since v1.44.0. if l.cfg.LintersSettings.Gci.LocalPrefixes != "" { - l.warn("The configuration option `linters.gci.local-prefixes` is deprecated, please use `prefix()` inside `linters.gci.sections`.") + l.log.Warnf("The configuration option `linters.gci.local-prefixes` is deprecated, please use `prefix()` inside `linters.gci.sections`.") } // Deprecated since v1.33.0. if l.cfg.LintersSettings.Godot.CheckAll { - l.warn("The configuration option `linters.godot.check-all` is deprecated, please use `linters.godot.scope: all`.") + l.log.Warnf("The configuration option `linters.godot.check-all` is deprecated, please use `linters.godot.scope: all`.") } // Deprecated since v1.44.0. if len(l.cfg.LintersSettings.Gomnd.Settings) > 0 { - l.warn("The configuration option `linters.gomnd.settings` is deprecated. Please use the options " + + l.log.Warnf("The configuration option `linters.gomnd.settings` is deprecated. Please use the options " + "`linters.gomnd.checks`,`linters.gomnd.ignored-numbers`,`linters.gomnd.ignored-files`,`linters.gomnd.ignored-functions`.") } // Deprecated since v1.47.0 if l.cfg.LintersSettings.Gofumpt.LangVersion != "" { - l.warn("The configuration option `linters.gofumpt.lang-version` is deprecated, please use global `run.go`.") + l.log.Warnf("The configuration option `linters.gofumpt.lang-version` is deprecated, please use global `run.go`.") } // Deprecated since v1.47.0 if l.cfg.LintersSettings.Staticcheck.GoVersion != "" { - l.warn("The configuration option `linters.staticcheck.go` is deprecated, please use global `run.go`.") + l.log.Warnf("The configuration option `linters.staticcheck.go` is deprecated, please use global `run.go`.") } // Deprecated since v1.47.0 if l.cfg.LintersSettings.Gosimple.GoVersion != "" { - l.warn("The configuration option `linters.gosimple.go` is deprecated, please use global `run.go`.") + l.log.Warnf("The configuration option `linters.gosimple.go` is deprecated, please use global `run.go`.") } // Deprecated since v1.47.0 if l.cfg.LintersSettings.Stylecheck.GoVersion != "" { - l.warn("The configuration option `linters.stylecheck.go` is deprecated, please use global `run.go`.") + l.log.Warnf("The configuration option `linters.stylecheck.go` is deprecated, please use global `run.go`.") } } @@ -408,14 +418,6 @@ func (l *Loader) handleEnableOnlyOption() error { return nil } -func (l *Loader) warn(format string) { - if l.cfg.InternalTest || l.cfg.InternalCmdTest || os.Getenv(logutils.EnvTestRun) == "1" { - return - } - - l.log.Warnf(format) -} - func customDecoderHook() viper.DecoderConfigOption { return viper.DecodeHook(mapstructure.ComposeDecodeHookFunc( // Default hooks (https://github.com/spf13/viper/blob/518241257478c557633ab36e474dfcaeb9a3c623/viper.go#L135-L138). From b5068ccf26a658c493e9cdcd2a8709d46fa9bf65 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 30 Mar 2024 12:49:56 +0100 Subject: [PATCH 2/2] feat: early validation --- pkg/commands/linters.go | 2 +- pkg/commands/run.go | 2 +- pkg/config/config.go | 8 ++++---- pkg/config/loader.go | 8 ++++++++ pkg/lint/lintersdb/validator.go | 5 ----- 5 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/commands/linters.go b/pkg/commands/linters.go index 926c29480af4..a93814f0f851 100644 --- a/pkg/commands/linters.go +++ b/pkg/commands/linters.go @@ -61,7 +61,7 @@ func newLintersCommand(logger logutils.Log) *lintersCommand { func (c *lintersCommand) preRunE(cmd *cobra.Command, args []string) error { loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg, args) - err := loader.Load(config.LoadOptions{}) + err := loader.Load(config.LoadOptions{Validation: true}) if err != nil { return fmt.Errorf("can't load config: %w", err) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 2477f023cb37..dc323f101768 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -154,7 +154,7 @@ func (c *runCommand) persistentPreRunE(cmd *cobra.Command, args []string) error loader := config.NewLoader(c.log.Child(logutils.DebugKeyConfigReader), c.viper, cmd.Flags(), c.opts.LoaderOptions, c.cfg, args) - err := loader.Load(config.LoadOptions{CheckDeprecation: true}) + err := loader.Load(config.LoadOptions{CheckDeprecation: true, Validation: true}) if err != nil { return fmt.Errorf("can't load config: %w", err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index af27a91b55ff..59f6eef0da91 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -33,12 +33,12 @@ func (c *Config) GetConfigDir() string { func (c *Config) Validate() error { validators := []func() error{ - c.Issues.Validate, - c.Severity.Validate, + c.Run.Validate, + c.Output.Validate, c.LintersSettings.Validate, c.Linters.Validate, - c.Output.Validate, - c.Run.Validate, + c.Issues.Validate, + c.Severity.Validate, } for _, v := range validators { diff --git a/pkg/config/loader.go b/pkg/config/loader.go index c383a135d1a6..e373abd63b9f 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -26,6 +26,7 @@ type LoaderOptions struct { type LoadOptions struct { CheckDeprecation bool + Validation bool } type Loader struct { @@ -78,6 +79,13 @@ func (l *Loader) Load(opts LoadOptions) error { return err } + if opts.Validation { + err = l.cfg.Validate() + if err != nil { + return err + } + } + return nil } diff --git a/pkg/lint/lintersdb/validator.go b/pkg/lint/lintersdb/validator.go index 76bd195155da..079d8198fa3c 100644 --- a/pkg/lint/lintersdb/validator.go +++ b/pkg/lint/lintersdb/validator.go @@ -22,11 +22,6 @@ func NewValidator(m *Manager) *Validator { // Validate validates the configuration by calling all other validators for different // sections in the configuration and then some additional linter validation functions. func (v Validator) Validate(cfg *config.Config) error { - err := cfg.Validate() - if err != nil { - return err - } - validators := []func(cfg *config.Linters) error{ v.validateLintersNames, v.validatePresets,