dev: split ContextLoader (#4516)

This commit is contained in:
Ludovic Fernandez 2024-03-17 22:52:28 +01:00 committed by GitHub
parent 2c0a8ee50e
commit 7ad5ccb65b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 344 additions and 225 deletions

View File

@ -87,8 +87,8 @@ type runCommand struct {
debugf logutils.DebugFunc
reportData *report.Data
contextLoader *lint.ContextLoader
goenv *goutil.Env
contextBuilder *lint.ContextBuilder
goenv *goutil.Env
fileCache *fsutils.FileCache
lineCache *fsutils.LineCache
@ -182,7 +182,7 @@ func (c *runCommand) persistentPostRunE(_ *cobra.Command, _ []string) error {
return nil
}
func (c *runCommand) preRunE(_ *cobra.Command, _ []string) error {
func (c *runCommand) preRunE(_ *cobra.Command, args []string) error {
dbManager, err := lintersdb.NewManager(c.log.Child(logutils.DebugKeyLintersDB), c.cfg,
lintersdb.NewLinterBuilder(), lintersdb.NewPluginModuleBuilder(c.log), lintersdb.NewPluginGoBuilder(c.log))
if err != nil {
@ -210,8 +210,11 @@ func (c *runCommand) preRunE(_ *cobra.Command, _ []string) error {
return fmt.Errorf("failed to build packages cache: %w", err)
}
c.contextLoader = lint.NewContextLoader(c.cfg, c.log.Child(logutils.DebugKeyLoader), c.goenv,
c.lineCache, c.fileCache, pkgCache, load.NewGuard())
guard := load.NewGuard()
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)
if err = initHashSalt(c.buildInfo.Version, c.cfg); err != nil {
return fmt.Errorf("failed to init hash salt: %w", err)
@ -373,7 +376,7 @@ func (c *runCommand) runAnalysis(ctx context.Context, args []string) ([]result.I
return nil, err
}
lintCtx, err := c.contextLoader.Load(ctx, c.log.Child(logutils.DebugKeyLintersContext), lintersToRun)
lintCtx, err := c.contextBuilder.Build(ctx, c.log.Child(logutils.DebugKeyLintersContext), lintersToRun)
if err != nil {
return nil, fmt.Errorf("context loading failed: %w", err)
}

View File

@ -38,6 +38,7 @@ func (c *Config) Validate() error {
c.LintersSettings.Validate,
c.Linters.Validate,
c.Output.Validate,
c.Run.Validate,
}
for _, v := range validators {

View File

@ -1,6 +1,11 @@
package config
import "time"
import (
"fmt"
"slices"
"strings"
"time"
)
// Run encapsulates the config options for running the linter analysis.
type Run struct {
@ -29,6 +34,17 @@ type Run struct {
// Deprecated: use Output.ShowStats instead.
ShowStats bool `mapstructure:"show-stats"`
// It's obtain by flags and use for the tests and the context loader.
// Only used by skipDirs processor. TODO(ldez) remove it in next PR.
Args []string // Internal needs.
}
func (r *Run) Validate() error {
// go help modules
allowedMods := []string{"mod", "readonly", "vendor"}
if r.ModulesDownloadMode != "" && !slices.Contains(allowedMods, r.ModulesDownloadMode) {
return fmt.Errorf("invalid modules download path %s, only (%s) allowed", r.ModulesDownloadMode, strings.Join(allowedMods, "|"))
}
return nil
}

75
pkg/config/run_test.go Normal file
View File

@ -0,0 +1,75 @@
package config
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestRun_Validate(t *testing.T) {
testCases := []struct {
desc string
settings *Run
}{
{
desc: "modules-download-mode: mod",
settings: &Run{
ModulesDownloadMode: "mod",
},
},
{
desc: "modules-download-mode: readonly",
settings: &Run{
ModulesDownloadMode: "readonly",
},
},
{
desc: "modules-download-mode: vendor",
settings: &Run{
ModulesDownloadMode: "vendor",
},
},
{
desc: "modules-download-mode: empty",
settings: &Run{
ModulesDownloadMode: "",
},
},
}
for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
err := test.settings.Validate()
require.NoError(t, err)
})
}
}
func TestRun_Validate_error(t *testing.T) {
testCases := []struct {
desc string
settings *Run
expected string
}{
{
desc: "modules-download-mode: invalid",
settings: &Run{
ModulesDownloadMode: "invalid",
},
expected: "invalid modules download path invalid, only (mod|readonly|vendor) allowed",
},
}
for _, test := range testCases {
test := test
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
err := test.settings.Validate()
require.EqualError(t, err, test.expected)
})
}
}

64
pkg/lint/context.go Normal file
View File

@ -0,0 +1,64 @@
package lint
import (
"context"
"fmt"
"github.com/golangci/golangci-lint/internal/pkgcache"
"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/golinters/goanalysis/load"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/logutils"
)
type ContextBuilder struct {
cfg *config.Config
pkgLoader *PackageLoader
fileCache *fsutils.FileCache
pkgCache *pkgcache.Cache
loadGuard *load.Guard
}
func NewContextBuilder(cfg *config.Config, pkgLoader *PackageLoader,
fileCache *fsutils.FileCache, pkgCache *pkgcache.Cache, loadGuard *load.Guard,
) *ContextBuilder {
return &ContextBuilder{
cfg: cfg,
pkgLoader: pkgLoader,
fileCache: fileCache,
pkgCache: pkgCache,
loadGuard: loadGuard,
}
}
func (cl *ContextBuilder) Build(ctx context.Context, log logutils.Log, linters []*linter.Config) (*linter.Context, error) {
pkgs, deduplicatedPkgs, err := cl.pkgLoader.Load(ctx, linters)
if err != nil {
return nil, fmt.Errorf("failed to load packages: %w", err)
}
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,
PkgCache: cl.pkgCache,
LoadGuard: cl.loadGuard,
}
return ret, nil
}

View File

@ -22,7 +22,6 @@ type Context struct {
Cfg *config.Config
FileCache *fsutils.FileCache
LineCache *fsutils.LineCache
Log logutils.Log
PkgCache *pkgcache.Cache

View File

@ -13,183 +13,97 @@ import (
"golang.org/x/tools/go/packages"
"github.com/golangci/golangci-lint/internal/pkgcache"
"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/golinters/goanalysis/load"
"github.com/golangci/golangci-lint/pkg/goutil"
"github.com/golangci/golangci-lint/pkg/lint/linter"
"github.com/golangci/golangci-lint/pkg/logutils"
)
type ContextLoader struct {
cfg *config.Config
log logutils.Log
debugf logutils.DebugFunc
goenv *goutil.Env
// PackageLoader loads packages based on [golang.org/x/tools/go/packages.Load].
type PackageLoader struct {
log logutils.Log
debugf logutils.DebugFunc
cfg *config.Config
args []string
pkgTestIDRe *regexp.Regexp
lineCache *fsutils.LineCache
fileCache *fsutils.FileCache
pkgCache *pkgcache.Cache
loadGuard *load.Guard
goenv *goutil.Env
loadGuard *load.Guard
}
func NewContextLoader(cfg *config.Config, log logutils.Log, goenv *goutil.Env,
lineCache *fsutils.LineCache, fileCache *fsutils.FileCache, pkgCache *pkgcache.Cache, loadGuard *load.Guard,
) *ContextLoader {
return &ContextLoader{
// NewPackageLoader creates a new PackageLoader.
func NewPackageLoader(log logutils.Log, cfg *config.Config, args []string, goenv *goutil.Env, loadGuard *load.Guard) *PackageLoader {
return &PackageLoader{
cfg: cfg,
args: args,
log: log,
debugf: logutils.Debug(logutils.DebugKeyLoader),
goenv: goenv,
pkgTestIDRe: regexp.MustCompile(`^(.*) \[(.*)\.test\]`),
lineCache: lineCache,
fileCache: fileCache,
pkgCache: pkgCache,
loadGuard: loadGuard,
}
}
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)
// Load loads packages.
func (l *PackageLoader) Load(ctx context.Context, linters []*linter.Config) (pkgs, deduplicatedPkgs []*packages.Package, err error) {
loadMode := findLoadMode(linters)
pkgs, err = l.loadPackages(ctx, loadMode)
if err != nil {
return nil, fmt.Errorf("failed to load packages: %w", err)
return nil, 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
return pkgs, l.filterDuplicatePackages(pkgs), nil
}
func (cl *ContextLoader) prepareBuildContext() {
// Set GOROOT to have working cross-compilation: cross-compiled binaries
// have invalid GOROOT. XXX: can't use runtime.GOROOT().
goroot := cl.goenv.Get(goutil.EnvGoRoot)
if goroot == "" {
return
func (l *PackageLoader) loadPackages(ctx context.Context, loadMode packages.LoadMode) ([]*packages.Package, error) {
defer func(startedAt time.Time) {
l.log.Infof("Go packages loading at mode %s took %s", stringifyLoadMode(loadMode), time.Since(startedAt))
}(time.Now())
l.prepareBuildContext()
conf := &packages.Config{
Mode: loadMode,
Tests: l.cfg.Run.AnalyzeTests,
Context: ctx,
BuildFlags: l.makeBuildFlags(),
Logf: l.debugf,
// TODO: use fset, parsefile, overlay
}
os.Setenv(string(goutil.EnvGoRoot), goroot)
build.Default.GOROOT = goroot
build.Default.BuildTags = cl.cfg.Run.BuildTags
args := l.buildArgs()
l.debugf("Built loader args are %s", args)
pkgs, err := packages.Load(conf, args...)
if err != nil {
return nil, fmt.Errorf("failed to load with go/packages: %w", err)
}
if loadMode&packages.NeedSyntax == 0 {
// Needed e.g. for go/analysis loading.
fset := token.NewFileSet()
packages.Visit(pkgs, nil, func(pkg *packages.Package) {
pkg.Fset = fset
l.loadGuard.AddMutexForPkg(pkg)
})
}
l.debugPrintLoadedPackages(pkgs)
if err := l.parseLoadedPackagesErrors(pkgs); err != nil {
return nil, err
}
return l.filterTestMainPackages(pkgs), nil
}
func (cl *ContextLoader) findLoadMode(linters []*linter.Config) packages.LoadMode {
loadMode := packages.LoadMode(0)
for _, lc := range linters {
loadMode |= lc.LoadMode
}
return loadMode
}
func (cl *ContextLoader) buildArgs() []string {
args := cl.cfg.Run.Args
if len(args) == 0 {
return []string{"./..."}
}
var retArgs []string
for _, arg := range args {
if strings.HasPrefix(arg, ".") || filepath.IsAbs(arg) {
retArgs = append(retArgs, arg)
} else {
// go/packages doesn't work well if we don't have the prefix ./ for local packages
retArgs = append(retArgs, fmt.Sprintf(".%c%s", filepath.Separator, arg))
}
}
return retArgs
}
func (cl *ContextLoader) makeBuildFlags() ([]string, error) {
var buildFlags []string
if len(cl.cfg.Run.BuildTags) != 0 {
// go help build
buildFlags = append(buildFlags, "-tags", strings.Join(cl.cfg.Run.BuildTags, " "))
cl.log.Infof("Using build tags: %v", cl.cfg.Run.BuildTags)
}
mod := cl.cfg.Run.ModulesDownloadMode
if mod != "" {
// go help modules
allowedMods := []string{"mod", "readonly", "vendor"}
var ok bool
for _, am := range allowedMods {
if am == mod {
ok = true
break
}
}
if !ok {
return nil, fmt.Errorf("invalid modules download path %s, only (%s) allowed", mod, strings.Join(allowedMods, "|"))
}
buildFlags = append(buildFlags, fmt.Sprintf("-mod=%s", cl.cfg.Run.ModulesDownloadMode))
}
return buildFlags, nil
}
func stringifyLoadMode(mode packages.LoadMode) string {
m := map[packages.LoadMode]string{
packages.NeedCompiledGoFiles: "compiled_files",
packages.NeedDeps: "deps",
packages.NeedExportFile: "exports_file",
packages.NeedFiles: "files",
packages.NeedImports: "imports",
packages.NeedName: "name",
packages.NeedSyntax: "syntax",
packages.NeedTypes: "types",
packages.NeedTypesInfo: "types_info",
packages.NeedTypesSizes: "types_sizes",
}
var flags []string
for flag, flagStr := range m {
if mode&flag != 0 {
flags = append(flags, flagStr)
}
}
return fmt.Sprintf("%d (%s)", mode, strings.Join(flags, "|"))
}
func (cl *ContextLoader) debugPrintLoadedPackages(pkgs []*packages.Package) {
cl.debugf("loaded %d pkgs", len(pkgs))
for i, pkg := range pkgs {
var syntaxFiles []string
for _, sf := range pkg.Syntax {
syntaxFiles = append(syntaxFiles, pkg.Fset.Position(sf.Pos()).Filename)
}
cl.debugf("Loaded pkg #%d: ID=%s GoFiles=%s CompiledGoFiles=%s Syntax=%s",
i, pkg.ID, pkg.GoFiles, pkg.CompiledGoFiles, syntaxFiles)
}
}
func (cl *ContextLoader) parseLoadedPackagesErrors(pkgs []*packages.Package) error {
func (l *PackageLoader) parseLoadedPackagesErrors(pkgs []*packages.Package) error {
for _, pkg := range pkgs {
var errs []packages.Error
for _, err := range pkg.Errors {
@ -217,54 +131,8 @@ func (cl *ContextLoader) parseLoadedPackagesErrors(pkgs []*packages.Package) err
return nil
}
func (cl *ContextLoader) loadPackages(ctx context.Context, loadMode packages.LoadMode) ([]*packages.Package, error) {
defer func(startedAt time.Time) {
cl.log.Infof("Go packages loading at mode %s took %s", stringifyLoadMode(loadMode), time.Since(startedAt))
}(time.Now())
cl.prepareBuildContext()
buildFlags, err := cl.makeBuildFlags()
if err != nil {
return nil, fmt.Errorf("failed to make build flags for go list: %w", err)
}
conf := &packages.Config{
Mode: loadMode,
Tests: cl.cfg.Run.AnalyzeTests,
Context: ctx,
BuildFlags: buildFlags,
Logf: cl.debugf,
// TODO: use fset, parsefile, overlay
}
args := cl.buildArgs()
cl.debugf("Built loader args are %s", args)
pkgs, err := packages.Load(conf, args...)
if err != nil {
return nil, fmt.Errorf("failed to load with go/packages: %w", err)
}
if loadMode&packages.NeedSyntax == 0 {
// Needed e.g. for go/analysis loading.
fset := token.NewFileSet()
packages.Visit(pkgs, nil, func(pkg *packages.Package) {
pkg.Fset = fset
cl.loadGuard.AddMutexForPkg(pkg)
})
}
cl.debugPrintLoadedPackages(pkgs)
if err := cl.parseLoadedPackagesErrors(pkgs); err != nil {
return nil, err
}
return cl.filterTestMainPackages(pkgs), nil
}
func (cl *ContextLoader) tryParseTestPackage(pkg *packages.Package) (name string, isTest bool) {
matches := cl.pkgTestIDRe.FindStringSubmatch(pkg.ID)
func (l *PackageLoader) tryParseTestPackage(pkg *packages.Package) (name string, isTest bool) {
matches := l.pkgTestIDRe.FindStringSubmatch(pkg.ID)
if matches == nil {
return "", false
}
@ -272,36 +140,21 @@ func (cl *ContextLoader) tryParseTestPackage(pkg *packages.Package) (name string
return matches[1], true
}
func (cl *ContextLoader) filterTestMainPackages(pkgs []*packages.Package) []*packages.Package {
var retPkgs []*packages.Package
for _, pkg := range pkgs {
if pkg.Name == "main" && strings.HasSuffix(pkg.PkgPath, ".test") {
// it's an implicit testmain package
cl.debugf("skip pkg ID=%s", pkg.ID)
continue
}
retPkgs = append(retPkgs, pkg)
}
return retPkgs
}
func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*packages.Package {
func (l *PackageLoader) filterDuplicatePackages(pkgs []*packages.Package) []*packages.Package {
packagesWithTests := map[string]bool{}
for _, pkg := range pkgs {
name, isTest := cl.tryParseTestPackage(pkg)
name, isTest := l.tryParseTestPackage(pkg)
if !isTest {
continue
}
packagesWithTests[name] = true
}
cl.debugf("package with tests: %#v", packagesWithTests)
l.debugf("package with tests: %#v", packagesWithTests)
var retPkgs []*packages.Package
for _, pkg := range pkgs {
_, isTest := cl.tryParseTestPackage(pkg)
_, isTest := l.tryParseTestPackage(pkg)
if !isTest && packagesWithTests[pkg.PkgPath] {
// If tests loading is enabled,
// for package with files a.go and a_test.go go/packages loads two packages:
@ -309,7 +162,7 @@ func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*pa
// 2. ID=".../a [.../a.test]" GoFiles=[a.go a_test.go]
// We need only the second package, otherwise we can get warnings about unused variables/fields/functions
// in a.go if they are used only in a_test.go.
cl.debugf("skip pkg ID=%s because we load it with test package", pkg.ID)
l.debugf("skip pkg ID=%s because we load it with test package", pkg.ID)
continue
}
@ -318,3 +171,111 @@ func (cl *ContextLoader) filterDuplicatePackages(pkgs []*packages.Package) []*pa
return retPkgs
}
func (l *PackageLoader) filterTestMainPackages(pkgs []*packages.Package) []*packages.Package {
var retPkgs []*packages.Package
for _, pkg := range pkgs {
if pkg.Name == "main" && strings.HasSuffix(pkg.PkgPath, ".test") {
// it's an implicit testmain package
l.debugf("skip pkg ID=%s", pkg.ID)
continue
}
retPkgs = append(retPkgs, pkg)
}
return retPkgs
}
func (l *PackageLoader) debugPrintLoadedPackages(pkgs []*packages.Package) {
l.debugf("loaded %d pkgs", len(pkgs))
for i, pkg := range pkgs {
var syntaxFiles []string
for _, sf := range pkg.Syntax {
syntaxFiles = append(syntaxFiles, pkg.Fset.Position(sf.Pos()).Filename)
}
l.debugf("Loaded pkg #%d: ID=%s GoFiles=%s CompiledGoFiles=%s Syntax=%s",
i, pkg.ID, pkg.GoFiles, pkg.CompiledGoFiles, syntaxFiles)
}
}
func (l *PackageLoader) prepareBuildContext() {
// Set GOROOT to have working cross-compilation: cross-compiled binaries
// have invalid GOROOT. XXX: can't use runtime.GOROOT().
goroot := l.goenv.Get(goutil.EnvGoRoot)
if goroot == "" {
return
}
os.Setenv(string(goutil.EnvGoRoot), goroot)
build.Default.GOROOT = goroot
build.Default.BuildTags = l.cfg.Run.BuildTags
}
func (l *PackageLoader) buildArgs() []string {
if len(l.args) == 0 {
return []string{"./..."}
}
var retArgs []string
for _, arg := range l.args {
if strings.HasPrefix(arg, ".") || filepath.IsAbs(arg) {
retArgs = append(retArgs, arg)
} else {
// go/packages doesn't work well if we don't have the prefix ./ for local packages
retArgs = append(retArgs, fmt.Sprintf(".%c%s", filepath.Separator, arg))
}
}
return retArgs
}
func (l *PackageLoader) makeBuildFlags() []string {
var buildFlags []string
if len(l.cfg.Run.BuildTags) != 0 {
// go help build
buildFlags = append(buildFlags, "-tags", strings.Join(l.cfg.Run.BuildTags, " "))
l.log.Infof("Using build tags: %v", l.cfg.Run.BuildTags)
}
if l.cfg.Run.ModulesDownloadMode != "" {
// go help modules
buildFlags = append(buildFlags, fmt.Sprintf("-mod=%s", l.cfg.Run.ModulesDownloadMode))
}
return buildFlags
}
func findLoadMode(linters []*linter.Config) packages.LoadMode {
loadMode := packages.LoadMode(0)
for _, lc := range linters {
loadMode |= lc.LoadMode
}
return loadMode
}
func stringifyLoadMode(mode packages.LoadMode) string {
m := map[packages.LoadMode]string{
packages.NeedCompiledGoFiles: "compiled_files",
packages.NeedDeps: "deps",
packages.NeedExportFile: "exports_file",
packages.NeedFiles: "files",
packages.NeedImports: "imports",
packages.NeedName: "name",
packages.NeedSyntax: "syntax",
packages.NeedTypes: "types",
packages.NeedTypesInfo: "types_info",
packages.NeedTypesSizes: "types_sizes",
}
var flags []string
for flag, flagStr := range m {
if mode&flag != 0 {
flags = append(flags, flagStr)
}
}
return fmt.Sprintf("%d (%s)", mode, strings.Join(flags, "|"))
}