From 0cdcd98fa1a36a76e308caa60db7a88cc492f576 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:06:50 +0100 Subject: [PATCH 01/15] chore: organise processor --- .../processors/autogenerated_exclude.go | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 9f4de3b4a385..e785cb7ecd25 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -12,6 +12,8 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +var _ Processor = &AutogeneratedExclude{} + var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude) type ageFileSummary struct { @@ -30,8 +32,6 @@ func NewAutogeneratedExclude() *AutogeneratedExclude { } } -var _ Processor = &AutogeneratedExclude{} - func (p *AutogeneratedExclude) Name() string { return "autogenerated_exclude" } @@ -40,11 +40,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" { @@ -69,29 +65,6 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error 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 - ) - - 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) - return true - } - } - - autogenDebugf("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 { @@ -130,4 +103,31 @@ func getDoc(filePath string) (string, error) { return strings.Join(docLines, "\n"), nil } -func (p *AutogeneratedExclude) Finish() {} +// 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 + ) + + 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) + return true + } + } + + autogenDebugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) + return false +} + +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" +} From f3d0accd0d825d93e58b52c974698e4dd0538fcd Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:18:27 +0100 Subject: [PATCH 02/15] chore: replace isSpecialAutogeneratedFile by isGoFile --- pkg/result/processors/autogenerated_exclude.go | 8 +++----- pkg/result/processors/skip_dirs.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index e785cb7ecd25..c2f478d9d9c4 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -52,7 +52,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return true, nil } - if isSpecialAutogeneratedFile(issue.FilePath()) { + if !isGoFile(issue.FilePath()) { return false, nil } @@ -126,8 +126,6 @@ func isGeneratedFileByComment(doc string) bool { return false } -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 isGoFile(name string) bool { + return filepath.Ext(name) == ".go" } diff --git a/pkg/result/processors/skip_dirs.go b/pkg/result/processors/skip_dirs.go index c2468b7613a9..7c4e0b9c0cbe 100644 --- a/pkg/result/processors/skip_dirs.go +++ b/pkg/result/processors/skip_dirs.go @@ -81,7 +81,7 @@ func (p *SkipDirs) Process(issues []result.Issue) ([]result.Issue, error) { func (p *SkipDirs) shouldPassIssue(issue *result.Issue) bool { 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()) } return true From 21629112dcdead04813b59e096eeddd3671cd662 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:25:35 +0100 Subject: [PATCH 03/15] chore: develop getOrCreateFileSummary --- .../processors/autogenerated_exclude.go | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index c2f478d9d9c4..f94cda770601 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -56,36 +56,29 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, nil } - fs, err := p.getOrCreateFileSummary(issue) - if err != nil { - return false, err - } - - // don't report issues for autogenerated files - return !fs.isGenerated, nil -} - -func (p *AutogeneratedExclude) getOrCreateFileSummary(issue *result.Issue) (*ageFileSummary, error) { fs := p.fileSummaryCache[issue.FilePath()] if fs != nil { - return fs, nil + return !fs.isGenerated, nil } fs = &ageFileSummary{} p.fileSummaryCache[issue.FilePath()] = fs if issue.FilePath() == "" { - return nil, errors.New("no file path for issue") + return false, errors.New("no file path for issue") } doc, err := getDoc(issue.FilePath()) if err != nil { - return nil, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) + return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } fs.isGenerated = isGeneratedFileByComment(doc) + autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) - return fs, nil + + // don't report issues for autogenerated files + return !fs.isGenerated, nil } func getDoc(filePath string) (string, error) { From 10134f379c4ff142a3e20e16de734c888d4f4b95 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:35:25 +0100 Subject: [PATCH 04/15] chore: remove useless name type --- pkg/result/processors/autogenerated_exclude.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index f94cda770601..5ea2a18663ef 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -20,15 +20,13 @@ type ageFileSummary struct { isGenerated bool } -type ageFileSummaryCache map[string]*ageFileSummary - type AutogeneratedExclude struct { - fileSummaryCache ageFileSummaryCache + fileSummaryCache map[string]*ageFileSummary } func NewAutogeneratedExclude() *AutogeneratedExclude { return &AutogeneratedExclude{ - fileSummaryCache: ageFileSummaryCache{}, + fileSummaryCache: map[string]*ageFileSummary{}, } } From 55866d9f78dfaead54ddbfdf0e2e85a93b9020e3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:39:19 +0100 Subject: [PATCH 05/15] chore: rename isGeneratedFileByComment to isGeneratedFileLax --- .../processors/autogenerated_exclude.go | 26 +++++++++++-------- .../processors/autogenerated_exclude_test.go | 4 +-- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 5ea2a18663ef..e0104a8b8da1 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -12,6 +12,12 @@ import ( "github.com/golangci/golangci-lint/pkg/result" ) +const ( + genCodeGenerated = "code generated" + genDoNotEdit = "do not edit" + genAutoFile = "autogenerated file" // easyjson +) + var _ Processor = &AutogeneratedExclude{} var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude) @@ -71,7 +77,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } - fs.isGenerated = isGeneratedFileByComment(doc) + fs.isGenerated = isGeneratedFileLax(doc) autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) @@ -94,26 +100,24 @@ func getDoc(filePath string) (string, error) { return strings.Join(docLines, "\n"), 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 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) + return true } } autogenDebugf("doc of len %d doesn't contain any of markers: %s", len(doc), markers) + return false } diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index 531b80782b61..4ba89c92ac26 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -55,7 +55,7 @@ func TestIsAutogeneratedDetection(t *testing.T) { generatedCases := strings.Split(all, "\n\n") for _, gc := range generatedCases { - isGenerated := isGeneratedFileByComment(gc) + isGenerated := isGeneratedFileLax(gc) assert.True(t, isGenerated) } @@ -64,7 +64,7 @@ func TestIsAutogeneratedDetection(t *testing.T) { "test", } for _, ngc := range notGeneratedCases { - isGenerated := isGeneratedFileByComment(ngc) + isGenerated := isGeneratedFileLax(ngc) assert.False(t, isGenerated) } } From a9c9b1f14499a8c34378fff9ad8b58e787310e94 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:44:31 +0100 Subject: [PATCH 06/15] chore: isGeneratedFileLax as method --- .../processors/autogenerated_exclude.go | 34 +++++++++---------- .../processors/autogenerated_exclude_test.go | 8 +++-- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index e0104a8b8da1..1d9cf22e2ed7 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -77,7 +77,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } - fs.isGenerated = isGeneratedFileLax(doc) + fs.isGenerated = p.isGeneratedFileLax(doc) autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) @@ -85,25 +85,10 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return !fs.isGenerated, nil } -func getDoc(filePath string) (string, error) { - fset := token.NewFileSet() - syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) - if err != nil { - return "", fmt.Errorf("failed to parse file: %w", err) - } - - var docLines []string - for _, c := range syntax.Comments { - docLines = append(docLines, strings.TrimSpace(c.Text())) - } - - return strings.Join(docLines, "\n"), nil -} - // 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 isGeneratedFileLax(doc string) bool { +func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile} doc = strings.ToLower(doc) @@ -121,6 +106,21 @@ func isGeneratedFileLax(doc string) bool { return false } +func getDoc(filePath string) (string, error) { + fset := token.NewFileSet() + syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) + if err != nil { + return "", fmt.Errorf("failed to parse file: %w", err) + } + + var docLines []string + for _, c := range syntax.Comments { + docLines = append(docLines, strings.TrimSpace(c.Text())) + } + + return strings.Join(docLines, "\n"), nil +} + func isGoFile(name string) bool { return filepath.Ext(name) == ".go" } diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index 4ba89c92ac26..ca9750e9f852 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestIsAutogeneratedDetection(t *testing.T) { +func TestAutogeneratedExclude_isGeneratedFileLax(t *testing.T) { all := ` // generated by stringer -type Pill pill.go; DO NOT EDIT @@ -53,9 +53,11 @@ func TestIsAutogeneratedDetection(t *testing.T) { // AUTOGENERATED FILE: easyjson file.go ` + p := NewAutogeneratedExclude() + generatedCases := strings.Split(all, "\n\n") for _, gc := range generatedCases { - isGenerated := isGeneratedFileLax(gc) + isGenerated := p.isGeneratedFileLax(gc) assert.True(t, isGenerated) } @@ -64,7 +66,7 @@ func TestIsAutogeneratedDetection(t *testing.T) { "test", } for _, ngc := range notGeneratedCases { - isGenerated := isGeneratedFileLax(ngc) + isGenerated := p.isGeneratedFileLax(ngc) assert.False(t, isGenerated) } } From 836fc8d9e76ab4e583e87409e738f0a219c407a6 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:50:46 +0100 Subject: [PATCH 07/15] chore: debugf as field --- pkg/result/processors/autogenerated_exclude.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 1d9cf22e2ed7..eccd693542b3 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -20,18 +20,19 @@ const ( var _ Processor = &AutogeneratedExclude{} -var autogenDebugf = logutils.Debug(logutils.DebugKeyAutogenExclude) - type ageFileSummary struct { isGenerated bool } type AutogeneratedExclude struct { + debugf logutils.DebugFunc + fileSummaryCache map[string]*ageFileSummary } func NewAutogeneratedExclude() *AutogeneratedExclude { return &AutogeneratedExclude{ + debugf: logutils.Debug(logutils.DebugKeyAutogenExclude), fileSummaryCache: map[string]*ageFileSummary{}, } } @@ -79,7 +80,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error fs.isGenerated = p.isGeneratedFileLax(doc) - autogenDebugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) + p.debugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) // don't report issues for autogenerated files return !fs.isGenerated, nil @@ -95,13 +96,13 @@ func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { 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 } From f37298204adc538e008f0b803747ed5c5ed4b673 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 16:56:39 +0100 Subject: [PATCH 08/15] chore: rename getDoc to getPackageComments --- pkg/result/processors/autogenerated_exclude.go | 4 ++-- pkg/result/processors/autogenerated_exclude_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index eccd693542b3..a3276109dc5a 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -73,7 +73,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, errors.New("no file path for issue") } - doc, err := getDoc(issue.FilePath()) + doc, err := getPackageComments(issue.FilePath()) if err != nil { return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } @@ -107,7 +107,7 @@ func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { return false } -func getDoc(filePath string) (string, error) { +func getPackageComments(filePath string) (string, error) { fset := token.NewFileSet() syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) if err != nil { diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index ca9750e9f852..ed2974f747a2 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -71,7 +71,7 @@ func TestAutogeneratedExclude_isGeneratedFileLax(t *testing.T) { } } -func TestGetDoc(t *testing.T) { +func Test_getPackageComments(t *testing.T) { testCases := []struct { fpath string doc string @@ -101,7 +101,7 @@ this one line comment also`, } for _, tc := range testCases { - doc, err := getDoc(tc.fpath) + doc, err := getPackageComments(tc.fpath) require.NoError(t, err) assert.Equal(t, tc.doc, doc) } @@ -109,8 +109,8 @@ this one line comment also`, // Issue 954: Some lines can be very long, e.g. auto-generated // embedded resources. Reported on file of 86.2KB. -func TestGetDocFileWithLongLine(t *testing.T) { +func Test_getPackageComments_fileWithLongLine(t *testing.T) { fpath := filepath.Join("testdata", "autogen_exclude_long_line.go") - _, err := getDoc(fpath) + _, err := getPackageComments(fpath) assert.NoError(t, err) } From 59520fedfa3d57871bece150059c59ab7a991b89 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 17:17:31 +0100 Subject: [PATCH 09/15] tests: clean up isGeneratedFileLax tests --- .../processors/autogenerated_exclude_test.go | 94 +++++++++---------- 1 file changed, 46 insertions(+), 48 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index ed2974f747a2..9769b5f27f2e 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -2,72 +2,70 @@ package processors import ( "path/filepath" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestAutogeneratedExclude_isGeneratedFileLax(t *testing.T) { - all := ` - // generated by stringer -type Pill pill.go; 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 - 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. -// Generate with: go run gen.go -full -output md5block.go - -// generated by "go run gen.go". DO NOT EDIT. - -// 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 // +func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { + p := NewAutogeneratedExclude() -// Generated by running + comments := []string{ + ` // generated by stringer -type Pill pill.go; 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 - 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. +// Generate with: go run gen.go -full -output md5block.go`, + `// generated by "go run gen.go". DO NOT EDIT.`, + `// 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 // --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 * THIS FILE SHOULD NOT BE EDITED BY HAND -*/ - -// AUTOGENERATED FILE: easyjson file.go -` + */`, + `// AUTOGENERATED FILE: easyjson file.go`, + } - p := NewAutogeneratedExclude() + for _, comment := range comments { + comment := comment + t.Run(comment, func(t *testing.T) { + t.Parallel() - generatedCases := strings.Split(all, "\n\n") - for _, gc := range generatedCases { - isGenerated := p.isGeneratedFileLax(gc) - assert.True(t, isGenerated) + generated := p.isGeneratedFileLax(comment) + assert.True(t, generated) + }) } +} + +func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { + p := NewAutogeneratedExclude() - notGeneratedCases := []string{ + comments := []string{ "code not generated by", "test", } - for _, ngc := range notGeneratedCases { - isGenerated := p.isGeneratedFileLax(ngc) - assert.False(t, isGenerated) + + for _, comment := range comments { + comment := comment + t.Run(comment, func(t *testing.T) { + t.Parallel() + + generated := p.isGeneratedFileLax(comment) + assert.False(t, generated) + }) } } From 8ff189b8676ccb483a3a43373355d59c4da43d4c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 17:57:00 +0100 Subject: [PATCH 10/15] feat: option to follow strict Go autogenerated file detection --- pkg/config/issues.go | 11 ++-- pkg/lint/runner.go | 2 +- .../processors/autogenerated_exclude.go | 57 +++++++++++++++++-- .../processors/autogenerated_exclude_test.go | 37 +++++++++++- .../processors/testdata/autogen_go_strict.go | 7 +++ .../testdata/autogen_go_strict_invalid.go | 6 ++ 6 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 pkg/result/processors/testdata/autogen_go_strict.go create mode 100644 pkg/result/processors/testdata/autogen_go_strict_invalid.go diff --git a/pkg/config/issues.go b/pkg/config/issues.go index 2e14f6cc5d59..dac949049775 100644 --- a/pkg/config/issues.go +++ b/pkg/config/issues.go @@ -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"` diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index aa26ea082911..35212a2c90aa 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -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(), diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index a3276109dc5a..b781a55a96fc 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -6,6 +6,7 @@ import ( "go/parser" "go/token" "path/filepath" + "regexp" "strings" "github.com/golangci/golangci-lint/pkg/logutils" @@ -27,12 +28,17 @@ type ageFileSummary struct { type AutogeneratedExclude struct { debugf logutils.DebugFunc + strict bool + strictPattern *regexp.Regexp + fileSummaryCache map[string]*ageFileSummary } -func NewAutogeneratedExclude() *AutogeneratedExclude { +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]*ageFileSummary{}, } } @@ -61,6 +67,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, nil } + // The file is already known. fs := p.fileSummaryCache[issue.FilePath()] if fs != nil { return !fs.isGenerated, nil @@ -73,12 +80,20 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, errors.New("no file path for issue") } - doc, err := getPackageComments(issue.FilePath()) - if err != nil { - return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) - } + 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 := getPackageComments(issue.FilePath()) + if err != nil { + return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) + } - fs.isGenerated = p.isGeneratedFileLax(doc) + fs.isGenerated = p.isGeneratedFileLax(doc) + } p.debugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) @@ -107,6 +122,36 @@ func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool { return false } +// 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) + } + + if file == nil || len(file.Comments) == 0 { + return false, 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 getPackageComments(filePath string) (string, error) { fset := token.NewFileSet() syntax, err := parser.ParseFile(fset, filePath, nil, parser.PackageClauseOnly|parser.ParseComments) diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index 9769b5f27f2e..a5a65220399d 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -9,7 +9,7 @@ import ( ) func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { - p := NewAutogeneratedExclude() + p := NewAutogeneratedExclude(false) comments := []string{ ` // generated by stringer -type Pill pill.go; DO NOT EDIT`, @@ -51,7 +51,7 @@ func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) { } func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { - p := NewAutogeneratedExclude() + p := NewAutogeneratedExclude(false) comments := []string{ "code not generated by", @@ -69,6 +69,39 @@ func TestAutogeneratedExclude_isGeneratedFileLax_nonGenerated(t *testing.T) { } } +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_getPackageComments(t *testing.T) { testCases := []struct { fpath string diff --git a/pkg/result/processors/testdata/autogen_go_strict.go b/pkg/result/processors/testdata/autogen_go_strict.go new file mode 100644 index 000000000000..0f32d4af731f --- /dev/null +++ b/pkg/result/processors/testdata/autogen_go_strict.go @@ -0,0 +1,7 @@ +// foo foo foo. +// foo foo foo. +// foo foo foo. + +// Code generated by example — DO NOT EDIT. + +package testdata diff --git a/pkg/result/processors/testdata/autogen_go_strict_invalid.go b/pkg/result/processors/testdata/autogen_go_strict_invalid.go new file mode 100644 index 000000000000..c804e3e5f934 --- /dev/null +++ b/pkg/result/processors/testdata/autogen_go_strict_invalid.go @@ -0,0 +1,6 @@ +package testdata + +// Code generated by example; DO NOT EDIT. +func _() { + +} From a261faef5fb033552eae2ff151f46611280ab219 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 17:57:20 +0100 Subject: [PATCH 11/15] docs: update reference and schema --- .golangci.next.reference.yml | 6 ++++++ jsonschema/golangci.next.jsonschema.json | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 71dff87c68d0..47eea0880f38 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2814,6 +2814,12 @@ issues: # Default: false exclude-case-sensitive: false + # Use strict Go autogenerated file detection. + # 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: [] diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index e2a30045d528..f843cdcbedce 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3342,6 +3342,11 @@ "type": "boolean", "default": false }, + "exclude-autogenerated-strict": { + "description": "Use strict Go autogenerated file detection.", + "type": "boolean", + "default": false + }, "include": { "description": "The list of ids of default excludes to include or disable.", "type": "array", From c466665889bdee37a938fc2d368eb00102036b64 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 18:07:29 +0100 Subject: [PATCH 12/15] chore: use a more accurate name --- pkg/result/processors/autogenerated_exclude.go | 4 ++-- pkg/result/processors/autogenerated_exclude_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index b781a55a96fc..5b8d8fedd541 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -87,7 +87,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } } else { - doc, err := getPackageComments(issue.FilePath()) + doc, err := getComments(issue.FilePath()) if err != nil { return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } @@ -152,7 +152,7 @@ func (p *AutogeneratedExclude) isGeneratedFileStrict(filePath string) (bool, err return false, nil } -func getPackageComments(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 { diff --git a/pkg/result/processors/autogenerated_exclude_test.go b/pkg/result/processors/autogenerated_exclude_test.go index a5a65220399d..fbf48847a9cb 100644 --- a/pkg/result/processors/autogenerated_exclude_test.go +++ b/pkg/result/processors/autogenerated_exclude_test.go @@ -102,7 +102,7 @@ func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) { } } -func Test_getPackageComments(t *testing.T) { +func Test_getComments(t *testing.T) { testCases := []struct { fpath string doc string @@ -132,7 +132,7 @@ this one line comment also`, } for _, tc := range testCases { - doc, err := getPackageComments(tc.fpath) + doc, err := getComments(tc.fpath) require.NoError(t, err) assert.Equal(t, tc.doc, doc) } @@ -140,8 +140,8 @@ this one line comment also`, // Issue 954: Some lines can be very long, e.g. auto-generated // embedded resources. Reported on file of 86.2KB. -func Test_getPackageComments_fileWithLongLine(t *testing.T) { +func Test_getComments_fileWithLongLine(t *testing.T) { fpath := filepath.Join("testdata", "autogen_exclude_long_line.go") - _, err := getPackageComments(fpath) + _, err := getComments(fpath) assert.NoError(t, err) } From 593f61a0b96cfceaff935f62e1fd7c0db23f5e8a Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 18:12:41 +0100 Subject: [PATCH 13/15] docs: replace 'detection' by 'convention' --- .golangci.next.reference.yml | 2 +- jsonschema/golangci.next.jsonschema.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 47eea0880f38..47069aeb54bb 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2814,7 +2814,7 @@ issues: # Default: false exclude-case-sensitive: false - # Use strict Go autogenerated file detection. + # Use strict Go autogenerated file convention. # https://go.dev/s/generatedcode # By default a lax pattern is applied. # Default: false diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index f843cdcbedce..4413d839a2e4 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3343,7 +3343,7 @@ "default": false }, "exclude-autogenerated-strict": { - "description": "Use strict Go autogenerated file detection.", + "description": "Use strict Go autogenerated file convention.", "type": "boolean", "default": false }, From cdb5dee49099f541331d60177d3ed894674152c3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 18:14:02 +0100 Subject: [PATCH 14/15] docs: rephrase --- .golangci.next.reference.yml | 2 +- jsonschema/golangci.next.jsonschema.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index 47069aeb54bb..30825bad3ff7 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2814,7 +2814,7 @@ issues: # Default: false exclude-case-sensitive: false - # Use strict Go autogenerated file convention. + # To follow strict Go autogenerated file convention. # https://go.dev/s/generatedcode # By default a lax pattern is applied. # Default: false diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index 4413d839a2e4..5b614cc03947 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -3343,7 +3343,7 @@ "default": false }, "exclude-autogenerated-strict": { - "description": "Use strict Go autogenerated file convention.", + "description": "To follow strict Go autogenerated file convention", "type": "boolean", "default": false }, From f6acfcb4b7ea711d336fa0e312ea9f25f840bcc8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 14 Mar 2024 19:49:21 +0100 Subject: [PATCH 15/15] review: improve struct naming --- .../processors/autogenerated_exclude.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/result/processors/autogenerated_exclude.go b/pkg/result/processors/autogenerated_exclude.go index 5b8d8fedd541..9575ca4e74d8 100644 --- a/pkg/result/processors/autogenerated_exclude.go +++ b/pkg/result/processors/autogenerated_exclude.go @@ -21,8 +21,8 @@ const ( var _ Processor = &AutogeneratedExclude{} -type ageFileSummary struct { - isGenerated bool +type fileSummary struct { + generated bool } type AutogeneratedExclude struct { @@ -31,7 +31,7 @@ type AutogeneratedExclude struct { strict bool strictPattern *regexp.Regexp - fileSummaryCache map[string]*ageFileSummary + fileSummaryCache map[string]*fileSummary } func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude { @@ -39,7 +39,7 @@ func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude { debugf: logutils.Debug(logutils.DebugKeyAutogenExclude), strict: strict, strictPattern: regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`), - fileSummaryCache: map[string]*ageFileSummary{}, + fileSummaryCache: map[string]*fileSummary{}, } } @@ -70,10 +70,10 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error // The file is already known. fs := p.fileSummaryCache[issue.FilePath()] if fs != nil { - return !fs.isGenerated, nil + return !fs.generated, nil } - fs = &ageFileSummary{} + fs = &fileSummary{} p.fileSummaryCache[issue.FilePath()] = fs if issue.FilePath() == "" { @@ -82,7 +82,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error if p.strict { var err error - fs.isGenerated, err = p.isGeneratedFileStrict(issue.FilePath()) + 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) } @@ -92,13 +92,13 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error return false, fmt.Errorf("failed to get doc of file %s: %w", issue.FilePath(), err) } - fs.isGenerated = p.isGeneratedFileLax(doc) + fs.generated = p.isGeneratedFileLax(doc) } - p.debugf("file %q is generated: %t", issue.FilePath(), fs.isGenerated) + 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 } // isGeneratedFileLax reports whether the source file is generated code.