Skip to content

dev: unifying processors code style #4592

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 1 commit into from
Mar 29, 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
2 changes: 1 addition & 1 deletion pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti
processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
processors.NewDiff(cfg.Issues.Diff, cfg.Issues.DiffFromRevision, cfg.Issues.DiffPatchFilePath, cfg.Issues.WholeFiles),
processors.NewDiff(&cfg.Issues),
processors.NewMaxPerFileFromLinter(cfg),
processors.NewMaxSameIssues(cfg.Issues.MaxSameIssues, log.Child(logutils.DebugKeyMaxSameIssues), cfg),
processors.NewMaxFromLinter(cfg.Issues.MaxIssuesPerLinter, log.Child(logutils.DebugKeyMaxFromLinter), cfg),
Expand Down
4 changes: 2 additions & 2 deletions pkg/result/processors/autogenerated_exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const (
genAutoFile = "autogenerated file" // easyjson
)

var _ Processor = &AutogeneratedExclude{}
var _ Processor = (*AutogeneratedExclude)(nil)

type fileSummary struct {
generated bool
Expand All @@ -41,7 +41,7 @@ func NewAutogeneratedExclude(strict bool) *AutogeneratedExclude {
}
}

func (p *AutogeneratedExclude) Name() string {
func (*AutogeneratedExclude) Name() string {
return "autogenerated_exclude"
}

Expand Down
56 changes: 29 additions & 27 deletions pkg/result/processors/cgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,49 +9,51 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

var _ Processor = (*Cgo)(nil)

type Cgo struct {
goCacheDir string
}

var _ Processor = Cgo{}

func NewCgo(goenv *goutil.Env) *Cgo {
return &Cgo{
goCacheDir: goenv.Get(goutil.EnvGoCache),
}
}

func (p Cgo) Name() string {
func (Cgo) Name() string {
return "cgo"
}

func (p Cgo) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, func(issue *result.Issue) (bool, error) {
// some linters (e.g. gosec, deadcode) return incorrect filepaths for cgo issues,
// also cgo files have strange issues looking like false positives.

// cache dir contains all preprocessed files including cgo files

issueFilePath := issue.FilePath()
if !filepath.IsAbs(issue.FilePath()) {
absPath, err := filepath.Abs(issue.FilePath())
if err != nil {
return false, fmt.Errorf("failed to build abs path for %q: %w", issue.FilePath(), err)
}
issueFilePath = absPath
}
return filterIssuesErr(issues, p.shouldPassIssue)
}

if p.goCacheDir != "" && strings.HasPrefix(issueFilePath, p.goCacheDir) {
return false, nil
}
func (Cgo) Finish() {}

func (p Cgo) shouldPassIssue(issue *result.Issue) (bool, error) {
// some linters (e.g. gosec, deadcode) return incorrect filepaths for cgo issues,
// also cgo files have strange issues looking like false positives.

// cache dir contains all preprocessed files including cgo files

if filepath.Base(issue.FilePath()) == "_cgo_gotypes.go" {
// skip cgo warning for go1.10
return false, nil
issueFilePath := issue.FilePath()
if !filepath.IsAbs(issue.FilePath()) {
absPath, err := filepath.Abs(issue.FilePath())
if err != nil {
return false, fmt.Errorf("failed to build abs path for %q: %w", issue.FilePath(), err)
}
issueFilePath = absPath
}

return true, nil
})
}
if p.goCacheDir != "" && strings.HasPrefix(issueFilePath, p.goCacheDir) {
return false, nil
}

func (Cgo) Finish() {}
if filepath.Base(issue.FilePath()) == "_cgo_gotypes.go" {
// skip cgo warning for go1.10
return false, nil
}

return true, nil
}
17 changes: 9 additions & 8 deletions pkg/result/processors/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@ import (

"github.com/golangci/revgrep"

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

const envGolangciDiffProcessorPatch = "GOLANGCI_DIFF_PROCESSOR_PATCH"

var _ Processor = (*Diff)(nil)

type Diff struct {
onlyNew bool
fromRev string
Expand All @@ -22,19 +25,17 @@ type Diff struct {
patch string
}

var _ Processor = Diff{}

func NewDiff(onlyNew bool, fromRev, patchFilePath string, wholeFiles bool) *Diff {
func NewDiff(cfg *config.Issues) *Diff {
return &Diff{
onlyNew: onlyNew,
fromRev: fromRev,
patchFilePath: patchFilePath,
wholeFiles: wholeFiles,
onlyNew: cfg.Diff,
fromRev: cfg.DiffFromRevision,
patchFilePath: cfg.DiffPatchFilePath,
wholeFiles: cfg.WholeFiles,
patch: os.Getenv(envGolangciDiffProcessorPatch),
}
}

func (p Diff) Name() string {
func (Diff) Name() string {
return "diff"
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/result/processors/exclude.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

var _ Processor = Exclude{}
var _ Processor = (*Exclude)(nil)

type Exclude struct {
name string
Expand Down Expand Up @@ -49,4 +49,4 @@ func (p Exclude) Process(issues []result.Issue) ([]result.Issue, error) {
}), nil
}

func (p Exclude) Finish() {}
func (Exclude) Finish() {}
8 changes: 5 additions & 3 deletions pkg/result/processors/exclude_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

var _ Processor = ExcludeRules{}
var _ Processor = (*ExcludeRules)(nil)

type excludeRule struct {
baseRule
Expand Down Expand Up @@ -50,23 +50,25 @@ func NewExcludeRules(log logutils.Log, files *fsutils.Files, opts ExcludeRulesOp
return p
}

func (p ExcludeRules) Name() string { return p.name }

func (p ExcludeRules) Process(issues []result.Issue) ([]result.Issue, error) {
if len(p.rules) == 0 {
return issues, nil
}

return filterIssues(issues, func(issue *result.Issue) bool {
for _, rule := range p.rules {
rule := rule
if rule.match(issue, p.files, p.log) {
return false
}
}

return true
}), nil
}

func (p ExcludeRules) Name() string { return p.name }

func (ExcludeRules) Finish() {}

func createRules(rules []ExcludeRule, prefix string) []excludeRule {
Expand Down
94 changes: 48 additions & 46 deletions pkg/result/processors/filename_unadjuster.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/golangci/golangci-lint/pkg/result"
)

var _ Processor = (*FilenameUnadjuster)(nil)

type posMapper func(pos token.Position) token.Position

type adjustMap struct {
Expand All @@ -30,50 +32,6 @@ type FilenameUnadjuster struct {
loggedUnadjustments map[string]bool
}

var _ Processor = &FilenameUnadjuster{}

func processUnadjusterPkg(m *adjustMap, pkg *packages.Package, log logutils.Log) {
fset := token.NewFileSet() // it's more memory efficient to not store all in one fset

for _, filename := range pkg.CompiledGoFiles {
// It's important to call func here to run GC
processUnadjusterFile(filename, m, log, fset)
}
}

func processUnadjusterFile(filename string, m *adjustMap, log logutils.Log, fset *token.FileSet) {
syntax, err := parser.ParseFile(fset, filename, nil, parser.ParseComments)
if err != nil {
// Error will be reported by typecheck
return
}

adjustedFilename := fset.PositionFor(syntax.Pos(), true).Filename
if adjustedFilename == "" {
return
}

unadjustedFilename := fset.PositionFor(syntax.Pos(), false).Filename
if unadjustedFilename == "" || unadjustedFilename == adjustedFilename {
return
}

if !strings.HasSuffix(unadjustedFilename, ".go") {
return // file.go -> /caches/cgo-xxx
}

m.Lock()
defer m.Unlock()
m.m[adjustedFilename] = func(adjustedPos token.Position) token.Position {
tokenFile := fset.File(syntax.Pos())
if tokenFile == nil {
log.Warnf("Failed to get token file for %s", adjustedFilename)
return adjustedPos
}
return fset.PositionFor(tokenFile.Pos(adjustedPos.Offset), false)
}
}

func NewFilenameUnadjuster(pkgs []*packages.Package, log logutils.Log) *FilenameUnadjuster {
m := adjustMap{m: map[string]posMapper{}}

Expand All @@ -97,7 +55,7 @@ func NewFilenameUnadjuster(pkgs []*packages.Package, log logutils.Log) *Filename
}
}

func (p *FilenameUnadjuster) Name() string {
func (*FilenameUnadjuster) Name() string {
return "filename_unadjuster"
}

Expand Down Expand Up @@ -128,4 +86,48 @@ func (p *FilenameUnadjuster) Process(issues []result.Issue) ([]result.Issue, err
}), nil
}

func (p *FilenameUnadjuster) Finish() {}
func (*FilenameUnadjuster) Finish() {}

func processUnadjusterPkg(m *adjustMap, pkg *packages.Package, log logutils.Log) {
fset := token.NewFileSet() // it's more memory efficient to not store all in one fset

for _, filename := range pkg.CompiledGoFiles {
// It's important to call func here to run GC
processUnadjusterFile(filename, m, log, fset)
}
}

func processUnadjusterFile(filename string, m *adjustMap, log logutils.Log, fset *token.FileSet) {
syntax, err := parser.ParseFile(fset, filename, nil, parser.ParseComments)
if err != nil {
// Error will be reported by typecheck
return
}

adjustedFilename := fset.PositionFor(syntax.Pos(), true).Filename
if adjustedFilename == "" {
return
}

unadjustedFilename := fset.PositionFor(syntax.Pos(), false).Filename
if unadjustedFilename == "" || unadjustedFilename == adjustedFilename {
return
}

if !strings.HasSuffix(unadjustedFilename, ".go") {
return // file.go -> /caches/cgo-xxx
}

m.Lock()
defer m.Unlock()

m.m[adjustedFilename] = func(adjustedPos token.Position) token.Position {
tokenFile := fset.File(syntax.Pos())
if tokenFile == nil {
log.Warnf("Failed to get token file for %s", adjustedFilename)
return adjustedPos
}

return fset.PositionFor(tokenFile.Pos(adjustedPos.Offset), false)
}
}
Loading