Skip to content

Commit

Permalink
feat: rename exclude-autogenerated-strict to exclude-generated-strict (
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Mar 15, 2024
1 parent d239c9b commit 39617e4
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 27 deletions.
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)
})
}
}

0 comments on commit 39617e4

Please sign in to comment.