Skip to content
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

dev: processor optimizations #5316

Merged
merged 6 commits into from
Jan 14, 2025
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/commands/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (c *runCommand) preRunE(_ *cobra.Command, args []string) error {

pkgLoader := lint.NewPackageLoader(c.log.Child(logutils.DebugKeyLoader), c.cfg, args, c.goenv, guard)

c.contextBuilder = lint.NewContextBuilder(c.cfg, pkgLoader, c.fileCache, pkgCache, guard)
c.contextBuilder = lint.NewContextBuilder(c.cfg, pkgLoader, pkgCache, guard)

if err = initHashSalt(c.buildInfo.Version, c.cfg); err != nil {
return fmt.Errorf("failed to init hash salt: %w", err)
Expand Down
58 changes: 24 additions & 34 deletions pkg/golinters/misspell/misspell.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,50 +12,38 @@ import (

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/goanalysis"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/golinters/internal"
)

const linterName = "misspell"

func New(settings *config.MisspellSettings) *goanalysis.Linter {
analyzer := &analysis.Analyzer{
Name: linterName,
Doc: goanalysis.TheOnlyanalyzerDoc,
Run: goanalysis.DummyRun,
replacer, err := createMisspellReplacer(settings)
if err != nil {
internal.LinterLogger.Fatalf("%s: %v", linterName, err)
}

return goanalysis.NewLinter(
linterName,
"Finds commonly misspelled English words",
[]*analysis.Analyzer{analyzer},
nil,
).WithContextSetter(func(lintCtx *linter.Context) {
replacer, ruleErr := createMisspellReplacer(settings)

analyzer.Run = func(pass *analysis.Pass) (any, error) {
if ruleErr != nil {
return nil, ruleErr
}

err := runMisspell(lintCtx, pass, replacer, settings.Mode)
if err != nil {
return nil, err
a := &analysis.Analyzer{
Name: linterName,
Doc: "Finds commonly misspelled English words",
Run: func(pass *analysis.Pass) (any, error) {
for _, file := range pass.Files {
err := runMisspellOnFile(pass, file, replacer, settings.Mode)
if err != nil {
return nil, err
}
}

return nil, nil
}
}).WithLoadMode(goanalysis.LoadModeSyntax)
}

func runMisspell(lintCtx *linter.Context, pass *analysis.Pass, replacer *misspell.Replacer, mode string) error {
for _, file := range pass.Files {
err := runMisspellOnFile(lintCtx, pass, file, replacer, mode)
if err != nil {
return err
}
},
}

return nil
return goanalysis.NewLinter(
a.Name,
a.Doc,
[]*analysis.Analyzer{a},
nil,
).WithLoadMode(goanalysis.LoadModeSyntax)
}

func createMisspellReplacer(settings *config.MisspellSettings) (*misspell.Replacer, error) {
Expand Down Expand Up @@ -90,13 +78,15 @@ func createMisspellReplacer(settings *config.MisspellSettings) (*misspell.Replac
return replacer, nil
}

func runMisspellOnFile(lintCtx *linter.Context, pass *analysis.Pass, file *ast.File, replacer *misspell.Replacer, mode string) error {
func runMisspellOnFile(pass *analysis.Pass, file *ast.File, replacer *misspell.Replacer, mode string) error {
position, isGoFile := goanalysis.GetGoFilePosition(pass, file)
if !isGoFile {
return nil
}

fileContent, err := lintCtx.FileCache.GetFileBytes(position.Filename)
// Uses the non-adjusted file to work with cgo:
// if we read the real file, the positions are wrong in some cases.
fileContent, err := pass.ReadFile(pass.Fset.PositionFor(file.Pos(), false).Filename)
if err != nil {
return fmt.Errorf("can't get file %s contents: %w", position.Filename, err)
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/golinters/misspell/testdata/misspell_cgo.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package testdata
import "C"

import (
"fmt"
"unsafe"
)

Expand All @@ -28,3 +29,11 @@ func Misspell() {

// the word langauge should be ignored here: it's set in config
// the word Dialogue should be ignored here: it's set in config

func _() error {
cs := C.CString("Hello from stdio\n")
C.myprint(cs)
C.free(unsafe.Pointer(cs))

return fmt.Errorf("an unknown error ocurred") // want "`ocurred` is a misspelling of `occurred`"
}
8 changes: 2 additions & 6 deletions pkg/lint/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/golangci/golangci-lint/internal/cache"
"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/exitcodes"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/goanalysis/load"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/logutils"
Expand All @@ -18,19 +17,17 @@ type ContextBuilder struct {

pkgLoader *PackageLoader

fileCache *fsutils.FileCache
pkgCache *cache.Cache
pkgCache *cache.Cache

loadGuard *load.Guard
}

func NewContextBuilder(cfg *config.Config, pkgLoader *PackageLoader,
fileCache *fsutils.FileCache, pkgCache *cache.Cache, loadGuard *load.Guard,
pkgCache *cache.Cache, loadGuard *load.Guard,
) *ContextBuilder {
return &ContextBuilder{
cfg: cfg,
pkgLoader: pkgLoader,
fileCache: fileCache,
pkgCache: pkgCache,
loadGuard: loadGuard,
}
Expand All @@ -55,7 +52,6 @@ func (cl *ContextBuilder) Build(ctx context.Context, log logutils.Log, linters [

Cfg: cl.cfg,
Log: log,
FileCache: cl.fileCache,
PkgCache: cl.pkgCache,
LoadGuard: cl.loadGuard,
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/lint/linter/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/golangci/golangci-lint/internal/cache"
"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/fsutils"
"github.com/golangci/golangci-lint/pkg/goanalysis/load"
"github.com/golangci/golangci-lint/pkg/logutils"
)
Expand All @@ -20,9 +19,8 @@ type Context struct {
// version for each of packages
OriginalPackages []*packages.Package

Cfg *config.Config
FileCache *fsutils.FileCache
Log logutils.Log
Cfg *config.Config
Log logutils.Log

PkgCache *cache.Cache
LoadGuard *load.Guard
Expand Down
11 changes: 8 additions & 3 deletions pkg/lint/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,19 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti

return &Runner{
Processors: []processors.Processor{
// Must be the first processor.
processors.NewPathAbsoluter(log),

processors.NewCgo(goenv),

// Must go after Cgo.
// Must be after Cgo.
processors.NewFilenameUnadjuster(lintCtx.Packages, log.Child(logutils.DebugKeyFilenameUnadjuster)),

// Must go after FilenameUnadjuster.
// Must be after FilenameUnadjuster.
processors.NewInvalidIssue(log.Child(logutils.DebugKeyInvalidIssue)),

// Must be before diff, nolint and exclude autogenerated processor at least.
processors.NewPathPrettifier(),
processors.NewPathPrettifier(log),
skipFilesProcessor,
skipDirsProcessor, // must be after path prettifier

Expand All @@ -88,13 +91,15 @@ func NewRunner(log logutils.Log, cfg *config.Config, args []string, goenv *gouti

processors.NewExclude(&cfg.Issues),
processors.NewExcludeRules(log.Child(logutils.DebugKeyExcludeRules), files, &cfg.Issues),

processors.NewNolint(log.Child(logutils.DebugKeyNolint), dbManager, enabledLinters),

processors.NewUniqByLine(cfg),
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),

processors.NewSourceCode(lineCache, log.Child(logutils.DebugKeySourceCode)),
processors.NewPathShortener(),
processors.NewSeverity(log.Child(logutils.DebugKeySeverityRules), files, &cfg.Severity),
Expand Down
2 changes: 2 additions & 0 deletions pkg/logutils/logutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ const (
DebugKeyLoader = "loader" // Debugs packages loading (including `go/packages` internal debugging).
DebugKeyMaxFromLinter = "max_from_linter"
DebugKeyMaxSameIssues = "max_same_issues"
DebugKeyPathAbsoluter = "path_absoluter"
DebugKeyPathPrettifier = "path_prettifier"
DebugKeyPkgCache = "pkgcache"
DebugKeyRunner = "runner"
DebugKeySeverityRules = "severity_rules"
Expand Down
30 changes: 10 additions & 20 deletions pkg/result/processors/cgo.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package processors

import (
"fmt"
"path/filepath"
"strings"

Expand All @@ -11,6 +10,10 @@ import (

var _ Processor = (*Cgo)(nil)

// Cgo some linters (e.g. gosec, deadcode) return incorrect filepaths for cgo issues,
// also cgo files have strange issues looking like false positives.
//
// Require absolute filepath.
type Cgo struct {
goCacheDir string
}
Expand All @@ -21,32 +24,19 @@ func NewCgo(goenv *goutil.Env) *Cgo {
}
}

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

func (p Cgo) Process(issues []result.Issue) ([]result.Issue, error) {
func (p *Cgo) Process(issues []result.Issue) ([]result.Issue, error) {
return filterIssuesErr(issues, p.shouldPassIssue)
}

func (Cgo) Finish() {}
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

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
}

if p.goCacheDir != "" && strings.HasPrefix(issueFilePath, p.goCacheDir) {
func (p *Cgo) shouldPassIssue(issue *result.Issue) (bool, error) {
// [p.goCacheDir] contains all preprocessed files including cgo files.
if p.goCacheDir != "" && strings.HasPrefix(issue.FilePath(), p.goCacheDir) {
return false, nil
}

Expand Down
23 changes: 14 additions & 9 deletions pkg/result/processors/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,32 +36,36 @@ func NewDiff(cfg *config.Issues) *Diff {
}
}

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

func (p Diff) Process(issues []result.Issue) ([]result.Issue, error) {
if !p.onlyNew && p.fromRev == "" && p.patchFilePath == "" && p.patch == "" { // no need to work
func (p *Diff) Process(issues []result.Issue) ([]result.Issue, error) {
if !p.onlyNew && p.fromRev == "" && p.patchFilePath == "" && p.patch == "" {
return issues, nil
}

var patchReader io.Reader
if p.patchFilePath != "" {
switch {
case p.patchFilePath != "":
patch, err := os.ReadFile(p.patchFilePath)
if err != nil {
return nil, fmt.Errorf("can't read from patch file %s: %w", p.patchFilePath, err)
}

patchReader = bytes.NewReader(patch)
} else if p.patch != "" {

case p.patch != "":
patchReader = strings.NewReader(p.patch)
}

c := revgrep.Checker{
checker := revgrep.Checker{
Patch: patchReader,
RevisionFrom: p.fromRev,
WholeFiles: p.wholeFiles,
}
if err := c.Prepare(context.Background()); err != nil {

if err := checker.Prepare(context.Background()); err != nil {
return nil, fmt.Errorf("can't prepare diff by revgrep: %w", err)
}

Expand All @@ -71,15 +75,16 @@ func (p Diff) Process(issues []result.Issue) ([]result.Issue, error) {
return issue
}

hunkPos, isNew := c.IsNewIssue(issue)
hunkPos, isNew := checker.IsNewIssue(issue)
if !isNew {
return nil
}

newIssue := *issue
newIssue.HunkPos = hunkPos

return &newIssue
}), nil
}

func (Diff) Finish() {}
func (*Diff) Finish() {}
Loading
Loading