feat: option to strictly follow Go autogenerated file convention (#4507)

This commit is contained in:
Ludovic Fernandez 2024-03-15 01:17:23 +01:00 committed by GitHub
parent d736d092f6
commit b05e397ac9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 218 additions and 121 deletions

View File

@ -2814,6 +2814,12 @@ issues:
# Default: false # Default: false
exclude-case-sensitive: 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. # 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
# Default: [] # Default: []

View File

@ -3342,6 +3342,11 @@
"type": "boolean", "type": "boolean",
"default": false "default": false
}, },
"exclude-autogenerated-strict": {
"description": "To follow strict Go autogenerated file convention",
"type": "boolean",
"default": false
},
"include": { "include": {
"description": "The list of ids of default excludes to include or disable.", "description": "The list of ids of default excludes to include or disable.",
"type": "array", "type": "array",

View File

@ -109,6 +109,7 @@ type Issues struct {
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"`
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"`

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(), processors.NewAutogeneratedExclude(cfg.Issues.ExcludeAutogeneratedStrict),
// 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

@ -6,32 +6,43 @@ import (
"go/parser" "go/parser"
"go/token" "go/token"
"path/filepath" "path/filepath"
"regexp"
"strings" "strings"
"github.com/golangci/golangci-lint/pkg/logutils" "github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-lint/pkg/result"
) )
var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude) const (
genCodeGenerated = "code generated"
type ageFileSummary struct { genDoNotEdit = "do not edit"
isGenerated bool genAutoFile = "autogenerated file" // easyjson
} )
type ageFileSummaryCache map[string]*ageFileSummary
type AutogeneratedExclude struct {
fileSummaryCache ageFileSummaryCache
}
func NewAutogeneratedExclude() *AutogeneratedExclude {
return &AutogeneratedExclude{
fileSummaryCache: ageFileSummaryCache{},
}
}
var _ Processor = &AutogeneratedExclude{} var _ Processor = &AutogeneratedExclude{}
type fileSummary struct {
generated bool
}
type AutogeneratedExclude struct {
debugf logutils.DebugFunc
strict bool
strictPattern *regexp.Regexp
fileSummaryCache map[string]*fileSummary
}
func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude {
return &AutogeneratedExclude{
debugf: logutils.Debug(logutils.DebugKeyAutogenExclude),
strict: strict,
strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`),
fileSummaryCache: map[string]*fileSummary{},
}
}
func (p *AutogeneratedExclude) Name() string { func (p *AutogeneratedExclude) Name() string {
return "autogenerated_exclude" return "autogenerated_exclude"
} }
@ -40,11 +51,7 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e
return filterIssuesErr(issues, p.shouldPassIssue) return filterIssuesErr(issues, p.shouldPassIssue)
} }
func isSpecialAutogeneratedFile(filePath string) bool { func (p *AutogeneratedExclude) Finish() {}
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) shouldPassIssue(issue *result.Issue) (bool, error) { func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error) {
if issue.FromLinter == "typecheck" { if issue.FromLinter == "typecheck" {
@ -56,66 +63,96 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
return true, nil return true, nil
} }
if isSpecialAutogeneratedFile(issue.FilePath()) { if !isGoFile(issue.FilePath()) {
return false, nil return false, nil
} }
fs, err := p.getOrCreateFileSummary(issue) // The file is already known.
if err != nil { fs := p.fileSummaryCache[issue.FilePath()]
return false, err 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 // don't report issues for autogenerated files
return !fs.isGenerated, nil return !fs.generated, nil
} }
// isGenerated 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 // Using a bit laxer rules than https://go.dev/s/generatedcode to match more generated code.
// match more generated code. See #48 and #72. // See https://github.com/golangci/golangci-lint/issues/48 and https://github.com/golangci/golangci-lint/issues/72.
func isGeneratedFileByComment(doc string) bool { func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool {
const (
genCodeGenerated = "code generated"
genDoNotEdit = "do not edit"
genAutoFile = "autogenerated file" // easyjson
)
markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile} markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile}
doc = strings.ToLower(doc) doc = strings.ToLower(doc)
for _, marker := range markers { for _, marker := range markers {
if strings.Contains(doc, marker) { 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 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 return false
} }
func (p *AutogeneratedExclude) getOrCreateFileSummary(issue *result.Issue) (*ageFileSummary, error) { // Based on https://go.dev/s/generatedcode
fs := p.fileSummaryCache[issue.FilePath()] // > This line must appear before the first non-comment, non-blank text in the file.
if fs != nil { func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, error) {
return fs, nil file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
}
fs = &ageFileSummary{}
p.fileSummaryCache[issue.FilePath()] = fs
if issue.FilePath() == "" {
return nil, errors.New("no file path for issue")
}
doc, err := getDoc(issue.FilePath())
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) return false, fmt.Errorf("failed to parse file: %w", err)
} }
fs.isGenerated = isGeneratedFileByComment(doc) if file == nil || len(file.Comments) == 0 {
autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) return false, nil
return fs, nil }
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
}
}
}
return false, nil
} }
func getDoc(filePath string) (string, error) { func getComments(filePath string) (string, error) {
fset := token.NewFileSet() fset := token.NewFileSet()
syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
if err != nil { if err != nil {
@ -130,4 +167,6 @@ func getDoc(filePath string) (string, error) {
return strings.Join(docLines, "\n"), nil return strings.Join(docLines, "\n"), nil
} }
func (p *AutogeneratedExclude) Finish() {} func isGoFile(name string) bool {
return filepath.Ext(name) == ".go"
}

View File

@ -2,74 +2,107 @@ package processors
import ( import (
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestIsAutogeneratedDetection(t *testing.T) { func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) {
all := ` p := NewAutogeneratedExclude(false)
// generated by stringer -type Pill pill.go; DO NOT EDIT
// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT comments := []string{
` // generated by stringer -type Pill pill.go; DO NOT EDIT`,
// Code generated by vfsgen; 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 -godefs - DO NOT EDIT`,
`/* Created by cgo - DO NOT EDIT. */`,
/* Created by cgo - DO NOT EDIT. */ `// Generated by stringer -i a.out.go -o anames.go -p ppc64
// 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.
// DO NOT EDIT // Generate with: go run gen.go -full -output md5block.go`,
// generated by: x86map -fmt=decoder ../x86.csv `// generated by "go run gen.go". DO NOT EDIT.`,
`// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.`,
// DO NOT EDIT. `// GENERATED BY make_perl_groups.pl; DO NOT EDIT.`,
// Generate with: go run gen.go -full -output md5block.go `// generated by mknacl.sh - do not edit`,
`// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //`,
// generated by "go run gen.go". DO NOT EDIT. `// Generated by running
// 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 // 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 // --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 * CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap
* THIS FILE SHOULD NOT BE EDITED BY HAND * THIS FILE SHOULD NOT BE EDITED BY HAND
*/ */`,
`// AUTOGENERATED FILE: easyjson file.go`,
// AUTOGENERATED FILE: easyjson file.go
`
generatedCases := strings.Split(all, "\n\n")
for _, gc := range generatedCases {
isGenerated := isGeneratedFileByComment(gc)
assert.True(t, isGenerated)
} }
notGeneratedCases := []string{ for _, comment := range comments {
"code not generated by", comment := comment
"test", t.Run(comment, func(t *testing.T) {
} t.Parallel()
for _, ngc := range notGeneratedCases {
isGenerated := isGeneratedFileByComment(ngc) generated := p.isGeneratedFileLax(comment)
assert.False(t, isGenerated) assert.True(t, generated)
})
} }
} }
func TestGetDoc(t *testing.T) { func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) {
p := NewAutogeneratedExclude(false)
comments := []string{
"code not generated by",
"test",
}
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 Test_getComments(t *testing.T) {
testCases := []struct { testCases := []struct {
fpath string fpath string
doc string doc string
@ -99,7 +132,7 @@ this one line comment also`,
} }
for _, tc := range testCases { for _, tc := range testCases {
doc, err := getDoc(tc.fpath) doc, err := getComments(tc.fpath)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, tc.doc, doc) 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 // 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 TestGetDocFileWithLongLine(t *testing.T) { func Test_getComments_fileWithLongLine(t *testing.T) {
fpath := filepath.Join("testdata", "autogen_exclude_long_line.go") fpath := filepath.Join("testdata", "autogen_exclude_long_line.go")
_, err := getDoc(fpath) _, err := getComments(fpath)
assert.NoError(t, err) assert.NoError(t, err)
} }

View File

@ -81,7 +81,7 @@ func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) {
func (p *SkipDirs) shouldPassIssue(issue *result.Issue) bool { func (p *SkipDirs) shouldPassIssue(issue *result.Issue) bool {
if filepath.IsAbs(issue.FilePath()) { 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()) p.log.Warnf("Got abs path %s in skip dirs processor, it should be relative", issue.FilePath())
} }
return true return true

View File

@ -0,0 +1,7 @@
// foo foo foo.
// foo foo foo.
// foo foo foo.
// Code generated by example — DO NOT EDIT.
package testdata

View File

@ -0,0 +1,6 @@
package testdata
// Code generated by example; DO NOT EDIT.
func _() {
}