diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 6938157dacbb..5399bbdb1f7e 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2845,6 +2845,11 @@ issues: - ".*\\.my\\.go$" - lib/bad.go + # Which files do not excluded as a generated they will be analyzed and issues will be reported. + # Default: [] + exclude-generated-skip-files: + - ".*pb.go$" + # To follow strictly the Go generated file convention. # # If set to true, source files that have lines matching only the following regular expression will be excluded: diff --git a/pkg/config/issues.go b/pkg/config/issues.go index 6d48694948d3..2f834c518f91 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -104,12 +104,13 @@ 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"` - ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-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"` + ExcludeGeneratedSkipFiles []string `mapstructure:"exclude-generated-skip-files"` + ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-strict"` + UseDefaultExcludes bool `mapstructure:"exclude-use-default"` ExcludeFiles []string `mapstructure:"exclude-files"` ExcludeDirs []string `mapstructure:"exclude-dirs"` diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index f4877e1e89c6..213eb5a8b286 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -60,6 +60,12 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti return nil, fmt.Errorf("failed to get enabled linters: %w", err) } + autogeneratedExclude, err := processors.NewAutogeneratedExclude( + cfg.Issues.ExcludeGeneratedStrict, cfg.Issues.ExcludeGeneratedSkipFiles) + if err != nil { + return nil, fmt.Errorf("failed to construct autogenerated exclude: %w", err) + } + return &Runner{ Processors: []processors.Processor{ processors.NewCgo(goenv), @@ -75,7 +81,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), + autogeneratedExclude, // 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..76198819607e 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" + "github.com/golangci/golangci-lint/pkg/fsutils" "github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/result" ) @@ -27,19 +28,34 @@ type fileSummary struct { type AutogeneratedExclude struct { debugf logutils.DebugFunc + skipFilesRe []*regexp.Regexp + strict bool strictPattern *regexp.Regexp fileSummaryCache map[string]*fileSummary } -func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude { +func NewAutogeneratedExclude(strict bool, skipFilesPattern []string) (*AutogeneratedExclude, error) { + var patternsRe []*regexp.Regexp + for _, p := range skipFilesPattern { + 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 &AutogeneratedExclude{ debugf: logutils.Debug(logutils.DebugKeyAutogenExclude), + skipFilesRe: patternsRe, strict: strict, strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`), fileSummaryCache: map[string]*fileSummary{}, - } + }, nil } func (*AutogeneratedExclude) Name() string { @@ -53,6 +69,12 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e func (*AutogeneratedExclude) Finish() {} func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error) { + for _, pattern := range p.skipFilesRe { + if pattern.MatchString(issue.FilePath()) { + return true, nil + } + } + if issue.FromLinter == typeCheckName { // don't hide typechecking errors in generated files: users expect to see why the project isn't compiling return true, nil diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index b2a51caf28b1..ea664456100d 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -14,7 +14,8 @@ import ( ) func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { - p := NewAutogeneratedExclude(false) + p, err := NewAutogeneratedExclude(false, nil) + require.NoError(t, err) comments := []string{ ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, @@ -56,7 +57,8 @@ func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { } func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { - p := NewAutogeneratedExclude(false) + p, err := NewAutogeneratedExclude(false, nil) + require.NoError(t, err) comments := []string{ "code not generated by", @@ -75,7 +77,8 @@ func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { } func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) { - p := NewAutogeneratedExclude(true) + p, err := NewAutogeneratedExclude(true, nil) + require.NoError(t, err) testCases := []struct { desc string @@ -153,22 +156,25 @@ func Test_getComments_fileWithLongLine(t *testing.T) { func Test_shouldPassIssue(t *testing.T) { testCases := []struct { - desc string - strict bool - issue *result.Issue - assert assert.BoolAssertionFunc + desc string + include []string + strict bool + issue *result.Issue + assert assert.BoolAssertionFunc }{ { - desc: "typecheck issue", - strict: false, + desc: "typecheck issue", + include: nil, + strict: false, issue: &result.Issue{ FromLinter: "typecheck", }, assert: assert.True, }, { - desc: "lax ", - strict: false, + desc: "lax ", + include: nil, + strict: false, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -178,8 +184,9 @@ func Test_shouldPassIssue(t *testing.T) { assert: assert.False, }, { - desc: "strict ", - strict: true, + desc: "strict ", + include: nil, + strict: true, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -188,6 +195,30 @@ func Test_shouldPassIssue(t *testing.T) { }, assert: assert.True, }, + { + desc: "include ", + include: []string{"(.*)?invalid.go"}, + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"), + }, + }, + assert: assert.True, + }, + { + desc: "include other pattern", + include: []string{"(.*)?.pb.go"}, + strict: false, + issue: &result.Issue{ + FromLinter: "example", + Pos: token.Position{ + Filename: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"), + }, + }, + assert: assert.False, + }, } for _, test := range testCases { @@ -195,7 +226,8 @@ func Test_shouldPassIssue(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - p := NewAutogeneratedExclude(test.strict) + p, err := NewAutogeneratedExclude(test.strict, test.include) + require.NoError(t, err) pass, err := p.shouldPassIssue(test.issue) require.NoError(t, err) @@ -213,13 +245,15 @@ func Test_shouldPassIssue_error(t *testing.T) { testCases := []struct { desc string + include []string strict bool issue *result.Issue expected string }{ { - desc: "non-existing file (lax)", - strict: false, + desc: "non-existing file (lax)", + include: nil, + strict: false, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -230,8 +264,9 @@ 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)", + include: nil, + strict: true, issue: &result.Issue{ FromLinter: "example", Pos: token.Position{ @@ -248,7 +283,8 @@ func Test_shouldPassIssue_error(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - p := NewAutogeneratedExclude(test.strict) + p, err := NewAutogeneratedExclude(test.strict, test.include) + require.NoError(t, err) pass, err := p.shouldPassIssue(test.issue) @@ -258,3 +294,46 @@ func Test_shouldPassIssue_error(t *testing.T) { }) } } + +func Test_NewAutogeneratedExclude(t *testing.T) { + testCases := []struct { + desc string + include []string + strict bool + assert assert.ValueAssertionFunc + expectedError string + }{ + { + desc: "success ", + include: []string{".pb.go"}, + strict: false, + + assert: assert.NotNil, + expectedError: "", + }, + { + desc: "regexp compilation error ", + include: []string{"*bad_regexp*"}, + strict: false, + + assert: assert.Nil, + expectedError: "can't compile regexp \"*bad_regexp*\": error parsing regexp: missing argument to repetition operator: `*`", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + p, err := NewAutogeneratedExclude(test.strict, test.include) + test.assert(t, p) + + if test.expectedError == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, test.expectedError) + } + }) + } +}