From b53505e708d412e45c0bd1386f914377fafcf20f Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 4 Oct 2019 17:18:43 -0400 Subject: [PATCH] internal/lsp: cache analysis using memoize package This change moves to the approach of caching the analysis using the memoize package. This means that we will do less work, as we no longer need to recompute results that are unchanged. The cache key for an analysis is simply the key of the CheckPackageHandle, along with the name of the analyzer. Change-Id: I0e589ccf088ff1de5670401b7207ffa77a254ceb Reviewed-on: https://go-review.googlesource.com/c/tools/+/200817 Run-TryBot: Rebecca Stambler Reviewed-by: Ian Cottrell --- internal/lsp/{source => cache}/analysis.go | 252 ++++++++++++--------- internal/lsp/cache/check.go | 2 - internal/lsp/cache/pkg.go | 105 +-------- internal/lsp/cache/snapshot.go | 4 + internal/lsp/source/diagnostics.go | 53 ++--- internal/lsp/source/suggested_fix.go | 2 +- internal/lsp/source/view.go | 10 +- 7 files changed, 191 insertions(+), 237 deletions(-) rename internal/lsp/{source => cache}/analysis.go (54%) diff --git a/internal/lsp/source/analysis.go b/internal/lsp/cache/analysis.go similarity index 54% rename from internal/lsp/source/analysis.go rename to internal/lsp/cache/analysis.go index 2c7961efcc..1f1316b42e 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/cache/analysis.go @@ -1,10 +1,4 @@ -// Copyright 2019 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. - -// This file is largely based on go/analysis/internal/checker/checker.go. - -package source +package cache import ( "context" @@ -13,69 +7,65 @@ import ( "go/types" "reflect" "sort" - "strings" "sync" - "time" "golang.org/x/sync/errgroup" "golang.org/x/tools/go/analysis" - "golang.org/x/tools/internal/telemetry/trace" + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/memoize" + "golang.org/x/tools/internal/telemetry/log" errors "golang.org/x/xerrors" ) -func analyze(ctx context.Context, v View, cphs []CheckPackageHandle, analyzers []*analysis.Analyzer) ([]*Action, error) { - ctx, done := trace.StartSpan(ctx, "source.analyze") - defer done() +func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) (map[*analysis.Analyzer][]*analysis.Diagnostic, error) { + var roots []*actionHandle + for _, a := range analyzers { + ah, err := s.actionHandle(ctx, packageID(id), source.ParseFull, a) + if err != nil { + return nil, err + } + ah.isroot = true + roots = append(roots, ah) + } + + // Check if the context has been canceled before running the analyses. if ctx.Err() != nil { return nil, ctx.Err() } - // Build nodes for initial packages. - var roots []*Action - for _, a := range analyzers { - for _, cph := range cphs { - pkg, err := cph.Check(ctx) - if err != nil { - return nil, err - } - root, err := pkg.GetActionGraph(ctx, a) - if err != nil { - return nil, err - } - root.isroot = true - root.view = v - roots = append(roots, root) + results := make(map[*analysis.Analyzer][]*analysis.Diagnostic) + for _, ah := range roots { + diagnostics, _, err := ah.analyze(ctx) + if err != nil { + log.Error(ctx, "no results", err) + continue } + results[ah.analyzer] = diagnostics } - - // Execute the graph in parallel. - if err := execAll(ctx, v.Session().Cache().FileSet(), roots); err != nil { - return nil, err - } - return roots, nil + return results, nil } // 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 { - once sync.Once - Analyzer *analysis.Analyzer - Pkg Package - Deps []*Action - diagnostics []analysis.Diagnostic +type actionHandle struct { + handle *memoize.Handle + analyzer *analysis.Analyzer + deps []*actionHandle + pkg *pkg pass *analysis.Pass isroot bool objectFacts map[objectFactKey]analysis.Fact packageFacts map[packageFactKey]analysis.Fact - inputs map[*analysis.Analyzer]interface{} - result interface{} - err error - duration time.Duration - view View +} + +type actionData struct { + diagnostics []*analysis.Diagnostic + result interface{} + err error } type objectFactKey struct { @@ -88,46 +78,105 @@ type packageFactKey struct { typ reflect.Type } -func (act *Action) String() string { - return fmt.Sprintf("%s@%s", act.Analyzer, act.Pkg.PkgPath()) +func (s *snapshot) actionHandle(ctx context.Context, id packageID, mode source.ParseMode, a *analysis.Analyzer) (*actionHandle, error) { + cph := s.getPackage(id, mode) + if cph == nil { + return nil, errors.Errorf("no CheckPackageHandle for %s:%v", id, mode == source.ParseExported) + } + if len(cph.key) == 0 { + return nil, errors.Errorf("no key for CheckPackageHandle %s", id) + } + pkg, err := cph.check(ctx) + if err != nil { + return nil, err + } + ah := &actionHandle{ + analyzer: a, + pkg: pkg, + } + // Add a dependency on each required analyzers. + for _, req := range a.Requires { + reqActionHandle, err := s.actionHandle(ctx, id, mode, req) + if err != nil { + return nil, err + } + ah.deps = append(ah.deps, reqActionHandle) + } + // An analysis that consumes/produces facts + // must run on the package's dependencies too. + if len(a.FactTypes) > 0 { + importIDs := make([]string, 0, len(cph.m.deps)) + for _, importID := range cph.m.deps { + importIDs = append(importIDs, string(importID)) + } + sort.Strings(importIDs) // for determinism + for _, importID := range importIDs { + depActionHandle, err := s.actionHandle(ctx, packageID(importID), source.ParseExported, a) + if err != nil { + return nil, err + } + ah.deps = append(ah.deps, depActionHandle) + } + } + h := s.view.session.cache.store.Bind(actionKey(a, cph), func(ctx context.Context) interface{} { + data := &actionData{} + data.diagnostics, data.result, data.err = ah.exec(ctx, s.view.session.cache.fset) + return data + }) + ah.handle = h + return ah, nil } -func execAll(ctx context.Context, fset *token.FileSet, actions []*Action) error { +func (ah *actionHandle) analyze(ctx context.Context) ([]*analysis.Diagnostic, interface{}, error) { + v := ah.handle.Get(ctx) + if v == nil { + return nil, nil, errors.Errorf("no analyses for %s", ah.pkg.ID()) + } + data := v.(*actionData) + return data.diagnostics, data.result, data.err +} + +func actionKey(a *analysis.Analyzer, cph *checkPackageHandle) string { + return hashContents([]byte(fmt.Sprintf("%s %s", a, string(cph.key)))) +} + +func (act *actionHandle) String() string { + return fmt.Sprintf("%s@%s", act.analyzer, act.pkg.PkgPath()) +} + +func execAll(ctx context.Context, fset *token.FileSet, actions []*actionHandle) (map[*actionHandle][]*analysis.Diagnostic, map[*actionHandle]interface{}, error) { + var ( + mu sync.Mutex + diagnostics = make(map[*actionHandle][]*analysis.Diagnostic) + results = make(map[*actionHandle]interface{}) + ) + g, ctx := errgroup.WithContext(ctx) for _, act := range actions { act := act g.Go(func() error { - return act.exec(ctx, fset) + d, r, err := act.analyze(ctx) + if err != nil { + return err + } + + mu.Lock() + defer mu.Unlock() + + diagnostics[act] = d + results[act] = r + + return nil }) } - return g.Wait() + return diagnostics, results, g.Wait() } -func (act *Action) exec(ctx context.Context, fset *token.FileSet) error { - var err error - act.once.Do(func() { - err = act.execOnce(ctx, fset) - }) - return err -} - -func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { +func (act *actionHandle) exec(ctx context.Context, fset *token.FileSet) (diagnostics []*analysis.Diagnostic, result interface{}, err error) { // Analyze dependencies. - if err := execAll(ctx, fset, act.Deps); err != nil { - return err - } - - // Report an error if any dependency failed. - var failed []string - for _, dep := range act.Deps { - if dep.err != nil { - failed = append(failed, fmt.Sprintf("%s: %v", dep.String(), dep.err)) - } - } - if failed != nil { - sort.Strings(failed) - act.err = errors.Errorf("failed prerequisites: %s", strings.Join(failed, ", ")) - return act.err + _, depResults, err := execAll(ctx, fset, act.deps) + if err != nil { + return nil, nil, err } // Plumb the output values of the dependencies @@ -135,14 +184,13 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { inputs := make(map[*analysis.Analyzer]interface{}) act.objectFacts = make(map[objectFactKey]analysis.Fact) act.packageFacts = make(map[packageFactKey]analysis.Fact) - for _, dep := range act.Deps { - if dep.Pkg == act.Pkg { + 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.Analyzer] = dep.result - - } else if dep.Analyzer == act.Analyzer { // (always true) + inputs[dep.analyzer] = depResults[dep] + } else if dep.analyzer == act.analyzer { // (always true) // Same analysis, different package (vertical edge): // serialized facts produced by prerequisite analysis // become available to this analysis pass. @@ -152,14 +200,14 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { // Run the analysis. pass := &analysis.Pass{ - Analyzer: act.Analyzer, + Analyzer: act.analyzer, Fset: fset, - Files: act.Pkg.GetSyntax(ctx), - Pkg: act.Pkg.GetTypes(), - TypesInfo: act.Pkg.GetTypesInfo(), - TypesSizes: act.Pkg.GetTypesSizes(), + Files: act.pkg.GetSyntax(ctx), + Pkg: act.pkg.GetTypes(), + TypesInfo: act.pkg.GetTypesInfo(), + TypesSizes: act.pkg.GetTypesSizes(), ResultOf: inputs, - Report: func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) }, + Report: func(d analysis.Diagnostic) { diagnostics = append(diagnostics, &d) }, ImportObjectFact: act.importObjectFact, ExportObjectFact: act.exportObjectFact, ImportPackageFact: act.importPackageFact, @@ -169,13 +217,13 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { } act.pass = pass - if act.Pkg.IsIllTyped() { - act.err = errors.Errorf("analysis skipped due to errors in package: %v", act.Pkg.GetErrors()) + if act.pkg.IsIllTyped() { + return nil, nil, errors.Errorf("analysis skipped due to errors in package: %v", act.pkg.GetErrors()) } else { - act.result, act.err = pass.Analyzer.Run(pass) - if act.err == nil { - if got, want := reflect.TypeOf(act.result), pass.Analyzer.ResultType; got != want { - act.err = errors.Errorf( + result, err = pass.Analyzer.Run(pass) + if err == nil { + if got, want := reflect.TypeOf(result), pass.Analyzer.ResultType; got != want { + err = errors.Errorf( "internal error: on package %s, analyzer %s returned a result of type %v, but declared ResultType %v", pass.Pkg.Path(), pass.Analyzer, got, want) } @@ -186,17 +234,17 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { pass.ExportObjectFact = nil pass.ExportPackageFact = nil - return act.err + return diagnostics, result, err } // inheritFacts populates act.facts with // those it obtains from its dependency, dep. -func inheritFacts(act, dep *Action) { +func inheritFacts(act, dep *actionHandle) { 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.GetTypes()) { + if !exportedFrom(key.obj, dep.pkg.types) { continue } act.objectFacts[key] = fact @@ -236,7 +284,7 @@ func exportedFrom(obj types.Object, pkg *types.Package) bool { // 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 { +func (act *actionHandle) importObjectFact(obj types.Object, ptr analysis.Fact) bool { if obj == nil { panic("nil object") } @@ -249,14 +297,14 @@ func (act *Action) importObjectFact(obj types.Object, ptr analysis.Fact) bool { } // exportObjectFact implements Pass.ExportObjectFact. -func (act *Action) exportObjectFact(obj types.Object, fact analysis.Fact) { +func (act *actionHandle) exportObjectFact(obj types.Object, fact analysis.Fact) { if act.pass.ExportObjectFact == nil { panic(fmt.Sprintf("%s: Pass.ExportObjectFact(%s, %T) called after Run", act, obj, fact)) } - if obj.Pkg() != act.Pkg.GetTypes() { + if obj.Pkg() != act.pkg.types { panic(fmt.Sprintf("internal error: in analysis %s of package %s: Fact.Set(%s, %T): can't set facts on objects belonging another package", - act.Analyzer, act.Pkg, obj, fact)) + act.analyzer, act.pkg.ID(), obj, fact)) } key := objectFactKey{obj, factType(fact)} @@ -264,7 +312,7 @@ func (act *Action) exportObjectFact(obj types.Object, fact analysis.Fact) { } // allObjectFacts implements Pass.AllObjectFacts. -func (act *Action) allObjectFacts() []analysis.ObjectFact { +func (act *actionHandle) allObjectFacts() []analysis.ObjectFact { 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]}) @@ -275,7 +323,7 @@ func (act *Action) allObjectFacts() []analysis.ObjectFact { // 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 { +func (act *actionHandle) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool { if pkg == nil { panic("nil package") } @@ -288,7 +336,7 @@ func (act *Action) importPackageFact(pkg *types.Package, ptr analysis.Fact) bool } // exportPackageFact implements Pass.ExportPackageFact. -func (act *Action) exportPackageFact(fact analysis.Fact) { +func (act *actionHandle) exportPackageFact(fact analysis.Fact) { if act.pass.ExportPackageFact == nil { panic(fmt.Sprintf("%s: Pass.ExportPackageFact(%T) called after Run", act, fact)) } @@ -306,7 +354,7 @@ func factType(fact analysis.Fact) reflect.Type { } // allObjectFacts implements Pass.AllObjectFacts. -func (act *Action) allPackageFacts() []analysis.PackageFact { +func (act *actionHandle) allPackageFacts() []analysis.PackageFact { 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]}) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 69321cf1e9..42ce1c8a4d 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -14,7 +14,6 @@ import ( "sort" "sync" - "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/lsp/telemetry" @@ -279,7 +278,6 @@ func (imp *importer) typeCheck(ctx context.Context, cph *checkPackageHandle) (*p Selections: make(map[*ast.SelectorExpr]*types.Selection), Scopes: make(map[ast.Node]*types.Scope), }, - analyses: make(map[*analysis.Analyzer]*analysisEntry), } // If the package comes back with errors from `go list`, // don't bother type-checking it. diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index 513e66f758..01cc760ae9 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -8,7 +8,6 @@ import ( "context" "go/ast" "go/types" - "sort" "sync" "golang.org/x/tools/go/analysis" @@ -33,13 +32,6 @@ type pkg struct { typesInfo *types.Info typesSizes types.Sizes - // The analysis cache holds analysis information for all the packages in a view. - // Each graph node (action) is one unit of analysis. - // Edges express package-to-package (vertical) dependencies, - // and analysis-to-analysis (horizontal) dependencies. - mu sync.Mutex - analyses map[*analysis.Analyzer]*analysisEntry - diagMu sync.Mutex diagnostics map[*analysis.Analyzer][]source.Diagnostic } @@ -50,95 +42,6 @@ type pkg struct { type packageID string type packagePath string -type analysisEntry struct { - done chan struct{} - succeeded bool - *source.Action -} - -func (p *pkg) GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*source.Action, error) { - p.mu.Lock() - e, ok := p.analyses[a] - if ok { - // cache hit - p.mu.Unlock() - - // wait for entry to become ready or the context to be cancelled - select { - case <-e.done: - // If the goroutine we are waiting on was cancelled, we should retry. - // If errors other than cancelation/timeout become possible, it may - // no longer be appropriate to always retry here. - if !e.succeeded { - return p.GetActionGraph(ctx, a) - } - case <-ctx.Done(): - return nil, ctx.Err() - } - } else { - // cache miss - e = &analysisEntry{ - done: make(chan struct{}), - Action: &source.Action{ - Analyzer: a, - Pkg: p, - }, - } - p.analyses[a] = e - p.mu.Unlock() - - defer func() { - // If we got an error, clear out our defunct cache entry. We don't cache - // errors since they could depend on our dependencies, which can change. - // Currently the only possible error is context.Canceled, though, which - // should also not be cached. - if !e.succeeded { - p.mu.Lock() - delete(p.analyses, a) - p.mu.Unlock() - } - - // Always close done so waiters don't get stuck. - close(e.done) - }() - - // This goroutine becomes responsible for populating - // the entry and broadcasting its readiness. - - // Add a dependency on each required analyzers. - for _, req := range a.Requires { - act, err := p.GetActionGraph(ctx, req) - if err != nil { - return nil, err - } - e.Deps = append(e.Deps, act) - } - - // An analysis that consumes/produces facts - // must run on the package's dependencies too. - if len(a.FactTypes) > 0 { - importPaths := make([]string, 0, len(p.imports)) - for importPath := range p.imports { - importPaths = append(importPaths, string(importPath)) - } - sort.Strings(importPaths) // for determinism - for _, importPath := range importPaths { - dep, err := p.GetImport(ctx, importPath) - if err != nil { - return nil, err - } - act, err := dep.GetActionGraph(ctx, a) - if err != nil { - return nil, err - } - e.Deps = append(e.Deps, act) - } - } - e.succeeded = true - } - return e.Action, nil -} - func (p *pkg) ID() string { return string(p.id) } @@ -208,11 +111,11 @@ func (p *pkg) SetDiagnostics(a *analysis.Analyzer, diags []source.Diagnostic) { p.diagnostics[a] = diags } -func (pkg *pkg) FindDiagnostic(pdiag protocol.Diagnostic) (*source.Diagnostic, error) { - pkg.diagMu.Lock() - defer pkg.diagMu.Unlock() +func (p *pkg) FindDiagnostic(pdiag protocol.Diagnostic) (*source.Diagnostic, error) { + p.diagMu.Lock() + defer p.diagMu.Unlock() - for a, diagnostics := range pkg.diagnostics { + for a, diagnostics := range p.diagnostics { if a.Name != pdiag.Source { continue } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index ab76e40457..19c77afd3a 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -34,6 +34,10 @@ type snapshot struct { packages map[packageKey]*checkPackageHandle } +func (s *snapshot) View() source.View { + return s.view +} + func (s *snapshot) getImportedBy(id packageID) []packageID { s.mu.Lock() defer s.mu.Unlock() diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index a1f0e8d6d0..969594a896 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -41,7 +41,7 @@ func Diagnostics(ctx context.Context, view View, f File, disabledAnalyses map[st ctx, done := trace.StartSpan(ctx, "source.Diagnostics", telemetry.File.Of(f.URI())) defer done() - _, cphs, err := view.CheckPackageHandles(ctx, f) + snapshot, cphs, err := view.CheckPackageHandles(ctx, f) if err != nil { return nil, "", err } @@ -82,7 +82,7 @@ func Diagnostics(ctx context.Context, view View, f File, disabledAnalyses map[st // Run diagnostics for the package that this URI belongs to. if !diagnostics(ctx, view, pkg, reports) { // If we don't have any list, parse, or type errors, run analyses. - if err := analyses(ctx, view, cph, disabledAnalyses, reports); err != nil { + if err := analyses(ctx, snapshot, cph, disabledAnalyses, reports); err != nil { log.Error(ctx, "failed to run analyses", err, telemetry.File.Of(f.URI())) } } @@ -188,14 +188,16 @@ func spanToRange(ctx context.Context, view View, pkg Package, spn span.Span, isT return m.Range(spn) } -func analyses(ctx context.Context, view View, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, reports map[span.URI][]Diagnostic) error { +func analyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, reports map[span.URI][]Diagnostic) error { // Type checking and parsing succeeded. Run analyses. - if err := runAnalyses(ctx, view, cph, disabledAnalyses, func(a *analysis.Analyzer, diag analysis.Diagnostic) error { - diagnostic, err := toDiagnostic(ctx, view, diag, a.Name) - if err != nil { - return err + if err := runAnalyses(ctx, snapshot, cph, disabledAnalyses, func(diags []*analysis.Diagnostic, a *analysis.Analyzer) error { + for _, diag := range diags { + diagnostic, err := toDiagnostic(ctx, snapshot.View(), diag, a.Name) + if err != nil { + return err + } + addReport(snapshot.View(), reports, diagnostic.URI, diagnostic) } - addReport(view, reports, diagnostic.URI, diagnostic) return nil }); err != nil { return err @@ -203,16 +205,13 @@ func analyses(ctx context.Context, view View, cph CheckPackageHandle, disabledAn return nil } -func toDiagnostic(ctx context.Context, view View, diag analysis.Diagnostic, category string) (Diagnostic, error) { +func toDiagnostic(ctx context.Context, view View, diag *analysis.Diagnostic, category string) (Diagnostic, error) { r := span.NewRange(view.Session().Cache().FileSet(), diag.Pos, diag.End) spn, err := r.Span() if err != nil { // The diagnostic has an invalid position, so we don't have a valid span. return Diagnostic{}, err } - if diag.Category != "" { - category += "." + category - } f, err := view.GetFile(ctx, spn.URI()) if err != nil { return Diagnostic{}, err @@ -246,6 +245,9 @@ func toDiagnostic(ctx context.Context, view View, diag analysis.Diagnostic, cate if onlyDeletions(fixes) { tags = append(tags, protocol.Unnecessary) } + if diag.Category != "" { + category += "." + diag.Category + } return Diagnostic{ URI: spn.URI(), Range: rng, @@ -309,33 +311,28 @@ func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.UR } } -func runAnalyses(ctx context.Context, view View, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, report func(a *analysis.Analyzer, diag analysis.Diagnostic) error) error { +func runAnalyses(ctx context.Context, snapshot Snapshot, cph CheckPackageHandle, disabledAnalyses map[string]struct{}, report func(diag []*analysis.Diagnostic, a *analysis.Analyzer) error) error { var analyzers []*analysis.Analyzer - for _, a := range view.Options().Analyzers { + for _, a := range snapshot.View().Options().Analyzers { if _, ok := disabledAnalyses[a.Name]; ok { continue } analyzers = append(analyzers, a) } - roots, err := analyze(ctx, view, []CheckPackageHandle{cph}, analyzers) + diagnostics, err := snapshot.Analyze(ctx, cph.ID(), analyzers) if err != nil { return err } // Report diagnostics and errors from root analyzers. - for _, r := range roots { - var sdiags []Diagnostic - for _, diag := range r.diagnostics { - if r.err != nil { - // TODO(matloob): This isn't quite right: we might return a failed prerequisites error, - // which isn't super useful... - return r.err - } - if err := report(r.Analyzer, diag); err != nil { - return err - } - sdiag, err := toDiagnostic(ctx, view, diag, r.Analyzer.Name) + var sdiags []Diagnostic + for a, diags := range diagnostics { + if err := report(diags, a); err != nil { + return err + } + for _, diag := range diags { + sdiag, err := toDiagnostic(ctx, snapshot.View(), diag, a.Name) if err != nil { return err } @@ -345,7 +342,7 @@ func runAnalyses(ctx context.Context, view View, cph CheckPackageHandle, disable if err != nil { return err } - pkg.SetDiagnostics(r.Analyzer, sdiags) + pkg.SetDiagnostics(a, sdiags) } return nil } diff --git a/internal/lsp/source/suggested_fix.go b/internal/lsp/source/suggested_fix.go index dd5f54aea7..b417097b24 100644 --- a/internal/lsp/source/suggested_fix.go +++ b/internal/lsp/source/suggested_fix.go @@ -13,7 +13,7 @@ type SuggestedFix struct { Edits map[span.URI][]protocol.TextEdit } -func suggestedFixes(ctx context.Context, view View, pkg Package, diag analysis.Diagnostic) ([]SuggestedFix, error) { +func suggestedFixes(ctx context.Context, view View, pkg Package, diag *analysis.Diagnostic) ([]SuggestedFix, error) { var fixes []SuggestedFix for _, fix := range diag.SuggestedFixes { edits := make(map[span.URI][]protocol.TextEdit) diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index d23d7cb537..f4a2346c2f 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -111,6 +111,7 @@ type CheckPackageHandle interface { // Cached returns the Package for the CheckPackageHandle if it has already been stored. Cached(ctx context.Context) (Package, error) + // MissingDependencies reports any unresolved imports. MissingDependencies() []string } @@ -256,6 +257,12 @@ type View interface { type Snapshot interface { // Handle returns the FileHandle for the given file. Handle(ctx context.Context, f File) FileHandle + + // View returns the View associated with this snapshot. + View() View + + // Analyze runs the analyses for the given package at this snapshot. + Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) (map[*analysis.Analyzer][]*analysis.Diagnostic, error) } // File represents a source file of any type. @@ -284,9 +291,6 @@ type Package interface { // GetImport returns the CheckPackageHandle for a package imported by this package. GetImport(ctx context.Context, pkgPath string) (Package, error) - // GetActionGraph returns the action graph for the given package. - GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*Action, error) - // FindFile returns the AST and type information for a file that may // belong to or be part of a dependency of the given package. FindFile(ctx context.Context, uri span.URI) (ParseGoHandle, Package, error)