From 0c01d5778c30e169388f106b0a97cb9daf25230f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Tue, 21 May 2024 20:55:55 +0200 Subject: [PATCH] feat: allow the analysis of generated files --- .golangci.next.reference.yml | 18 +++++----- jsonschema/golangci.next.jsonschema.json | 8 ++--- pkg/commands/flagsets.go | 3 ++ pkg/config/issues.go | 12 ++++--- pkg/config/loader.go | 6 ++++ pkg/lint/runner.go | 2 +- .../processors/autogenerated_exclude.go | 18 +++++++--- .../processors/autogenerated_exclude_test.go | 34 +++++++++---------- 8 files changed, 62 insertions(+), 39 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 9caca3d783ee..470bebebc1dc 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2871,17 +2871,17 @@ issues: - ".*\\.my\\.go$" - lib/bad.go - # To follow strictly the Go generated file convention. + # Mode of the generated files analysis. # - # If set to true, 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 + # - `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`: xxx # - # By default, a lax pattern is applied: - # sources are excluded if they contain lines `autogenerated file`, `code generated`, `do not edit`, etc. - # Default: false - exclude-generated-strict: true + # 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 diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 5bb029478c40..b37834d9b169 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3517,10 +3517,10 @@ "type": "boolean", "default": false }, - "exclude-generated-strict": { - "description": "To follow strict Go generated file convention", - "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.", diff --git a/pkg/commands/flagsets.go b/pkg/commands/flagsets.go index f70474cad4a6..608f6b9de581 100644 --- a/pkg/commands/flagsets.go +++ b/pkg/commands/flagsets.go @@ -103,6 +103,9 @@ func setupIssuesFlagSet(v *viper.Viper, fs *pflag.FlagSet) { internal.AddFlagAndBind(v, fs, fs.Bool, "exclude-dirs-use-default", "issues.exclude-dirs-use-default", true, getDefaultDirectoryExcludeHelp()) + internal.AddFlagAndBind(v, fs, fs.String, "exclude-generated", "issues.exclude-generated", processors.AutogeneratedModeLax, + 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 " + diff --git a/pkg/config/issues.go b/pkg/config/issues.go index 6d48694948d3..2ee9364aaa91 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -108,12 +108,14 @@ type Issues struct { ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` ExcludePatterns []string `mapstructure:"exclude"` ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` - ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-strict"` UseDefaultExcludes bool `mapstructure:"exclude-use-default"` - ExcludeFiles []string `mapstructure:"exclude-files"` - ExcludeDirs []string `mapstructure:"exclude-dirs"` - UseDefaultExcludeDirs bool `mapstructure:"exclude-dirs-use-default"` + 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"` @@ -124,6 +126,8 @@ type Issues struct { Diff bool `mapstructure:"new"` NeedFix bool `mapstructure:"fix"` + + ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-strict"` // Deprecated: use ExcludeGenerated instead. } func (i *Issues) Validate() error { diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 3dfe1cfd4d5f..70cbcafbfc71 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -357,6 +357,12 @@ func (l *Loader) handleDeprecation() error { } } + // Deprecated since v1.59.0 + if l.cfg.Issues.ExcludeGeneratedStrict { + l.log.Warnf("The configuration option `issues.exclude-generated-strict` is deprecated, please use `issues.exclude-generated`") + l.cfg.Issues.ExcludeGenerated = "strict" // Don't use the constants to avoid cyclic dependencies. + } + l.handleLinterOptionDeprecations() return nil diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index f4877e1e89c6..f583121ed869 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -75,7 +75,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti skipFilesProcessor, skipDirsProcessor, // must be after path prettifier - processors.NewAutogeneratedExclude(cfg.Issues.ExcludeGeneratedStrict), + processors.NewAutogeneratedExclude(cfg.Issues.ExcludeGenerated), // Must be before exclude because users see already marked output and configure excluding by it. processors.NewIdentifierMarker(), diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 2fca5117f2b6..5cc5e530ceda 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -12,6 +12,12 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +const ( + AutogeneratedModeLax = "lax" + AutogeneratedModeStrict = "strict" + AutogeneratedModeDisable = "disable" +) + const ( genCodeGenerated = "code generated" genDoNotEdit = "do not edit" @@ -27,16 +33,16 @@ type fileSummary struct { type AutogeneratedExclude struct { debugf logutils.DebugFunc - strict bool + mode string strictPattern *regexp.Regexp fileSummaryCache map[string]*fileSummary } -func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude { +func NewAutogeneratedExclude(mode string) *AutogeneratedExclude { return &AutogeneratedExclude{ debugf: logutils.Debug(logutils.DebugKeyAutogenExclude), - strict: strict, + mode: mode, strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`), fileSummaryCache: map[string]*fileSummary{}, } @@ -47,6 +53,10 @@ func (*AutogeneratedExclude) Name() string { } func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, error) { + if p.mode == AutogeneratedModeDisable { + return issues, nil + } + return filterIssuesErr(issues, p.shouldPassIssue) } @@ -71,7 +81,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error fs = &fileSummary{} p.fileSummaryCache[issue.FilePath()] = fs - if p.strict { + if p.mode == AutogeneratedModeStrict { var err error fs.generated, err = p.isGeneratedFileStrict(issue.FilePath()) if err != nil { diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index b2a51caf28b1..34c5c512f8db 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -14,7 +14,7 @@ import ( ) func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { - p := NewAutogeneratedExclude(false) + p := NewAutogeneratedExclude(AutogeneratedModeLax) comments := []string{ ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, @@ -56,7 +56,7 @@ func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { } func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { - p := NewAutogeneratedExclude(false) + p := NewAutogeneratedExclude(AutogeneratedModeLax) comments := []string{ "code not generated by", @@ -75,7 +75,7 @@ func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { } func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) { - p := NewAutogeneratedExclude(true) + p := NewAutogeneratedExclude(AutogeneratedModeStrict) testCases := []struct { desc string @@ -154,21 +154,21 @@ func Test_getComments_fileWithLongLine(t *testing.T) { func Test_shouldPassIssue(t *testing.T) { testCases := []struct { desc string - strict bool + mode string issue *result.Issue assert assert.BoolAssertionFunc }{ { - desc: "typecheck issue", - strict: false, + desc: "typecheck issue", + mode: AutogeneratedModeLax, issue: &result.Issue{ FromLinter: "typecheck", }, assert: assert.True, }, { - desc: "lax ", - strict: false, + desc: "lax ", + mode: AutogeneratedModeLax, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -178,8 +178,8 @@ func Test_shouldPassIssue(t *testing.T) { assert: assert.False, }, { - desc: "strict ", - strict: true, + desc: "strict ", + mode: AutogeneratedModeStrict, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -195,7 +195,7 @@ func Test_shouldPassIssue(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - p := NewAutogeneratedExclude(test.strict) + p := NewAutogeneratedExclude(test.mode) pass, err := p.shouldPassIssue(test.issue) require.NoError(t, err) @@ -213,13 +213,13 @@ func Test_shouldPassIssue_error(t *testing.T) { testCases := []struct { desc string - strict bool + mode string issue *result.Issue expected string }{ { - desc: "non-existing file (lax)", - strict: false, + desc: "non-existing file (lax)", + mode: AutogeneratedModeLax, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -230,8 +230,8 @@ func Test_shouldPassIssue_error(t *testing.T) { filepath.FromSlash("no-existing.go"), notFoundMsg), }, { - desc: "non-existing file (strict)", - strict: true, + desc: "non-existing file (strict)", + mode: AutogeneratedModeStrict, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -248,7 +248,7 @@ func Test_shouldPassIssue_error(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - p := NewAutogeneratedExclude(test.strict) + p := NewAutogeneratedExclude(test.mode) pass, err := p.shouldPassIssue(test.issue)