diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 30825bad3ff7..85916490ff04 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2814,11 +2814,17 @@ issues: # Default: false exclude-case-sensitive: false - # To follow strict Go autogenerated file convention. + # To follow strictly the Go generated file convention. + # + # If set to true, source files that have lines matching only the following regular expression will be excluded: + # `^// 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 - # By default a lax pattern is applied. + # + # 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-autogenerated-strict: true + exclude-generated-strict: true # 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 5b614cc03947..bc84e042e35d 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3342,8 +3342,8 @@ "type": "boolean", "default": false }, - "exclude-autogenerated-strict": { - "description": "To follow strict Go autogenerated file convention", + "exclude-generated-strict": { + "description": "To follow strict Go generated file convention", "type": "boolean", "default": false }, diff --git a/pkg/config/issues.go b/pkg/config/issues.go index dac949049775..2ac29eacba99 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -105,12 +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"` - ExcludeAutogeneratedStrict bool `mapstructure:"exclude-autogenerated-strict"` - 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"` + ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-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 35212a2c90aa..0de5298c9add 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(cfg.Issues.ExcludeAutogeneratedStrict), + processors.NewAutogeneratedExclude(cfg.Issues.ExcludeGeneratedStrict), // 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 9575ca4e74d8..9f4092425f29 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -63,6 +63,10 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return true, nil } + if issue.FilePath() == "" { + return false, errors.New("no file path for issue") + } + if !isGoFile(issue.FilePath()) { return false, nil } @@ -76,20 +80,16 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error 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) + return false, fmt.Errorf("failed to get doc (strict) 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) + return false, fmt.Errorf("failed to get doc (lax) of file %s: %w", issue.FilePath(), err) } fs.generated = p.isGeneratedFileLax(doc) @@ -102,7 +102,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error } // 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. +// The function uses a bit laxer rules than isGeneratedFileStrict 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} @@ -122,8 +122,12 @@ func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { return false } -// Based on https://go.dev/s/generatedcode -// > This line must appear before the first non-comment, non-blank text in the file. +// isGeneratedFileStrict returns true if the source file has a line that matches the regular expression: +// +// ^// Code generated .* DO NOT EDIT\.$ +// +// This line must appear before the first non-comment, non-blank text in the file. +// Based on https://go.dev/s/generatedcode. func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, error) { file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments) if err != nil { diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index fbf48847a9cb..c3291e4e3c2d 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -1,11 +1,16 @@ package processors import ( + "fmt" + "go/token" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/golangci/golangci-lint/pkg/result" ) func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { @@ -79,12 +84,12 @@ func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) { }{ { desc: "", - filepath: filepath.FromSlash("./testdata/autogen_go_strict.go"), + filepath: filepath.FromSlash("testdata/autogen_go_strict.go"), assert: assert.True, }, { desc: "", - filepath: filepath.FromSlash("./testdata/autogen_go_strict_invalid.go"), + filepath: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"), assert: assert.False, }, } @@ -108,7 +113,7 @@ func Test_getComments(t *testing.T) { doc string }{ { - fpath: filepath.Join("testdata", "autogen_exclude.go"), + fpath: filepath.FromSlash("testdata/autogen_exclude.go"), doc: `first line second line third line @@ -116,11 +121,11 @@ this text also and this text also`, }, { - fpath: filepath.Join("testdata", "autogen_exclude_doc.go"), + fpath: filepath.FromSlash("testdata/autogen_exclude_doc.go"), doc: `DO NOT EDIT`, }, { - fpath: filepath.Join("testdata", "autogen_exclude_block_comment.go"), + fpath: filepath.FromSlash("testdata/autogen_exclude_block_comment.go"), doc: `* first line * * second line @@ -141,7 +146,147 @@ 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 Test_getComments_fileWithLongLine(t *testing.T) { - fpath := filepath.Join("testdata", "autogen_exclude_long_line.go") + fpath := filepath.FromSlash("testdata/autogen_exclude_long_line.go") _, err := getComments(fpath) assert.NoError(t, err) } + +func Test_shouldPassIssue(t *testing.T) { + testCases := []struct { + desc string + strict bool + issue *result.Issue + assert assert.BoolAssertionFunc + }{ + { + desc: "typecheck issue", + strict: false, + issue: &result.Issue{ + FromLinter: "typecheck", + }, + assert: assert.True, + }, + { + desc: "go.mod", + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("/a/b/c/go.mod"), + }, + }, + assert: assert.True, + }, + { + desc: "non Go file", + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("/a/b/c/test.txt"), + }, + }, + assert: assert.False, + }, + { + desc: "lax ", + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"), + }, + }, + assert: assert.False, + }, + { + desc: "strict ", + strict: true, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"), + }, + }, + assert: assert.True, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + p := NewAutogeneratedExclude(test.strict) + + pass, err := p.shouldPassIssue(test.issue) + require.NoError(t, err) + + test.assert(t, pass) + }) + } +} + +func Test_shouldPassIssue_error(t *testing.T) { + notFoundMsg := "no such file or directory" + if runtime.GOOS == "windows" { + notFoundMsg = "The system cannot find the file specified." + } + + testCases := []struct { + desc string + strict bool + issue *result.Issue + expected string + }{ + { + desc: "missing Filename", + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: "", + }, + }, + expected: "no file path for issue", + }, + { + desc: "non-existing file (lax)", + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("no-existing.go"), + }, + }, + expected: fmt.Sprintf("failed to get doc (lax) of file %[1]s: failed to parse file: open %[1]s: %[2]s", + filepath.FromSlash("no-existing.go"), notFoundMsg), + }, + { + desc: "non-existing file (strict)", + strict: true, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("no-existing.go"), + }, + }, + expected: fmt.Sprintf("failed to get doc (strict) of file %[1]s: failed to parse file: open %[1]s: %[2]s", + filepath.FromSlash("no-existing.go"), notFoundMsg), + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + p := NewAutogeneratedExclude(test.strict) + + pass, err := p.shouldPassIssue(test.issue) + + assert.EqualError(t, err, test.expected) + assert.False(t, pass) + }) + } +}