diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 71dff87c68d0..30825bad3ff7 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2814,6 +2814,12 @@ issues: # Default: false exclude-case-sensitive: false + # To follow strict Go autogenerated file convention. + # https://go.dev/s/generatedcode + # By default a lax pattern is applied. + # Default: false + exclude-autogenerated-strict: true + # The list of ids of default excludes to include or disable. # https://golangci-lint.run/usage/false-positives/#default-exclusions # Default: [] diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index e2a30045d528..5b614cc03947 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3342,6 +3342,11 @@ "type": "boolean", "default": false }, + "exclude-autogenerated-strict": { + "description": "To follow strict Go autogenerated file convention", + "type": "boolean", + "default": false + }, "include": { "description": "The list of ids of default excludes to include or disable.", "type": "array", diff --git a/pkg/config/issues.go b/pkg/config/issues.go index 2e14f6cc5d59..dac949049775 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -105,11 +105,12 @@ var DefaultExcludePatterns = []ExcludePattern{ } 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"` + IncludeDefaultExcludes []string `mapstructure:"include"` + ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` + ExcludePatterns []string `mapstructure:"exclude"` + ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` + ExcludeAutogeneratedStrict bool `mapstructure:"exclude-autogenerated-strict"` + UseDefaultExcludes bool `mapstructure:"exclude-use-default"` MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"` MaxSameIssues int `mapstructure:"max-same-issues"` diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index aa26ea082911..35212a2c90aa 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -72,7 +72,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env, skipFilesProcessor, skipDirsProcessor, // must be after path prettifier - processors.NewAutogeneratedExclude(), + processors.NewAutogeneratedExclude(cfg.Issues.ExcludeAutogeneratedStrict), // 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 9f4de3b4a385..9575ca4e74d8 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -6,32 +6,43 @@ import ( "go/parser" "go/token" "path/filepath" + "regexp" "strings" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) -var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude) +const ( + genCodeGenerated = "code generated" + genDoNotEdit = "do not edit" + genAutoFile = "autogenerated file" // easyjson +) -type ageFileSummary struct { - isGenerated bool -} +var _ Processor = &AutogeneratedExclude{} -type ageFileSummaryCache map[string]*ageFileSummary +type fileSummary struct { + generated bool +} type AutogeneratedExclude struct { - fileSummaryCache ageFileSummaryCache + debugf logutils.DebugFunc + + strict bool + strictPattern *regexp.Regexp + + fileSummaryCache map[string]*fileSummary } -func NewAutogeneratedExclude() *AutogeneratedExclude { +func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude { return &AutogeneratedExclude{ - fileSummaryCache: ageFileSummaryCache{}, + debugf: logutils.Debug(logutils.DebugKeyAutogenExclude), + strict: strict, + strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`), + fileSummaryCache: map[string]*fileSummary{}, } } -var _ Processor = &AutogeneratedExclude{} - func (p *AutogeneratedExclude) Name() string { return "autogenerated_exclude" } @@ -40,11 +51,7 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e return filterIssuesErr(issues, p.shouldPassIssue) } -func isSpecialAutogeneratedFile(filePath string) bool { - fileName := filepath.Base(filePath) - // fake files or generation definitions to which //line points to for generated files - return filepath.Ext(fileName) != ".go" -} +func (p *AutogeneratedExclude) Finish() {} func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error) { if issue.FromLinter == "typecheck" { @@ -56,66 +63,96 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return true, nil } - if isSpecialAutogeneratedFile(issue.FilePath()) { + if !isGoFile(issue.FilePath()) { return false, nil } - fs, err := p.getOrCreateFileSummary(issue) - if err != nil { - return false, err + // The file is already known. + fs := p.fileSummaryCache[issue.FilePath()] + if fs != nil { + return !fs.generated, nil } + fs = &fileSummary{} + p.fileSummaryCache[issue.FilePath()] = fs + + if issue.FilePath() == "" { + return false, errors.New("no file path for issue") + } + + if p.strict { + var err error + fs.generated, err = p.isGeneratedFileStrict(issue.FilePath()) + if err != nil { + return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) + } + } else { + doc, err := getComments(issue.FilePath()) + if err != nil { + return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) + } + + fs.generated = p.isGeneratedFileLax(doc) + } + + p.debugf("file %q is generated: %t", issue.FilePath(), fs.generated) + // don't report issues for autogenerated files - return !fs.isGenerated, nil + return !fs.generated, nil } -// isGenerated reports whether the source file is generated code. -// Using a bit laxer rules than https://go.dev/s/generatedcode to -// match more generated code. See #48 and #72. -func isGeneratedFileByComment(doc string) bool { - const ( - genCodeGenerated = "code generated" - genDoNotEdit = "do not edit" - genAutoFile = "autogenerated file" // easyjson - ) - +// isGeneratedFileLax reports whether the source file is generated code. +// Using a bit laxer rules than https://go.dev/s/generatedcode to match more generated code. +// See https://github.com/golangci/golangci-lint/issues/48 and https://github.com/golangci/golangci-lint/issues/72. +func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile} + doc = strings.ToLower(doc) + for _, marker := range markers { if strings.Contains(doc, marker) { - autogenDebugf("doc contains marker %q: file is generated", marker) + p.debugf("doc contains marker %q: file is generated", marker) + return true } } - autogenDebugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) + p.debugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) + return false } -func (p *AutogeneratedExclude) getOrCreateFileSummary(issue *result.Issue) (*ageFileSummary, error) { - fs := p.fileSummaryCache[issue.FilePath()] - if fs != nil { - return fs, nil +// Based on https://go.dev/s/generatedcode +// > This line must appear before the first non-comment, non-blank text in the file. +func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, error) { + file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments) + if err != nil { + return false, fmt.Errorf("failed to parse file: %w", err) } - fs = &ageFileSummary{} - p.fileSummaryCache[issue.FilePath()] = fs - - if issue.FilePath() == "" { - return nil, errors.New("no file path for issue") + if file == nil || len(file.Comments) == 0 { + return false, nil } - doc, err := getDoc(issue.FilePath()) - if err != nil { - return nil, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) + for _, comment := range file.Comments { + if comment.Pos() > file.Package { + return false, nil + } + + for _, line := range comment.List { + generated := p.strictPattern.MatchString(line.Text) + if generated { + p.debugf("doc contains ignore expression: file is generated") + + return true, nil + } + } } - fs.isGenerated = isGeneratedFileByComment(doc) - autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) - return fs, nil + return false, nil } -func getDoc(filePath string) (string, error) { +func getComments(filePath string) (string, error) { fset := token.NewFileSet() syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) if err != nil { @@ -130,4 +167,6 @@ func getDoc(filePath string) (string, error) { return strings.Join(docLines, "\n"), nil } -func (p *AutogeneratedExclude) Finish() {} +func isGoFile(name string) bool { + return filepath.Ext(name) == ".go" +} diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index 531b80782b61..fbf48847a9cb 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -2,74 +2,107 @@ package processors import ( "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestIsAutogeneratedDetection(t *testing.T) { - all := ` - // generated by stringer -type Pill pill.go; DO NOT EDIT - -// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT - -// Code generated by vfsgen; DO NOT EDIT - -// Created by cgo -godefs - DO NOT EDIT - -/* Created by cgo - DO NOT EDIT. */ - -// Generated by stringer -i a.out.go -o anames.go -p ppc64 -// Do not edit. - -// DO NOT EDIT -// generated by: x86map -fmt=decoder ../x86.csv - -// DO NOT EDIT. -// Generate with: go run gen.go -full -output md5block.go - -// generated by "go run gen.go". DO NOT EDIT. - -// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution. - -// GENERATED BY make_perl_groups.pl; DO NOT EDIT. - -// generated by mknacl.sh - do not edit - -// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT // - -// Generated by running +func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { + p := NewAutogeneratedExclude(false) + + comments := []string{ + ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, + `// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT`, + `// Code generated by vfsgen; DO NOT EDIT`, + `// Created by cgo -godefs - DO NOT EDIT`, + `/* Created by cgo - DO NOT EDIT. */`, + `// Generated by stringer -i a.out.go -o anames.go -p ppc64 +// Do not edit.`, + `// DO NOT EDIT +// generated by: x86map -fmt=decoder ../x86.csv`, + `// DO NOT EDIT. +// Generate with: go run gen.go -full -output md5block.go`, + `// generated by "go run gen.go". DO NOT EDIT.`, + `// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.`, + `// GENERATED BY make_perl_groups.pl; DO NOT EDIT.`, + `// generated by mknacl.sh - do not edit`, + `// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //`, + `// Generated by running // maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt // --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt -// DO NOT EDIT - -/* +// DO NOT EDIT`, + `/* * CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap * THIS FILE SHOULD NOT BE EDITED BY HAND -*/ + */`, + `// AUTOGENERATED FILE: easyjson file.go`, + } -// AUTOGENERATED FILE: easyjson file.go -` + for _, comment := range comments { + comment := comment + t.Run(comment, func(t *testing.T) { + t.Parallel() - generatedCases := strings.Split(all, "\n\n") - for _, gc := range generatedCases { - isGenerated := isGeneratedFileByComment(gc) - assert.True(t, isGenerated) + generated := p.isGeneratedFileLax(comment) + assert.True(t, generated) + }) } +} + +func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { + p := NewAutogeneratedExclude(false) - notGeneratedCases := []string{ + comments := []string{ "code not generated by", "test", } - for _, ngc := range notGeneratedCases { - isGenerated := isGeneratedFileByComment(ngc) - assert.False(t, isGenerated) + + for _, comment := range comments { + comment := comment + t.Run(comment, func(t *testing.T) { + t.Parallel() + + generated := p.isGeneratedFileLax(comment) + assert.False(t, generated) + }) + } +} + +func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) { + p := NewAutogeneratedExclude(true) + + testCases := []struct { + desc string + filepath string + assert assert.BoolAssertionFunc + }{ + { + desc: "", + filepath: filepath.FromSlash("./testdata/autogen_go_strict.go"), + assert: assert.True, + }, + { + desc: "", + filepath: filepath.FromSlash("./testdata/autogen_go_strict_invalid.go"), + assert: assert.False, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + generated, err := p.isGeneratedFileStrict(test.filepath) + require.NoError(t, err) + + test.assert(t, generated) + }) } } -func TestGetDoc(t *testing.T) { +func Test_getComments(t *testing.T) { testCases := []struct { fpath string doc string @@ -99,7 +132,7 @@ this one line comment also`, } for _, tc := range testCases { - doc, err := getDoc(tc.fpath) + doc, err := getComments(tc.fpath) require.NoError(t, err) assert.Equal(t, tc.doc, doc) } @@ -107,8 +140,8 @@ this one line comment also`, // Issue 954: Some lines can be very long, e.g. auto-generated // embedded resources. Reported on file of 86.2KB. -func TestGetDocFileWithLongLine(t *testing.T) { +func Test_getComments_fileWithLongLine(t *testing.T) { fpath := filepath.Join("testdata", "autogen_exclude_long_line.go") - _, err := getDoc(fpath) + _, err := getComments(fpath) assert.NoError(t, err) } diff --git a/pkg/result/processors/skip_dirs.go b/pkg/result/processors/skip_dirs.go index c2468b7613a9..7c4e0b9c0cbe 100644 --- a/pkg/result/processors/skip_dirs.go +++ b/pkg/result/processors/skip_dirs.go @@ -81,7 +81,7 @@ func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) { func (p *SkipDirs) shouldPassIssue(issue *result.Issue) bool { if filepath.IsAbs(issue.FilePath()) { - if !isSpecialAutogeneratedFile(issue.FilePath()) { + if isGoFile(issue.FilePath()) { p.log.Warnf("Got abs path %s in skip dirs processor, it should be relative", issue.FilePath()) } return true diff --git a/pkg/result/processors/testdata/autogen_go_strict.go b/pkg/result/processors/testdata/autogen_go_strict.go new file mode 100644 index 000000000000..0f32d4af731f --- /dev/null +++ b/pkg/result/processors/testdata/autogen_go_strict.go @@ -0,0 +1,7 @@ +// foo foo foo. +// foo foo foo. +// foo foo foo. + +// Code generated by example — DO NOT EDIT. + +package testdata diff --git a/pkg/result/processors/testdata/autogen_go_strict_invalid.go b/pkg/result/processors/testdata/autogen_go_strict_invalid.go new file mode 100644 index 000000000000..c804e3e5f934 --- /dev/null +++ b/pkg/result/processors/testdata/autogen_go_strict_invalid.go @@ -0,0 +1,6 @@ +package testdata + +// Code generated by example; DO NOT EDIT. +func _() { + +}