Skip to content
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

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

Merged
merged 15 commits into from
Mar 15, 2024
Merged
6 changes: 6 additions & 0 deletions .golangci.next.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2814,6 +2814,12 @@ issues:
# Default: 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
ldez marked this conversation as resolved.
Show resolved Hide resolved
exclude-autogenerated-strict: true
ldez marked this conversation as resolved.
Show resolved Hide resolved
ldez marked this conversation as resolved.
Show resolved Hide resolved

# The list of ids of default excludes to include or disable.
# https://golangci-lint.run/usage/false-positives/#default-exclusions
# Default: []
Expand Down
5 changes: 5 additions & 0 deletions jsonschema/golangci.next.jsonschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3342,6 +3342,11 @@
"type": "boolean",
"default": false
},
"exclude-autogenerated-strict": {
"description": "To follow strict Go autogenerated file convention",
"type": "boolean",
"default": false
},
ldez marked this conversation as resolved.
Show resolved Hide resolved
"include": {
"description": "The list of ids of default excludes to include or disable.",
"type": "array",
Expand Down
11 changes: 6 additions & 5 deletions pkg/config/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +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"`
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"`
ExcludeAutogeneratedStrict bool `mapstructure:"exclude-autogenerated-strict"`
UseDefaultExcludes bool `mapstructure:"exclude-use-default"`

MaxIssuesPerLinter int `mapstructure:"max-issues-per-linter"`
MaxSameIssues int `mapstructure:"max-same-issues"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env,
skipFilesProcessor,
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.
processors.NewIdentifierMarker(),
Expand Down
131 changes: 85 additions & 46 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,43 @@ import (
"go/parser"
"go/token"
"path/filepath"
"regexp"
"strings"

"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
)

var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude)
const (
genCodeGenerated = "code generated"
genDoNotEdit = "do not edit"
genAutoFile = "autogenerated file" // easyjson
)

var _ Processor = &AutogeneratedExclude{}

type ageFileSummary struct {
ldez marked this conversation as resolved.
Show resolved Hide resolved
isGenerated bool
}

type ageFileSummaryCache map[string]*ageFileSummary

type AutogeneratedExclude struct {
fileSummaryCache ageFileSummaryCache
debugf logutils.DebugFunc

strict bool
strictPattern *regexp.Regexp

fileSummaryCache map[string]*ageFileSummary
}

func NewAutogeneratedExclude() *AutogeneratedExclude {
func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude {
return &AutogeneratedExclude{
fileSummaryCache: ageFileSummaryCache{},
debugf: logutils.Debug(logutils.DebugKeyAutogenExclude),
strict: strict,
strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`),
fileSummaryCache: map[string]*ageFileSummary{},
}
}

var _ Processor = &AutogeneratedExclude{}

func (p *AutogeneratedExclude) Name() string {
return "autogenerated_exclude"
}
Expand All @@ -40,11 +51,7 @@ func (p *AutogeneratedExclude) Process(issues []result.Issue) ([]result.Issue, e
return filterIssuesErr(issues, p.shouldPassIssue)
}

func isSpecialAutogeneratedFile(filePath string) bool {
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) Finish() {}

func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error) {
ldez marked this conversation as resolved.
Show resolved Hide resolved
if issue.FromLinter == "typecheck" {
Expand All @@ -56,66 +63,96 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
return true, nil
}

if isSpecialAutogeneratedFile(issue.FilePath()) {
if !isGoFile(issue.FilePath()) {
return false, nil
}

fs, err := p.getOrCreateFileSummary(issue)
if err != nil {
return false, err
// The file is already known.
fs := p.fileSummaryCache[issue.FilePath()]
if fs != nil {
return !fs.isGenerated, nil
}

fs = &ageFileSummary{}
p.fileSummaryCache[issue.FilePath()] = fs

if issue.FilePath() == "" {
return false, errors.New("no file path for issue")
}

if p.strict {
var err error
fs.isGenerated, 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)
ldez marked this conversation as resolved.
Show resolved Hide resolved
}

fs.isGenerated = p.isGeneratedFileLax(doc)
}

p.debugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated)

// don't report issues for autogenerated files
return !fs.isGenerated, nil
}

// isGenerated reports whether the source file is generated code.
// Using a bit laxer rules than https://go.dev/s/generatedcode to
// match more generated code. See #48 and #72.
func isGeneratedFileByComment(doc string) bool {
const (
genCodeGenerated = "code generated"
genDoNotEdit = "do not edit"
genAutoFile = "autogenerated file" // easyjson
)

// 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.
ldez marked this conversation as resolved.
Show resolved Hide resolved
// 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}

doc = strings.ToLower(doc)

for _, marker := range markers {
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
}
}

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
}

func (p *AutogeneratedExclude) getOrCreateFileSummary(issue *result.Issue) (*ageFileSummary, error) {
fs := p.fileSummaryCache[issue.FilePath()]
if fs != nil {
return fs, nil
// Based on https://go.dev/s/generatedcode
// > This line must appear before the first non-comment, non-blank text in the file.
ldez marked this conversation as resolved.
Show resolved Hide resolved
func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, error) {
file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
if err != nil {
return false, fmt.Errorf("failed to parse file: %w", err)
}

fs = &ageFileSummary{}
p.fileSummaryCache[issue.FilePath()] = fs

if issue.FilePath() == "" {
return nil, errors.New("no file path for issue")
if file == nil || len(file.Comments) == 0 {
return false, nil
}

doc, err := getDoc(issue.FilePath())
if err != nil {
return nil, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err)
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
}
}
}

fs.isGenerated = isGeneratedFileByComment(doc)
autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated)
return fs, nil
return false, nil
}

func getDoc(filePath string) (string, error) {
func getComments(filePath string) (string, error) {
fset := token.NewFileSet()
syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
if err != nil {
Expand All @@ -130,4 +167,6 @@ func getDoc(filePath string) (string, error) {
return strings.Join(docLines, "\n"), nil
}

func (p *AutogeneratedExclude) Finish() {}
func isGoFile(name string) bool {
return filepath.Ext(name) == ".go"
}
Loading
Loading