feat: replace run.skip-xxx options by issues.exclude-xxx options (#…

…4509)
golangci · Mar 16, 2024 · 4b7a55a · 4b7a55a
1 parent 1466b36
commit 4b7a55a
Show file tree

Hide file tree

Showing 18 changed files with 234 additions and 159 deletions.
diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml
@@ -28,31 +28,6 @@ run:
   build-tags:
     - mytag
 
-  # Which dirs to skip: issues from them won't be reported.
-  # Can use regexp here: `generated.*`, regexp is applied on full path,
-  # including the path prefix if one is set.
-  # Default dirs are skipped independently of this option's value (see skip-dirs-use-default).
-  # "/" will be replaced by current OS file path separator to properly work on Windows.
-  # Default: []
-  skip-dirs:
-    - src/external_libs
-    - autogenerated_by_my_lib
-
-  # Enables skipping of directories:
-  # - vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
-  # Default: true
-  skip-dirs-use-default: false
-
-  # Which files to skip: they will be analyzed, but issues from them won't be reported.
-  # There is no need to include all autogenerated files,
-  # we confidently recognize autogenerated files.
-  # If it's not, please let us know.
-  # "/" will be replaced by current OS file path separator to properly work on Windows.
-  # Default: []
-  skip-files:
-    - ".*\\.my\\.go$"
-    - lib/bad.go
-
   # If set, we pass it to "go list -mod={option}". From "go help modules":
   # If invoked with -mod=readonly, the go command is disallowed from the implicit
   # automatic updating of go.mod described above. Instead, it fails when any changes
@@ -2810,10 +2785,35 @@ issues:
   # Default: true
   exclude-use-default: false
 
-  # If set to true exclude and exclude-rules regular expressions become case-sensitive.
+  # If set to true, `exclude` and `exclude-rules` regular expressions become case-sensitive.
   # Default: false
   exclude-case-sensitive: false
 
+  # Which dirs to exclude: issues from them won't be reported.
+  # Can use regexp here: `generated.*`, regexp is applied on full path,
+  # including the path prefix if one is set.
+  # Default dirs are skipped independently of this option's value (see exclude-dirs-use-default).
+  # "/" will be replaced by current OS file path separator to properly work on Windows.
+  # Default: []
+  exclude-dirs:
+    - src/external_libs
+    - autogenerated_by_my_lib
+
+  # Enables exclude of directories:
+  # - vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  # Default: true
+  exclude-dirs-use-default: false
+
+  # Which files to exclude: they will be analyzed, but issues from them won't be reported.
+  # There is no need to include all autogenerated files,
+  # we confidently recognize autogenerated files.
+  # If it's not, please let us know.
+  # "/" will be replaced by current OS file path separator to properly work on Windows.
+  # Default: []
+  exclude-files:
+    - ".*\\.my\\.go$"
+    - lib/bad.go
+
   # To follow strictly the Go generated file convention.
   #
   # If set to true, source files that have lines matching only the following regular expression will be excluded:

diff --git a/.golangci.yml b/.golangci.yml
@@ -175,7 +175,7 @@ issues:
 
 run:
   timeout: 5m
-  skip-dirs:
+  skip-dirs: # TODO(ldez): should be replaced by `issues.exclude-dirs` after the next release.
     - test/testdata_etc # test files
     - internal/cache # extracted from Go code
     - internal/renameio # extracted from Go code

diff --git a/docs/src/docs/usage/false-positives.mdx b/docs/src/docs/usage/false-positives.mdx
@@ -74,7 +74,7 @@ issues:
 
 ### Exclude Issues by Path
 
-Exclude issues in path by `run.skip-dirs`, `run.skip-files` or `issues.exclude-rules` config options.
+Exclude issues in path by `issues.exclude-dirs`, `issues.exclude-files` or `issues.exclude-rules` config options.
 
 Beware that the paths that get matched here are relative to the current working directory.
 When the configuration contains path patterns that check for specific directories,
@@ -103,19 +103,19 @@ issues:
         - goconst
 ```
 
-In the following example, all the reports related to the files (`skip-files`) are excluded:
+In the following example, all the reports related to the files (`exclude-files`) are excluded:
 
 ```yml
-run:
-  skip-files:
+issues:
+  exclude-files:
     - path/to/a/file.go
 ```
 
-In the following example, all the reports related to the directories (`skip-dirs`) are excluded:
+In the following example, all the reports related to the directories (`exclude-dirs`) are excluded:
 
 ```yml
-run:
-  skip-dirs:
+issues:
+  exclude-dirs:
     - path/to/a/dir/
 ```
 

diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json
@@ -422,33 +422,6 @@
           "default": [],
           "examples": [["mytag"]]
         },
-        "skip-dirs": {
-          "description": "Which directories to skip: issues from them won't be reported.",
-          "type": "array",
-          "items": {
-            "description": "You can use regexp here. The regexp is applied on the full path.\n\"/\" will be replaced by current OS file path separator to properly work on Windows.",
-            "type": "string",
-            "examples": ["generated.*"]
-          },
-          "default": [],
-          "examples": [["src/external_libs", "autogenerated_by_my_lib"]]
-        },
-        "skip-dirs-use-default": {
-          "description": "Enable skipping of directories \"vendor\", \"third_party\", \"testdata\", \"examples\", \"Godeps\", and \"builtin\".",
-          "type": "boolean",
-          "default": true
-        },
-        "skip-files": {
-          "description": "Which files to skip: they will be analyzed, but issues from them will not be reported.",
-          "type": "array",
-          "items": {
-            "description": "You can use regexp here. There is no need to include all autogenerated files, we confidently recognize them. If that is not the case, please let us know.\n\"/\" will be replaced by current OS file path separator to properly work on Windows.",
-            "type": "string",
-            "examples": [".*\\.my\\.go$"]
-          },
-          "default": [],
-          "examples": [[".*\\.my\\.go$", "lib/bad.go"]]
-        },
         "modules-download-mode": {
           "description": "Option to pass to \"go list -mod={option}\".\nSee \"go help modules\" for more information.",
           "enum": ["mod", "readonly", "vendor"]
@@ -3347,6 +3320,33 @@
           "type": "boolean",
           "default": false
         },
+        "exclude-dirs": {
+          "description": "Which directories to exclude: issues from them won't be reported.",
+          "type": "array",
+          "items": {
+            "description": "You can use regexp here. The regexp is applied on the full path.\n\"/\" will be replaced by current OS file path separator to properly work on Windows.",
+            "type": "string",
+            "examples": ["generated.*"]
+          },
+          "default": [],
+          "examples": [["src/external_libs", "autogenerated_by_my_lib"]]
+        },
+        "exclude-dirs-use-default": {
+          "description": "Enable exclusion of directories \"vendor\", \"third_party\", \"testdata\", \"examples\", \"Godeps\", and \"builtin\".",
+          "type": "boolean",
+          "default": true
+        },
+        "exclude-files": {
+          "description": "Which files to exclude: they will be analyzed, but issues from them will not be reported.",
+          "type": "array",
+          "items": {
+            "description": "You can use regexp here. There is no need to include all autogenerated files, we confidently recognize them. If that is not the case, please let us know.\n\"/\" will be replaced by current OS file path separator to properly work on Windows.",
+            "type": "string",
+            "examples": [".*\\.my\\.go$"]
+          },
+          "default": [],
+          "examples": [[".*\\.my\\.go$", "lib/bad.go"]]
+        },
         "include": {
           "description": "The list of ids of default excludes to include or disable.",
           "type": "array",

diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go
@@ -12,22 +12,22 @@ import (
 	"github.com/golangci/golangci-lint/pkg/config"
 	"github.com/golangci/golangci-lint/pkg/exitcodes"
 	"github.com/golangci/golangci-lint/pkg/lint/lintersdb"
+	"github.com/golangci/golangci-lint/pkg/packages"
 )
 
 func setupLintersFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
-	fs.StringSliceP("disable", "D", nil, color.GreenString("Disable specific linter")) // Hack see Loader.applyStringSliceHack
+	internal.AddHackedStringSliceP(fs, "disable", "D", color.GreenString("Disable specific linter"))
 	internal.AddFlagAndBind(v, fs, fs.Bool, "disable-all", "linters.disable-all", false, color.GreenString("Disable all linters"))
 
-	fs.StringSliceP("enable", "E", nil, color.GreenString("Enable specific linter")) // Hack see Loader.applyStringSliceHack
+	internal.AddHackedStringSliceP(fs, "enable", "E", color.GreenString("Enable specific linter"))
 	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,
 		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,
-		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(), "|"))))
+	internal.AddHackedStringSliceP(fs, "presets", "p",
+		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(), "|"))))
 
 	fs.StringSlice("enable-only", nil,
 		color.GreenString("Override linters configuration section to only run the specific linter(s)")) // Flags only.
@@ -42,14 +42,16 @@ func setupRunFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 	internal.AddFlagAndBind(v, fs, fs.Int, "issues-exit-code", "run.issues-exit-code", exitcodes.IssuesFound,
 		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.AddHackedStringSlice(fs, "build-tags", color.GreenString("Build tags"))
 
 	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, 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, color.GreenString("Regexps of files to skip")) // Hack see Loader.applyStringSliceHack
+
+	internal.AddDeprecatedHackedStringSlice(fs, "skip-files", color.GreenString("Regexps of files to skip"))
+	internal.AddDeprecatedHackedStringSlice(fs, "skip-dirs", color.GreenString("Regexps of directories to skip"))
+	internal.AddDeprecatedFlagAndBind(v, fs, fs.Bool, "skip-dirs-use-default", "run.skip-dirs-use-default", true,
+		getDefaultDirectoryExcludeHelp())
 
 	const allowParallelDesc = "Allow multiple parallel golangci-lint instances running. " +
 		"If false (default) - golangci-lint acquires file lock on start."
@@ -80,7 +82,7 @@ func setupOutputFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 
 //nolint:gomnd
 func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
-	fs.StringSliceP("exclude", "e", nil, color.GreenString("Exclude issue by regexp")) // Hack see Loader.applyStringSliceHack
+	internal.AddHackedStringSliceP(fs, "exclude", "e", color.GreenString("Exclude issue by regexp"))
 	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,
@@ -91,6 +93,11 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 	internal.AddFlagAndBind(v, fs, fs.Int, "max-same-issues", "issues.max-same-issues", 3,
 		color.GreenString("Maximum count of issues with the same text. Set to 0 to disable"))
 
+	internal.AddHackedStringSlice(fs, "exclude-files", color.GreenString("Regexps of files to exclude"))
+	internal.AddHackedStringSlice(fs, "exclude-dirs", color.GreenString("Regexps of directories to exclude"))
+	internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-dirs-use-default", "issues.exclude-dirs-use-default", true,
+		getDefaultDirectoryExcludeHelp())
+
 	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 " +
@@ -107,3 +114,24 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) {
 	internal.AddFlagAndBind(v, fs, fs.Bool, "fix", "issues.fix", false,
 		color.GreenString("Fix found issues (if it's supported by the linter)"))
 }
+
+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/internal/vibra.go b/pkg/commands/internal/vibra.go
@@ -30,3 +30,30 @@ func AddFlagAndBindP[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagPFunc[T],
 		panic(fmt.Sprintf("failed to bind flag %s: %v", name, err))
 	}
 }
+
+// AddDeprecatedFlagAndBind similar to AddFlagAndBind but deprecate the flag.
+func AddDeprecatedFlagAndBind[T any](v *viper.Viper, fs *pflag.FlagSet, pfn FlagFunc[T], name, bind string, value T, usage string) {
+	AddFlagAndBind(v, fs, pfn, name, bind, value, usage)
+	deprecateFlag(fs, name)
+}
+
+// AddHackedStringSliceP Hack for slice, see Loader.applyStringSliceHack.
+func AddHackedStringSliceP(fs *pflag.FlagSet, name, shorthand, usage string) {
+	fs.StringSliceP(name, shorthand, nil, usage)
+}
+
+// AddHackedStringSlice Hack for slice, see Loader.applyStringSliceHack.
+func AddHackedStringSlice(fs *pflag.FlagSet, name, usage string) {
+	AddHackedStringSliceP(fs, name, "", usage)
+}
+
+// AddDeprecatedHackedStringSlice similar to AddHackedStringSlice but deprecate the flag.
+func AddDeprecatedHackedStringSlice(fs *pflag.FlagSet, name, usage string) {
+	AddHackedStringSlice(fs, name, usage)
+	deprecateFlag(fs, name)
+}
+
+func deprecateFlag(fs *pflag.FlagSet, name string) {
+	_ = fs.MarkHidden(name)
+	_ = fs.MarkDeprecated(name, "check the documentation for more information.")
+}
diff --git a/pkg/commands/run.go b/pkg/commands/run.go
@@ -38,7 +38,6 @@ import (
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
 	"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"
@@ -426,11 +425,7 @@ func (c *runCommand) printDeprecatedLinterMessages(enabledLinters map[string]*li
 }
 
 func (c *runCommand) printStats(issues []result.Issue) {
-	if c.cfg.Run.ShowStats {
-		c.log.Warnf("The configuration option `run.show-stats` is deprecated, please use `output.show-stats`")
-	}
-
-	if !c.cfg.Run.ShowStats && !c.cfg.Output.ShowStats {
+	if !c.cfg.Output.ShowStats {
 		return
 	}
 
@@ -465,7 +460,7 @@ func (c *runCommand) setupExitCode(ctx context.Context) {
 		return
 	}
 
-	needFailOnWarnings := os.Getenv(lintersdb.EnvTestRun) == "1" || os.Getenv(envFailOnWarnings) == "1"
+	needFailOnWarnings := os.Getenv(logutils.EnvTestRun) == "1" || os.Getenv(envFailOnWarnings) == "1"
 	if needFailOnWarnings && len(c.reportData.Warnings) != 0 {
 		c.exitCode = exitcodes.WarningInTest
 		return
@@ -578,27 +573,6 @@ func setupConfigFileFlagSet(fs *pflag.FlagSet, cfg *config.LoaderOptions) {
 	fs.BoolVar(&cfg.NoConfig, "no-config", false, color.GreenString("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,
 		color.GreenString("Print avg and max memory usage of golangci-lint and total time"))

diff --git a/pkg/config/issues.go b/pkg/config/issues.go
@@ -112,6 +112,10 @@ type Issues struct {
 	ExcludeGeneratedStrict bool          `mapstructure:"exclude-generated-strict"`
 	UseDefaultExcludes     bool          `mapstructure:"exclude-use-default"`
 
+	ExcludeFiles          []string `mapstructure:"exclude-files"`
+	ExcludeDirs           []string `mapstructure:"exclude-dirs"`
+	UseDefaultExcludeDirs bool     `mapstructure:"exclude-dirs-use-default"`
+
 	MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"`
 	MaxSameIssues      int `mapstructure:"max-same-issues"`