Skip to content

feat: allow the analysis of generated files #4740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions .golangci.next.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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`: disable the generated files exclusion.
#
# 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
Expand Down
8 changes: 4 additions & 4 deletions jsonschema/golangci.next.jsonschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
3 changes: 3 additions & 0 deletions pkg/commands/flagsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 " +
Expand Down
12 changes: 8 additions & 4 deletions pkg/config/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions pkg/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
18 changes: 14 additions & 4 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{},
}
Expand All @@ -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)
}

Expand All @@ -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 {
Expand Down
34 changes: 17 additions & 17 deletions pkg/result/processors/autogenerated_exclude_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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)
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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)

Expand Down
Loading