fix: filter invalid issues before other processors ()

This commit is contained in:
Ludovic Fernandez 2024-03-20 17:25:22 +01:00 committed by GitHub
parent 921d53570b
commit cd890db217
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 174 additions and 59 deletions

@ -68,6 +68,9 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti
// Must go after Cgo.
processors.NewFilenameUnadjuster(lintCtx.Packages, log.Child(logutils.DebugKeyFilenameUnadjuster)),
// Must go after FilenameUnadjuster.
processors.NewInvalidIssue(log.Child(logutils.DebugKeyInvalidIssue)),
// Must be before diff, nolint and exclude autogenerated processor at least.
processors.NewPathPrettifier(),
skipFilesProcessor,

@ -25,6 +25,7 @@ const (
DebugKeyExcludeRules = "exclude_rules"
DebugKeyExec = "exec"
DebugKeyFilenameUnadjuster = "filename_unadjuster"
DebugKeyInvalidIssue = "invalid_issue"
DebugKeyForbidigo = "forbidigo"
DebugKeyGoEnv = "goenv"
DebugKeyLinter = "linter"

@ -1,7 +1,6 @@
package processors
import (
"errors"
"fmt"
"go/parser"
"go/token"
@ -59,18 +58,6 @@ func (p *AutogeneratedExclude) shouldPassIssue(issue *result.Issue) (bool, error
return true, nil
}
if filepath.Base(issue.FilePath()) == "go.mod" {
return true, nil
}
if issue.FilePath() == "" {
return false, errors.New("no file path for issue")
}
if !isGoFile(issue.FilePath()) {
return false, nil
}
// The file is already known.
fs := p.fileSummaryCache[issue.FilePath()]
if fs != nil {

@ -166,28 +166,6 @@ func Test_shouldPassIssue(t *testing.T) {
},
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,
@ -239,17 +217,6 @@ func Test_shouldPassIssue_error(t *testing.T) {
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,

@ -0,0 +1,54 @@
package processors
import (
"path/filepath"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
)
var _ Processor = InvalidIssue{}
type InvalidIssue struct {
log logutils.Log
}
func NewInvalidIssue(log logutils.Log) *InvalidIssue {
return &InvalidIssue{log: log}
}
func (p InvalidIssue) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, p.shouldPassIssue)
}
func (p InvalidIssue) Name() string {
return "invalid_issue"
}
func (p InvalidIssue) Finish() {}
func (p InvalidIssue) shouldPassIssue(issue *result.Issue) (bool, error) {
if issue.FromLinter == "typecheck" {
return true, nil
}
if issue.FilePath() == "" {
// contextcheck has a known bug https://github.com/kkHAIKE/contextcheck/issues/21
if issue.FromLinter != "contextcheck" {
p.log.Warnf("no file path for the issue: probably a bug inside the linter %q: %#v", issue.FromLinter, issue)
}
return false, nil
}
if filepath.Base(issue.FilePath()) == "go.mod" {
return true, nil
}
if !isGoFile(issue.FilePath()) {
p.log.Infof("issue related to file %s is skipped", issue.FilePath())
return false, nil
}
return true, nil
}

@ -0,0 +1,109 @@
package processors
import (
"go/token"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/golangci/golangci-lint/pkg/logutils"
"github.com/golangci/golangci-lint/pkg/result"
)
func TestInvalidIssue_Process(t *testing.T) {
logger := logutils.NewStderrLog(logutils.DebugKeyInvalidIssue)
logger.SetLevel(logutils.LogLevelDebug)
p := NewInvalidIssue(logger)
testCases := []struct {
desc string
issues []result.Issue
expected []result.Issue
}{
{
desc: "typecheck",
issues: []result.Issue{
{FromLinter: "typecheck"},
},
expected: []result.Issue{
{FromLinter: "typecheck"},
},
},
{
desc: "Go file",
issues: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "test.go",
},
},
},
expected: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "test.go",
},
},
},
},
{
desc: "go.mod",
issues: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "go.mod",
},
},
},
expected: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "go.mod",
},
},
},
},
{
desc: "non Go file",
issues: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "test.txt",
},
},
},
expected: []result.Issue{},
},
{
desc: "no filename",
issues: []result.Issue{
{
FromLinter: "example",
Pos: token.Position{
Filename: "",
},
},
},
expected: []result.Issue{},
},
}
for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
after, err := p.Process(test.issues)
require.NoError(t, err)
assert.Equal(t, test.expected, after)
})
}
}

@ -1,7 +1,6 @@
package processors
import (
"errors"
"go/ast"
"go/parser"
"go/token"
@ -99,19 +98,15 @@ func (p *Nolint) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, p.shouldPassIssue)
}
func (p *Nolint) getOrCreateFileData(issue *result.Issue) (*fileData, error) {
func (p *Nolint) getOrCreateFileData(issue *result.Issue) *fileData {
fd := p.cache[issue.FilePath()]
if fd != nil {
return fd, nil
return fd
}
fd = &fileData{}
p.cache[issue.FilePath()] = fd
if issue.FilePath() == "" {
return nil, errors.New("no file path for issue")
}
// TODO: migrate this parsing to go/analysis facts
// or cache them somehow per file.
@ -120,12 +115,14 @@ func (p *Nolint) getOrCreateFileData(issue *result.Issue) (*fileData, error) {
f, err := parser.ParseFile(fset, issue.FilePath(), nil, parser.ParseComments)
if err != nil {
// Don't report error because it's already must be reporter by typecheck or go/analysis.
return fd, nil
return fd
}
fd.ignoredRanges = p.buildIgnoredRangesForFile(f, fset, issue.FilePath())
nolintDebugf("file %s: built nolint ranges are %+v", issue.FilePath(), fd.ignoredRanges)
return fd, nil
return fd
}
func (p *Nolint) buildIgnoredRangesForFile(f *ast.File, fset *token.FileSet, filePath string) []ignoredRange {
@ -161,10 +158,7 @@ func (p *Nolint) shouldPassIssue(issue *result.Issue) (bool, error) {
nolintDebugf("checking that lint issue was used for %s: %v", issue.ExpectedNoLintLinter, issue)
}
fd, err := p.getOrCreateFileData(issue)
if err != nil {
return false, err
}
fd := p.getOrCreateFileData(issue)
for _, ir := range fd.ignoredRanges {
if ir.doesMatch(issue) {