Skip to content

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

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

Merged
merged 3 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .golangci.next.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2814,11 +2814,17 @@ issues:
# Default: 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
# 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
exclude-autogenerated-strict: true
exclude-generated-strict: true

# The list of ids of default excludes to include or disable.
# https://golangci-lint.run/usage/false-positives/#default-exclusions
Expand Down
4 changes: 2 additions & 2 deletions jsonschema/golangci.next.jsonschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3342,8 +3342,8 @@
"type": "boolean",
"default": false
},
"exclude-autogenerated-strict": {
"description": "To follow strict Go autogenerated file convention",
"exclude-generated-strict": {
"description": "To follow strict Go generated file convention",
"type": "boolean",
"default": false
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/config/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +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"`
ExcludeAutogeneratedStrict bool `mapstructure:"exclude-autogenerated-strict"`
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"`
ExcludeGeneratedStrict bool `mapstructure:"exclude-generated-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(cfg.Issues.ExcludeAutogeneratedStrict),
processors.NewAutogeneratedExclude(cfg.Issues.ExcludeGeneratedStrict),

// Must be before exclude because users see already marked output and configure excluding by it.
processors.NewIdentifierMarker(),
Expand Down
22 changes: 13 additions & 9 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
return true, nil
}

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

if !isGoFile(issue.FilePath()) {
return false, nil
}
Expand All @@ -76,20 +80,16 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
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)
return false, fmt.Errorf("failed to get doc (strict) 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)
return false, fmt.Errorf("failed to get doc (lax) of file %s: %w", issue.FilePath(), err)
}

fs.generated = p.isGeneratedFileLax(doc)
Expand All @@ -102,7 +102,7 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
}

// 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.
func (p *AutogeneratedExclude) isGeneratedFileLax(doc string) bool {
markers := []string{genCodeGenerated, genDoNotEdit, genAutoFile}
Expand All @@ -122,8 +122,12 @@ 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.
// isGeneratedFileStrict returns true if the source file has a line that matches the regular expression:
//
// ^// 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) {
file, err := parser.ParseFile(token.NewFileSet(), filePath, nil, parser.PackageClauseOnly|parser.ParseComments)
if err != nil {
Expand Down
157 changes: 151 additions & 6 deletions pkg/result/processors/autogenerated_exclude_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package processors

import (
"fmt"
"go/token"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

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

func TestAutogeneratedExclude_isGeneratedFileLax_generated(t *testing.T) {
Expand Down Expand Up @@ -79,12 +84,12 @@ func TestAutogeneratedExclude_isGeneratedFileStrict(t *testing.T) {
}{
{
desc: "",
filepath: filepath.FromSlash("./testdata/autogen_go_strict.go"),
filepath: filepath.FromSlash("testdata/autogen_go_strict.go"),
assert: assert.True,
},
{
desc: "",
filepath: filepath.FromSlash("./testdata/autogen_go_strict_invalid.go"),
filepath: filepath.FromSlash("testdata/autogen_go_strict_invalid.go"),
assert: assert.False,
},
}
Expand All @@ -108,19 +113,19 @@ func Test_getComments(t *testing.T) {
doc string
}{
{
fpath: filepath.Join("testdata", "autogen_exclude.go"),
fpath: filepath.FromSlash("testdata/autogen_exclude.go"),
doc: `first line
second line
third line
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`,
},
{
fpath: filepath.Join("testdata", "autogen_exclude_block_comment.go"),
fpath: filepath.FromSlash("testdata/autogen_exclude_block_comment.go"),
doc: `* first line
*
* second line
Expand All @@ -141,7 +146,147 @@ 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_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)
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)
})
}
}