From 8077a19cfb0a8d67d0e1835838af8d70863c0306 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 15:45:05 +0100 Subject: [PATCH 01/19] chore(runners.go): extract cache related elements --- pkg/goanalysis/runners.go | 162 ------------------------------ pkg/goanalysis/runners_cache.go | 172 ++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 162 deletions(-) create mode 100644 pkg/goanalysis/runners_cache.go diff --git a/pkg/goanalysis/runners.go b/pkg/goanalysis/runners.go index 033cba81d5de..a9aee03a2bf8 100644 --- a/pkg/goanalysis/runners.go +++ b/pkg/goanalysis/runners.go @@ -2,17 +2,10 @@ package goanalysis import ( "fmt" - "runtime" - "sort" - "strings" - "sync" - "sync/atomic" - "time" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" - "github.com/golangci/golangci-lint/internal/cache" "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/logutils" @@ -119,158 +112,3 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st } return issues } - -func getIssuesCacheKey(analyzers []*analysis.Analyzer) string { - return "lint/result:" + analyzersHashID(analyzers) -} - -func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.Package]bool, - issues []result.Issue, lintCtx *linter.Context, analyzers []*analysis.Analyzer, -) { - startedAt := time.Now() - perPkgIssues := map[*packages.Package][]result.Issue{} - for ind := range issues { - i := &issues[ind] - perPkgIssues[i.Pkg] = append(perPkgIssues[i.Pkg], *i) - } - - var savedIssuesCount int64 = 0 - lintResKey := getIssuesCacheKey(analyzers) - - workerCount := runtime.GOMAXPROCS(-1) - var wg sync.WaitGroup - wg.Add(workerCount) - - pkgCh := make(chan *packages.Package, len(allPkgs)) - for i := 0; i < workerCount; i++ { - go func() { - defer wg.Done() - for pkg := range pkgCh { - pkgIssues := perPkgIssues[pkg] - encodedIssues := make([]EncodingIssue, 0, len(pkgIssues)) - for ind := range pkgIssues { - i := &pkgIssues[ind] - encodedIssues = append(encodedIssues, EncodingIssue{ - FromLinter: i.FromLinter, - Text: i.Text, - Severity: i.Severity, - Pos: i.Pos, - LineRange: i.LineRange, - Replacement: i.Replacement, - ExpectNoLint: i.ExpectNoLint, - ExpectedNoLintLinter: i.ExpectedNoLintLinter, - }) - } - - atomic.AddInt64(&savedIssuesCount, int64(len(encodedIssues))) - if err := lintCtx.PkgCache.Put(pkg, cache.HashModeNeedAllDeps, lintResKey, encodedIssues); err != nil { - lintCtx.Log.Infof("Failed to save package %s issues (%d) to cache: %s", pkg, len(pkgIssues), err) - } else { - issuesCacheDebugf("Saved package %s issues (%d) to cache", pkg, len(pkgIssues)) - } - } - }() - } - - for _, pkg := range allPkgs { - if pkgsFromCache[pkg] { - continue - } - - pkgCh <- pkg - } - close(pkgCh) - wg.Wait() - - lintCtx.PkgCache.Close() - - issuesCacheDebugf("Saved %d issues from %d packages to cache in %s", savedIssuesCount, len(allPkgs), time.Since(startedAt)) -} - -func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context, - analyzers []*analysis.Analyzer, -) (issuesFromCache []result.Issue, pkgsFromCache map[*packages.Package]bool) { - startedAt := time.Now() - - lintResKey := getIssuesCacheKey(analyzers) - type cacheRes struct { - issues []result.Issue - loadErr error - } - pkgToCacheRes := make(map[*packages.Package]*cacheRes, len(pkgs)) - for _, pkg := range pkgs { - pkgToCacheRes[pkg] = &cacheRes{} - } - - workerCount := runtime.GOMAXPROCS(-1) - var wg sync.WaitGroup - wg.Add(workerCount) - - pkgCh := make(chan *packages.Package, len(pkgs)) - for range workerCount { - go func() { - defer wg.Done() - for pkg := range pkgCh { - var pkgIssues []EncodingIssue - err := lintCtx.PkgCache.Get(pkg, cache.HashModeNeedAllDeps, lintResKey, &pkgIssues) - cacheRes := pkgToCacheRes[pkg] - cacheRes.loadErr = err - if err != nil { - continue - } - if len(pkgIssues) == 0 { - continue - } - - issues := make([]result.Issue, 0, len(pkgIssues)) - for i := range pkgIssues { - issue := &pkgIssues[i] - issues = append(issues, result.Issue{ - FromLinter: issue.FromLinter, - Text: issue.Text, - Severity: issue.Severity, - Pos: issue.Pos, - LineRange: issue.LineRange, - Replacement: issue.Replacement, - Pkg: pkg, - ExpectNoLint: issue.ExpectNoLint, - ExpectedNoLintLinter: issue.ExpectedNoLintLinter, - }) - } - cacheRes.issues = issues - } - }() - } - - for _, pkg := range pkgs { - pkgCh <- pkg - } - close(pkgCh) - wg.Wait() - - loadedIssuesCount := 0 - pkgsFromCache = map[*packages.Package]bool{} - for pkg, cacheRes := range pkgToCacheRes { - if cacheRes.loadErr == nil { - loadedIssuesCount += len(cacheRes.issues) - pkgsFromCache[pkg] = true - issuesFromCache = append(issuesFromCache, cacheRes.issues...) - issuesCacheDebugf("Loaded package %s issues (%d) from cache", pkg, len(cacheRes.issues)) - } else { - issuesCacheDebugf("Didn't load package %s issues from cache: %s", pkg, cacheRes.loadErr) - } - } - issuesCacheDebugf("Loaded %d issues from cache in %s, analyzing %d/%d packages", - loadedIssuesCount, time.Since(startedAt), len(pkgs)-len(pkgsFromCache), len(pkgs)) - return issuesFromCache, pkgsFromCache -} - -func analyzersHashID(analyzers []*analysis.Analyzer) string { - names := make([]string, 0, len(analyzers)) - for _, a := range analyzers { - names = append(names, a.Name) - } - - sort.Strings(names) - return strings.Join(names, ",") -} diff --git a/pkg/goanalysis/runners_cache.go b/pkg/goanalysis/runners_cache.go new file mode 100644 index 000000000000..8c244688b42d --- /dev/null +++ b/pkg/goanalysis/runners_cache.go @@ -0,0 +1,172 @@ +package goanalysis + +import ( + "runtime" + "sort" + "strings" + "sync" + "sync/atomic" + "time" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/packages" + + "github.com/golangci/golangci-lint/internal/cache" + "github.com/golangci/golangci-lint/pkg/lint/linter" + "github.com/golangci/golangci-lint/pkg/result" +) + +func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.Package]bool, + issues []result.Issue, lintCtx *linter.Context, analyzers []*analysis.Analyzer, +) { + startedAt := time.Now() + perPkgIssues := map[*packages.Package][]result.Issue{} + for ind := range issues { + i := &issues[ind] + perPkgIssues[i.Pkg] = append(perPkgIssues[i.Pkg], *i) + } + + var savedIssuesCount int64 = 0 + lintResKey := getIssuesCacheKey(analyzers) + + workerCount := runtime.GOMAXPROCS(-1) + var wg sync.WaitGroup + wg.Add(workerCount) + + pkgCh := make(chan *packages.Package, len(allPkgs)) + for i := 0; i < workerCount; i++ { + go func() { + defer wg.Done() + for pkg := range pkgCh { + pkgIssues := perPkgIssues[pkg] + encodedIssues := make([]EncodingIssue, 0, len(pkgIssues)) + for ind := range pkgIssues { + i := &pkgIssues[ind] + encodedIssues = append(encodedIssues, EncodingIssue{ + FromLinter: i.FromLinter, + Text: i.Text, + Severity: i.Severity, + Pos: i.Pos, + LineRange: i.LineRange, + Replacement: i.Replacement, + ExpectNoLint: i.ExpectNoLint, + ExpectedNoLintLinter: i.ExpectedNoLintLinter, + }) + } + + atomic.AddInt64(&savedIssuesCount, int64(len(encodedIssues))) + if err := lintCtx.PkgCache.Put(pkg, cache.HashModeNeedAllDeps, lintResKey, encodedIssues); err != nil { + lintCtx.Log.Infof("Failed to save package %s issues (%d) to cache: %s", pkg, len(pkgIssues), err) + } else { + issuesCacheDebugf("Saved package %s issues (%d) to cache", pkg, len(pkgIssues)) + } + } + }() + } + + for _, pkg := range allPkgs { + if pkgsFromCache[pkg] { + continue + } + + pkgCh <- pkg + } + close(pkgCh) + wg.Wait() + + lintCtx.PkgCache.Close() + + issuesCacheDebugf("Saved %d issues from %d packages to cache in %s", savedIssuesCount, len(allPkgs), time.Since(startedAt)) +} + +func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context, + analyzers []*analysis.Analyzer, +) (issuesFromCache []result.Issue, pkgsFromCache map[*packages.Package]bool) { + startedAt := time.Now() + + lintResKey := getIssuesCacheKey(analyzers) + type cacheRes struct { + issues []result.Issue + loadErr error + } + pkgToCacheRes := make(map[*packages.Package]*cacheRes, len(pkgs)) + for _, pkg := range pkgs { + pkgToCacheRes[pkg] = &cacheRes{} + } + + workerCount := runtime.GOMAXPROCS(-1) + var wg sync.WaitGroup + wg.Add(workerCount) + + pkgCh := make(chan *packages.Package, len(pkgs)) + for range workerCount { + go func() { + defer wg.Done() + for pkg := range pkgCh { + var pkgIssues []EncodingIssue + err := lintCtx.PkgCache.Get(pkg, cache.HashModeNeedAllDeps, lintResKey, &pkgIssues) + cacheRes := pkgToCacheRes[pkg] + cacheRes.loadErr = err + if err != nil { + continue + } + if len(pkgIssues) == 0 { + continue + } + + issues := make([]result.Issue, 0, len(pkgIssues)) + for i := range pkgIssues { + issue := &pkgIssues[i] + issues = append(issues, result.Issue{ + FromLinter: issue.FromLinter, + Text: issue.Text, + Severity: issue.Severity, + Pos: issue.Pos, + LineRange: issue.LineRange, + Replacement: issue.Replacement, + Pkg: pkg, + ExpectNoLint: issue.ExpectNoLint, + ExpectedNoLintLinter: issue.ExpectedNoLintLinter, + }) + } + cacheRes.issues = issues + } + }() + } + + for _, pkg := range pkgs { + pkgCh <- pkg + } + close(pkgCh) + wg.Wait() + + loadedIssuesCount := 0 + pkgsFromCache = map[*packages.Package]bool{} + for pkg, cacheRes := range pkgToCacheRes { + if cacheRes.loadErr == nil { + loadedIssuesCount += len(cacheRes.issues) + pkgsFromCache[pkg] = true + issuesFromCache = append(issuesFromCache, cacheRes.issues...) + issuesCacheDebugf("Loaded package %s issues (%d) from cache", pkg, len(cacheRes.issues)) + } else { + issuesCacheDebugf("Didn't load package %s issues from cache: %s", pkg, cacheRes.loadErr) + } + } + issuesCacheDebugf("Loaded %d issues from cache in %s, analyzing %d/%d packages", + loadedIssuesCount, time.Since(startedAt), len(pkgs)-len(pkgsFromCache), len(pkgs)) + return issuesFromCache, pkgsFromCache +} + +func getIssuesCacheKey(analyzers []*analysis.Analyzer) string { + return "lint/result:" + analyzersHashID(analyzers) +} + +func analyzersHashID(analyzers []*analysis.Analyzer) string { + names := make([]string, 0, len(analyzers)) + for _, a := range analyzers { + names = append(names, a.Name) + } + + sort.Strings(names) + return strings.Join(names, ",") +} From b87262beedb9cf9f9aef0dd6a937ef4fce0efe82 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 15:46:27 +0100 Subject: [PATCH 02/19] chore(runner_action.go): extract cache related elements --- pkg/goanalysis/runner_action.go | 109 ----------------------- pkg/goanalysis/runner_action_cache.go | 123 ++++++++++++++++++++++++++ pkg/goanalysis/runner_facts.go | 5 -- 3 files changed, 123 insertions(+), 114 deletions(-) create mode 100644 pkg/goanalysis/runner_action_cache.go diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index 0ef8c094c772..b6cf12bb483b 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -4,16 +4,13 @@ import ( "errors" "fmt" "go/types" - "io" "reflect" "runtime/debug" "time" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" - "golang.org/x/tools/go/types/objectpath" - "github.com/golangci/golangci-lint/internal/cache" "github.com/golangci/golangci-lint/internal/errorutil" "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" ) @@ -66,27 +63,6 @@ func (act *action) String() string { return fmt.Sprintf("%s@%s", act.a, act.pkg) } -func (act *action) loadCachedFacts() bool { - if act.loadCachedFactsDone { // can't be set in parallel - return act.loadCachedFactsOk - } - - res := func() bool { - if act.isInitialPkg { - return true // load cached facts only for non-initial packages - } - - if len(act.a.FactTypes) == 0 { - return true // no need to load facts - } - - return act.loadPersistedFacts() - }() - act.loadCachedFactsDone = true - act.loadCachedFactsOk = res - return res -} - func (act *action) waitUntilDependingAnalyzersWorked() { for _, dep := range act.deps { if dep.pkg == act.pkg { @@ -287,91 +263,6 @@ func (act *action) factType(fact analysis.Fact) reflect.Type { return t } -func (act *action) persistFactsToCache() error { - analyzer := act.a - if len(analyzer.FactTypes) == 0 { - return nil - } - - // Merge new facts into the package and persist them. - var facts []Fact - for key, fact := range act.packageFacts { - if key.pkg != act.pkg.Types { - // The fact is from inherited facts from another package - continue - } - facts = append(facts, Fact{ - Path: "", - Fact: fact, - }) - } - for key, fact := range act.objectFacts { - obj := key.obj - if obj.Pkg() != act.pkg.Types { - // The fact is from inherited facts from another package - continue - } - - path, err := objectpath.For(obj) - if err != nil { - // The object is not globally addressable - continue - } - - facts = append(facts, Fact{ - Path: string(path), - Fact: fact, - }) - } - - factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name) - - key := fmt.Sprintf("%s/facts", analyzer.Name) - return act.r.pkgCache.Put(act.pkg, cache.HashModeNeedAllDeps, key, facts) -} - -func (act *action) loadPersistedFacts() bool { - var facts []Fact - key := fmt.Sprintf("%s/facts", act.a.Name) - if err := act.r.pkgCache.Get(act.pkg, cache.HashModeNeedAllDeps, key, &facts); err != nil { - if !errors.Is(err, cache.ErrMissing) && !errors.Is(err, io.EOF) { - act.r.log.Warnf("Failed to get persisted facts: %s", err) - } - - factsCacheDebugf("No cached facts for package %q and analyzer %s", act.pkg.Name, act.a.Name) - return false - } - - factsCacheDebugf("Loaded %d cached facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name) - - for _, f := range facts { - if f.Path == "" { // this is a package fact - key := packageFactKey{act.pkg.Types, act.factType(f.Fact)} - act.packageFacts[key] = f.Fact - continue - } - obj, err := objectpath.Object(act.pkg.Types, objectpath.Path(f.Path)) - if err != nil { - // Be lenient about these errors. For example, when - // analyzing io/ioutil from source, we may get a fact - // for methods on the devNull type, and objectpath - // will happily create a path for them. However, when - // we later load io/ioutil from export data, the path - // no longer resolves. - // - // If an exported type embeds the unexported type, - // then (part of) the unexported type will become part - // of the type information and our path will resolve - // again. - continue - } - factKey := objectFactKey{obj, act.factType(f.Fact)} - act.objectFacts[factKey] = f.Fact - } - - return true -} - func (act *action) markDepsForAnalyzingSource() { // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing // this action. diff --git a/pkg/goanalysis/runner_action_cache.go b/pkg/goanalysis/runner_action_cache.go new file mode 100644 index 000000000000..6ed1270dc80a --- /dev/null +++ b/pkg/goanalysis/runner_action_cache.go @@ -0,0 +1,123 @@ +package goanalysis + +import ( + "errors" + "fmt" + "io" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/types/objectpath" + + "github.com/golangci/golangci-lint/internal/cache" +) + +type Fact struct { + Path string // non-empty only for object facts + Fact analysis.Fact +} + +func (act *action) loadCachedFacts() bool { + if act.loadCachedFactsDone { // can't be set in parallel + return act.loadCachedFactsOk + } + + res := func() bool { + if act.isInitialPkg { + return true // load cached facts only for non-initial packages + } + + if len(act.a.FactTypes) == 0 { + return true // no need to load facts + } + + return act.loadPersistedFacts() + }() + act.loadCachedFactsDone = true + act.loadCachedFactsOk = res + return res +} + +func (act *action) persistFactsToCache() error { + analyzer := act.a + if len(analyzer.FactTypes) == 0 { + return nil + } + + // Merge new facts into the package and persist them. + var facts []Fact + for key, fact := range act.packageFacts { + if key.pkg != act.pkg.Types { + // The fact is from inherited facts from another package + continue + } + facts = append(facts, Fact{ + Path: "", + Fact: fact, + }) + } + for key, fact := range act.objectFacts { + obj := key.obj + if obj.Pkg() != act.pkg.Types { + // The fact is from inherited facts from another package + continue + } + + path, err := objectpath.For(obj) + if err != nil { + // The object is not globally addressable + continue + } + + facts = append(facts, Fact{ + Path: string(path), + Fact: fact, + }) + } + + factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name) + + key := fmt.Sprintf("%s/facts", analyzer.Name) + return act.r.pkgCache.Put(act.pkg, cache.HashModeNeedAllDeps, key, facts) +} + +func (act *action) loadPersistedFacts() bool { + var facts []Fact + key := fmt.Sprintf("%s/facts", act.a.Name) + if err := act.r.pkgCache.Get(act.pkg, cache.HashModeNeedAllDeps, key, &facts); err != nil { + if !errors.Is(err, cache.ErrMissing) && !errors.Is(err, io.EOF) { + act.r.log.Warnf("Failed to get persisted facts: %s", err) + } + + factsCacheDebugf("No cached facts for package %q and analyzer %s", act.pkg.Name, act.a.Name) + return false + } + + factsCacheDebugf("Loaded %d cached facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name) + + for _, f := range facts { + if f.Path == "" { // this is a package fact + key := packageFactKey{act.pkg.Types, act.factType(f.Fact)} + act.packageFacts[key] = f.Fact + continue + } + obj, err := objectpath.Object(act.pkg.Types, objectpath.Path(f.Path)) + if err != nil { + // Be lenient about these errors. For example, when + // analyzing io/ioutil from source, we may get a fact + // for methods on the devNull type, and objectpath + // will happily create a path for them. However, when + // we later load io/ioutil from export data, the path + // no longer resolves. + // + // If an exported type embeds the unexported type, + // then (part of) the unexported type will become part + // of the type information and our path will resolve + // again. + continue + } + factKey := objectFactKey{obj, act.factType(f.Fact)} + act.objectFacts[factKey] = f.Fact + } + + return true +} diff --git a/pkg/goanalysis/runner_facts.go b/pkg/goanalysis/runner_facts.go index 1d0fb974e752..c8e262a14b83 100644 --- a/pkg/goanalysis/runner_facts.go +++ b/pkg/goanalysis/runner_facts.go @@ -20,11 +20,6 @@ type packageFactKey struct { typ reflect.Type } -type Fact struct { - Path string // non-empty only for object facts - Fact analysis.Fact -} - // inheritFacts populates act.facts with // those it obtains from its dependency, dep. func inheritFacts(act, dep *action) { From 2fe04a53bac09aedc869fae1ca790b3518dc29f9 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:10:30 +0100 Subject: [PATCH 03/19] chore: add new base file --- pkg/goanalysis/runner.go | 5 ----- pkg/goanalysis/runner_base.go | 8 ++++++++ 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 pkg/goanalysis/runner_base.go diff --git a/pkg/goanalysis/runner.go b/pkg/goanalysis/runner.go index dfcdcbaaa4d5..ac03c71ecc2d 100644 --- a/pkg/goanalysis/runner.go +++ b/pkg/goanalysis/runner.go @@ -1,8 +1,3 @@ -// checker is a partial copy of https://github.com/golang/tools/blob/master/go/analysis/internal/checker -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - // Package goanalysis defines the implementation of the checker commands. // The same code drives the multi-analysis driver, the single-analysis // driver that is conventionally provided for convenience along with diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go new file mode 100644 index 000000000000..fb7a410bdc36 --- /dev/null +++ b/pkg/goanalysis/runner_base.go @@ -0,0 +1,8 @@ +// Copyright 2018 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. +// +// Partial copy of https://github.com/golang/tools/blob/master/go/analysis/internal/checker +// FIXME add a commit hash. + +package goanalysis From 76047f2cff7d5444d2bf88a76f818b9ba6cda2ce Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:11:01 +0100 Subject: [PATCH 04/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 11 ----------- pkg/goanalysis/runner_base.go | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index b6cf12bb483b..0805ed39a3a1 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -244,17 +244,6 @@ func (act *action) exportPackageFact(fact analysis.Fact) { act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact) } -func (act *action) allPackageFacts() []analysis.PackageFact { - out := make([]analysis.PackageFact, 0, len(act.packageFacts)) - for key, fact := range act.packageFacts { - out = append(out, analysis.PackageFact{ - Package: key.pkg, - Fact: fact, - }) - } - return out -} - func (act *action) factType(fact analysis.Fact) reflect.Type { t := reflect.TypeOf(fact) if t.Kind() != reflect.Ptr { diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index fb7a410bdc36..42ae65ccc21f 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -6,3 +6,17 @@ // FIXME add a commit hash. package goanalysis + +import "golang.org/x/tools/go/analysis" + +// NOTE(ldez) no alteration. +func (act *action) allPackageFacts() []analysis.PackageFact { + out := make([]analysis.PackageFact, 0, len(act.packageFacts)) + for key, fact := range act.packageFacts { + out = append(out, analysis.PackageFact{ + Package: key.pkg, + Fact: fact, + }) + } + return out +} From 904d637666be69fde199089ad8c4c0cf07be3f14 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:11:53 +0100 Subject: [PATCH 05/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 8 -------- pkg/goanalysis/runner_base.go | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index 0805ed39a3a1..f4ceb2370b2e 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -244,14 +244,6 @@ func (act *action) exportPackageFact(fact analysis.Fact) { act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact) } -func (act *action) factType(fact analysis.Fact) reflect.Type { - t := reflect.TypeOf(fact) - if t.Kind() != reflect.Ptr { - act.r.log.Fatalf("invalid Fact type: got %T, want pointer", t) - } - return t -} - func (act *action) markDepsForAnalyzingSource() { // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing // this action. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 42ae65ccc21f..6bc54addd3e1 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -7,7 +7,20 @@ package goanalysis -import "golang.org/x/tools/go/analysis" +import ( + "reflect" + + "golang.org/x/tools/go/analysis" +) + +// NOTE(ldez) altered: add receiver to handle logs. +func (act *action) factType(fact analysis.Fact) reflect.Type { + t := reflect.TypeOf(fact) + if t.Kind() != reflect.Ptr { + act.r.log.Fatalf("invalid Fact type: got %T, want pointer", t) + } + return t +} // NOTE(ldez) no alteration. func (act *action) allPackageFacts() []analysis.PackageFact { From e3153377d6332da75db909dfe6853524d3c649d2 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:12:38 +0100 Subject: [PATCH 06/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 8 -------- pkg/goanalysis/runner_base.go | 9 +++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index f4ceb2370b2e..84235a7094b3 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -236,14 +236,6 @@ func (act *action) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool return false } -// exportPackageFact implements Pass.ExportPackageFact. -func (act *action) exportPackageFact(fact analysis.Fact) { - key := packageFactKey{act.pass.Pkg, act.factType(fact)} - act.packageFacts[key] = fact // clobber any existing entry - factsDebugf("%s: package %s has fact %s\n", - act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact) -} - func (act *action) markDepsForAnalyzingSource() { // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing // this action. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 6bc54addd3e1..84b1c13ba727 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -13,6 +13,15 @@ import ( "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; act.factType. +// exportPackageFact implements Pass.ExportPackageFact. +func (act *action) exportPackageFact(fact analysis.Fact) { + key := packageFactKey{act.pass.Pkg, act.factType(fact)} + act.packageFacts[key] = fact // clobber any existing entry + factsDebugf("%s: package %s has fact %s\n", + act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact) +} + // NOTE(ldez) altered: add receiver to handle logs. func (act *action) factType(fact analysis.Fact) reflect.Type { t := reflect.TypeOf(fact) From 64951cf43854a94bd3b37b921f6baf6a4b290f24 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:13:07 +0100 Subject: [PATCH 07/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 11 ----------- pkg/goanalysis/runner_base.go | 14 +++++++++++++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index 84235a7094b3..f8e5c06918ec 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -210,17 +210,6 @@ func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { } } -func (act *action) allObjectFacts() []analysis.ObjectFact { - out := make([]analysis.ObjectFact, 0, len(act.objectFacts)) - for key, fact := range act.objectFacts { - out = append(out, analysis.ObjectFact{ - Object: key.obj, - Fact: fact, - }) - } - return out -} - // importPackageFact implements Pass.ImportPackageFact. // Given a non-nil pointer ptr of type *T, where *T satisfies Fact, // fact copies the fact value to *ptr. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 84b1c13ba727..009b01c95484 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -13,7 +13,19 @@ import ( "golang.org/x/tools/go/analysis" ) -// NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; act.factType. +// NOTE(ldez) no alteration. +func (act *action) allObjectFacts() []analysis.ObjectFact { + out := make([]analysis.ObjectFact, 0, len(act.objectFacts)) + for key, fact := range act.objectFacts { + out = append(out, analysis.ObjectFact{ + Object: key.obj, + Fact: fact, + }) + } + return out +} + +// NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; `act.factType`. // exportPackageFact implements Pass.ExportPackageFact. func (act *action) exportPackageFact(fact analysis.Fact) { key := packageFactKey{act.pass.Pkg, act.factType(fact)} From 9ffeb58bcf3c6c4cfe7aad833afdf0988321de7d Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:15:29 +0100 Subject: [PATCH 08/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 16 ---------------- pkg/goanalysis/runner_base.go | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index f8e5c06918ec..c8f1e524df2d 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -194,22 +194,6 @@ func (act *action) importObjectFact(obj types.Object, ptr analysis.Fact) bool { return false } -// exportObjectFact implements Pass.ExportObjectFact. -func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { - if obj.Pkg() != act.pkg.Types { - act.r.log.Panicf("internal error: in analysis %s of package %s: Fact.Set(%s, %T): can't set facts on objects belonging another package", - act.a, act.pkg, obj, fact) - } - - key := objectFactKey{obj, act.factType(fact)} - act.objectFacts[key] = fact // clobber any existing entry - if isFactsExportDebug { - objstr := types.ObjectString(obj, (*types.Package).Name) - factsExportDebugf("%s: object %s has fact %s\n", - act.pkg.Fset.Position(obj.Pos()), objstr, fact) - } -} - // importPackageFact implements Pass.ImportPackageFact. // Given a non-nil pointer ptr of type *T, where *T satisfies Fact, // fact copies the fact value to *ptr. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 009b01c95484..b2f16acd754d 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -8,11 +8,29 @@ package goanalysis import ( + "go/types" "reflect" "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; `act.factType`. +// exportObjectFact implements Pass.ExportObjectFact. +func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { + if obj.Pkg() != act.pkg.Types { + act.r.log.Panicf("internal error: in analysis %s of package %s: Fact.Set(%s, %T): can't set facts on objects belonging another package", + act.a, act.pkg, obj, fact) + } + + key := objectFactKey{obj, act.factType(fact)} + act.objectFacts[key] = fact // clobber any existing entry + if isFactsExportDebug { + objstr := types.ObjectString(obj, (*types.Package).Name) + factsExportDebugf("%s: object %s has fact %s\n", + act.pkg.Fset.Position(obj.Pos()), objstr, fact) + } +} + // NOTE(ldez) no alteration. func (act *action) allObjectFacts() []analysis.ObjectFact { out := make([]analysis.ObjectFact, 0, len(act.objectFacts)) From 2f93c099d94d27d4bded6bb5d1509ccf8d2da6ef Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:16:35 +0100 Subject: [PATCH 09/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 15 --------------- pkg/goanalysis/runner_base.go | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index c8f1e524df2d..40de2eccef30 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -179,21 +179,6 @@ func (act *action) analyze() { } } -// importObjectFact implements Pass.ImportObjectFact. -// Given a non-nil pointer ptr of type *T, where *T satisfies Fact, -// importObjectFact copies the fact value to *ptr. -func (act *action) importObjectFact(obj types.Object, ptr analysis.Fact) bool { - if obj == nil { - panic("nil object") - } - key := objectFactKey{obj, act.factType(ptr)} - if v, ok := act.objectFacts[key]; ok { - reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem()) - return true - } - return false -} - // importPackageFact implements Pass.ImportPackageFact. // Given a non-nil pointer ptr of type *T, where *T satisfies Fact, // fact copies the fact value to *ptr. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index b2f16acd754d..8a02bb5fbb45 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -14,6 +14,22 @@ import ( "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) altered: logger; `act.factType` +// importObjectFact implements Pass.ImportObjectFact. +// Given a non-nil pointer ptr of type *T, where *T satisfies Fact, +// importObjectFact copies the fact value to *ptr. +func (act *action) importObjectFact(obj types.Object, ptr analysis.Fact) bool { + if obj == nil { + panic("nil object") + } + key := objectFactKey{obj, act.factType(ptr)} + if v, ok := act.objectFacts[key]; ok { + reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem()) + return true + } + return false +} + // NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; `act.factType`. // exportObjectFact implements Pass.ExportObjectFact. func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { From 41d64351a3b99e612568c89be6ffa17e35ede5cb Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:17:58 +0100 Subject: [PATCH 10/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_base.go | 23 +++++++++++++++++++++++ pkg/goanalysis/runner_facts.go | 22 ---------------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 8a02bb5fbb45..b085357d2257 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -14,6 +14,29 @@ import ( "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) no alteration. +// exportedFrom reports whether obj may be visible to a package that imports pkg. +// This includes not just the exported members of pkg, but also unexported +// constants, types, fields, and methods, perhaps belonging to other packages, +// that find there way into the API. +// This is an over-approximation of the more accurate approach used by +// gc export data, which walks the type graph, but it's much simpler. +// +// TODO(adonovan): do more accurate filtering by walking the type graph. +func exportedFrom(obj types.Object, pkg *types.Package) bool { + switch obj := obj.(type) { + case *types.Func: + return obj.Exported() && obj.Pkg() == pkg || + obj.Type().(*types.Signature).Recv() != nil + case *types.Var: + return obj.Exported() && obj.Pkg() == pkg || + obj.IsField() + case *types.TypeName, *types.Const: + return true + } + return false // Nil, Builtin, Label, or PkgName +} + // NOTE(ldez) altered: logger; `act.factType` // importObjectFact implements Pass.ImportObjectFact. // Given a non-nil pointer ptr of type *T, where *T satisfies Fact, diff --git a/pkg/goanalysis/runner_facts.go b/pkg/goanalysis/runner_facts.go index c8e262a14b83..028eae4d48eb 100644 --- a/pkg/goanalysis/runner_facts.go +++ b/pkg/goanalysis/runner_facts.go @@ -96,25 +96,3 @@ func codeFact(fact analysis.Fact) (analysis.Fact, error) { } return newFact, nil } - -// exportedFrom reports whether obj may be visible to a package that imports pkg. -// This includes not just the exported members of pkg, but also unexported -// constants, types, fields, and methods, perhaps belonging to other packages, -// that find there way into the API. -// This is an over-approximation of the more accurate approach used by -// gc export data, which walks the type graph, but it's much simpler. -// -// TODO(adonovan): do more accurate filtering by walking the type graph. -func exportedFrom(obj types.Object, pkg *types.Package) bool { - switch obj := obj.(type) { - case *types.Func: - return obj.Exported() && obj.Pkg() == pkg || - obj.Type().(*types.Signature).Recv() != nil - case *types.Var: - return obj.Exported() && obj.Pkg() == pkg || - obj.IsField() - case *types.TypeName, *types.Const: - return true - } - return false // Nil, Builtin, Label, or PkgName -} From b5bc8286fcc2c9df8b0048df42cb26654a29b34f Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:18:45 +0100 Subject: [PATCH 11/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_base.go | 32 ++++++++++++++++++++++++++++++++ pkg/goanalysis/runner_facts.go | 31 ------------------------------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index b085357d2257..5581291b47dc 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -8,12 +8,44 @@ package goanalysis import ( + "bytes" + "encoding/gob" + "fmt" "go/types" "reflect" "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) no alteration. +// codeFact encodes then decodes a fact, +// just to exercise that logic. +func codeFact(fact analysis.Fact) (analysis.Fact, error) { + // We encode facts one at a time. + // A real modular driver would emit all facts + // into one encoder to improve gob efficiency. + var buf bytes.Buffer + if err := gob.NewEncoder(&buf).Encode(fact); err != nil { + return nil, err + } + + // Encode it twice and assert that we get the same bits. + // This helps detect nondeterministic Gob encoding (e.g. of maps). + var buf2 bytes.Buffer + if err := gob.NewEncoder(&buf2).Encode(fact); err != nil { + return nil, err + } + if !bytes.Equal(buf.Bytes(), buf2.Bytes()) { + return nil, fmt.Errorf("encoding of %T fact is nondeterministic", fact) + } + + newFact := reflect.New(reflect.TypeOf(fact).Elem()).Interface().(analysis.Fact) + if err := gob.NewDecoder(&buf).Decode(newFact); err != nil { + return nil, err + } + return newFact, nil +} + // NOTE(ldez) no alteration. // exportedFrom reports whether obj may be visible to a package that imports pkg. // This includes not just the exported members of pkg, but also unexported diff --git a/pkg/goanalysis/runner_facts.go b/pkg/goanalysis/runner_facts.go index 028eae4d48eb..f5ac14f506fd 100644 --- a/pkg/goanalysis/runner_facts.go +++ b/pkg/goanalysis/runner_facts.go @@ -1,9 +1,6 @@ package goanalysis import ( - "bytes" - "encoding/gob" - "fmt" "go/types" "reflect" @@ -68,31 +65,3 @@ func inheritFacts(act, dep *action) { act.packageFacts[key] = fact } } - -// codeFact encodes then decodes a fact, -// just to exercise that logic. -func codeFact(fact analysis.Fact) (analysis.Fact, error) { - // We encode facts one at a time. - // A real modular driver would emit all facts - // into one encoder to improve gob efficiency. - var buf bytes.Buffer - if err := gob.NewEncoder(&buf).Encode(fact); err != nil { - return nil, err - } - - // Encode it twice and assert that we get the same bits. - // This helps detect nondeterministic Gob encoding (e.g. of maps). - var buf2 bytes.Buffer - if err := gob.NewEncoder(&buf2).Encode(fact); err != nil { - return nil, err - } - if !bytes.Equal(buf.Bytes(), buf2.Bytes()) { - return nil, fmt.Errorf("encoding of %T fact is nondeterministic", fact) - } - - newFact := reflect.New(reflect.TypeOf(fact).Elem()).Interface().(analysis.Fact) - if err := gob.NewDecoder(&buf).Decode(newFact); err != nil { - return nil, err - } - return newFact, nil -} From a7c5baf056e0d2eb638d10397a8aef2e577b016e Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:20:32 +0100 Subject: [PATCH 12/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_base.go | 50 +++++++++++++++++++++++++++++++++ pkg/goanalysis/runner_facts.go | 51 ---------------------------------- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 5581291b47dc..fd619e13c04b 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -17,6 +17,56 @@ import ( "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) altered: logger; serialize. +// inheritFacts populates act.facts with +// those it obtains from its dependency, dep. +func inheritFacts(act, dep *action) { + const serialize = false + + for key, fact := range dep.objectFacts { + // Filter out facts related to objects + // that are irrelevant downstream + // (equivalently: not in the compiler export data). + if !exportedFrom(key.obj, dep.pkg.Types) { + factsInheritDebugf("%v: discarding %T fact from %s for %s: %s", act, fact, dep, key.obj, fact) + continue + } + + // Optionally serialize/deserialize fact + // to verify that it works across address spaces. + if serialize { + var err error + fact, err = codeFact(fact) + if err != nil { + act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) + } + } + + factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.obj, fact) + act.objectFacts[key] = fact + } + + for key, fact := range dep.packageFacts { + // TODO: filter out facts that belong to + // packages not mentioned in the export data + // to prevent side channels. + + // Optionally serialize/deserialize fact + // to verify that it works across address spaces + // and is deterministic. + if serialize { + var err error + fact, err = codeFact(fact) + if err != nil { + act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) + } + } + + factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.pkg.Path(), fact) + act.packageFacts[key] = fact + } +} + // NOTE(ldez) no alteration. // codeFact encodes then decodes a fact, // just to exercise that logic. diff --git a/pkg/goanalysis/runner_facts.go b/pkg/goanalysis/runner_facts.go index f5ac14f506fd..f3de83bf64e7 100644 --- a/pkg/goanalysis/runner_facts.go +++ b/pkg/goanalysis/runner_facts.go @@ -3,8 +3,6 @@ package goanalysis import ( "go/types" "reflect" - - "golang.org/x/tools/go/analysis" ) type objectFactKey struct { @@ -16,52 +14,3 @@ type packageFactKey struct { pkg *types.Package typ reflect.Type } - -// inheritFacts populates act.facts with -// those it obtains from its dependency, dep. -func inheritFacts(act, dep *action) { - serialize := false - - for key, fact := range dep.objectFacts { - // Filter out facts related to objects - // that are irrelevant downstream - // (equivalently: not in the compiler export data). - if !exportedFrom(key.obj, dep.pkg.Types) { - factsInheritDebugf("%v: discarding %T fact from %s for %s: %s", act, fact, dep, key.obj, fact) - continue - } - - // Optionally serialize/deserialize fact - // to verify that it works across address spaces. - if serialize { - var err error - fact, err = codeFact(fact) - if err != nil { - act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) - } - } - - factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.obj, fact) - act.objectFacts[key] = fact - } - - for key, fact := range dep.packageFacts { - // TODO: filter out facts that belong to - // packages not mentioned in the export data - // to prevent side channels. - - // Optionally serialize/deserialize fact - // to verify that it works across address spaces - // and is deterministic. - if serialize { - var err error - fact, err = codeFact(fact) - if err != nil { - act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) - } - } - - factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.pkg.Path(), fact) - act.packageFacts[key] = fact - } -} From 62e053614ca0da92b5d4bebe3da3e252d7fcb106 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:21:34 +0100 Subject: [PATCH 13/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 93 -------------------------------- pkg/goanalysis/runner_base.go | 94 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 93 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index 40de2eccef30..de76816ccd03 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -1,18 +1,15 @@ package goanalysis import ( - "errors" "fmt" "go/types" "reflect" "runtime/debug" - "time" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/internal/errorutil" - "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" ) type actionAllocator struct { @@ -89,96 +86,6 @@ func (act *action) analyzeSafe() { act.r.sw.TrackStage(act.a.Name, act.analyze) } -func (act *action) analyze() { - defer close(act.analysisDoneCh) // unblock actions depending on this action - - if !act.needAnalyzeSource { - return - } - - defer func(now time.Time) { - analyzeDebugf("go/analysis: %s: %s: analyzed package %q in %s", act.r.prefix, act.a.Name, act.pkg.Name, time.Since(now)) - }(time.Now()) - - // Report an error if any dependency failures. - var depErrors error - for _, dep := range act.deps { - if dep.err == nil { - continue - } - - depErrors = errors.Join(depErrors, errors.Unwrap(dep.err)) - } - if depErrors != nil { - act.err = fmt.Errorf("failed prerequisites: %w", depErrors) - return - } - - // Plumb the output values of the dependencies - // into the inputs of this action. Also facts. - inputs := make(map[*analysis.Analyzer]any) - startedAt := time.Now() - for _, dep := range act.deps { - if dep.pkg == act.pkg { - // Same package, different analysis (horizontal edge): - // in-memory outputs of prerequisite analyzers - // become inputs to this analysis pass. - inputs[dep.a] = dep.result - } else if dep.a == act.a { // (always true) - // Same analysis, different package (vertical edge): - // serialized facts produced by prerequisite analysis - // become available to this analysis pass. - inheritFacts(act, dep) - } - } - factsDebugf("%s: Inherited facts in %s", act, time.Since(startedAt)) - - // Run the analysis. - pass := &analysis.Pass{ - Analyzer: act.a, - Fset: act.pkg.Fset, - Files: act.pkg.Syntax, - OtherFiles: act.pkg.OtherFiles, - Pkg: act.pkg.Types, - TypesInfo: act.pkg.TypesInfo, - TypesSizes: act.pkg.TypesSizes, - ResultOf: inputs, - Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, - ImportObjectFact: act.importObjectFact, - ExportObjectFact: act.exportObjectFact, - ImportPackageFact: act.importPackageFact, - ExportPackageFact: act.exportPackageFact, - AllObjectFacts: act.allObjectFacts, - AllPackageFacts: act.allPackageFacts, - } - act.pass = pass - act.r.passToPkgGuard.Lock() - act.r.passToPkg[pass] = act.pkg - act.r.passToPkgGuard.Unlock() - - if act.pkg.IllTyped { - // It looks like there should be !pass.Analyzer.RunDespiteErrors - // but govet's cgocall crashes on it. Govet itself contains !pass.Analyzer.RunDespiteErrors condition here, - // but it exits before it if packages.Load have failed. - act.err = fmt.Errorf("analysis skipped: %w", &pkgerrors.IllTypedError{Pkg: act.pkg}) - } else { - startedAt = time.Now() - act.result, act.err = pass.Analyzer.Run(pass) - analyzedIn := time.Since(startedAt) - if analyzedIn > time.Millisecond*10 { - debugf("%s: run analyzer in %s", act, analyzedIn) - } - } - - // disallow calls after Run - pass.ExportObjectFact = nil - pass.ExportPackageFact = nil - - if err := act.persistFactsToCache(); err != nil { - act.r.log.Warnf("Failed to persist facts to cache: %s", err) - } -} - // importPackageFact implements Pass.ImportPackageFact. // Given a non-nil pointer ptr of type *T, where *T satisfies Fact, // fact copies the fact value to *ptr. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index fd619e13c04b..c5d49e648dbc 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -10,13 +10,107 @@ package goanalysis import ( "bytes" "encoding/gob" + "errors" "fmt" "go/types" "reflect" + "time" + "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" "golang.org/x/tools/go/analysis" ) +// NOTE(ldez) altered version of `func (act *action) execOnce()`. +func (act *action) analyze() { + defer close(act.analysisDoneCh) // unblock actions depending on this action + + if !act.needAnalyzeSource { + return + } + + defer func(now time.Time) { + analyzeDebugf("go/analysis: %s: %s: analyzed package %q in %s", act.r.prefix, act.a.Name, act.pkg.Name, time.Since(now)) + }(time.Now()) + + // Report an error if any dependency failures. + var depErrors error + for _, dep := range act.deps { + if dep.err == nil { + continue + } + + depErrors = errors.Join(depErrors, errors.Unwrap(dep.err)) + } + if depErrors != nil { + act.err = fmt.Errorf("failed prerequisites: %w", depErrors) + return + } + + // Plumb the output values of the dependencies + // into the inputs of this action. Also facts. + inputs := make(map[*analysis.Analyzer]any) + startedAt := time.Now() + for _, dep := range act.deps { + if dep.pkg == act.pkg { + // Same package, different analysis (horizontal edge): + // in-memory outputs of prerequisite analyzers + // become inputs to this analysis pass. + inputs[dep.a] = dep.result + } else if dep.a == act.a { // (always true) + // Same analysis, different package (vertical edge): + // serialized facts produced by prerequisite analysis + // become available to this analysis pass. + inheritFacts(act, dep) + } + } + factsDebugf("%s: Inherited facts in %s", act, time.Since(startedAt)) + + // Run the analysis. + pass := &analysis.Pass{ + Analyzer: act.a, + Fset: act.pkg.Fset, + Files: act.pkg.Syntax, + OtherFiles: act.pkg.OtherFiles, + Pkg: act.pkg.Types, + TypesInfo: act.pkg.TypesInfo, + TypesSizes: act.pkg.TypesSizes, + ResultOf: inputs, + Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, + ImportObjectFact: act.importObjectFact, + ExportObjectFact: act.exportObjectFact, + ImportPackageFact: act.importPackageFact, + ExportPackageFact: act.exportPackageFact, + AllObjectFacts: act.allObjectFacts, + AllPackageFacts: act.allPackageFacts, + } + act.pass = pass + act.r.passToPkgGuard.Lock() + act.r.passToPkg[pass] = act.pkg + act.r.passToPkgGuard.Unlock() + + if act.pkg.IllTyped { + // It looks like there should be !pass.Analyzer.RunDespiteErrors + // but govet's cgocall crashes on it. Govet itself contains !pass.Analyzer.RunDespiteErrors condition here, + // but it exits before it if packages.Load have failed. + act.err = fmt.Errorf("analysis skipped: %w", &pkgerrors.IllTypedError{Pkg: act.pkg}) + } else { + startedAt = time.Now() + act.result, act.err = pass.Analyzer.Run(pass) + analyzedIn := time.Since(startedAt) + if analyzedIn > time.Millisecond*10 { + debugf("%s: run analyzer in %s", act, analyzedIn) + } + } + + // disallow calls after Run + pass.ExportObjectFact = nil + pass.ExportPackageFact = nil + + if err := act.persistFactsToCache(); err != nil { + act.r.log.Warnf("Failed to persist facts to cache: %s", err) + } +} + // NOTE(ldez) altered: logger; serialize. // inheritFacts populates act.facts with // those it obtains from its dependency, dep. From d88ac1a69c38455ac76d6abf86943b70119c5eb0 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:22:25 +0100 Subject: [PATCH 14/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 4 ---- pkg/goanalysis/runner_base.go | 20 +++++++++++++++++++- pkg/goanalysis/runner_facts.go | 16 ---------------- 3 files changed, 19 insertions(+), 21 deletions(-) delete mode 100644 pkg/goanalysis/runner_facts.go diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index de76816ccd03..d341b146db92 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -56,10 +56,6 @@ type action struct { needAnalyzeSource bool } -func (act *action) String() string { - return fmt.Sprintf("%s@%s", act.a, act.pkg) -} - func (act *action) waitUntilDependingAnalyzersWorked() { for _, dep := range act.deps { if dep.pkg == act.pkg { diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index c5d49e648dbc..a65347645f9c 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -16,10 +16,28 @@ import ( "reflect" "time" - "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" "golang.org/x/tools/go/analysis" + + "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" ) +// NOTE(ldez) no alteration. +type objectFactKey struct { + obj types.Object + typ reflect.Type +} + +// NOTE(ldez) no alteration. +type packageFactKey struct { + pkg *types.Package + typ reflect.Type +} + +// NOTE(ldez) no alteration. +func (act *action) String() string { + return fmt.Sprintf("%s@%s", act.a, act.pkg) +} + // NOTE(ldez) altered version of `func (act *action) execOnce()`. func (act *action) analyze() { defer close(act.analysisDoneCh) // unblock actions depending on this action diff --git a/pkg/goanalysis/runner_facts.go b/pkg/goanalysis/runner_facts.go deleted file mode 100644 index f3de83bf64e7..000000000000 --- a/pkg/goanalysis/runner_facts.go +++ /dev/null @@ -1,16 +0,0 @@ -package goanalysis - -import ( - "go/types" - "reflect" -) - -type objectFactKey struct { - obj types.Object - typ reflect.Type -} - -type packageFactKey struct { - pkg *types.Package - typ reflect.Type -} From 00feff85c09916c34c9c8c510578110657fba4a8 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:23:09 +0100 Subject: [PATCH 15/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 24 ------------------------ pkg/goanalysis/runner_base.go | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index d341b146db92..c59d556001a6 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -7,7 +7,6 @@ import ( "runtime/debug" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/internal/errorutil" ) @@ -33,29 +32,6 @@ func (actAlloc *actionAllocator) alloc() *action { return act } -// An action represents one unit of analysis work: the application of -// one analysis to one package. Actions form a DAG, both within a -// package (as different analyzers are applied, either in sequence or -// parallel), and across packages (as dependencies are analyzed). -type action struct { - a *analysis.Analyzer - pkg *packages.Package - pass *analysis.Pass - deps []*action - objectFacts map[objectFactKey]analysis.Fact - packageFacts map[packageFactKey]analysis.Fact - result any - diagnostics []analysis.Diagnostic - err error - r *runner - analysisDoneCh chan struct{} - loadCachedFactsDone bool - loadCachedFactsOk bool - isroot bool - isInitialPkg bool - needAnalyzeSource bool -} - func (act *action) waitUntilDependingAnalyzersWorked() { for _, dep := range act.deps { if dep.pkg == act.pkg { diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index a65347645f9c..c0b27f267b9a 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -17,10 +17,35 @@ import ( "time" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/packages" "github.com/golangci/golangci-lint/pkg/goanalysis/pkgerrors" ) +// NOTE(ldez) altered: custom fields; remove 'once' and 'duration'. +// An action represents one unit of analysis work: the application of +// one analysis to one package. Actions form a DAG, both within a +// package (as different analyzers are applied, either in sequence or +// parallel), and across packages (as dependencies are analyzed). +type action struct { + a *analysis.Analyzer + pkg *packages.Package + pass *analysis.Pass + deps []*action + objectFacts map[objectFactKey]analysis.Fact + packageFacts map[packageFactKey]analysis.Fact + result any + diagnostics []analysis.Diagnostic + err error + r *runner + analysisDoneCh chan struct{} + loadCachedFactsDone bool + loadCachedFactsOk bool + isroot bool + isInitialPkg bool + needAnalyzeSource bool +} + // NOTE(ldez) no alteration. type objectFactKey struct { obj types.Object From d98733b5142c26bec02bcd2dbaf95ee11ce9403c Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:30:49 +0100 Subject: [PATCH 16/19] chore: isolate code from x/tools --- pkg/goanalysis/runner_action.go | 19 ------------------- pkg/goanalysis/runner_base.go | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/pkg/goanalysis/runner_action.go b/pkg/goanalysis/runner_action.go index c59d556001a6..152cab181408 100644 --- a/pkg/goanalysis/runner_action.go +++ b/pkg/goanalysis/runner_action.go @@ -2,12 +2,8 @@ package goanalysis import ( "fmt" - "go/types" - "reflect" "runtime/debug" - "golang.org/x/tools/go/analysis" - "github.com/golangci/golangci-lint/internal/errorutil" ) @@ -58,21 +54,6 @@ func (act *action) analyzeSafe() { act.r.sw.TrackStage(act.a.Name, act.analyze) } -// importPackageFact implements Pass.ImportPackageFact. -// Given a non-nil pointer ptr of type *T, where *T satisfies Fact, -// fact copies the fact value to *ptr. -func (act *action) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool { - if pkg == nil { - panic("nil package") - } - key := packageFactKey{pkg, act.factType(ptr)} - if v, ok := act.packageFacts[key]; ok { - reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem()) - return true - } - return false -} - func (act *action) markDepsForAnalyzingSource() { // Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing // this action. diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index c0b27f267b9a..6d555e76b660 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -301,6 +301,22 @@ func (act *action) allObjectFacts() []analysis.ObjectFact { return out } +// NOTE(ldez) altered: `act.factType` +// importPackageFact implements Pass.ImportPackageFact. +// Given a non-nil pointer ptr of type *T, where *T satisfies Fact, +// fact copies the fact value to *ptr. +func (act *action) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool { + if pkg == nil { + panic("nil package") + } + key := packageFactKey{pkg, act.factType(ptr)} + if v, ok := act.packageFacts[key]; ok { + reflect.ValueOf(ptr).Elem().Set(reflect.ValueOf(v).Elem()) + return true + } + return false +} + // NOTE(ldez) altered: removes code related to `act.pass.ExportPackageFact`; logger; `act.factType`. // exportPackageFact implements Pass.ExportPackageFact. func (act *action) exportPackageFact(fact analysis.Fact) { From 669e5be7a184dcf013c5152097de0fc1a228df08 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 16:34:12 +0100 Subject: [PATCH 17/19] chore: sync code from x/tools --- pkg/goanalysis/runner_base.go | 114 ++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 46 deletions(-) diff --git a/pkg/goanalysis/runner_base.go b/pkg/goanalysis/runner_base.go index 6d555e76b660..d868f8f5dac5 100644 --- a/pkg/goanalysis/runner_base.go +++ b/pkg/goanalysis/runner_base.go @@ -2,8 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. // -// Partial copy of https://github.com/golang/tools/blob/master/go/analysis/internal/checker -// FIXME add a commit hash. +// Partial copy of https://github.com/golang/tools/blob/dba5486c2a1d03519930812112b23ed2c45c04fc/go/analysis/internal/checker/checker.go package goanalysis @@ -28,20 +27,22 @@ import ( // package (as different analyzers are applied, either in sequence or // parallel), and across packages (as dependencies are analyzed). type action struct { - a *analysis.Analyzer - pkg *packages.Package - pass *analysis.Pass - deps []*action - objectFacts map[objectFactKey]analysis.Fact - packageFacts map[packageFactKey]analysis.Fact - result any - diagnostics []analysis.Diagnostic - err error + a *analysis.Analyzer + pkg *packages.Package + pass *analysis.Pass + isroot bool + deps []*action + objectFacts map[objectFactKey]analysis.Fact + packageFacts map[packageFactKey]analysis.Fact + result any + diagnostics []analysis.Diagnostic + err error + + // NOTE(ldez) custom fields. r *runner analysisDoneCh chan struct{} loadCachedFactsDone bool loadCachedFactsOk bool - isroot bool isInitialPkg bool needAnalyzeSource bool } @@ -78,12 +79,11 @@ func (act *action) analyze() { // Report an error if any dependency failures. var depErrors error for _, dep := range act.deps { - if dep.err == nil { - continue + if dep.err != nil { + depErrors = errors.Join(depErrors, errors.Unwrap(dep.err)) } - - depErrors = errors.Join(depErrors, errors.Unwrap(dep.err)) } + if depErrors != nil { act.err = fmt.Errorf("failed prerequisites: %w", depErrors) return @@ -92,7 +92,10 @@ func (act *action) analyze() { // Plumb the output values of the dependencies // into the inputs of this action. Also facts. inputs := make(map[*analysis.Analyzer]any) + act.objectFacts = make(map[objectFactKey]analysis.Fact) + act.packageFacts = make(map[packageFactKey]analysis.Fact) startedAt := time.Now() + for _, dep := range act.deps { if dep.pkg == act.pkg { // Same package, different analysis (horizontal edge): @@ -106,17 +109,29 @@ func (act *action) analyze() { inheritFacts(act, dep) } } + factsDebugf("%s: Inherited facts in %s", act, time.Since(startedAt)) + module := &analysis.Module{} // possibly empty (non nil) in go/analysis drivers. + if mod := act.pkg.Module; mod != nil { + module.Path = mod.Path + module.Version = mod.Version + module.GoVersion = mod.GoVersion + } + // Run the analysis. pass := &analysis.Pass{ - Analyzer: act.a, - Fset: act.pkg.Fset, - Files: act.pkg.Syntax, - OtherFiles: act.pkg.OtherFiles, - Pkg: act.pkg.Types, - TypesInfo: act.pkg.TypesInfo, - TypesSizes: act.pkg.TypesSizes, + Analyzer: act.a, + Fset: act.pkg.Fset, + Files: act.pkg.Syntax, + OtherFiles: act.pkg.OtherFiles, + IgnoredFiles: act.pkg.IgnoredFiles, + Pkg: act.pkg.Types, + TypesInfo: act.pkg.TypesInfo, + TypesSizes: act.pkg.TypesSizes, + TypeErrors: act.pkg.TypeErrors, + Module: module, + ResultOf: inputs, Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, ImportObjectFact: act.importObjectFact, @@ -126,6 +141,7 @@ func (act *action) analyze() { AllObjectFacts: act.allObjectFacts, AllPackageFacts: act.allPackageFacts, } + act.pass = pass act.r.passToPkgGuard.Lock() act.r.passToPkg[pass] = act.pkg @@ -138,7 +154,9 @@ func (act *action) analyze() { act.err = fmt.Errorf("analysis skipped: %w", &pkgerrors.IllTypedError{Pkg: act.pkg}) } else { startedAt = time.Now() + act.result, act.err = pass.Analyzer.Run(pass) + analyzedIn := time.Since(startedAt) if analyzedIn > time.Millisecond*10 { debugf("%s: run analyzer in %s", act, analyzedIn) @@ -149,7 +167,8 @@ func (act *action) analyze() { pass.ExportObjectFact = nil pass.ExportPackageFact = nil - if err := act.persistFactsToCache(); err != nil { + err := act.persistFactsToCache() + if err != nil { act.r.log.Warnf("Failed to persist facts to cache: %s", err) } } @@ -172,14 +191,15 @@ func inheritFacts(act, dep *action) { // Optionally serialize/deserialize fact // to verify that it works across address spaces. if serialize { - var err error - fact, err = codeFact(fact) + encodedFact, err := codeFact(fact) if err != nil { - act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) + act.r.log.Panicf("internal error: encoding of %T fact failed in %v: %v", fact, act, err) } + fact = encodedFact } factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.obj, fact) + act.objectFacts[key] = fact } @@ -192,14 +212,15 @@ func inheritFacts(act, dep *action) { // to verify that it works across address spaces // and is deterministic. if serialize { - var err error - fact, err = codeFact(fact) + encodedFact, err := codeFact(fact) if err != nil { act.r.log.Panicf("internal error: encoding of %T fact failed in %v", fact, act) } + fact = encodedFact } factsInheritDebugf("%v: inherited %T fact for %s: %s", act, fact, key.pkg.Path(), fact) + act.packageFacts[key] = fact } } @@ -248,8 +269,13 @@ func exportedFrom(obj types.Object, pkg *types.Package) bool { return obj.Exported() && obj.Pkg() == pkg || obj.Type().(*types.Signature).Recv() != nil case *types.Var: - return obj.Exported() && obj.Pkg() == pkg || - obj.IsField() + if obj.IsField() { + return true + } + // we can't filter more aggressively than this because we need + // to consider function parameters exported, but have no way + // of telling apart function parameters from local variables. + return obj.Pkg() == pkg case *types.TypeName, *types.Const: return true } @@ -284,6 +310,7 @@ func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { act.objectFacts[key] = fact // clobber any existing entry if isFactsExportDebug { objstr := types.ObjectString(obj, (*types.Package).Name) + factsExportDebugf("%s: object %s has fact %s\n", act.pkg.Fset.Position(obj.Pos()), objstr, fact) } @@ -291,14 +318,11 @@ func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) { // NOTE(ldez) no alteration. func (act *action) allObjectFacts() []analysis.ObjectFact { - out := make([]analysis.ObjectFact, 0, len(act.objectFacts)) - for key, fact := range act.objectFacts { - out = append(out, analysis.ObjectFact{ - Object: key.obj, - Fact: fact, - }) + facts := make([]analysis.ObjectFact, 0, len(act.objectFacts)) + for k := range act.objectFacts { + facts = append(facts, analysis.ObjectFact{Object: k.obj, Fact: act.objectFacts[k]}) } - return out + return facts } // NOTE(ldez) altered: `act.factType` @@ -322,6 +346,7 @@ func (act *action) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool func (act *action) exportPackageFact(fact analysis.Fact) { key := packageFactKey{act.pass.Pkg, act.factType(fact)} act.packageFacts[key] = fact // clobber any existing entry + factsDebugf("%s: package %s has fact %s\n", act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact) } @@ -330,19 +355,16 @@ func (act *action) exportPackageFact(fact analysis.Fact) { func (act *action) factType(fact analysis.Fact) reflect.Type { t := reflect.TypeOf(fact) if t.Kind() != reflect.Ptr { - act.r.log.Fatalf("invalid Fact type: got %T, want pointer", t) + act.r.log.Fatalf("invalid Fact type: got %T, want pointer", fact) } return t } // NOTE(ldez) no alteration. func (act *action) allPackageFacts() []analysis.PackageFact { - out := make([]analysis.PackageFact, 0, len(act.packageFacts)) - for key, fact := range act.packageFacts { - out = append(out, analysis.PackageFact{ - Package: key.pkg, - Fact: fact, - }) + facts := make([]analysis.PackageFact, 0, len(act.packageFacts)) + for k := range act.packageFacts { + facts = append(facts, analysis.PackageFact{Package: k.pkg, Fact: act.packageFacts[k]}) } - return out + return facts } From 8debab418bea8ca8992706fc768b9386cb373bbd Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Wed, 6 Nov 2024 15:40:52 +0100 Subject: [PATCH 18/19] fix: go version --- pkg/config/loader.go | 11 ----------- pkg/goanalysis/runner_loadingpackage.go | 16 +++++++++++----- pkg/lint/linter/config.go | 2 +- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/pkg/config/loader.go b/pkg/config/loader.go index efdbfce1f17d..948139496473 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -302,17 +302,6 @@ func (l *Loader) handleGoVersion() { l.cfg.LintersSettings.Gocritic.Go = trimmedGoVersion - // staticcheck related linters. - if l.cfg.LintersSettings.Staticcheck.GoVersion == "" { - l.cfg.LintersSettings.Staticcheck.GoVersion = trimmedGoVersion - } - if l.cfg.LintersSettings.Gosimple.GoVersion == "" { - l.cfg.LintersSettings.Gosimple.GoVersion = trimmedGoVersion - } - if l.cfg.LintersSettings.Stylecheck.GoVersion == "" { - l.cfg.LintersSettings.Stylecheck.GoVersion = trimmedGoVersion - } - os.Setenv("GOSECGOVERSION", l.cfg.Run.Go) } diff --git a/pkg/goanalysis/runner_loadingpackage.go b/pkg/goanalysis/runner_loadingpackage.go index 614cc1c006c0..44d6769586e9 100644 --- a/pkg/goanalysis/runner_loadingpackage.go +++ b/pkg/goanalysis/runner_loadingpackage.go @@ -10,6 +10,7 @@ import ( "go/types" "os" "reflect" + "strings" "sync" "sync/atomic" @@ -153,10 +154,15 @@ func (lp *loadingPackage) loadFromSource(loadMode LoadMode) error { return imp.Types, nil } - // TODO(ldez) temporary workaround - rv, err := goutil.CleanRuntimeVersion() - if err != nil { - return err + var goVersion string + if pkg.Module != nil && pkg.Module.GoVersion != "" { + goVersion = "go" + strings.TrimPrefix(pkg.Module.GoVersion, "go") + } else { + var err error + goVersion, err = goutil.CleanRuntimeVersion() + if err != nil { + return err + } } tc := &types.Config{ @@ -164,7 +170,7 @@ func (lp *loadingPackage) loadFromSource(loadMode LoadMode) error { Error: func(err error) { pkg.Errors = append(pkg.Errors, lp.convertError(err)...) }, - GoVersion: rv, // TODO(ldez) temporary workaround + GoVersion: goVersion, Sizes: types.SizesFor(build.Default.Compiler, build.Default.GOARCH), } diff --git a/pkg/lint/linter/config.go b/pkg/lint/linter/config.go index 57c51fa75e4c..6d6d4b17e7d4 100644 --- a/pkg/lint/linter/config.go +++ b/pkg/lint/linter/config.go @@ -81,7 +81,7 @@ func (lc *Config) IsSlowLinter() bool { } func (lc *Config) WithLoadFiles() *Config { - lc.LoadMode |= packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles + lc.LoadMode |= packages.NeedName | packages.NeedFiles | packages.NeedCompiledGoFiles | packages.NeedModule return lc } From c9d2b11d8e0a2afa633fcc1848b908793b106dc3 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 7 Nov 2024 22:13:24 +0100 Subject: [PATCH 19/19] chore: factorize fact cache key --- pkg/goanalysis/runner_action_cache.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/goanalysis/runner_action_cache.go b/pkg/goanalysis/runner_action_cache.go index 6ed1270dc80a..fbc2f82fa98d 100644 --- a/pkg/goanalysis/runner_action_cache.go +++ b/pkg/goanalysis/runner_action_cache.go @@ -76,14 +76,14 @@ func (act *action) persistFactsToCache() error { factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name) - key := fmt.Sprintf("%s/facts", analyzer.Name) - return act.r.pkgCache.Put(act.pkg, cache.HashModeNeedAllDeps, key, facts) + return act.r.pkgCache.Put(act.pkg, cache.HashModeNeedAllDeps, factCacheKey(analyzer), facts) } func (act *action) loadPersistedFacts() bool { var facts []Fact - key := fmt.Sprintf("%s/facts", act.a.Name) - if err := act.r.pkgCache.Get(act.pkg, cache.HashModeNeedAllDeps, key, &facts); err != nil { + + err := act.r.pkgCache.Get(act.pkg, cache.HashModeNeedAllDeps, factCacheKey(act.a), &facts) + if err != nil { if !errors.Is(err, cache.ErrMissing) && !errors.Is(err, io.EOF) { act.r.log.Warnf("Failed to get persisted facts: %s", err) } @@ -121,3 +121,7 @@ func (act *action) loadPersistedFacts() bool { return true } + +func factCacheKey(a *analysis.Analyzer) string { + return fmt.Sprintf("%s/facts", a.Name) +}