From 0df1bb0860f7b25091b9227cccd45f69a28ab37a Mon Sep 17 00:00:00 2001 From: dominiquelefevre <54854047+dominiquelefevre@users.noreply.github.com> Date: Sat, 22 Jun 2024 18:12:49 +0300 Subject: [PATCH] Fix a performance regression #995 (#998) * Support go workspaces when detecting the go version. When a module is part of a workspace, a call to `go list -m` lists all modules in the workspace, and we need to parse multiple modinfos. * Do not invoke `go list` for every package. * Add a go language version override config option for golangci-lint. --- lint/config.go | 7 ++++ lint/linter.go | 98 +++++++++++++++++++++++++++++++++----------------- 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/lint/config.go b/lint/config.go index 7e51a93..d7ecd96 100644 --- a/lint/config.go +++ b/lint/config.go @@ -1,5 +1,9 @@ package lint +import ( + goversion "github.com/hashicorp/go-version" +) + // Arguments is type used for the arguments of a rule. type Arguments = []interface{} @@ -61,4 +65,7 @@ type Config struct { WarningCode int `toml:"warningCode"` Directives DirectivesConfig `toml:"directive"` Exclude []string `toml:"exclude"` + // If set, overrides the go language version specified in go.mod of + // packages being linted, and assumes this specific language version. + GoVersion *goversion.Version } diff --git a/lint/linter.go b/lint/linter.go index 18ab523..3c97f30 100644 --- a/lint/linter.go +++ b/lint/linter.go @@ -63,16 +63,52 @@ var ( func (l *Linter) Lint(packages [][]string, ruleSet []Rule, config Config) (<-chan Failure, error) { failures := make(chan Failure) + perModVersions := make(map[string]*goversion.Version) + perPkgVersions := make([]*goversion.Version, len(packages)) + for n, files := range packages { + if len(files) == 0 { + continue + } + if config.GoVersion != nil { + perPkgVersions[n] = config.GoVersion + continue + } + + dir, err := filepath.Abs(filepath.Dir(files[0])) + if err != nil { + return nil, err + } + + alreadyKnownMod := false + for d, v := range perModVersions { + if strings.HasPrefix(dir, d) { + perPkgVersions[n] = v + alreadyKnownMod = true + break + } + } + if alreadyKnownMod { + continue + } + + d, v, err := detectGoMod(dir) + if err != nil { + return nil, err + } + perModVersions[d] = v + perPkgVersions[n] = v + } + var wg sync.WaitGroup - for _, pkg := range packages { + for n := range packages { wg.Add(1) - go func(pkg []string) { - if err := l.lintPackage(pkg, ruleSet, config, failures); err != nil { + go func(pkg []string, gover *goversion.Version) { + if err := l.lintPackage(pkg, gover, ruleSet, config, failures); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } defer wg.Done() - }(pkg) + }(packages[n], perPkgVersions[n]) } go func() { @@ -83,20 +119,15 @@ func (l *Linter) Lint(packages [][]string, ruleSet []Rule, config Config) (<-cha return failures, nil } -func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, failures chan Failure) error { +func (l *Linter) lintPackage(filenames []string, gover *goversion.Version, ruleSet []Rule, config Config, failures chan Failure) error { if len(filenames) == 0 { return nil } - goVersion, err := detectGoVersion(filepath.Dir(filenames[0])) - if err != nil { - return err - } - pkg := &Package{ fset: token.NewFileSet(), files: map[string]*File{}, - goVersion: goVersion, + goVersion: gover, } for _, filename := range filenames { content, err := l.readFile(filename) @@ -124,37 +155,38 @@ func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, return nil } -func detectGoVersion(dir string) (ver *goversion.Version, err error) { +func detectGoMod(dir string) (rootDir string, ver *goversion.Version, err error) { // https://github.com/golang/go/issues/44753#issuecomment-790089020 cmd := exec.Command("go", "list", "-m", "-json") cmd.Dir = dir - raw, err := cmd.Output() + out, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("command go list: %w", err) + return "", nil, fmt.Errorf("command go list: %w", err) } - var v struct { - GoMod string `json:"GoMod"` - GoVersion string `json:"GoVersion"` - } - if err = json.Unmarshal(raw, &v); err != nil { - return nil, fmt.Errorf("can't parse the output of go list: %w", err) - } - - if v.GoMod == "" { - // this package is outside a module, so assume - // an old-style source directory - - if v := os.Getenv("GOVERSION"); v != "" { - return goversion.NewVersion(strings.TrimPrefix(v, "go")) + // NOTE: A package may be part of a go workspace. In this case `go list -m` + // lists all modules in the workspace, so we need to go through them all. + d := json.NewDecoder(bytes.NewBuffer(out)) + for d.More() { + var v struct { + GoMod string `json:"GoMod"` + GoVersion string `json:"GoVersion"` + Dir string `json:"Dir"` + } + if err = d.Decode(&v); err != nil { + return "", nil, err + } + if v.GoMod == "" { + return "", nil, fmt.Errorf("not part of a module: %q", dir) + } + if v.Dir != "" && strings.HasPrefix(dir, v.Dir) { + rootDir = v.Dir + ver, err = goversion.NewVersion(strings.TrimPrefix(v.GoVersion, "go")) + return rootDir, ver, err } - - // assume the last version that does not have generics - return goversion.Must(goversion.NewVersion("1.17")), nil } - - return goversion.NewVersion(strings.TrimPrefix(v.GoVersion, "go")) + return "", nil, fmt.Errorf("not part of a module: %q", dir) } // isGenerated reports whether the source file is generated code