From 6fda81008dfcea4898994a1a9c96d49e7e0022e5 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Mon, 11 Mar 2024 16:19:53 +0100 Subject: [PATCH] dev: clean context loader (#4480) --- pkg/commands/run.go | 7 ++-- pkg/lint/load.go | 69 +++++++++++++++++--------------------- pkg/lint/runner.go | 81 +++++++++++++++++++++++---------------------- 3 files changed, 75 insertions(+), 82 deletions(-) diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 0202cc4d..0b4501ef 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -363,19 +363,18 @@ func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.I c.reportData.AddLinter(lc.Name(), isEnabled, lc.EnabledByDefault) } - lintCtx, err := c.contextLoader.Load(ctx, lintersToRun) + lintCtx, err := c.contextLoader.Load(ctx, c.log.Child(logutils.DebugKeyLintersContext), lintersToRun) if err != nil { return nil, fmt.Errorf("context loading failed: %w", err) } - lintCtx.Log = c.log.Child(logutils.DebugKeyLintersContext) runner, err := lint.NewRunner(c.log.Child(logutils.DebugKeyRunner), - c.cfg, c.goenv, c.lineCache, c.fileCache, c.dbManager, lintCtx.Packages) + c.cfg, c.goenv, c.lineCache, c.fileCache, c.dbManager, lintCtx) if err != nil { return nil, err } - return runner.Run(ctx, lintersToRun, lintCtx) + return runner.Run(ctx, lintersToRun) } func (c *runCommand) setOutputToDevNull() (savedStdout, savedStderr *os.File) { diff --git a/pkg/lint/load.go b/pkg/lint/load.go index babad5ba..7f892e94 100644 --- a/pkg/lint/load.go +++ b/pkg/lint/load.go @@ -50,6 +50,37 @@ func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env, } } +func (cl *ContextLoader) Load(ctx context.Context, log logutils.Log, linters []*linter.Config) (*linter.Context, error) { + loadMode := cl.findLoadMode(linters) + pkgs, err := cl.loadPackages(ctx, loadMode) + if err != nil { + return nil, fmt.Errorf("failed to load packages: %w", err) + } + + deduplicatedPkgs := cl.filterDuplicatePackages(pkgs) + + if len(deduplicatedPkgs) == 0 { + return nil, exitcodes.ErrNoGoFiles + } + + ret := &linter.Context{ + Packages: deduplicatedPkgs, + + // At least `unused` linters works properly only on original (not deduplicated) packages, + // see https://github.com/golangci/golangci-lint/pull/585. + OriginalPackages: pkgs, + + Cfg: cl.cfg, + Log: log, + FileCache: cl.fileCache, + LineCache: cl.lineCache, + PkgCache: cl.pkgCache, + LoadGuard: cl.loadGuard, + } + + return ret, nil +} + func (cl *ContextLoader) prepareBuildContext() { // Set GOROOT to have working cross-compilation: cross-compiled binaries // have invalid GOROOT. XXX: can't use runtime.GOROOT(). @@ -213,13 +244,6 @@ func (cl *ContextLoader) loadPackages(ctx context.Context, loadMode packages.Loa return nil, fmt.Errorf("failed to load with go/packages: %w", err) } - // Currently, go/packages doesn't guarantee that error will be returned - // if context was canceled. See - // https://github.com/golang/tools/commit/c5cec6710e927457c3c29d6c156415e8539a5111#r39261855 - if ctx.Err() != nil { - return nil, fmt.Errorf("timed out to load packages: %w", ctx.Err()) - } - if loadMode&packages.NeedSyntax == 0 { // Needed e.g. for go/analysis loading. fset := token.NewFileSet() @@ -293,34 +317,3 @@ func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*pa return retPkgs } - -func (cl *ContextLoader) Load(ctx context.Context, linters []*linter.Config) (*linter.Context, error) { - loadMode := cl.findLoadMode(linters) - pkgs, err := cl.loadPackages(ctx, loadMode) - if err != nil { - return nil, fmt.Errorf("failed to load packages: %w", err) - } - - deduplicatedPkgs := cl.filterDuplicatePackages(pkgs) - - if len(deduplicatedPkgs) == 0 { - return nil, exitcodes.ErrNoGoFiles - } - - ret := &linter.Context{ - Packages: deduplicatedPkgs, - - // At least `unused` linters works properly only on original (not deduplicated) packages, - // see https://github.com/golangci/golangci-lint/pull/585. - OriginalPackages: pkgs, - - Cfg: cl.cfg, - Log: cl.log, - FileCache: cl.fileCache, - LineCache: cl.lineCache, - PkgCache: cl.pkgCache, - LoadGuard: cl.loadGuard, - } - - return ret, nil -} diff --git a/pkg/lint/runner.go b/pkg/lint/runner.go index dc6b2e73..2362042c 100644 --- a/pkg/lint/runner.go +++ b/pkg/lint/runner.go @@ -7,8 +7,6 @@ import ( "runtime/debug" "strings" - gopackages "golang.org/x/tools/go/packages" - "github.com/golangci/golangci-lint/internal/errorutil" "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/fsutils" @@ -22,14 +20,21 @@ import ( "github.com/golangci/golangci-lint/pkg/timeutils" ) +type processorStat struct { + inCount int + outCount int +} + type Runner struct { + Log logutils.Log + + lintCtx *linter.Context Processors []processors.Processor - Log logutils.Log } func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env, lineCache *fsutils.LineCache, fileCache *fsutils.FileCache, - dbManager *lintersdb.Manager, pkgs []*gopackages.Package) (*Runner, error) { + dbManager *lintersdb.Manager, lintCtx *linter.Context) (*Runner, error) { // Beware that some processors need to add the path prefix when working with paths // because they get invoked before the path prefixer (exclude and severity rules) // or process other paths (skip files). @@ -75,7 +80,7 @@ func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env, processors.NewCgo(goenv), // Must go after Cgo. - processors.NewFilenameUnadjuster(pkgs, log.Child(logutils.DebugKeyFilenameUnadjuster)), + processors.NewFilenameUnadjuster(lintCtx.Packages, log.Child(logutils.DebugKeyFilenameUnadjuster)), // Must be before diff, nolint and exclude autogenerated processor at least. processors.NewPathPrettifier(), @@ -107,10 +112,38 @@ func NewRunner(log logutils.Log, cfg *config.Config, goenv *goutil.Env, processors.NewPathPrefixer(cfg.Output.PathPrefix), processors.NewSortResults(cfg), }, - Log: log, + lintCtx: lintCtx, + Log: log, }, nil } +func (r *Runner) Run(ctx context.Context, linters []*linter.Config) ([]result.Issue, error) { + sw := timeutils.NewStopwatch("linters", r.Log) + defer sw.Print() + + var ( + lintErrors error + issues []result.Issue + ) + + for _, lc := range linters { + lc := lc + sw.TrackStage(lc.Name(), func() { + linterIssues, err := r.runLinterSafe(ctx, r.lintCtx, lc) + if err != nil { + lintErrors = errors.Join(lintErrors, fmt.Errorf("can't run linter %s", lc.Linter.Name()), err) + r.Log.Warnf("Can't run linter %s: %v", lc.Linter.Name(), err) + + return + } + + issues = append(issues, linterIssues...) + }) + } + + return r.processLintResults(issues), lintErrors +} + func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, lc *linter.Config) (ret []result.Issue, err error) { defer func() { @@ -151,12 +184,7 @@ func (r *Runner) runLinterSafe(ctx context.Context, lintCtx *linter.Context, return issues, nil } -type processorStat struct { - inCount int - outCount int -} - -func (r Runner) processLintResults(inIssues []result.Issue) []result.Issue { +func (r *Runner) processLintResults(inIssues []result.Issue) []result.Issue { sw := timeutils.NewStopwatch("processing", r.Log) var issuesBefore, issuesAfter int @@ -187,7 +215,7 @@ func (r Runner) processLintResults(inIssues []result.Issue) []result.Issue { return outIssues } -func (r Runner) printPerProcessorStat(stat map[string]processorStat) { +func (r *Runner) printPerProcessorStat(stat map[string]processorStat) { parts := make([]string, 0, len(stat)) for name, ps := range stat { if ps.inCount != 0 { @@ -199,33 +227,6 @@ func (r Runner) printPerProcessorStat(stat map[string]processorStat) { } } -func (r Runner) Run(ctx context.Context, linters []*linter.Config, lintCtx *linter.Context) ([]result.Issue, error) { - sw := timeutils.NewStopwatch("linters", r.Log) - defer sw.Print() - - var ( - lintErrors error - issues []result.Issue - ) - - for _, lc := range linters { - lc := lc - sw.TrackStage(lc.Name(), func() { - linterIssues, err := r.runLinterSafe(ctx, lintCtx, lc) - if err != nil { - lintErrors = errors.Join(lintErrors, fmt.Errorf("can't run linter %s", lc.Linter.Name()), err) - r.Log.Warnf("Can't run linter %s: %v", lc.Linter.Name(), err) - - return - } - - issues = append(issues, linterIssues...) - }) - } - - return r.processLintResults(issues), lintErrors -} - func (r *Runner) processIssues(issues []result.Issue, sw *timeutils.Stopwatch, statPerProcessor map[string]processorStat) []result.Issue { for _, p := range r.Processors { var newIssues []result.Issue