Skip to content

Commit

Permalink
feat: option to strictly follow Go autogenerated file convention (#4507)
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez authored Mar 15, 2024
1 parent d736d09 commit b05e397
Show file tree
Hide file tree
Showing 9 changed files with 204 additions and 107 deletions.
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
exclude-autogenerated-strict: true

# 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
},
"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
137 changes: 88 additions & 49 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
)

type ageFileSummary struct {
isGenerated bool
}
var _ Processor = &AutogeneratedExclude{}

type ageFileSummaryCache map[string]*ageFileSummary
type fileSummary struct {
generated bool
}

type AutogeneratedExclude struct {
fileSummaryCache ageFileSummaryCache
debugf logutils.DebugFunc

strict bool
strictPattern *regexp.Regexp

fileSummaryCache map[string]*fileSummary
}

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]*fileSummary{},
}
}

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) {
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.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
return !fs.isGenerated, nil
return !fs.generated, 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.
// 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.
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

0 comments on commit b05e397

Please sign in to comment.