From c704960c5782ebed0705fdddc48bb5d00b934e2f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 18 Feb 2025 23:46:09 +0100 Subject: [PATCH 1/8] chore: remove v1 exclusion configuration --- .golangci.next.reference.yml | 182 +++++++--------- jsonschema/golangci.next.jsonschema.json | 103 +-------- pkg/commands/flagsets.go | 29 --- pkg/commands/run.go | 12 +- pkg/config/config.go | 1 - pkg/config/issues.go | 146 ------------- pkg/config/issues_test.go | 203 ------------------ pkg/config/linters_exclusions_test.go | 66 ++++++ pkg/config/loader.go | 23 +- pkg/lint/runner.go | 21 +- pkg/result/processors/exclusion_rules.go | 47 +--- pkg/result/processors/exclusion_rules_test.go | 16 +- pkg/result/processors/skip_dirs.go | 180 ---------------- pkg/result/processors/skip_dirs_test.go | 101 --------- pkg/result/processors/skip_files.go | 65 ------ pkg/result/processors/skip_files_test.go | 49 ----- scripts/website/dump_info/main.go | 14 +- 17 files changed, 161 insertions(+), 1097 deletions(-) delete mode 100644 pkg/config/issues_test.go create mode 100644 pkg/config/linters_exclusions_test.go delete mode 100644 pkg/result/processors/skip_dirs.go delete mode 100644 pkg/result/processors/skip_dirs_test.go delete mode 100644 pkg/result/processors/skip_files.go delete mode 100644 pkg/result/processors/skip_files_test.go diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 63cfb2a60dd9..13281226cfa7 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -261,6 +261,78 @@ linters: # Default: false fast: true + # Defines a set of rules to ignore issues. + # It does not skip the analysis, and so don't ignore "typecheck" errors. + exclusions: + # Mode of the generated files analysis. + # + # - `strict`: sources are excluded by following strictly the Go generated file convention. + # Source files that have lines matching only the following regular expression will be excluded: `^// Code generated .* DO NOT EDIT\.$` + # This line must appear before the first non-comment, non-blank text in the file. + # https://go.dev/s/generatedcode + # - `lax`: sources are excluded if they contain lines `autogenerated file`, `code generated`, `do not edit`, etc. + # - `disable`: disable the generated files exclusion. + # + # Default: lax + generated: strict + # Log a warning if an exclusion rule is unused. + # Default: false + warn-unused: true + # Predefined exclusion rules. + # Default: [] + presets: + - comments + - stdErrorHandling + - commonFalsePositives + - legacy + + # Excluding configuration per-path, per-linter, per-text and per-source. + rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + + # Run some linter only for test files by excluding its issues for everything else. + - path-except: _test\.go + linters: + - forbidigo + + # Exclude known linters from partially hard-vendored code, + # which is impossible to exclude via `nolint` comments. + # `/` will be replaced by current OS file path separator to properly work on Windows. + - path: internal/hmac/ + text: "weak cryptographic primitive" + linters: + - gosec + + # Exclude some `staticcheck` messages. + - linters: + - staticcheck + text: "SA9003:" + + # Exclude `lll` issues for long lines with `go:generate`. + - linters: + - lll + source: "^//go:generate " + + # Which file paths 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: [] + paths: + - ".*\\.my\\.go$" + - lib/bad.go + # Which file paths to not exclude. + # Default: [] + paths-except: + - ".*\\.my\\.go$" + - lib/bad.go formatters: # Enable specific formatter. @@ -3940,116 +4012,6 @@ linters-settings: issues: - # List of regexps of issue texts to exclude. - # - # But independently of this option we use default exclude patterns, - # it can be disabled by `exclude-use-default: false`. - # To list all excluded by default patterns execute `golangci-lint run --help` - # - # Default: https://golangci-lint.run/usage/false-positives/#default-exclusions - exclude: - - abcdef - - # Excluding configuration per-path, per-linter, per-text and per-source - exclude-rules: - # Exclude some linters from running on tests files. - - path: _test\.go - linters: - - gocyclo - - errcheck - - dupl - - gosec - - # Run some linter only for test files by excluding its issues for everything else. - - path-except: _test\.go - linters: - - forbidigo - - # Exclude known linters from partially hard-vendored code, - # which is impossible to exclude via `nolint` comments. - # `/` will be replaced by current OS file path separator to properly work on Windows. - - path: internal/hmac/ - text: "weak cryptographic primitive" - linters: - - gosec - - # Exclude some `staticcheck` messages. - - linters: - - staticcheck - text: "SA9003:" - - # Exclude `lll` issues for long lines with `go:generate`. - - linters: - - lll - source: "^//go:generate " - - # Independently of option `exclude` we use default exclude patterns, - # it can be disabled by this option. - # To list all excluded by default patterns execute `golangci-lint run --help`. - # Default: true - exclude-use-default: false - - # 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 - - # Mode of the generated files analysis. - # - # - `strict`: sources are excluded by following strictly the Go generated file convention. - # Source files that have lines matching only the following regular expression will be excluded: `^// Code generated .* DO NOT EDIT\.$` - # This line must appear before the first non-comment, non-blank text in the file. - # https://go.dev/s/generatedcode - # - `lax`: sources are excluded if they contain lines `autogenerated file`, `code generated`, `do not edit`, etc. - # - `disable`: disable the generated files exclusion. - # - # Default: lax - exclude-generated: strict - - # The list of ids of default excludes to include or disable. - # https://golangci-lint.run/usage/false-positives/#default-exclusions - # Default: [] - include: - - EXC0001 - - EXC0002 - - EXC0003 - - EXC0004 - - EXC0005 - - EXC0006 - - EXC0007 - - EXC0008 - - EXC0009 - - EXC0010 - - EXC0011 - - EXC0012 - - EXC0013 - - EXC0014 - - EXC0015 - # Maximum issues count per one linter. # Set to 0 to disable. # Default: 50 diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 771834a5b268..26d3e77c0a35 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -4254,7 +4254,7 @@ "properties": { "generated": { "enum": ["strict", "lax", "disable"], - "default": "lax" + "default": "strict" }, "warn-unused": { "type": "boolean", @@ -4374,107 +4374,6 @@ "type": "object", "additionalProperties": false, "properties": { - "exclude": { - "description": "List of regular expressions of issue texts to exclude.\nBut independently from this option we use default exclude patterns. Their usage can be controlled through `exclude-use-default`.", - "type": "array", - "items": { - "type": "string" - } - }, - "exclude-rules": { - "description": "Exclude configuration per-path, per-linter, per-text and per-source", - "type": "array", - "items": { - "type": "object", - "additionalProperties": false, - "properties": { - "path": { - "type": "string" - }, - "path-except": { - "type": "string" - }, - "linters": { - "type": "array", - "items": { - "$ref": "#/definitions/linter-names" - } - }, - "text": { - "type": "string" - }, - "source": { - "type": "string" - } - } - } - }, - "exclude-use-default": { - "description": "Independently from option `exclude` we use default exclude patterns. This behavior can be disabled by this option.", - "type": "boolean", - "default": true - }, - "exclude-case-sensitive": { - "description": "If set to true, exclude and exclude-rules regular expressions become case sensitive.", - "type": "boolean", - "default": false - }, - "exclude-generated": { - "description": "Mode of the generated files analysis.", - "enum": ["lax", "strict", "disable"], - "default": "lax" - }, - "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", - "items": { - "enum": [ - "EXC0001", - "EXC0002", - "EXC0003", - "EXC0004", - "EXC0005", - "EXC0006", - "EXC0007", - "EXC0008", - "EXC0009", - "EXC0010", - "EXC0011", - "EXC0012", - "EXC0013", - "EXC0014", - "EXC0015" - ] - }, - "default": [] - }, "max-issues-per-linter": { "description": "Maximum issues count per one linter. Set to 0 to disable.", "type": "integer", diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index 11459353fe61..ca0c53d599db 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -9,10 +9,8 @@ import ( "github.com/spf13/viper" "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" - "github.com/golangci/golangci-lint/pkg/result/processors" ) const defaultMaxIssuesPerLinter = 50 @@ -130,12 +128,6 @@ func setupOutputFormatsFlagSet(v *viper.Viper, fs *pflag.FlagSet) { //nolint:gomnd // magic numbers here is ok func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { - 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, - 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", defaultMaxIssuesPerLinter, 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, @@ -143,14 +135,6 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBind(v, fs, fs.Bool, "uniq-by-line", "issues.uniq-by-line", true, color.GreenString("Make issues output unique by line")) - 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, - formatList("Use or not use default excluded directories:", processors.StdExcludeDirRegexps)) - - internal.AddFlagAndBind(v, fs, fs.String, "exclude-generated", "issues.exclude-generated", config.GeneratedModeLax, - color.GreenString("Mode of the generated files analysis")) - 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 " + @@ -186,16 +170,3 @@ func formatList(head string, items []string, foot ...string) string { return strings.Join(parts, "\n") } - -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", color.BlueString(ep.ID), color.CyanString(ep.Linter), ep.Why), - fmt.Sprintf(` Pattern: %s`, color.YellowString(`'`+ep.Pattern+`'`)), - ) - } - - return strings.Join(parts, "\n") -} diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 00096542f5a8..319d6e23fd1b 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -232,7 +232,7 @@ func (c *runCommand) postRun(_ *cobra.Command, _ []string) { c.releaseFileLock() } -func (c *runCommand) execute(_ *cobra.Command, args []string) { +func (c *runCommand) execute(_ *cobra.Command, _ []string) { needTrackResources := logutils.IsVerbose() || c.opts.PrintResourcesUsage trackResourcesEndCh := make(chan struct{}) @@ -256,7 +256,7 @@ func (c *runCommand) execute(_ *cobra.Command, args []string) { go watchResources(ctx, trackResourcesEndCh, c.log, c.debugf) } - if err := c.runAndPrint(ctx, args); err != nil { + if err := c.runAndPrint(ctx); err != nil { c.log.Errorf("Running error: %s", err) if c.exitCode == exitcodes.Success { var exitErr *exitcodes.ExitError @@ -329,7 +329,7 @@ func (c *runCommand) stopTracing() error { return nil } -func (c *runCommand) runAndPrint(ctx context.Context, args []string) error { +func (c *runCommand) runAndPrint(ctx context.Context) error { if err := c.goenv.Discover(ctx); err != nil { c.log.Warnf("Failed to discover go env: %s", err) } @@ -350,7 +350,7 @@ func (c *runCommand) runAndPrint(ctx context.Context, args []string) error { c.printDeprecatedLinterMessages(enabledLintersMap) - issues, err := c.runAnalysis(ctx, args) + issues, err := c.runAnalysis(ctx) if err != nil { return err // XXX: don't lose type } @@ -376,7 +376,7 @@ func (c *runCommand) runAndPrint(ctx context.Context, args []string) error { } // runAnalysis executes the linters that have been enabled in the configuration. -func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.Issue, error) { +func (c *runCommand) runAnalysis(ctx context.Context) ([]result.Issue, error) { lintersToRun, err := c.dbManager.GetOptimizedLinters() if err != nil { return nil, err @@ -387,7 +387,7 @@ func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.I return nil, fmt.Errorf("context loading failed: %w", err) } - runner, err := lint.NewRunner(c.log.Child(logutils.DebugKeyRunner), c.cfg, args, + runner, err := lint.NewRunner(c.log.Child(logutils.DebugKeyRunner), c.cfg, c.goenv, c.lineCache, c.fileCache, c.dbManager, lintCtx) if err != nil { return nil, err diff --git a/pkg/config/config.go b/pkg/config/config.go index 7038371e9fa2..aa24e3dd33c2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -52,7 +52,6 @@ func (c *Config) Validate() error { c.Output.Validate, c.LintersSettings.Validate, c.Linters.Validate, - c.Issues.Validate, c.Severity.Validate, } diff --git a/pkg/config/issues.go b/pkg/config/issues.go index 6f546e7eaa3e..5dc630b001a1 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -1,118 +1,6 @@ package config -import ( - "fmt" -) - -var DefaultExcludePatterns = []ExcludePattern{ - { - ID: "EXC0001", - Pattern: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close" + - "|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked", - Linter: "errcheck", - Why: "Almost all programs ignore errors on these functions and in most cases it's ok.", - }, - { - ID: "EXC0002", // TODO(ldez): should be remove in v2 - Pattern: "(comment on exported (method|function|type|const)|" + - "should have( a package)? comment|comment should be of the form)", - Linter: "golint", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, - { - ID: "EXC0003", // TODO(ldez): should be remove in v2 - Pattern: "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this", - Linter: "golint", - Why: "False positive when tests are defined in package 'test'.", - }, - { - ID: "EXC0004", - Pattern: "(possible misuse of unsafe.Pointer|should have signature)", - Linter: "govet", - Why: "Common false positives.", - }, - { - ID: "EXC0005", - Pattern: "SA4011", // CheckScopedBreak - Linter: "staticcheck", - Why: "Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore.", - }, - { - ID: "EXC0006", - Pattern: "G103: Use of unsafe calls should be audited", - Linter: "gosec", - Why: "Too many false-positives on 'unsafe' usage.", - }, - { - ID: "EXC0007", - Pattern: "G204: Subprocess launched with variable", - Linter: "gosec", - Why: "Too many false-positives for parametrized shell calls.", - }, - { - ID: "EXC0008", - Pattern: "G104", // Errors unhandled. - Linter: "gosec", - Why: "Duplicated errcheck checks.", - }, - { - ID: "EXC0009", - Pattern: "(G301|G302|G307): Expect (directory permissions to be 0750|file permissions to be 0600) or less", - Linter: "gosec", - Why: "Too many issues in popular repos.", - }, - { - ID: "EXC0010", - Pattern: "G304: Potential file inclusion via variable", - Linter: "gosec", - Why: "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'.", - }, - { - ID: "EXC0011", - Pattern: "(ST1000|ST1020|ST1021|ST1022)", // CheckPackageComment, CheckExportedFunctionDocs, CheckExportedTypeDocs, CheckExportedVarDocs - Linter: "stylecheck", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, - { - ID: "EXC0012", - Pattern: `exported (.+) should have comment( \(or a comment on this block\))? or be unexported`, // rule: exported - Linter: "revive", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, - { - ID: "EXC0013", - Pattern: `package comment should be of the form "(.+)..."`, // rule: package-comments - Linter: "revive", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, - { - ID: "EXC0014", - Pattern: `comment on exported (.+) should be of the form "(.+)..."`, // rule: exported - Linter: "revive", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, - { - ID: "EXC0015", - Pattern: `should have a package comment`, // rule: package-comments - Linter: "revive", - Why: "Annoying issue about not having a comment. The rare codebase has such comments.", - }, -} - type Issues struct { - IncludeDefaultExcludes []string `mapstructure:"include"` - ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` - ExcludePatterns []string `mapstructure:"exclude"` - ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` - UseDefaultExcludes bool `mapstructure:"exclude-use-default"` - - ExcludeGenerated string `mapstructure:"exclude-generated"` - - 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"` UniqByLine bool `mapstructure:"uniq-by-line"` @@ -125,37 +13,3 @@ type Issues struct { NeedFix bool `mapstructure:"fix"` } - -func (i *Issues) Validate() error { - for i, rule := range i.ExcludeRules { - if err := rule.Validate(); err != nil { - return fmt.Errorf("error in exclude rule #%d: %w", i, err) - } - } - - return nil -} - -type ExcludePattern struct { - ID string - Pattern string - Linter string - Why string -} - -// TODO(ldez): this behavior must be changed in v2, because this is confusing. -func GetExcludePatterns(include []string) []ExcludePattern { - includeMap := make(map[string]struct{}, len(include)) - for _, inc := range include { - includeMap[inc] = struct{}{} - } - - var ret []ExcludePattern - for _, p := range DefaultExcludePatterns { - if _, ok := includeMap[p.ID]; !ok { - ret = append(ret, p) - } - } - - return ret -} diff --git a/pkg/config/issues_test.go b/pkg/config/issues_test.go deleted file mode 100644 index 26deaae46097..000000000000 --- a/pkg/config/issues_test.go +++ /dev/null @@ -1,203 +0,0 @@ -package config - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGetExcludePatterns(t *testing.T) { - patterns := GetExcludePatterns(nil) - - assert.Equal(t, DefaultExcludePatterns, patterns) -} - -func TestGetExcludePatterns_includes(t *testing.T) { - include := []string{DefaultExcludePatterns[0].ID, DefaultExcludePatterns[1].ID} - - exclude := GetExcludePatterns(include) - assert.Len(t, exclude, len(DefaultExcludePatterns)-len(include)) - - for _, p := range exclude { - assert.NotContains(t, include, p.ID) - assert.Contains(t, DefaultExcludePatterns, p) - } -} - -func TestExcludeRule_Validate(t *testing.T) { - testCases := []struct { - desc string - rule *ExcludeRule - expected string - }{ - { - desc: "empty rule", - rule: &ExcludeRule{}, - expected: "at least 2 of (text, source, path[-except], linters) should be set", - }, - { - desc: "only path rule", - rule: &ExcludeRule{ - BaseRule{ - Path: "test", - }, - }, - expected: "at least 2 of (text, source, path[-except], linters) should be set", - }, - { - desc: "only path-except rule", - rule: &ExcludeRule{ - BaseRule{ - PathExcept: "test", - }, - }, - expected: "at least 2 of (text, source, path[-except], linters) should be set", - }, - { - desc: "only text rule", - rule: &ExcludeRule{ - BaseRule{ - Text: "test", - }, - }, - expected: "at least 2 of (text, source, path[-except], linters) should be set", - }, - { - desc: "only source rule", - rule: &ExcludeRule{ - BaseRule{ - Source: "test", - }, - }, - expected: "at least 2 of (text, source, path[-except], linters) should be set", - }, - { - desc: "invalid path rule", - rule: &ExcludeRule{ - BaseRule{ - Path: "**test", - }, - }, - expected: "invalid path regex: error parsing regexp: missing argument to repetition operator: `*`", - }, - { - desc: "invalid path-except rule", - rule: &ExcludeRule{ - BaseRule{ - PathExcept: "**test", - }, - }, - expected: "invalid path-except regex: error parsing regexp: missing argument to repetition operator: `*`", - }, - { - desc: "invalid text rule", - rule: &ExcludeRule{ - BaseRule{ - Text: "**test", - }, - }, - expected: "invalid text regex: error parsing regexp: missing argument to repetition operator: `*`", - }, - { - desc: "invalid source rule", - rule: &ExcludeRule{ - BaseRule{ - Source: "**test", - }, - }, - expected: "invalid source regex: error parsing regexp: missing argument to repetition operator: `*`", - }, - { - desc: "path and path-expect", - rule: &ExcludeRule{ - BaseRule{ - Path: "test", - PathExcept: "test", - }, - }, - expected: "path and path-except should not be set at the same time", - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - err := test.rule.Validate() - require.EqualError(t, err, test.expected) - }) - } -} - -func TestExcludeRule_Validate_error(t *testing.T) { - testCases := []struct { - desc string - rule *ExcludeRule - }{ - { - desc: "path and linter", - rule: &ExcludeRule{ - BaseRule{ - Path: "test", - Linters: []string{"a"}, - }, - }, - }, - { - desc: "path-except and linter", - rule: &ExcludeRule{ - BaseRule{ - PathExcept: "test", - Linters: []string{"a"}, - }, - }, - }, - { - desc: "text and linter", - rule: &ExcludeRule{ - BaseRule{ - Text: "test", - Linters: []string{"a"}, - }, - }, - }, - { - desc: "source and linter", - rule: &ExcludeRule{ - BaseRule{ - Source: "test", - Linters: []string{"a"}, - }, - }, - }, - { - desc: "path and text", - rule: &ExcludeRule{ - BaseRule{ - Path: "test", - Text: "test", - }, - }, - }, - { - desc: "path and text and linter", - rule: &ExcludeRule{ - BaseRule{ - Path: "test", - Text: "test", - Linters: []string{"a"}, - }, - }, - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - err := test.rule.Validate() - require.NoError(t, err) - }) - } -} diff --git a/pkg/config/linters_exclusions_test.go b/pkg/config/linters_exclusions_test.go new file mode 100644 index 000000000000..6e7f1631a6a9 --- /dev/null +++ b/pkg/config/linters_exclusions_test.go @@ -0,0 +1,66 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestLinterExclusions_Validate(t *testing.T) { + testCases := []struct { + desc string + exclusions *LinterExclusions + }{ + { + desc: "empty configuration", + exclusions: &LinterExclusions{}, + }, + { + desc: "valid preset", + exclusions: &LinterExclusions{ + Presets: []string{ExclusionPresetComments}, + }, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := test.exclusions.Validate() + require.NoError(t, err) + }) + } +} + +func TestLinterExclusions_Validate_error(t *testing.T) { + testCases := []struct { + desc string + exclusions *LinterExclusions + expected string + }{ + { + desc: "invalid preset name", + exclusions: &LinterExclusions{ + Presets: []string{"foo"}, + }, + expected: "invalid preset: foo", + }, + { + desc: "invalid rule: empty rule", + exclusions: &LinterExclusions{ + Rules: []ExcludeRule{{BaseRule: BaseRule{}}}, + }, + expected: "error in exclude rule #0: at least 2 of (text, source, path[-except], linters) should be set", + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + err := test.exclusions.Validate() + require.EqualError(t, err, test.expected) + }) + } +} diff --git a/pkg/config/loader.go b/pkg/config/loader.go index b36c16d20d87..efd777775336 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -1,7 +1,6 @@ package config import ( - "cmp" "context" "errors" "fmt" @@ -69,23 +68,7 @@ func (l *Loader) Load(opts LoadOptions) error { l.applyStringSliceHack() if l.cfg.Linters.LinterExclusions.Generated == "" { - // `l.cfg.Issues.ExcludeGenerated` is always non-empty because of the flag default value. - l.cfg.Linters.LinterExclusions.Generated = cmp.Or(l.cfg.Issues.ExcludeGenerated, GeneratedModeStrict) - } - - // Compatibility layer with v1. - // TODO(ldez): should be removed in v2. - if l.cfg.Issues.UseDefaultExcludes { - l.cfg.Linters.LinterExclusions.Presets = []string{ - ExclusionPresetComments, - ExclusionPresetStdErrorHandling, - ExclusionPresetCommonFalsePositives, - ExclusionPresetLegacy, - } - } - - if len(l.cfg.Issues.ExcludeRules) > 0 { - l.cfg.Linters.LinterExclusions.Rules = append(l.cfg.Linters.LinterExclusions.Rules, l.cfg.Issues.ExcludeRules...) + l.cfg.Linters.LinterExclusions.Generated = GeneratedModeStrict } l.handleFormatters() @@ -299,10 +282,6 @@ func (l *Loader) applyStringSliceHack() { l.appendStringSlice("disable", &l.cfg.Linters.Disable) l.appendStringSlice("presets", &l.cfg.Linters.Presets) l.appendStringSlice("build-tags", &l.cfg.Run.BuildTags) - l.appendStringSlice("exclude", &l.cfg.Issues.ExcludePatterns) - - l.appendStringSlice("exclude-dirs", &l.cfg.Issues.ExcludeDirs) - l.appendStringSlice("exclude-files", &l.cfg.Issues.ExcludeFiles) } func (l *Loader) appendStringSlice(name string, current *[]string) { diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index a44d55122a9f..12be1f7cd41e 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -33,7 +33,7 @@ type Runner struct { Processors []processors.Processor } -func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *goutil.Env, +func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env, lineCache *fsutils.LineCache, fileCache *fsutils.FileCache, dbManager *lintersdb.Manager, lintCtx *linter.Context, ) (*Runner, error) { @@ -52,21 +52,6 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti return nil, err } - skipFilesProcessor, err := processors.NewSkipFiles(cfg.Issues.ExcludeFiles, cfg.Output.PathPrefix) - if err != nil { - return nil, err - } - - skipDirs := cfg.Issues.ExcludeDirs - if cfg.Issues.UseDefaultExcludeDirs { - skipDirs = append(skipDirs, processors.StdExcludeDirRegexps...) - } - - skipDirsProcessor, err := processors.NewSkipDirs(log.Child(logutils.DebugKeySkipDirs), skipDirs, args, cfg.Output.PathPrefix) - if err != nil { - return nil, err - } - enabledLinters, err := dbManager.GetEnabledLintersMap() if err != nil { return nil, fmt.Errorf("failed to get enabled linters: %w", err) @@ -107,8 +92,6 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti // Must be after PathRelativity. exclusionPaths, - skipFilesProcessor, - skipDirsProcessor, processors.NewGeneratedFileFilter(cfg.Linters.LinterExclusions.Generated), @@ -116,7 +99,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti processors.NewIdentifierMarker(), processors.NewExclusionRules(log.Child(logutils.DebugKeyExclusionRules), files, - &cfg.Linters.LinterExclusions, &cfg.Issues), + &cfg.Linters.LinterExclusions), processors.NewNolintFilter(log.Child(logutils.DebugKeyNolintFilter), dbManager, enabledLinters), diff --git a/pkg/result/processors/exclusion_rules.go b/pkg/result/processors/exclusion_rules.go index 7730a53dd04c..098b34a0cb44 100644 --- a/pkg/result/processors/exclusion_rules.go +++ b/pkg/result/processors/exclusion_rules.go @@ -24,7 +24,7 @@ type ExclusionRules struct { } func NewExclusionRules(log logutils.Log, files *fsutils.Files, - cfg *config.LinterExclusions, oldCfg *config.Issues) *ExclusionRules { + cfg *config.LinterExclusions) *ExclusionRules { p := &ExclusionRules{ log: log, files: files, @@ -32,34 +32,9 @@ func NewExclusionRules(log logutils.Log, files *fsutils.Files, skippedCounter: map[string]int{}, } - // TODO(ldez) remove prefix in v2: the matching must be case sensitive, users can add `(?i)` inside the patterns if needed. - prefix := caseInsensitivePrefix - if oldCfg.ExcludeCaseSensitive { - prefix = "" - } - - excludeRules := slices.Concat(slices.Clone(cfg.Rules), - filterInclude(getLinterExclusionPresets(cfg.Presets), oldCfg.IncludeDefaultExcludes)) - - p.rules = parseRules(excludeRules, prefix, newExcludeRule) - - // TODO(ldez): should be removed in v2. - for _, pattern := range oldCfg.ExcludePatterns { - if pattern == "" { - continue - } - - r := &config.ExcludeRule{ - BaseRule: config.BaseRule{ - Path: `.+\.go`, - Text: pattern, - }, - } - - rule := newExcludeRule(r, prefix) + excludeRules := slices.Concat(slices.Clone(cfg.Rules), getLinterExclusionPresets(cfg.Presets)) - p.rules = append(p.rules, rule) - } + p.rules = parseRules(excludeRules, "", newExcludeRule) for _, rule := range p.rules { if rule.internalReference == "" { @@ -146,19 +121,3 @@ func (e excludeRule) String() string { return strings.Join(msg, ", ") } - -// TODO(ldez): must be removed in v2, only for compatibility with exclude-use-default/include. -func filterInclude(rules []config.ExcludeRule, refs []string) []config.ExcludeRule { - if len(refs) == 0 { - return rules - } - - var filteredRules []config.ExcludeRule - for _, rule := range rules { - if !slices.Contains(refs, rule.InternalReference) { - filteredRules = append(filteredRules, rule) - } - } - - return filteredRules -} diff --git a/pkg/result/processors/exclusion_rules_test.go b/pkg/result/processors/exclusion_rules_test.go index bf3f31fb23b9..512246c3fc81 100644 --- a/pkg/result/processors/exclusion_rules_test.go +++ b/pkg/result/processors/exclusion_rules_test.go @@ -50,7 +50,7 @@ func TestExclusionRules_Process_multiple(t *testing.T) { }, } - p := NewExclusionRules(nil, files, cfg, &config.Issues{}) + p := NewExclusionRules(nil, files, cfg) cases := []issueTestCase{ {Path: "e.go", Text: "exclude", Linter: "linter"}, @@ -104,7 +104,7 @@ func TestExclusionRules_Process_pathPrefix(t *testing.T) { }, } - p := NewExclusionRules(nil, files, cfg, &config.Issues{}) + p := NewExclusionRules(nil, files, cfg) cases := []issueTestCase{ {Path: "e.go"}, @@ -147,9 +147,9 @@ func TestExclusionRules_Process_text(t *testing.T) { }}, } - p := NewExclusionRules(nil, files, cfg, &config.Issues{}) + p := NewExclusionRules(nil, files, cfg) - texts := []string{"excLude", "1", "", "exclud", "notexclude"} + texts := []string{"exclude", "1", "", "exclud", "notexclude"} var issues []result.Issue for _, t := range texts { issues = append(issues, result.Issue{ @@ -172,7 +172,7 @@ func TestExclusionRules_Process_text(t *testing.T) { func TestExclusionRules_Process_empty(t *testing.T) { files := fsutils.NewFiles(fsutils.NewLineCache(fsutils.NewFileCache()), "") - p := NewExclusionRules(nil, files, &config.LinterExclusions{}, &config.Issues{}) + p := NewExclusionRules(nil, files, &config.LinterExclusions{}) processAssertSame(t, p, newIssueFromTextTestCase("test")) } @@ -209,7 +209,7 @@ func TestExclusionRules_Process_caseSensitive_multiple(t *testing.T) { }, } - p := NewExclusionRules(nil, files, cfg, &config.Issues{ExcludeCaseSensitive: true}) + p := NewExclusionRules(nil, files, cfg) cases := []issueTestCase{ {Path: "e.go", Text: "exclude", Linter: "linter"}, @@ -266,7 +266,7 @@ func TestExclusionRules_Process_caseSensitive_text(t *testing.T) { }, } - p := NewExclusionRules(nil, files, cfg, &config.Issues{ExcludeCaseSensitive: true}) + p := NewExclusionRules(nil, files, cfg) texts := []string{"exclude", "excLude", "1", "", "exclud", "notexclude"} @@ -292,7 +292,7 @@ func TestExclusionRules_Process_caseSensitive_text(t *testing.T) { func TestExclusionRules_Process_caseSensitive_empty(t *testing.T) { files := fsutils.NewFiles(fsutils.NewLineCache(fsutils.NewFileCache()), "") - p := NewExclusionRules(nil, files, &config.LinterExclusions{}, &config.Issues{ExcludeCaseSensitive: true}) + p := NewExclusionRules(nil, files, &config.LinterExclusions{}) processAssertSame(t, p, newIssueFromTextTestCase("test")) } diff --git a/pkg/result/processors/skip_dirs.go b/pkg/result/processors/skip_dirs.go deleted file mode 100644 index 8900c96a9253..000000000000 --- a/pkg/result/processors/skip_dirs.go +++ /dev/null @@ -1,180 +0,0 @@ -package processors - -import ( - "fmt" - "path/filepath" - "regexp" - - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/logutils" - "github.com/golangci/golangci-lint/pkg/result" -) - -var _ Processor = (*SkipDirs)(nil) - -var StdExcludeDirRegexps = []string{ - normalizePathRegex("vendor"), - normalizePathRegex("third_party"), - normalizePathRegex("testdata"), - normalizePathRegex("examples"), - normalizePathRegex("Godeps"), - normalizePathRegex("builtin"), -} - -type skipStat struct { - pattern string - count int -} - -// SkipDirs filters reports based on directory names. -// It uses the shortest relative paths and `path-prefix` option. -// TODO(ldez): should be removed in v2. -type SkipDirs struct { - patterns []*regexp.Regexp - pathPrefix string - - log logutils.Log - - skippedDirs map[string]*skipStat - absArgsDirs []string - skippedDirsCache map[string]bool -} - -func NewSkipDirs(log logutils.Log, patterns, args []string, pathPrefix string) (*SkipDirs, error) { - var patternsRe []*regexp.Regexp - for _, p := range patterns { - p = fsutils.NormalizePathInRegex(p) - - patternRe, err := regexp.Compile(p) - if err != nil { - return nil, fmt.Errorf("can't compile regexp %q: %w", p, err) - } - - patternsRe = append(patternsRe, patternRe) - } - - absArgsDirs, err := absDirs(args) - if err != nil { - return nil, err - } - - return &SkipDirs{ - patterns: patternsRe, - pathPrefix: pathPrefix, - log: log, - skippedDirs: map[string]*skipStat{}, - absArgsDirs: absArgsDirs, - skippedDirsCache: map[string]bool{}, - }, nil -} - -func (*SkipDirs) Name() string { - return "skip_dirs" -} - -func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) { - if len(p.patterns) == 0 { - return issues, nil - } - - return filterIssues(issues, p.shouldPassIssue), nil -} - -func (p *SkipDirs) Finish() { - for dir, stat := range p.skippedDirs { - p.log.Infof("Skipped %d issues from dir %s by pattern %s", stat.count, dir, stat.pattern) - } -} - -func (p *SkipDirs) shouldPassIssue(issue *result.Issue) bool { - issueRelDir := filepath.Dir(issue.RelativePath) - - if toPass, ok := p.skippedDirsCache[issueRelDir]; ok { - if !toPass { - p.skippedDirs[issueRelDir].count++ - } - - return toPass - } - - issueAbsDir, err := filepath.Abs(issueRelDir) - if err != nil { - p.log.Warnf("Can't abs-ify path %q: %s", issueRelDir, err) - - return true - } - - toPass := p.shouldPassIssueDirs(issueRelDir, issueAbsDir) - - p.skippedDirsCache[issueRelDir] = toPass - - return toPass -} - -func (p *SkipDirs) shouldPassIssueDirs(issueRelDir, issueAbsDir string) bool { - for _, absArgDir := range p.absArgsDirs { - if absArgDir == issueAbsDir { - // we must not skip issues if they are from explicitly set dirs - // even if they match skip patterns - return true - } - } - - // We use issueRelDir for matching: it's the relative to the current - // work dir path of directory of source file with the issue. It can lead - // to unexpected behavior if we're analyzing files out of current work dir. - // The alternative solution is to find relative to args path, but it has - // disadvantages (https://github.com/golangci/golangci-lint/pull/313). - - path := fsutils.WithPathPrefix(p.pathPrefix, issueRelDir) - - for _, pattern := range p.patterns { - if pattern.MatchString(path) { - ps := pattern.String() - - if p.skippedDirs[issueRelDir] == nil { - p.skippedDirs[issueRelDir] = &skipStat{ - pattern: ps, - } - } - - p.skippedDirs[issueRelDir].count++ - - return false - } - } - - return true -} - -func absDirs(args []string) ([]string, error) { - if len(args) == 0 { - args = append(args, "./...") - } - - var absArgsDirs []string - for _, arg := range args { - base := filepath.Base(arg) - if base == "..." || isGoFile(base) { - arg = filepath.Dir(arg) - } - - absArg, err := filepath.Abs(arg) - if err != nil { - return nil, fmt.Errorf("failed to abs-ify arg %q: %w", arg, err) - } - - absArgsDirs = append(absArgsDirs, absArg) - } - - return absArgsDirs, nil -} - -func normalizePathRegex(e string) string { - return createPathRegex(e, filepath.Separator) -} - -func createPathRegex(e string, sep rune) string { - escapedSep := regexp.QuoteMeta(string(sep)) // needed for windows sep '\\' - return fmt.Sprintf(`(^|%[1]s)%[2]s($|%[1]s)`, escapedSep, e) -} diff --git a/pkg/result/processors/skip_dirs_test.go b/pkg/result/processors/skip_dirs_test.go deleted file mode 100644 index 7a71754c6aa7..000000000000 --- a/pkg/result/processors/skip_dirs_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package processors - -import ( - "path/filepath" - "regexp" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_absDirs(t *testing.T) { - testCases := []struct { - desc string - args []string - expected []string - }{ - { - desc: "empty", - expected: []string{mustAbs(t, ".")}, - }, - { - desc: "wildcard", - args: []string{"./..."}, - expected: []string{mustAbs(t, ".")}, - }, - { - desc: "wildcard directory", - args: []string{"foo/..."}, - expected: []string{mustAbs(t, "foo")}, - }, - { - desc: "Go file", - args: []string{"./foo/bar.go"}, - expected: []string{mustAbs(t, "foo")}, - }, - { - desc: "relative directory", - args: []string{filepath.FromSlash("./foo")}, - expected: []string{mustAbs(t, "foo")}, - }, - { - desc: "absolute directory", - args: []string{mustAbs(t, "foo")}, - expected: []string{mustAbs(t, "foo")}, - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - results, err := absDirs(test.args) - require.NoError(t, err) - - assert.Equal(t, test.expected, results) - }) - } -} - -func Test_createPathRegex(t *testing.T) { - matches := [][]string{ - {"dir"}, - {"root", "dir"}, - {"root", "dir", "subdir"}, - {"dir", "subdir"}, - } - - noMatches := [][]string{ - {"nodir"}, - {"dirno"}, - {"root", "dirno"}, - {"root", "nodir"}, - {"root", "dirno", "subdir"}, - {"root", "nodir", "subdir"}, - {"dirno", "subdir"}, - {"nodir", "subdir"}, - } - - for _, sep := range []rune{'/', '\\'} { - exp := regexp.MustCompile(createPathRegex("dir", sep)) - - for _, m := range matches { - assert.Regexp(t, exp, strings.Join(m, string(sep))) - } - - for _, m := range noMatches { - assert.NotRegexp(t, exp, strings.Join(m, string(sep))) - } - } -} - -func mustAbs(t *testing.T, p string) string { - t.Helper() - - abs, err := filepath.Abs(p) - require.NoError(t, err) - - return abs -} diff --git a/pkg/result/processors/skip_files.go b/pkg/result/processors/skip_files.go deleted file mode 100644 index 5907cf6777b2..000000000000 --- a/pkg/result/processors/skip_files.go +++ /dev/null @@ -1,65 +0,0 @@ -package processors - -import ( - "fmt" - "regexp" - - "github.com/golangci/golangci-lint/pkg/fsutils" - "github.com/golangci/golangci-lint/pkg/result" -) - -var _ Processor = (*SkipFiles)(nil) - -// SkipFiles filters reports based on filename. -// -// It uses the shortest relative paths and `path-prefix` option. -// TODO(ldez): should be removed in v2. -type SkipFiles struct { - patterns []*regexp.Regexp - pathPrefix string -} - -func NewSkipFiles(patterns []string, pathPrefix string) (*SkipFiles, error) { - var patternsRe []*regexp.Regexp - for _, p := range patterns { - p = fsutils.NormalizePathInRegex(p) - - patternRe, err := regexp.Compile(p) - if err != nil { - return nil, fmt.Errorf("can't compile regexp %q: %w", p, err) - } - - patternsRe = append(patternsRe, patternRe) - } - - return &SkipFiles{ - patterns: patternsRe, - pathPrefix: pathPrefix, - }, nil -} - -func (*SkipFiles) Name() string { - return "skip_files" -} - -func (p *SkipFiles) Process(issues []result.Issue) ([]result.Issue, error) { - if len(p.patterns) == 0 { - return issues, nil - } - - return filterIssues(issues, p.shouldPassIssue), nil -} - -func (*SkipFiles) Finish() {} - -func (p *SkipFiles) shouldPassIssue(issue *result.Issue) bool { - path := fsutils.WithPathPrefix(p.pathPrefix, issue.RelativePath) - - for _, pattern := range p.patterns { - if pattern.MatchString(path) { - return false - } - } - - return true -} diff --git a/pkg/result/processors/skip_files_test.go b/pkg/result/processors/skip_files_test.go deleted file mode 100644 index 254dba4ee09a..000000000000 --- a/pkg/result/processors/skip_files_test.go +++ /dev/null @@ -1,49 +0,0 @@ -package processors - -import ( - "path/filepath" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/golangci/golangci-lint/pkg/result" -) - -func newFileIssue(file string) result.Issue { - return result.Issue{RelativePath: file} -} - -func newTestSkipFiles(t *testing.T, patterns ...string) *SkipFiles { - p, err := NewSkipFiles(patterns, "") - require.NoError(t, err) - return p -} - -func TestSkipFiles(t *testing.T) { - processAssertSame(t, newTestSkipFiles(t), newFileIssue("any.go")) - - processAssertEmpty(t, newTestSkipFiles(t, "file"), - newFileIssue("file.go"), - newFileIssue("file"), - newFileIssue("nofile.go")) - - processAssertEmpty(t, newTestSkipFiles(t, ".*"), newFileIssue("any.go")) - - cleanPath := strings.ReplaceAll(filepath.FromSlash("a/b/c.go"), `\`, `\\`) - processAssertEmpty(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/c.go"))) - processAssertSame(t, newTestSkipFiles(t, cleanPath), newFileIssue(filepath.FromSlash("a/b/d.go"))) - - processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.pb.go"))) - processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go"), newFileIssue(filepath.FromSlash("a/b.go"))) - - processAssertEmpty(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.pb.go"))) - processAssertSame(t, newTestSkipFiles(t, ".*\\.pb\\.go$"), newFileIssue(filepath.FromSlash("a/b.go"))) -} - -func TestSkipFilesInvalidPattern(t *testing.T) { - p, err := NewSkipFiles([]string{"\\o"}, "") - require.Error(t, err) - assert.Nil(t, p) -} diff --git a/scripts/website/dump_info/main.go b/scripts/website/dump_info/main.go index db7e00b36c93..e0be74dbdea4 100644 --- a/scripts/website/dump_info/main.go +++ b/scripts/website/dump_info/main.go @@ -71,18 +71,8 @@ func saveLinters() error { } func saveDefaultExclusions() error { - var excludePatterns []types.ExcludePattern - - for _, pattern := range config.DefaultExcludePatterns { - excludePatterns = append(excludePatterns, types.ExcludePattern{ - ID: pattern.ID, - Pattern: pattern.Pattern, - Linter: pattern.Linter, - Why: pattern.Why, - }) - } - - return saveToJSONFile(filepath.Join("assets", "default-exclusions.json"), excludePatterns) + // FIXME + return saveToJSONFile(filepath.Join("assets", "default-exclusions.json"), nil) } func saveCLIHelp(dst string) error { From ee3af9fa0de6387d293d9ba0a49dade6eabd490b Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 19 Feb 2025 04:19:13 +0100 Subject: [PATCH 2/8] docs: replace default exclusions by exclusion presets --- assets/cli-help.json | 2 +- assets/default-exclusions.json | 92 ------------------- assets/exclusion-presets.json | 88 ++++++++++++++++++ assets/linters-info.json | 30 +++--- docs/src/docs/usage/false-positives.mdx | 7 +- pkg/lint/lintersdb/builder_linter.go | 2 +- pkg/result/processors/exclusion_presets.go | 4 +- scripts/website/dump_info/main.go | 18 +++- .../website/expand_templates/exclusions.go | 47 +++++----- scripts/website/expand_templates/main.go | 4 +- scripts/website/types/types.go | 11 ++- 11 files changed, 156 insertions(+), 149 deletions(-) delete mode 100644 assets/default-exclusions.json create mode 100644 assets/exclusion-presets.json diff --git a/assets/cli-help.json b/assets/cli-help.json index 4850452aa773..3549e07a8b2f 100644 --- a/assets/cli-help.json +++ b/assets/cli-help.json @@ -1,4 +1,4 @@ { "enable": "Enabled by default linters:\nerrcheck: Errcheck is a program for checking for unchecked errors in Go code. These unchecked errors can be critical bugs in some cases.\ngosimple: Linter for Go source code that specializes in simplifying code. [auto-fix]\ngovet: Vet examines Go source code and reports suspicious constructs. It is roughly the same as 'go vet' and uses its passes. [auto-fix]\nineffassign: Detects when assignments to existing variables are not used. [fast]\nstaticcheck: It's a set of rules from staticcheck. It's not the same thing as the staticcheck binary. The author of staticcheck doesn't support or approve the use of staticcheck as a library inside golangci-lint. [auto-fix]\nunused: Checks Go code for unused constants, variables, functions and types.", - "help": "Usage:\n golangci-lint run [flags]\n\nFlags:\n -c, --config PATH Read config from file path PATH\n --no-config Don't read config file\n -D, --disable strings Disable specific linter\n --disable-all Disable all linters\n -E, --enable strings Enable specific linter\n --enable-all Enable all linters\n --fast Enable only fast linters from enabled linters set (first run won't be fast)\n -p, --presets strings Enable presets of linters:\n - bugs\n - comment\n - complexity\n - error\n - format\n - import\n - metalinter\n - module\n - performance\n - sql\n - style\n - test\n - unused\n Run 'golangci-lint help linters' to see them.\n This option implies option --disable-all\n --enable-only strings Override linters configuration section to only run the specific linter(s)\n -j, --concurrency int Number of CPUs to use (Default: number of logical CPUs) (default 8)\n --modules-download-mode string Modules download mode. If not empty, passed as -mod=\u003cmode\u003e to go tools\n --issues-exit-code int Exit code when issues were found (default 1)\n --go string Targeted Go version\n --build-tags strings Build tags\n --timeout duration Timeout for total work. If \u003c= 0, the timeout is disabled (default 1m0s)\n --tests Analyze tests (*_test.go) (default true)\n --allow-parallel-runners Allow multiple parallel golangci-lint instances running.\n If false (default) - golangci-lint acquires file lock on start.\n --allow-serial-runners Allow multiple golangci-lint instances running, but serialize them around a lock.\n If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start.\n --out-format string Formats of output:\n - json\n - line-number\n - colored-line-number\n - tab\n - colored-tab\n - checkstyle\n - code-climate\n - html\n - junit-xml\n - junit-xml-extended\n - github-actions\n - teamcity\n - sarif\n (default \"colored-line-number\")\n --print-issued-lines Print lines of code with issue (default true)\n --print-linter-name Print linter name in issue line (default true)\n --sort-results Sort linter results\n --sort-order strings Sort order of linter results\n --path-prefix string Path prefix to add to output\n --show-stats Show statistics per linter\n -e, --exclude strings Exclude issue by regexp\n --exclude-use-default Use or not use default excludes:\n - EXC0001 (errcheck): Almost all programs ignore errors on these functions and in most cases it's ok.\n Pattern: 'Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked'\n - EXC0002 (golint): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: '(comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)'\n - EXC0003 (golint): False positive when tests are defined in package 'test'.\n Pattern: 'func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this'\n - EXC0004 (govet): Common false positives.\n Pattern: '(possible misuse of unsafe.Pointer|should have signature)'\n - EXC0005 (staticcheck): Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore.\n Pattern: 'SA4011'\n - EXC0006 (gosec): Too many false-positives on 'unsafe' usage.\n Pattern: 'G103: Use of unsafe calls should be audited'\n - EXC0007 (gosec): Too many false-positives for parametrized shell calls.\n Pattern: 'G204: Subprocess launched with variable'\n - EXC0008 (gosec): Duplicated errcheck checks.\n Pattern: 'G104'\n - EXC0009 (gosec): Too many issues in popular repos.\n Pattern: '(G301|G302|G307): Expect (directory permissions to be 0750|file permissions to be 0600) or less'\n - EXC0010 (gosec): False positive is triggered by 'src, err := ioutil.ReadFile(filename)'.\n Pattern: 'G304: Potential file inclusion via variable'\n - EXC0011 (stylecheck): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: '(ST1000|ST1020|ST1021|ST1022)'\n - EXC0012 (revive): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: 'exported (.+) should have comment( \\(or a comment on this block\\))? or be unexported'\n - EXC0013 (revive): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: 'package comment should be of the form \"(.+)...\"'\n - EXC0014 (revive): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: 'comment on exported (.+) should be of the form \"(.+)...\"'\n - EXC0015 (revive): Annoying issue about not having a comment. The rare codebase has such comments.\n Pattern: 'should have a package comment' (default true)\n --exclude-case-sensitive If set to true exclude and exclude rules regular expressions are case-sensitive\n --max-issues-per-linter int Maximum issues count per one linter. Set to 0 to disable (default 50)\n --max-same-issues int Maximum count of issues with the same text. Set to 0 to disable (default 3)\n --uniq-by-line Make issues output unique by line (default true)\n --exclude-files strings Regexps of files to exclude\n --exclude-dirs strings Regexps of directories to exclude\n --exclude-dirs-use-default Use or not use default excluded directories:\n - (^|/)vendor($|/)\n - (^|/)third_party($|/)\n - (^|/)testdata($|/)\n - (^|/)examples($|/)\n - (^|/)Godeps($|/)\n - (^|/)builtin($|/)\n (default true)\n --exclude-generated string Mode of the generated files analysis (default \"lax\")\n -n, --new Show only new issues: if there are unstaged changes or untracked files, only those changes are analyzed, else only changes in HEAD~ are analyzed.\n It's a super-useful option for integration of golangci-lint into existing large codebase.\n It's not practical to fix all existing issues at the moment of integration: much better to not allow issues in new code.\n For 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.\n --new-from-rev REV Show only new issues created after git revision REV\n --new-from-patch PATH Show only new issues created in git patch with file path PATH\n --new-from-merge-base string Show only new issues created after the best common ancestor (merge-base against HEAD)\n --whole-files Show issues in any part of update files (requires new-from-rev or new-from-patch)\n --fix Fix found issues (if it's supported by the linter)\n --cpu-profile-path string Path to CPU profile output file\n --mem-profile-path string Path to memory profile output file\n --print-resources-usage Print avg and max memory usage of golangci-lint and total time\n --trace-path string Path to trace output file\n\nGlobal Flags:\n --color string Use color when printing; can be 'always', 'auto', or 'never' (default \"auto\")\n -h, --help Help for a command\n -v, --verbose Verbose output\n" + "help": "Usage:\n golangci-lint run [flags]\n\nFlags:\n -c, --config PATH Read config from file path PATH\n --no-config Don't read config file\n -D, --disable strings Disable specific linter\n --disable-all Disable all linters\n -E, --enable strings Enable specific linter\n --enable-all Enable all linters\n --fast Enable only fast linters from enabled linters set (first run won't be fast)\n -p, --presets strings Enable presets of linters:\n - bugs\n - comment\n - complexity\n - error\n - format\n - import\n - metalinter\n - module\n - performance\n - sql\n - style\n - test\n - unused\n Run 'golangci-lint help linters' to see them.\n This option implies option --disable-all\n --enable-only strings Override linters configuration section to only run the specific linter(s)\n -j, --concurrency int Number of CPUs to use (Default: number of logical CPUs) (default 8)\n --modules-download-mode string Modules download mode. If not empty, passed as -mod=\u003cmode\u003e to go tools\n --issues-exit-code int Exit code when issues were found (default 1)\n --go string Targeted Go version\n --build-tags strings Build tags\n --timeout duration Timeout for total work. If \u003c= 0, the timeout is disabled (default 1m0s)\n --tests Analyze tests (*_test.go) (default true)\n --allow-parallel-runners Allow multiple parallel golangci-lint instances running.\n If false (default) - golangci-lint acquires file lock on start.\n --allow-serial-runners Allow multiple golangci-lint instances running, but serialize them around a lock.\n If false (default) - golangci-lint exits with an error if it fails to acquire file lock on start.\n --sort-results Sort linter results\n --sort-order strings Sort order of linter results\n --path-prefix string Path prefix to add to output\n --show-stats Show statistics per linter\n --output.text.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.text.print-linter-name Print linter name in the end of issue text. (default true)\n --output.text.print-issued-lines Print lines of code with issue. (default true)\n --output.text.colors Use colors. (default true)\n --output.json.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.tab.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.tab.print-linter-name Print linter name in the end of issue text. (default true)\n --output.tab.colors Use colors. (default true)\n --output.html.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.checkstyle.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.code-climate.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.junit-xml.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.junit-xml.extended Support extra JUnit XML fields. (default true)\n --output.teamcity.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --output.sarif.path stdout Output path can be either stdout, `stderr` or path to the file to write to.\n --max-issues-per-linter int Maximum issues count per one linter. Set to 0 to disable (default 50)\n --max-same-issues int Maximum count of issues with the same text. Set to 0 to disable (default 3)\n --uniq-by-line Make issues output unique by line (default true)\n -n, --new Show only new issues: if there are unstaged changes or untracked files, only those changes are analyzed, else only changes in HEAD~ are analyzed.\n It's a super-useful option for integration of golangci-lint into existing large codebase.\n It's not practical to fix all existing issues at the moment of integration: much better to not allow issues in new code.\n For 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.\n --new-from-rev REV Show only new issues created after git revision REV\n --new-from-patch PATH Show only new issues created in git patch with file path PATH\n --new-from-merge-base string Show only new issues created after the best common ancestor (merge-base against HEAD)\n --whole-files Show issues in any part of update files (requires new-from-rev or new-from-patch)\n --fix Fix found issues (if it's supported by the linter)\n --cpu-profile-path string Path to CPU profile output file\n --mem-profile-path string Path to memory profile output file\n --print-resources-usage Print avg and max memory usage of golangci-lint and total time\n --trace-path string Path to trace output file\n\nGlobal Flags:\n --color string Use color when printing; can be 'always', 'auto', or 'never' (default \"auto\")\n -h, --help Help for a command\n -v, --verbose Verbose output\n" } diff --git a/assets/default-exclusions.json b/assets/default-exclusions.json deleted file mode 100644 index 5b985e5496a0..000000000000 --- a/assets/default-exclusions.json +++ /dev/null @@ -1,92 +0,0 @@ -[ - { - "id": "EXC0001", - "pattern": "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked", - "linter": "errcheck", - "why": "Almost all programs ignore errors on these functions and in most cases it's ok." - }, - { - "id": "EXC0002", - "pattern": "(comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)", - "linter": "golint", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - }, - { - "id": "EXC0003", - "pattern": "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this", - "linter": "golint", - "why": "False positive when tests are defined in package 'test'." - }, - { - "id": "EXC0004", - "pattern": "(possible misuse of unsafe.Pointer|should have signature)", - "linter": "govet", - "why": "Common false positives." - }, - { - "id": "EXC0005", - "pattern": "SA4011", - "linter": "staticcheck", - "why": "Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore." - }, - { - "id": "EXC0006", - "pattern": "G103: Use of unsafe calls should be audited", - "linter": "gosec", - "why": "Too many false-positives on 'unsafe' usage." - }, - { - "id": "EXC0007", - "pattern": "G204: Subprocess launched with variable", - "linter": "gosec", - "why": "Too many false-positives for parametrized shell calls." - }, - { - "id": "EXC0008", - "pattern": "G104", - "linter": "gosec", - "why": "Duplicated errcheck checks." - }, - { - "id": "EXC0009", - "pattern": "(G301|G302|G307): Expect (directory permissions to be 0750|file permissions to be 0600) or less", - "linter": "gosec", - "why": "Too many issues in popular repos." - }, - { - "id": "EXC0010", - "pattern": "G304: Potential file inclusion via variable", - "linter": "gosec", - "why": "False positive is triggered by 'src, err := ioutil.ReadFile(filename)'." - }, - { - "id": "EXC0011", - "pattern": "(ST1000|ST1020|ST1021|ST1022)", - "linter": "stylecheck", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - }, - { - "id": "EXC0012", - "pattern": "exported (.+) should have comment( \\(or a comment on this block\\))? or be unexported", - "linter": "revive", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - }, - { - "id": "EXC0013", - "pattern": "package comment should be of the form \"(.+)...\"", - "linter": "revive", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - }, - { - "id": "EXC0014", - "pattern": "comment on exported (.+) should be of the form \"(.+)...\"", - "linter": "revive", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - }, - { - "id": "EXC0015", - "pattern": "should have a package comment", - "linter": "revive", - "why": "Annoying issue about not having a comment. The rare codebase has such comments." - } -] diff --git a/assets/exclusion-presets.json b/assets/exclusion-presets.json new file mode 100644 index 000000000000..9f3c91f1d740 --- /dev/null +++ b/assets/exclusion-presets.json @@ -0,0 +1,88 @@ +{ + "comments": [ + { + "linters": [ + "stylecheck" + ], + "text": "(ST1000|ST1020|ST1021|ST1022)" + }, + { + "linters": [ + "revive" + ], + "text": "exported (.+) should have comment( \\(or a comment on this block\\))? or be unexported" + }, + { + "linters": [ + "revive" + ], + "text": "package comment should be of the form \"(.+)...\"" + }, + { + "linters": [ + "revive" + ], + "text": "comment on exported (.+) should be of the form \"(.+)...\"" + }, + { + "linters": [ + "revive" + ], + "text": "should have a package comment" + } + ], + "commonFalsePositives": [ + { + "linters": [ + "gosec" + ], + "text": "G103: Use of unsafe calls should be audited" + }, + { + "linters": [ + "gosec" + ], + "text": "G204: Subprocess launched with variable" + }, + { + "linters": [ + "gosec" + ], + "text": "G304: Potential file inclusion via variable" + } + ], + "legacy": [ + { + "linters": [ + "govet" + ], + "text": "(possible misuse of unsafe.Pointer|should have signature)" + }, + { + "linters": [ + "staticcheck" + ], + "text": "SA4011" + }, + { + "linters": [ + "gosec" + ], + "text": "G104" + }, + { + "linters": [ + "gosec" + ], + "text": "(G301|G302|G307): Expect (directory permissions to be 0750|file permissions to be 0600) or less" + } + ], + "stdErrorHandling": [ + { + "linters": [ + "errcheck" + ], + "text": "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked" + } + ] +} diff --git a/assets/linters-info.json b/assets/linters-info.json index a6bdba3f629b..6142f725cae0 100644 --- a/assets/linters-info.json +++ b/assets/linters-info.json @@ -519,6 +519,19 @@ "isSlow": false, "since": "v1.28.0" }, + { + "name": "golines", + "desc": "Checks if code is formatted, and fixes long lines", + "loadMode": 8199, + "inPresets": [ + "format" + ], + "originalURL": "https://github.com/segmentio/golines", + "internal": false, + "canAutoFix": true, + "isSlow": false, + "since": "v2.0.0" + }, { "name": "goheader", "desc": "Checks if file header matches to pattern", @@ -1182,23 +1195,6 @@ "isSlow": true, "since": "v1.40.0" }, - { - "name": "tenv", - "desc": "tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17", - "loadMode": 8767, - "inPresets": [ - "test" - ], - "originalURL": "https://github.com/sivchari/tenv", - "internal": false, - "isSlow": true, - "since": "v1.43.0", - "deprecation": { - "since": "v1.64.0", - "message": "Duplicate feature another linter.", - "replacement": "usetesting" - } - }, { "name": "testableexamples", "desc": "linter checks if examples are testable (have an expected output)", diff --git a/docs/src/docs/usage/false-positives.mdx b/docs/src/docs/usage/false-positives.mdx index 54f93568563b..75c6d6e2edea 100644 --- a/docs/src/docs/usage/false-positives.mdx +++ b/docs/src/docs/usage/false-positives.mdx @@ -169,12 +169,11 @@ You can see more examples of using `//nolint` in [our tests](https://github.com/ Use `//nolint` instead of `// nolint` because machine-readable comments should have no space by Go convention. -## Default Exclusions +## Exclusion Presets -Some exclusions are considered as common, to help golangci-lint users those common exclusions are used as default exclusions. +Some exclusions are considered as common, to help golangci-lint users those common exclusions are provided through presets. -If you don't want to use it you can set `issues.exclude-use-default` to `false`. -{.DefaultExclusions} +{.ExclusionPresets} ### Default Directory Exclusions diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 25469d703cd4..46b8e37fe71b 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -382,7 +382,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithURL("https://github.com/mvdan/gofumpt"), linter.NewConfig(golines.New(&cfg.LintersSettings.GoLines)). - WithSince("v1.64.0"). + WithSince("v2.0.0"). WithPresets(linter.PresetFormatting). WithAutoFix(). WithURL("https://github.com/segmentio/golines"), diff --git a/pkg/result/processors/exclusion_presets.go b/pkg/result/processors/exclusion_presets.go index 17299b90c5ee..7043b47eca10 100644 --- a/pkg/result/processors/exclusion_presets.go +++ b/pkg/result/processors/exclusion_presets.go @@ -2,7 +2,7 @@ package processors import "github.com/golangci/golangci-lint/pkg/config" -var linterExclusionPresets = map[string][]config.ExcludeRule{ +var LinterExclusionPresets = map[string][]config.ExcludeRule{ config.ExclusionPresetComments: { { // Annoying issue about not having a comment. The rare codebase has such comments. @@ -129,7 +129,7 @@ func getLinterExclusionPresets(names []string) []config.ExcludeRule { var rules []config.ExcludeRule for _, name := range names { - if p, ok := linterExclusionPresets[name]; ok { + if p, ok := LinterExclusionPresets[name]; ok { rules = append(rules, p...) } } diff --git a/scripts/website/dump_info/main.go b/scripts/website/dump_info/main.go index e0be74dbdea4..c1227d1e4406 100644 --- a/scripts/website/dump_info/main.go +++ b/scripts/website/dump_info/main.go @@ -12,6 +12,7 @@ import ( "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/result/processors" "github.com/golangci/golangci-lint/scripts/website/types" ) @@ -71,8 +72,21 @@ func saveLinters() error { } func saveDefaultExclusions() error { - // FIXME - return saveToJSONFile(filepath.Join("assets", "default-exclusions.json"), nil) + data := make(map[string][]types.ExcludeRule) + + for name, rules := range processors.LinterExclusionPresets { + for _, rule := range rules { + data[name] = append(data[name], types.ExcludeRule{ + Linters: rule.Linters, + Path: rule.Path, + PathExcept: rule.PathExcept, + Text: rule.Text, + Source: rule.Source, + }) + } + } + + return saveToJSONFile(filepath.Join("assets", "exclusion-presets.json"), data) } func saveCLIHelp(dst string) error { diff --git a/scripts/website/expand_templates/exclusions.go b/scripts/website/expand_templates/exclusions.go index 31678d5b7bb9..5804e3469908 100644 --- a/scripts/website/expand_templates/exclusions.go +++ b/scripts/website/expand_templates/exclusions.go @@ -3,22 +3,32 @@ package main import ( "bytes" "path/filepath" - "strings" "text/template" "github.com/golangci/golangci-lint/scripts/website/types" ) -const exclusionTmpl = `{{ $tick := "` + "`" + `" }} -### {{ .ID }} - -- linter: {{ $tick }}{{ .Linter }}{{ $tick }} -- pattern: {{ $tick }}{{ .Pattern }}{{ $tick }} -- why: {{ .Why }} -` - -func getDefaultExclusions() (string, error) { - defaultExcludePatterns, err := readJSONFile[[]types.ExcludePattern](filepath.Join("assets", "default-exclusions.json")) +const exclusionTmpl = `{{- $tick := "` + "`" + `" -}} +{{- range $name, $rules := . }} +### {{ $tick }}{{ $name }}{{ $tick }} +{{ range $rule := $rules }} +{{ $tick }}{{ range $linter := $rule.Linters }}{{ $linter }}{{ end }}{{ $tick }}: +{{ if $rule.Path -}} +- Path: {{ $tick }}{{ $rule.Path }}{{ $tick }} +{{ end -}} +{{ if $rule.PathExcept -}} +- Path Except: {{ $tick }}{{ $rule.PathExcept }}{{ $tick }} +{{ end -}} +{{ if $rule.Text -}} +- Text: {{ $tick }}{{ $rule.Text }}{{ $tick }} +{{ end -}} +{{ if $rule.Source -}} +- Source: {{ $tick }}{{ $rule.Source }}{{ $tick }} +{{ end -}} +{{ end }}{{ end }}` + +func getExclusionPresets() (string, error) { + linterExclusionPresets, err := readJSONFile[map[string][]types.ExcludeRule](filepath.Join("assets", "exclusion-presets.json")) if err != nil { return "", err } @@ -30,18 +40,9 @@ func getDefaultExclusions() (string, error) { return "", err } - for _, pattern := range defaultExcludePatterns { - data := map[string]any{ - "ID": pattern.ID, - "Linter": pattern.Linter, - "Pattern": strings.ReplaceAll(pattern.Pattern, "`", "`"), - "Why": pattern.Why, - } - - err := tmpl.Execute(bufferString, data) - if err != nil { - return "", err - } + err = tmpl.Execute(bufferString, linterExclusionPresets) + if err != nil { + return "", err } return bufferString.String(), nil diff --git a/scripts/website/expand_templates/main.go b/scripts/website/expand_templates/main.go index 9a36acb1dcd2..c7dd8dfe2835 100644 --- a/scripts/website/expand_templates/main.go +++ b/scripts/website/expand_templates/main.go @@ -115,7 +115,7 @@ func buildTemplateContext() (map[string]string, error) { return nil, fmt.Errorf("get the latest version: %w", err) } - exclusions, err := getDefaultExclusions() + exclusions, err := getExclusionPresets() if err != nil { return nil, fmt.Errorf("default exclusions: %w", err) } @@ -127,7 +127,7 @@ func buildTemplateContext() (map[string]string, error) { "LintersCommandOutputEnabledOnly": helps.Enable, "EnabledByDefaultLinters": getLintersListMarkdown(true), "DisabledByDefaultLinters": getLintersListMarkdown(false), - "DefaultExclusions": exclusions, + "ExclusionPresets": exclusions, "ThanksList": getThanksList(), "RunHelpText": helps.Help, "ChangeLog": string(changeLog), diff --git a/scripts/website/types/types.go b/scripts/website/types/types.go index 7a5fcc4e342d..fbdaf5db29ce 100644 --- a/scripts/website/types/types.go +++ b/scripts/website/types/types.go @@ -9,11 +9,12 @@ type CLIHelp struct { Help string `json:"help"` } -type ExcludePattern struct { - ID string `json:"id,omitempty"` - Pattern string `json:"pattern,omitempty"` - Linter string `json:"linter,omitempty"` - Why string `json:"why,omitempty"` +type ExcludeRule struct { + Linters []string `json:"linters,omitempty"` + Path string `json:"path,omitempty"` + PathExcept string `json:"path-except,omitempty"` + Text string `json:"text,omitempty"` + Source string `json:"source,omitempty"` } type Deprecation struct { From b574814536b6a013a4a157545a844df87b10860e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 19 Feb 2025 22:17:27 +0100 Subject: [PATCH 3/8] chore: update CI and configuration --- .github/workflows/pr.yml | 12 ++- .golangci.yml | 119 +++++++++++---------- pkg/commands/flagsets.go | 1 - pkg/result/processors/exclusion_presets.go | 2 +- 4 files changed, 71 insertions(+), 63 deletions(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index f8e6cfd28d3b..5687d95ef625 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -41,10 +41,11 @@ jobs: # - 1.18beta1 -> 1.18.0-beta.1 # - 1.18rc1 -> 1.18.0-rc.1 go-version: ${{ env.GO_VERSION }} - - name: lint - uses: golangci/golangci-lint-action@v6.5.0 - with: - version: latest + # TODO(ldez): must add uncommented when golangci-lint-action@v7.0.0 (with golangci-lint v2 support) will be created. +# - name: lint +# uses: golangci/golangci-lint-action@v6.5.0 +# with: +# version: latest tests-on-windows: needs: golangci-lint # run after golangci-lint action to not produce duplicated errors @@ -147,7 +148,8 @@ jobs: - run: ./golangci-lint config - run: ./golangci-lint config path - - run: ./golangci-lint config verify --schema jsonschema/golangci.jsonschema.json + # TODO(ldez) after v2: golangci.next.jsonschema.json -> golangci.jsonschema.json + - run: ./golangci-lint config verify --schema jsonschema/golangci.next.jsonschema.json - run: ./golangci-lint help - run: ./golangci-lint help linters diff --git a/.golangci.yml b/.golangci.yml index b2c0240c7805..87fe4f183efe 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,6 +11,8 @@ # # See the file `.golangci.reference.yml` to have a list of all available configuration options. +version: "2" + linters: disable-all: true # This list of linters is not a recommendation (same thing for all this configuration file). @@ -31,8 +33,6 @@ linters: - gocritic - gocyclo - godox - - gofmt - - goimports - mnd - goprintffuncname - gosec @@ -53,7 +53,68 @@ linters: - unparam - unused - whitespace + exclusions: + presets: + - comments + - stdErrorHandling + - commonFalsePositives + - legacy + paths: + - test/testdata_etc # test files + - internal/go # extracted from Go code + - internal/x # extracted from x/tools code + - pkg/goformatters/gci/internal # extracted from gci code + - pkg/goanalysis/runner_checker.go # extracted from x/tools code + rules: + - path: (.+)_test\.go + linters: + - dupl + - mnd + - lll + + # Deprecated linter options. + - path: pkg/golinters/errcheck/errcheck.go + linters: [staticcheck] + text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead" + - path: pkg/golinters/errcheck/errcheck.go + linters: [staticcheck] + text: "SA1019: errCfg.Ignore is deprecated: use ExcludeFunctions instead" + - path: pkg/golinters/govet/govet.go + linters: [staticcheck] + text: "SA1019: cfg.CheckShadowing is deprecated: the linter should be enabled inside Enable." + - path: pkg/golinters/godot/godot.go + linters: [staticcheck] + text: "SA1019: settings.CheckAll is deprecated: use Scope instead" + - path: pkg/goformatters/gci/gci.go + linters: [staticcheck] + text: "SA1019: settings.LocalPrefixes is deprecated: use Sections instead." + # Related to `run.go`, it cannot be removed. + - path: pkg/golinters/gofumpt/gofumpt.go + linters: [staticcheck] + text: "SA1019: settings.LangVersion is deprecated: use the global `run.go` instead." + + # Based on existing code, the modifications should be limited to make maintenance easier. + - path: pkg/golinters/unused/unused.go + linters: [gocritic] + text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)" + + # Related to the result of computation but divided multiple times by 1024. + - path: test/bench/bench_test.go + linters: [gosec] + text: "G115: integer overflow conversion uint64 -> int" + +formatters: + enable: + - gofmt + - goimports + settings: + gofmt: + rewrite-rules: + - pattern: 'interface{}' + replacement: 'any' + goimports: + local-prefixes: github.com/golangci/golangci-lint linters-settings: depguard: @@ -95,12 +156,6 @@ linters-settings: godox: keywords: - FIXME - gofmt: - rewrite-rules: - - pattern: 'interface{}' - replacement: 'any' - goimports: - local-prefixes: github.com/golangci/golangci-lint mnd: # don't include the "operation" and "assign" checks: @@ -146,53 +201,5 @@ linters-settings: - name: unused-parameter - name: unused-receiver -issues: - exclude-rules: - - path: (.+)_test\.go - linters: - - dupl - - mnd - - lll - - # Deprecated linter options. - - path: pkg/golinters/errcheck/errcheck.go - linters: [staticcheck] - text: "SA1019: errCfg.Exclude is deprecated: use ExcludeFunctions instead" - - path: pkg/golinters/errcheck/errcheck.go - linters: [staticcheck] - text: "SA1019: errCfg.Ignore is deprecated: use ExcludeFunctions instead" - - path: pkg/golinters/govet/govet.go - linters: [staticcheck] - text: "SA1019: cfg.CheckShadowing is deprecated: the linter should be enabled inside Enable." - - path: pkg/golinters/godot/godot.go - linters: [staticcheck] - text: "SA1019: settings.CheckAll is deprecated: use Scope instead" - - path: pkg/goformatters/gci/gci.go - linters: [staticcheck] - text: "SA1019: settings.LocalPrefixes is deprecated: use Sections instead." - - # Related to `run.go`, it cannot be removed. - - path: pkg/golinters/gofumpt/gofumpt.go - linters: [staticcheck] - text: "SA1019: settings.LangVersion is deprecated: use the global `run.go` instead." - - # Based on existing code, the modifications should be limited to make maintenance easier. - - path: pkg/golinters/unused/unused.go - linters: [gocritic] - text: "rangeValCopy: each iteration copies 160 bytes \\(consider pointers or indexing\\)" - - # Related to the result of computation but divided multiple times by 1024. - - path: test/bench/bench_test.go - linters: [gosec] - text: "G115: integer overflow conversion uint64 -> int" - - exclude-dirs: - - test/testdata_etc # test files - - internal/go # extracted from Go code - - internal/x # extracted from x/tools code - - pkg/goformatters/gci/internal # extracted from gci code - exclude-files: - - pkg/goanalysis/runner_checker.go # extracted from x/tools code - run: timeout: 5m diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index ca0c53d599db..4260c8712af1 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -126,7 +126,6 @@ func setupOutputFormatsFlagSet(v *viper.Viper, fs *pflag.FlagSet) { color.GreenString(outputPathDesc)) } -//nolint:gomnd // magic numbers here is ok func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBind(v, fs, fs.Int, "max-issues-per-linter", "issues.max-issues-per-linter", defaultMaxIssuesPerLinter, color.GreenString("Maximum issues count per one linter. Set to 0 to disable")) diff --git a/pkg/result/processors/exclusion_presets.go b/pkg/result/processors/exclusion_presets.go index 7043b47eca10..fb2f9cf3c154 100644 --- a/pkg/result/processors/exclusion_presets.go +++ b/pkg/result/processors/exclusion_presets.go @@ -54,7 +54,7 @@ var LinterExclusionPresets = map[string][]config.ExcludeRule{ { // Almost all programs ignore errors on these functions and in most cases it's ok. BaseRule: config.BaseRule{ - Text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close" + + Text: "(?i)Error return value of .((os\\.)?std(out|err)\\..*|.*Close" + "|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked", Linters: []string{"errcheck"}, InternalReference: "EXC0001", From 5108d71bc76b4cbddf2cfa907a732f3b474b24bf Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 19 Feb 2025 23:54:06 +0100 Subject: [PATCH 4/8] tests: update linter tests --- pkg/golinters/errcheck/testdata/errcheck.go | 8 ++++---- pkg/golinters/exhaustive/testdata/exhaustive_generated.go | 4 ++-- pkg/golinters/gosec/testdata/gosec.go | 2 +- pkg/golinters/revive/testdata/revive.go | 2 ++ pkg/golinters/revive/testdata/revive.yml | 1 - pkg/golinters/revive/testdata/revive_default.go | 1 + pkg/golinters/stylecheck/testdata/fix/in/stylecheck.go | 2 ++ pkg/golinters/stylecheck/testdata/fix/out/stylecheck.go | 2 ++ pkg/golinters/stylecheck/testdata/stylecheck.go | 1 + pkg/golinters/stylecheck/testdata/stylecheck_cgo.go | 1 + .../stylecheck/testdata/stylecheck_not_in_megacheck.go | 1 + 11 files changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/golinters/errcheck/testdata/errcheck.go b/pkg/golinters/errcheck/testdata/errcheck.go index 3227cac20f58..834cf261b17a 100644 --- a/pkg/golinters/errcheck/testdata/errcheck.go +++ b/pkg/golinters/errcheck/testdata/errcheck.go @@ -21,7 +21,7 @@ func IgnoreCloseMissingErrHandling() error { return err } - f.Close() + f.Close() // want "Error return value of `f.Close` is not checked" return nil } @@ -30,14 +30,14 @@ func IgnoreCloseInDeferMissingErrHandling() { if err != nil { panic(err) } - defer resp.Body.Close() + defer resp.Body.Close() // want "Error return value of `resp.Body.Close` is not checked" panic(resp) } func IgnoreStdxWrite() { - os.Stdout.Write([]byte{}) - os.Stderr.Write([]byte{}) + os.Stdout.Write([]byte{}) // want "Error return value of `os.Stdout.Write` is not checked" + os.Stderr.Write([]byte{}) // want "Error return value of `os.Stderr.Write` is not checked" } func IgnoreBufferWrites(buf *bytes.Buffer) { diff --git a/pkg/golinters/exhaustive/testdata/exhaustive_generated.go b/pkg/golinters/exhaustive/testdata/exhaustive_generated.go index fcc946971810..b86ba27d5a7a 100644 --- a/pkg/golinters/exhaustive/testdata/exhaustive_generated.go +++ b/pkg/golinters/exhaustive/testdata/exhaustive_generated.go @@ -1,9 +1,9 @@ +// Code generated by some program. DO NOT EDIT. + //golangcitest:args -Eexhaustive //golangcitest:expected_exitcode 0 package testdata -// Code generated by some program. DO NOT EDIT. - // Should not report missing cases in the switch statement below, because this // is a generated file as indicated by the above comment // (golang.org/s/generatedcode), and check-generated setting is false. diff --git a/pkg/golinters/gosec/testdata/gosec.go b/pkg/golinters/gosec/testdata/gosec.go index 7e660728a72f..bd984d6cacf0 100644 --- a/pkg/golinters/gosec/testdata/gosec.go +++ b/pkg/golinters/gosec/testdata/gosec.go @@ -34,5 +34,5 @@ func GosecG204SubprocWithFunc() { return "/tmp/dummy" } - exec.Command("ls", arg()).Run() // want "G204: Subprocess launched with a potential tainted input or cmd arguments" + exec.Command("ls", arg()).Run() // want "G104: Errors unhandled" } diff --git a/pkg/golinters/revive/testdata/revive.go b/pkg/golinters/revive/testdata/revive.go index 735f1b757bf6..4aac94527616 100644 --- a/pkg/golinters/revive/testdata/revive.go +++ b/pkg/golinters/revive/testdata/revive.go @@ -7,6 +7,8 @@ import ( "time" ) +// want +2 "exported: exported function SampleRevive should have comment or be unexported" + func SampleRevive(t *time.Duration) error { if t == nil { return nil diff --git a/pkg/golinters/revive/testdata/revive.yml b/pkg/golinters/revive/testdata/revive.yml index 76abf43b7867..0a901a6396a3 100644 --- a/pkg/golinters/revive/testdata/revive.yml +++ b/pkg/golinters/revive/testdata/revive.yml @@ -6,7 +6,6 @@ linters-settings: severity: warning rules: - name: exported - - name: package-comments - name: cognitive-complexity arguments: [ 7 ] - name: line-length-limit diff --git a/pkg/golinters/revive/testdata/revive_default.go b/pkg/golinters/revive/testdata/revive_default.go index a29d96c1e17e..18f436d5ad6d 100644 --- a/pkg/golinters/revive/testdata/revive_default.go +++ b/pkg/golinters/revive/testdata/revive_default.go @@ -1,4 +1,5 @@ //golangcitest:args -Erevive +// Package testdata ... package testdata import ( diff --git a/pkg/golinters/stylecheck/testdata/fix/in/stylecheck.go b/pkg/golinters/stylecheck/testdata/fix/in/stylecheck.go index b20df7227cb8..2469f148320d 100644 --- a/pkg/golinters/stylecheck/testdata/fix/in/stylecheck.go +++ b/pkg/golinters/stylecheck/testdata/fix/in/stylecheck.go @@ -1,3 +1,5 @@ +// Package testdata ... +// //golangcitest:args -Estylecheck //golangcitest:expected_exitcode 0 package testdata diff --git a/pkg/golinters/stylecheck/testdata/fix/out/stylecheck.go b/pkg/golinters/stylecheck/testdata/fix/out/stylecheck.go index 28e83cff5d86..2450d9309b18 100644 --- a/pkg/golinters/stylecheck/testdata/fix/out/stylecheck.go +++ b/pkg/golinters/stylecheck/testdata/fix/out/stylecheck.go @@ -1,3 +1,5 @@ +// Package testdata ... +// //golangcitest:args -Estylecheck //golangcitest:expected_exitcode 0 package testdata diff --git a/pkg/golinters/stylecheck/testdata/stylecheck.go b/pkg/golinters/stylecheck/testdata/stylecheck.go index 44a8cd1f723a..07c29e5ae999 100644 --- a/pkg/golinters/stylecheck/testdata/stylecheck.go +++ b/pkg/golinters/stylecheck/testdata/stylecheck.go @@ -1,4 +1,5 @@ //golangcitest:args -Estylecheck +// Package testdata ... package testdata func Stylecheck(x int) { diff --git a/pkg/golinters/stylecheck/testdata/stylecheck_cgo.go b/pkg/golinters/stylecheck/testdata/stylecheck_cgo.go index 5fd796c94d14..e2747585bba4 100644 --- a/pkg/golinters/stylecheck/testdata/stylecheck_cgo.go +++ b/pkg/golinters/stylecheck/testdata/stylecheck_cgo.go @@ -1,4 +1,5 @@ //golangcitest:args -Estylecheck +// Package testdata ... package testdata /* diff --git a/pkg/golinters/stylecheck/testdata/stylecheck_not_in_megacheck.go b/pkg/golinters/stylecheck/testdata/stylecheck_not_in_megacheck.go index 18dbae909592..1d03aa148f65 100644 --- a/pkg/golinters/stylecheck/testdata/stylecheck_not_in_megacheck.go +++ b/pkg/golinters/stylecheck/testdata/stylecheck_not_in_megacheck.go @@ -1,5 +1,6 @@ //golangcitest:args -Emegacheck //golangcitest:expected_exitcode 0 +// Package testdata ... package testdata func StylecheckNotInMegacheck(x int) { From e01da5334ae664c05d654ce8a17cdaa8401d5bc9 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 19 Feb 2025 23:54:22 +0100 Subject: [PATCH 5/8] tests: update global tests --- test/run_test.go | 58 +++++++------------ test/testdata/cgo/main.go | 2 + test/testdata/configs/default_exclude.yml | 9 ++- test/testdata/configs/path-except.yml | 15 ++--- test/testdata/default_exclude.go | 6 +- test/testdata/skipdirs/examples/with_issue.go | 11 ---- .../skipdirs/examples_no_skip/with_issue.go | 11 ---- .../skipdirs/skip_me/nested/with_issue.go | 11 ---- test/testdata/unsafe/pkg.go | 2 + 9 files changed, 42 insertions(+), 83 deletions(-) delete mode 100644 test/testdata/skipdirs/examples/with_issue.go delete mode 100644 test/testdata/skipdirs/examples_no_skip/with_issue.go delete mode 100644 test/testdata/skipdirs/skip_me/nested/with_issue.go diff --git a/test/run_test.go b/test/run_test.go index f540fe7d9730..842ea7e68bed 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -14,8 +14,18 @@ import ( const minimalPkg = "minimalpkg" func TestAutogeneratedNoIssues(t *testing.T) { + binPath := testshared.InstallGolangciLint(t) + + cfg := ` + linters: + exclusions: + generated: lax + ` + testshared.NewRunnerBuilder(t). + WithConfig(cfg). WithTargetPath(testdataDir, "autogenerated"). + WithBinPath(binPath). Runner(). Install(). Run(). @@ -270,7 +280,6 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { desc: "lite loading", args: []string{ "--output.text.print-issued-lines=false", - "--exclude-use-default=false", "-Erevive", }, target: "quicktemplate", @@ -285,7 +294,6 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { desc: "full loading", args: []string{ "--output.text.print-issued-lines=false", - "--exclude-use-default=false", "-Erevive,govet", }, target: "quicktemplate", @@ -316,12 +324,21 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { } func TestUnsafeOk(t *testing.T) { + binPath := testshared.InstallGolangciLint(t) + + cfg := ` + linters: + exclusions: + presets: + - commonFalsePositives + ` + testshared.NewRunnerBuilder(t). - WithNoConfig(). + WithConfig(cfg). WithArgs("--enable-all"). WithTargetPath(testdataDir, "unsafe"). + WithBinPath(binPath). Runner(). - Install(). Run(). ExpectNoIssues() } @@ -361,39 +378,6 @@ func TestSortedResults(t *testing.T) { } } -func TestSkippedDirsNoMatchArg(t *testing.T) { - dir := filepath.Join(testdataDir, "skipdirs", "skip_me", "nested") - - testshared.NewRunnerBuilder(t). - WithNoConfig(). - WithArgs( - "--output.text.print-issued-lines=false", - "--exclude-dirs", dir, - "-Erevive", - ). - WithTargetPath(dir). - Runner(). - Install(). - Run(). - ExpectExitCode(exitcodes.IssuesFound). - ExpectOutputEq("testdata/skipdirs/skip_me/nested/with_issue.go:8:9: " + - "indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)\n") -} - -func TestSkippedDirsTestdata(t *testing.T) { - testshared.NewRunnerBuilder(t). - WithNoConfig(). - WithArgs( - "--output.text.print-issued-lines=false", - "-Erevive", - ). - WithTargetPath(testdataDir, "skipdirs", "..."). - Runner(). - Install(). - Run(). - ExpectNoIssues() // all was skipped because in testdata -} - func TestIdentifierUsedOnlyInTests(t *testing.T) { testshared.NewRunnerBuilder(t). WithNoConfig(). diff --git a/test/testdata/cgo/main.go b/test/testdata/cgo/main.go index cbb692fe7068..253c1d6b31f8 100644 --- a/test/testdata/cgo/main.go +++ b/test/testdata/cgo/main.go @@ -1,3 +1,4 @@ +// Package cgoexample ... package cgoexample /* @@ -14,6 +15,7 @@ import ( "unsafe" ) +// Example ... func Example() { cs := C.CString("Hello from stdio\n") C.myprint(cs) diff --git a/test/testdata/configs/default_exclude.yml b/test/testdata/configs/default_exclude.yml index 9abbdb17985f..db4e40af937f 100644 --- a/test/testdata/configs/default_exclude.yml +++ b/test/testdata/configs/default_exclude.yml @@ -1,5 +1,8 @@ version: "2" -issues: - include: - - EXC0011 # include issues about comments from `stylecheck` +linters: + exclusions: + presets: + - stdErrorHandling + - commonFalsePositives + - legacy diff --git a/test/testdata/configs/path-except.yml b/test/testdata/configs/path-except.yml index 3f2261a241b0..44a60814e00b 100644 --- a/test/testdata/configs/path-except.yml +++ b/test/testdata/configs/path-except.yml @@ -6,10 +6,11 @@ linters-settings: - fmt\.Print.* - time.Sleep(# no sleeping!)? -issues: - exclude-rules: - # Apply forbidigo only to test files, exclude - # it everywhere else. - - path-except: _test\.go - linters: - - forbidigo +linters: + exclusions: + rules: + # Apply forbidigo only to test files, exclude + # it everywhere else. + - path-except: _test\.go + linters: + - forbidigo diff --git a/test/testdata/default_exclude.go b/test/testdata/default_exclude.go index c97eb7fbd414..dd78e76b84a2 100644 --- a/test/testdata/default_exclude.go +++ b/test/testdata/default_exclude.go @@ -4,18 +4,18 @@ /*Package testdata ...*/ package testdata -// InvalidFuncComment, both revive and stylecheck will complain about this, // want stylecheck:`ST1020: comment on exported function ExportedFunc1 should be of the form "ExportedFunc1 ..."` +// InvalidFuncComment, both revive and stylecheck will complain about this, // want `exported: comment on exported function ExportedFunc1 should be of the form "ExportedFunc1 ..."` // if include EXC0011, only the warning from revive will be ignored. // And only the warning from stylecheck will start with "ST1020". func ExportedFunc1() { } -// InvalidFuncComment // want stylecheck:`ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."` +// InvalidFuncComment // want `ST1020: comment on exported function ExportedFunc2 should be of the form "ExportedFunc2 ..."` // //nolint:revive func ExportedFunc2() { } //nolint:stylecheck -func IgnoreAll() { +func IgnoreAll() { // want "exported: exported function IgnoreAll should have comment or be unexported" } diff --git a/test/testdata/skipdirs/examples/with_issue.go b/test/testdata/skipdirs/examples/with_issue.go deleted file mode 100644 index e82aec73fdcd..000000000000 --- a/test/testdata/skipdirs/examples/with_issue.go +++ /dev/null @@ -1,11 +0,0 @@ -package main - -import "fmt" - -func main() { - if true { - return - } else { - fmt.Printf("") - } -} diff --git a/test/testdata/skipdirs/examples_no_skip/with_issue.go b/test/testdata/skipdirs/examples_no_skip/with_issue.go deleted file mode 100644 index e82aec73fdcd..000000000000 --- a/test/testdata/skipdirs/examples_no_skip/with_issue.go +++ /dev/null @@ -1,11 +0,0 @@ -package main - -import "fmt" - -func main() { - if true { - return - } else { - fmt.Printf("") - } -} diff --git a/test/testdata/skipdirs/skip_me/nested/with_issue.go b/test/testdata/skipdirs/skip_me/nested/with_issue.go deleted file mode 100644 index e82aec73fdcd..000000000000 --- a/test/testdata/skipdirs/skip_me/nested/with_issue.go +++ /dev/null @@ -1,11 +0,0 @@ -package main - -import "fmt" - -func main() { - if true { - return - } else { - fmt.Printf("") - } -} diff --git a/test/testdata/unsafe/pkg.go b/test/testdata/unsafe/pkg.go index 0a7e5874a70f..1df14f16923b 100644 --- a/test/testdata/unsafe/pkg.go +++ b/test/testdata/unsafe/pkg.go @@ -1,3 +1,4 @@ +// Package pkg ... package pkg import ( @@ -5,6 +6,7 @@ import ( "unsafe" ) +// F ... func F() { x := 123 // of type int p := unsafe.Pointer(&x) From 3214cac0975b8386af049d6f015640169f03aebf Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 20 Feb 2025 02:51:23 +0100 Subject: [PATCH 6/8] tests: Windows ... --- test/run_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/test/run_test.go b/test/run_test.go index 842ea7e68bed..2b3803ff8bf4 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -1,7 +1,9 @@ package test import ( + "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/require" @@ -14,6 +16,17 @@ import ( const minimalPkg = "minimalpkg" func TestAutogeneratedNoIssues(t *testing.T) { + _, ci := os.LookupEnv("CI") + if runtime.GOOS == "windows" && ci { + // Tests on Windows and GitHub action that use a file produce a warning and so an exit code 2: + // level=warning msg=\"[config_reader] Can't pretty print config file path: can't get relative path for path C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\Temp\\\\golangci_lint_test3234185281.yml and root D:\\\\a\\\\golangci-lint\\\\golangci-lint\\\\test: Rel: can't make C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\Temp\\\\golangci_lint_test3234185281.yml relative to D:\\\\a\\\\golangci-lint\\\\golangci-lint\\\\test\" + // + // In the context of a test that ExpectNoIssues this is problem. + // + // NOTE(ldez): I don't want to create flags only for running tests on Windows + GitHub Action. + t.Skip("on Windows + GitHub Action") + } + binPath := testshared.InstallGolangciLint(t) cfg := ` @@ -27,7 +40,6 @@ func TestAutogeneratedNoIssues(t *testing.T) { WithTargetPath(testdataDir, "autogenerated"). WithBinPath(binPath). Runner(). - Install(). Run(). ExpectNoIssues() } @@ -324,6 +336,17 @@ func TestLineDirectiveProcessedFiles(t *testing.T) { } func TestUnsafeOk(t *testing.T) { + _, ci := os.LookupEnv("CI") + if runtime.GOOS == "windows" && ci { + // Tests on Windows and GitHub action that use a file produce a warning and so an exit code 2: + // level=warning msg=\"[config_reader] Can't pretty print config file path: can't get relative path for path C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\Temp\\\\golangci_lint_test3234185281.yml and root D:\\\\a\\\\golangci-lint\\\\golangci-lint\\\\test: Rel: can't make C:\\\\Users\\\\runneradmin\\\\AppData\\\\Local\\\\Temp\\\\golangci_lint_test3234185281.yml relative to D:\\\\a\\\\golangci-lint\\\\golangci-lint\\\\test\" + // + // In the context of a test that ExpectNoIssues this is problem. + // + // NOTE(ldez): I don't want to create flags only for running tests on Windows + GitHub Action. + t.Skip("on Windows + GitHub Action") + } + binPath := testshared.InstallGolangciLint(t) cfg := ` From e401b34148c8ac2433659bd6fb016467665eb5f4 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 20 Feb 2025 14:07:27 +0100 Subject: [PATCH 7/8] review --- .golangci.next.reference.yml | 13 +++++-------- docs/src/docs/usage/false-positives.mdx | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 13281226cfa7..d3427c28602b 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -262,15 +262,15 @@ linters: fast: true # Defines a set of rules to ignore issues. - # It does not skip the analysis, and so don't ignore "typecheck" errors. + # It does not skip the analysis, and so does not ignore "typecheck" errors. exclusions: # Mode of the generated files analysis. # - # - `strict`: sources are excluded by following strictly the Go generated file convention. + # - `strict`: sources are excluded by strictly following the Go generated file convention. # Source files that have lines matching only the following regular expression will be excluded: `^// Code generated .* DO NOT EDIT\.$` # This line must appear before the first non-comment, non-blank text in the file. # https://go.dev/s/generatedcode - # - `lax`: sources are excluded if they contain lines `autogenerated file`, `code generated`, `do not edit`, etc. + # - `lax`: sources are excluded if they contain lines like `autogenerated file`, `code generated`, `do not edit`, etc. # - `disable`: disable the generated files exclusion. # # Default: lax @@ -303,7 +303,7 @@ linters: # Exclude known linters from partially hard-vendored code, # which is impossible to exclude via `nolint` comments. - # `/` will be replaced by current OS file path separator to properly work on Windows. + # `/` will be replaced by the current OS file path separator to properly work on Windows. - path: internal/hmac/ text: "weak cryptographic primitive" linters: @@ -320,10 +320,7 @@ linters: source: "^//go:generate " # Which file paths 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. + # "/" will be replaced by the current OS file path separator to properly work on Windows. # Default: [] paths: - ".*\\.my\\.go$" diff --git a/docs/src/docs/usage/false-positives.mdx b/docs/src/docs/usage/false-positives.mdx index 75c6d6e2edea..1a1c5dabd770 100644 --- a/docs/src/docs/usage/false-positives.mdx +++ b/docs/src/docs/usage/false-positives.mdx @@ -171,7 +171,7 @@ Use `//nolint` instead of `// nolint` because machine-readable comments should h ## Exclusion Presets -Some exclusions are considered as common, to help golangci-lint users those common exclusions are provided through presets. +Some exclusions are considered common. To help golangci-lint users those common exclusions are provided through presets. {.ExclusionPresets} From 7b3982556c02b6e5764bf7ed308018533c931b05 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 20 Feb 2025 19:25:48 +0100 Subject: [PATCH 8/8] feat: new preset names --- .golangci.next.reference.yml | 4 ++-- .golangci.yml | 4 ++-- assets/exclusion-presets.json | 4 ++-- jsonschema/golangci.next.jsonschema.json | 4 ++-- pkg/config/linters_exclusions.go | 4 ++-- test/run_test.go | 2 +- test/testdata/configs/default_exclude.yml | 4 ++-- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index d3427c28602b..cdedd51dd6f7 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -282,8 +282,8 @@ linters: # Default: [] presets: - comments - - stdErrorHandling - - commonFalsePositives + - std-error-handling + - common-false-positives - legacy # Excluding configuration per-path, per-linter, per-text and per-source. diff --git a/.golangci.yml b/.golangci.yml index 87fe4f183efe..73acb099b724 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -56,8 +56,8 @@ linters: exclusions: presets: - comments - - stdErrorHandling - - commonFalsePositives + - std-error-handling + - common-false-positives - legacy paths: - test/testdata_etc # test files diff --git a/assets/exclusion-presets.json b/assets/exclusion-presets.json index 9f3c91f1d740..2c1e0740b8c1 100644 --- a/assets/exclusion-presets.json +++ b/assets/exclusion-presets.json @@ -31,7 +31,7 @@ "text": "should have a package comment" } ], - "commonFalsePositives": [ + "common-false-positives": [ { "linters": [ "gosec" @@ -77,7 +77,7 @@ "text": "(G301|G302|G307): Expect (directory permissions to be 0750|file permissions to be 0600) or less" } ], - "stdErrorHandling": [ + "std-error-handling": [ { "linters": [ "errcheck" diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 26d3e77c0a35..cea3c8aed5b0 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -4265,8 +4265,8 @@ "items": { "enum": [ "comments", - "stdErrorHandling", - "commonFalsePositives", + "std-error-handling", + "common-false-positives", "legacy" ] } diff --git a/pkg/config/linters_exclusions.go b/pkg/config/linters_exclusions.go index 20c1429ca6ff..57d08b483e4c 100644 --- a/pkg/config/linters_exclusions.go +++ b/pkg/config/linters_exclusions.go @@ -13,8 +13,8 @@ const ( const ( ExclusionPresetComments = "comments" - ExclusionPresetStdErrorHandling = "stdErrorHandling" - ExclusionPresetCommonFalsePositives = "commonFalsePositives" + ExclusionPresetStdErrorHandling = "std-error-handling" + ExclusionPresetCommonFalsePositives = "common-false-positives" ExclusionPresetLegacy = "legacy" ) diff --git a/test/run_test.go b/test/run_test.go index 2b3803ff8bf4..8fad2e6fdb32 100644 --- a/test/run_test.go +++ b/test/run_test.go @@ -353,7 +353,7 @@ func TestUnsafeOk(t *testing.T) { linters: exclusions: presets: - - commonFalsePositives + - common-false-positives ` testshared.NewRunnerBuilder(t). diff --git a/test/testdata/configs/default_exclude.yml b/test/testdata/configs/default_exclude.yml index db4e40af937f..087e4bcc30c0 100644 --- a/test/testdata/configs/default_exclude.yml +++ b/test/testdata/configs/default_exclude.yml @@ -3,6 +3,6 @@ version: "2" linters: exclusions: presets: - - stdErrorHandling - - commonFalsePositives + - std-error-handling + - common-false-positives - legacy