dev: processor optimizations (#5316)

Author: ldez

pkg/commands/run.go

@@ -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)

pkg/golinters/misspell/misspell.go

@@ -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) {
@@ -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)
 	}

pkg/golinters/misspell/testdata/misspell_cgo.go

@@ -13,6 +13,7 @@ package testdata
 import "C"
 
 import (
+	"fmt"
 	"unsafe"
 )
 
@@ -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`"
+}

pkg/lint/context.go

@@ -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"
@@ -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,
 	}
@@ -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,
 	}

pkg/lint/linter/context.go

@@ -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"
 )
@@ -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

pkg/lint/runner.go

@@ -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
 
@@ -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),

pkg/logutils/logutils.go

@@ -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"

pkg/result/processors/cgo.go

@@ -1,7 +1,6 @@
 package processors
 
 import (
-	"fmt"
 	"path/filepath"
 	"strings"
 
@@ -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
 }
@@ -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
 	}
 

pkg/result/processors/diff.go

@@ -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)
 	}
 
@@ -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() {}