feat: rename exclude-autogenerated-strict to exclude-generated-strict (#4514)

This commit is contained in:
Ludovic Fernandez 2024-03-15 17:42:34 +01:00 committed by GitHub
parent d239c9b020
commit 39617e4db3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 182 additions and 27 deletions

View File

@ -2814,11 +2814,17 @@ issues:
# Default: false # Default: false
exclude-case-sensitive: 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 # 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 # Default: false
exclude-autogenerated-strict: true exclude-generated-strict: true
# The list of ids of default excludes to include or disable. # The list of ids of default excludes to include or disable.
# https://golangci-lint.run/usage/false-positives/#default-exclusions # https://golangci-lint.run/usage/false-positives/#default-exclusions

View File

@ -3342,8 +3342,8 @@
"type": "boolean", "type": "boolean",
"default": false "default": false
}, },
"exclude-autogenerated-strict": { "exclude-generated-strict": {
"description": "To follow strict Go autogenerated file convention", "description": "To follow strict Go generated file convention",
"type": "boolean", "type": "boolean",
"default": false "default": false
}, },

View File

@ -105,12 +105,12 @@ var DefaultExcludePatterns = []ExcludePattern{
} }
type Issues struct { type Issues struct {
IncludeDefaultExcludes []string `mapstructure:"include"` IncludeDefaultExcludes []string `mapstructure:"include"`
ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"` ExcludeCaseSensitive bool `mapstructure:"exclude-case-sensitive"`
ExcludePatterns []string `mapstructure:"exclude"` ExcludePatterns []string `mapstructure:"exclude"`
ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"` ExcludeRules []ExcludeRule `mapstructure:"exclude-rules"`
ExcludeAutogeneratedStrict bool `mapstructure:"exclude-autogenerated-strict"` ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-strict"`
UseDefaultExcludes bool `mapstructure:"exclude-use-default"` UseDefaultExcludes bool `mapstructure:"exclude-use-default"`
MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"` MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"`
MaxSameIssues int `mapstructure:"max-same-issues"` MaxSameIssues int `mapstructure:"max-same-issues"`

View File

@ -72,7 +72,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env,
skipFilesProcessor, skipFilesProcessor,
skipDirsProcessor, // must be after path prettifier 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. // Must be before exclude because users see already marked output and configure excluding by it.
processors.NewIdentifierMarker(), processors.NewIdentifierMarker(),

View File

@ -63,6 +63,10 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
return true, nil return true, nil
} }
if issue.FilePath() == "" {
return false, errors.New("no file path for issue")
}
if !isGoFile(issue.FilePath()) { if !isGoFile(issue.FilePath()) {
return false, nil return false, nil
} }
@ -76,20 +80,16 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
fs = &fileSummary{} fs = &fileSummary{}
p.fileSummaryCache[issue.FilePath()] = fs p.fileSummaryCache[issue.FilePath()] = fs
if issue.FilePath() == "" {
return false, errors.New("no file path for issue")
}
if p.strict { if p.strict {
var err error var err error
fs.generated, err = p.isGeneratedFileStrict(issue.FilePath()) fs.generated, err = p.isGeneratedFileStrict(issue.FilePath())
if err != nil { 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 { } else {
doc, err := getComments(issue.FilePath()) doc, err := getComments(issue.FilePath())
if err != nil { 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) 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. // 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. // 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 { func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool {
markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile} markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile}
@ -122,8 +122,12 @@ func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool {
return false return false
} }
// Based on https://go.dev/s/generatedcode // isGeneratedFileStrict returns true if the source file has a line that matches the regular expression:
// > This line must appear before the first non-comment, non-blank text in the file. //
// ^// 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) { func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, error) {
file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments) file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
if err != nil { if err != nil {

View File

@ -1,11 +1,16 @@
package processors package processors
import ( import (
"fmt"
"go/token"
"path/filepath" "path/filepath"
"runtime"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/golangci/golangci-lint/pkg/result"
) )
func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) {
@ -79,12 +84,12 @@ func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) {
}{ }{
{ {
desc: "", desc: "",
filepath: filepath.FromSlash("./testdata/autogen_go_strict.go"), filepath: filepath.FromSlash("testdata/autogen_go_strict.go"),
assert: assert.True, assert: assert.True,
}, },
{ {
desc: "", desc: "",
filepath: filepath.FromSlash("./testdata/autogen_go_strict_invalid.go"), filepath: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"),
assert: assert.False, assert: assert.False,
}, },
} }
@ -108,7 +113,7 @@ func Test_getComments(t *testing.T) {
doc string doc string
}{ }{
{ {
fpath: filepath.Join("testdata", "autogen_exclude.go"), fpath: filepath.FromSlash("testdata/autogen_exclude.go"),
doc: `first line doc: `first line
second line second line
third line third line
@ -116,11 +121,11 @@ this text also
and 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`, 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 doc: `* first line
* *
* second line * second line
@ -141,7 +146,147 @@ this one line comment also`,
// Issue 954: Some lines can be very long, e.g. auto-generated // Issue 954: Some lines can be very long, e.g. auto-generated
// embedded resources. Reported on file of 86.2KB. // embedded resources. Reported on file of 86.2KB.
func Test_getComments_fileWithLongLine(t *testing.T) { 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) _, err := getComments(fpath)
assert.NoError(t, err) 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)
})
}
}