gopls/internal/lsp: split ActivePackages
ActivePackages used to compute a set of packages, then
type check them all, even though many clients need only
metadata. This change replaces it by ActiveMetadata,
which returns only metadata for the set of packages,
and exposes the LoadPackages operation allowing callers
to load only the subset they actually need.
Details:
- Snapshot.TypeCheck method is exposed.
- Snapshot.DiagnosePackage now takes a PackageID,
and calls for typechecking itself. Similarly Server.diagnosePkg.
- source.Analyze now takes a PackageID, noot Package
(which it previously used only for its ID).
- GCOptimizationDetails accepts a Metadata, not a Package.
- workspaceMetadata replaces workspacePackageID, activePackageIDs.
- Combined the two loops in Server.diagnoseChangedFiles,
and parallelized the diagnose calls in moddiagnostics,
to ensure that type checking happens in parallel.
Change-Id: Id0383a678ce45447f3313d1426e3f9b96050eaa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456275
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go
index e2c56a5..69bb393 100644
--- a/gopls/internal/lsp/cache/analysis.go
+++ b/gopls/internal/lsp/cache/analysis.go
@@ -462,8 +462,12 @@
return t
}
-func (s *snapshot) DiagnosePackage(ctx context.Context, spkg source.Package) (map[span.URI][]*source.Diagnostic, error) {
- pkg := spkg.(*pkg)
+func (s *snapshot) DiagnosePackage(ctx context.Context, id PackageID) (map[span.URI][]*source.Diagnostic, error) {
+ pkgs, err := s.TypeCheck(ctx, source.TypecheckFull, id)
+ if err != nil {
+ return nil, err
+ }
+ pkg := pkgs[0].(*pkg)
var errorAnalyzerDiag []*source.Diagnostic
if pkg.HasTypeErrors() {
// Apply type error analyzers.
diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go
index 9495f3f..fa30df1 100644
--- a/gopls/internal/lsp/cache/mod_tidy.go
+++ b/gopls/internal/lsp/cache/mod_tidy.go
@@ -192,12 +192,7 @@
// Add diagnostics for missing modules anywhere they are imported in the
// workspace.
// TODO(adonovan): opt: opportunities for parallelism abound.
- for _, id := range snapshot.workspacePackageIDs() {
- m := snapshot.Metadata(id)
- if m == nil {
- return nil, fmt.Errorf("no metadata for %s", id)
- }
-
+ for _, m := range snapshot.workspaceMetadata() {
// Read both lists of files of this package, in parallel.
goFiles, compiledGoFiles, err := readGoFiles(ctx, snapshot, m)
if err != nil {
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index ac92e8c..b439e9e 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -299,6 +299,7 @@
}
// The user may have a multiple directories in their GOPATH.
// Check if the workspace is within any of them.
+ // TODO(rfindley): this should probably be subject to "if GO111MODULES = off {...}".
for _, gp := range filepath.SplitList(s.view.gopath) {
if source.InDir(filepath.Join(gp, "src"), s.view.rootURI.Filename()) {
return true
@@ -653,7 +654,7 @@
ids = append(ids, m.ID)
}
- return s.loadPackages(ctx, mode, ids...)
+ return s.TypeCheck(ctx, mode, ids...)
}
func (s *snapshot) PackageForFile(ctx context.Context, uri span.URI, mode source.TypecheckMode, pkgPolicy source.PackageFilter) (source.Package, error) {
@@ -671,7 +672,7 @@
case source.WidestPackage:
metas = metas[len(metas)-1:]
}
- pkgs, err := s.loadPackages(ctx, mode, metas[0].ID)
+ pkgs, err := s.TypeCheck(ctx, mode, metas[0].ID)
if err != nil {
return nil, err
}
@@ -713,8 +714,8 @@
return metas, err
}
-// loadPackages type-checks the specified packages in the given mode.
-func (s *snapshot) loadPackages(ctx context.Context, mode source.TypecheckMode, ids ...PackageID) ([]source.Package, error) {
+// TypeCheck type-checks the specified packages in the given mode.
+func (s *snapshot) TypeCheck(ctx context.Context, mode source.TypecheckMode, ids ...PackageID) ([]source.Package, error) {
// Build all the handles...
var phs []*packageHandle
for _, id := range ids {
@@ -854,30 +855,14 @@
return s.meta.importedBy[id]
}
-func (s *snapshot) workspacePackageIDs() (ids []PackageID) {
+func (s *snapshot) workspaceMetadata() (meta []*source.Metadata) {
s.mu.Lock()
defer s.mu.Unlock()
for id := range s.workspacePackages {
- ids = append(ids, id)
+ meta = append(meta, s.meta.metadata[id])
}
- return ids
-}
-
-func (s *snapshot) activePackageIDs() (ids []PackageID) {
- if s.view.Options().MemoryMode == source.ModeNormal {
- return s.workspacePackageIDs()
- }
-
- s.mu.Lock()
- defer s.mu.Unlock()
-
- for id := range s.workspacePackages {
- if s.isActiveLocked(id) {
- ids = append(ids, id)
- }
- }
- return ids
+ return meta
}
func (s *snapshot) isActiveLocked(id PackageID) (active bool) {
@@ -1089,35 +1074,25 @@
return files
}
-func (s *snapshot) ActivePackages(ctx context.Context) ([]source.Package, error) {
- phs, err := s.activePackageHandles(ctx)
- if err != nil {
- return nil, err
- }
- var pkgs []source.Package
- for _, ph := range phs {
- pkg, err := ph.await(ctx, s)
- if err != nil {
- return nil, err
- }
- pkgs = append(pkgs, pkg)
- }
- return pkgs, nil
-}
-
-func (s *snapshot) activePackageHandles(ctx context.Context) ([]*packageHandle, error) {
+func (s *snapshot) ActiveMetadata(ctx context.Context) ([]*source.Metadata, error) {
if err := s.awaitLoaded(ctx); err != nil {
return nil, err
}
- var phs []*packageHandle
- for _, pkgID := range s.activePackageIDs() {
- ph, err := s.buildPackageHandle(ctx, pkgID, s.workspaceParseMode(pkgID))
- if err != nil {
- return nil, err
- }
- phs = append(phs, ph)
+
+ if s.view.Options().MemoryMode == source.ModeNormal {
+ return s.workspaceMetadata(), nil
}
- return phs, nil
+
+ // ModeDegradeClosed
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ var active []*source.Metadata
+ for id := range s.workspacePackages {
+ if s.isActiveLocked(id) {
+ active = append(active, s.Metadata(id))
+ }
+ }
+ return active, nil
}
// Symbols extracts and returns the symbols for each file in all the snapshot's views.
@@ -1394,8 +1369,8 @@
// Even if packages didn't fail to load, we still may want to show
// additional warnings.
if loadErr == nil {
- wsPkgs, _ := s.ActivePackages(ctx)
- if msg := shouldShowAdHocPackagesWarning(s, wsPkgs); msg != "" {
+ active, _ := s.ActiveMetadata(ctx)
+ if msg := shouldShowAdHocPackagesWarning(s, active); msg != "" {
return &source.CriticalError{
MainError: errors.New(msg),
}
@@ -1410,7 +1385,7 @@
// on-demand or via orphaned file reloading.
//
// TODO(rfindley): re-evaluate this heuristic.
- if containsCommandLineArguments(wsPkgs) {
+ if containsCommandLineArguments(active) {
err, diags := s.workspaceLayoutError(ctx)
if err != nil {
if ctx.Err() != nil {
@@ -1445,15 +1420,9 @@
If you are using modules, please open your editor to a directory in your module.
If you believe this warning is incorrect, please file an issue: https://github.com/golang/go/issues/new.`
-func shouldShowAdHocPackagesWarning(snapshot source.Snapshot, pkgs []source.Package) string {
+func shouldShowAdHocPackagesWarning(snapshot source.Snapshot, active []*source.Metadata) string {
if !snapshot.ValidBuildConfiguration() {
- for _, pkg := range pkgs {
- // TODO(adonovan): strength-reduce the parameter to []Metadata.
- m := snapshot.Metadata(pkg.ID())
- if m == nil {
- continue
- }
-
+ for _, m := range active {
// A blank entry in DepsByImpPath
// indicates a missing dependency.
for _, importID := range m.DepsByImpPath {
@@ -1466,9 +1435,9 @@
return ""
}
-func containsCommandLineArguments(pkgs []source.Package) bool {
- for _, pkg := range pkgs {
- if source.IsCommandLineArguments(pkg.ID()) {
+func containsCommandLineArguments(metas []*source.Metadata) bool {
+ for _, m := range metas {
+ if source.IsCommandLineArguments(m.ID) {
return true
}
}
diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go
index 1cf58f9..63c0d67 100644
--- a/gopls/internal/lsp/code_action.go
+++ b/gopls/internal/lsp/code_action.go
@@ -155,16 +155,19 @@
if ctx.Err() != nil {
return nil, ctx.Err()
}
- pkg, err := snapshot.PackageForFile(ctx, fh.URI(), source.TypecheckFull, source.WidestPackage)
+ metas, err := snapshot.MetadataForFile(ctx, fh.URI())
if err != nil {
return nil, err
}
-
- pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, pkg)
+ if len(metas) == 0 {
+ return nil, fmt.Errorf("no package containing file %q", fh.URI())
+ }
+ id := metas[len(metas)-1].ID // last => widest package
+ pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, id)
if err != nil {
return nil, err
}
- analysisDiags, err := source.Analyze(ctx, snapshot, pkg, true)
+ analysisDiags, err := source.Analyze(ctx, snapshot, id, true)
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/diagnostics.go b/gopls/internal/lsp/diagnostics.go
index dec452b..addfa96 100644
--- a/gopls/internal/lsp/diagnostics.go
+++ b/gopls/internal/lsp/diagnostics.go
@@ -15,6 +15,7 @@
"sync"
"time"
+ "golang.org/x/sync/errgroup"
"golang.org/x/tools/gopls/internal/lsp/debug/log"
"golang.org/x/tools/gopls/internal/lsp/mod"
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -177,7 +178,8 @@
ctx, done := event.Start(ctx, "Server.diagnoseChangedFiles", source.SnapshotLabels(snapshot)...)
defer done()
- packages := make(map[source.Package]struct{})
+ var group errgroup.Group
+ seen := make(map[*source.Metadata]bool)
for _, uri := range uris {
// If the change is only on-disk and the file is not open, don't
// directly request its package. It may not be a workspace package.
@@ -194,28 +196,30 @@
if snapshot.IsBuiltin(ctx, uri) {
continue
}
- pkgs, err := snapshot.PackagesForFile(ctx, uri, source.TypecheckFull, false)
+
+ // Find all packages that include this file and diagnose them in parallel.
+ metas, err := snapshot.MetadataForFile(ctx, uri)
if err != nil {
// TODO (findleyr): we should probably do something with the error here,
// but as of now this can fail repeatedly if load fails, so can be too
// noisy to log (and we'll handle things later in the slow pass).
continue
}
- for _, pkg := range pkgs {
- packages[pkg] = struct{}{}
+ for _, m := range metas {
+ if m.IsIntermediateTestVariant() {
+ continue
+ }
+ if !seen[m] {
+ seen[m] = true
+ m := m
+ group.Go(func() error {
+ s.diagnosePkg(ctx, snapshot, m, false)
+ return nil // error result is ignored
+ })
+ }
}
}
- var wg sync.WaitGroup
- for pkg := range packages {
- wg.Add(1)
-
- go func(pkg source.Package) {
- defer wg.Done()
-
- s.diagnosePkg(ctx, snapshot, pkg, false)
- }(pkg)
- }
- wg.Wait()
+ group.Wait() // ignore error
}
// diagnose is a helper function for running diagnostics with a given context.
@@ -268,14 +272,9 @@
}
store(workSource, "diagnosing go.work file", workReports, workErr, true)
- // All subsequent steps depend on the completion of
- // type-checking of the all active packages in the workspace.
- // This step may take many seconds initially.
- // (mod.Diagnostics would implicitly wait for this too,
- // but the control is clearer if it is explicit here.)
- activePkgs, activeErr := snapshot.ActivePackages(ctx)
-
// Diagnose go.mod file.
+ // (This step demands type checking of all active packages:
+ // the bottleneck in the startup sequence for a big workspace.)
modReports, modErr := mod.Diagnostics(ctx, snapshot)
if ctx.Err() != nil {
log.Trace.Log(ctx, "diagnose cancelled")
@@ -291,6 +290,7 @@
}
store(modVulncheckSource, "diagnosing vulnerabilities", vulnReports, vulnErr, false)
+ activeMetas, activeErr := snapshot.ActiveMetadata(ctx)
if s.shouldIgnoreError(ctx, snapshot, activeErr) {
return
}
@@ -313,7 +313,7 @@
// If there are no workspace packages, there is nothing to diagnose and
// there are no orphaned files.
- if len(activePkgs) == 0 {
+ if len(activeMetas) == 0 {
return
}
@@ -324,16 +324,16 @@
wg sync.WaitGroup
seen = map[span.URI]struct{}{}
)
- for _, pkg := range activePkgs {
- for _, pgf := range pkg.CompiledGoFiles() {
- seen[pgf.URI] = struct{}{}
+ for _, m := range activeMetas {
+ for _, uri := range m.CompiledGoFiles {
+ seen[uri] = struct{}{}
}
wg.Add(1)
- go func(pkg source.Package) {
+ go func(m *source.Metadata) {
defer wg.Done()
- s.diagnosePkg(ctx, snapshot, pkg, forceAnalysis)
- }(pkg)
+ s.diagnosePkg(ctx, snapshot, m, forceAnalysis)
+ }(m)
}
wg.Wait()
@@ -352,55 +352,55 @@
}
}
-func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, pkg source.Package, alwaysAnalyze bool) {
- ctx, done := event.Start(ctx, "Server.diagnosePkg", append(source.SnapshotLabels(snapshot), tag.Package.Of(string(pkg.ID())))...)
+func (s *Server) diagnosePkg(ctx context.Context, snapshot source.Snapshot, m *source.Metadata, alwaysAnalyze bool) {
+ ctx, done := event.Start(ctx, "Server.diagnosePkg", append(source.SnapshotLabels(snapshot), tag.Package.Of(string(m.ID)))...)
defer done()
enableDiagnostics := false
includeAnalysis := alwaysAnalyze // only run analyses for packages with open files
- for _, pgf := range pkg.CompiledGoFiles() {
- enableDiagnostics = enableDiagnostics || !snapshot.IgnoredFile(pgf.URI)
- includeAnalysis = includeAnalysis || snapshot.IsOpen(pgf.URI)
+ for _, uri := range m.CompiledGoFiles {
+ enableDiagnostics = enableDiagnostics || !snapshot.IgnoredFile(uri)
+ includeAnalysis = includeAnalysis || snapshot.IsOpen(uri)
}
// Don't show any diagnostics on ignored files.
if !enableDiagnostics {
return
}
- pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, pkg)
+ pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, m.ID)
if err != nil {
- event.Error(ctx, "warning: diagnosing package", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(pkg.ID())))...)
+ event.Error(ctx, "warning: diagnosing package", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(m.ID)))...)
return
}
- for _, cgf := range pkg.CompiledGoFiles() {
+ for _, uri := range m.CompiledGoFiles {
// builtin.go exists only for documentation purposes, and is not valid Go code.
// Don't report distracting errors
- if !snapshot.IsBuiltin(ctx, cgf.URI) {
- s.storeDiagnostics(snapshot, cgf.URI, typeCheckSource, pkgDiagnostics[cgf.URI], true)
+ if !snapshot.IsBuiltin(ctx, uri) {
+ s.storeDiagnostics(snapshot, uri, typeCheckSource, pkgDiagnostics[uri], true)
}
}
if includeAnalysis {
- reports, err := source.Analyze(ctx, snapshot, pkg, false)
+ reports, err := source.Analyze(ctx, snapshot, m.ID, false)
if err != nil {
- event.Error(ctx, "warning: analyzing package", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(pkg.ID())))...)
+ event.Error(ctx, "warning: analyzing package", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(m.ID)))...)
return
}
- for _, cgf := range pkg.CompiledGoFiles() {
- s.storeDiagnostics(snapshot, cgf.URI, analysisSource, reports[cgf.URI], true)
+ for _, uri := range m.CompiledGoFiles {
+ s.storeDiagnostics(snapshot, uri, analysisSource, reports[uri], true)
}
}
// If gc optimization details are requested, add them to the
// diagnostic reports.
s.gcOptimizationDetailsMu.Lock()
- _, enableGCDetails := s.gcOptimizationDetails[pkg.ID()]
+ _, enableGCDetails := s.gcOptimizationDetails[m.ID]
s.gcOptimizationDetailsMu.Unlock()
if enableGCDetails {
- gcReports, err := source.GCOptimizationDetails(ctx, snapshot, pkg)
+ gcReports, err := source.GCOptimizationDetails(ctx, snapshot, m)
if err != nil {
- event.Error(ctx, "warning: gc details", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(pkg.ID())))...)
+ event.Error(ctx, "warning: gc details", err, append(source.SnapshotLabels(snapshot), tag.Package.Of(string(m.ID)))...)
}
s.gcOptimizationDetailsMu.Lock()
- _, enableGCDetails := s.gcOptimizationDetails[pkg.ID()]
+ _, enableGCDetails := s.gcOptimizationDetails[m.ID]
// NOTE(golang/go#44826): hold the gcOptimizationDetails lock, and re-check
// whether gc optimization details are enabled, while storing gc_details
diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go
index 77a7e02..a0e4272 100644
--- a/gopls/internal/lsp/mod/diagnostics.go
+++ b/gopls/internal/lsp/mod/diagnostics.go
@@ -11,9 +11,11 @@
"fmt"
"sort"
"strings"
+ "sync"
"golang.org/x/mod/modfile"
"golang.org/x/mod/semver"
+ "golang.org/x/sync/errgroup"
"golang.org/x/tools/gopls/internal/govulncheck"
"golang.org/x/tools/gopls/internal/lsp/command"
"golang.org/x/tools/gopls/internal/lsp/protocol"
@@ -90,17 +92,31 @@
// TODO(rfindley): Try to avoid calling DiagnosePackage on all packages in the workspace here,
// for every go.mod file. If gc_details is enabled, it looks like this could lead to extra
// go command invocations (as gc details is not memoized).
- wspkgs, err := snapshot.ActivePackages(ctx)
+ active, err := snapshot.ActiveMetadata(ctx)
if err != nil && !source.IsNonFatalGoModError(err) {
event.Error(ctx, fmt.Sprintf("workspace packages: diagnosing %s", pm.URI), err)
}
if err == nil {
- for _, pkg := range wspkgs {
- pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, pkg)
- if err != nil {
- return nil, err
- }
- diagnostics = append(diagnostics, pkgDiagnostics[fh.URI()]...)
+ // Diagnose packages in parallel.
+ // (It's possible this is the first operation after the initial
+ // metadata load to demand type-checking of all the active packages.)
+ var group errgroup.Group
+ var mu sync.Mutex
+ for _, m := range active {
+ m := m
+ group.Go(func() error {
+ pkgDiagnostics, err := snapshot.DiagnosePackage(ctx, m.ID)
+ if err != nil {
+ return err
+ }
+ mu.Lock()
+ diagnostics = append(diagnostics, pkgDiagnostics[fh.URI()]...)
+ mu.Unlock()
+ return nil
+ })
+ }
+ if err := group.Wait(); err != nil {
+ return nil, err
}
}
diff --git a/gopls/internal/lsp/source/completion/package.go b/gopls/internal/lsp/source/completion/package.go
index 19c5249..abfa8b8 100644
--- a/gopls/internal/lsp/source/completion/package.go
+++ b/gopls/internal/lsp/source/completion/package.go
@@ -215,7 +215,7 @@
// file. This also includes test packages for these packages (<pkg>_test) and
// the directory name itself.
func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI span.URI, prefix string) (packages []candidate, err error) {
- workspacePackages, err := snapshot.ActivePackages(ctx)
+ active, err := snapshot.ActiveMetadata(ctx)
if err != nil {
return nil, err
}
@@ -245,18 +245,18 @@
// The `go` command by default only allows one package per directory but we
// support multiple package suggestions since gopls is build system agnostic.
- for _, pkg := range workspacePackages {
- if pkg.Name() == "main" || pkg.Name() == "" {
+ for _, m := range active {
+ if m.Name == "main" || m.Name == "" {
continue
}
- if _, ok := seenPkgs[pkg.Name()]; ok {
+ if _, ok := seenPkgs[m.Name]; ok {
continue
}
// Only add packages that are previously used in the current directory.
var relevantPkg bool
- for _, pgf := range pkg.CompiledGoFiles() {
- if filepath.Dir(pgf.URI.Filename()) == dirPath {
+ for _, uri := range m.CompiledGoFiles {
+ if filepath.Dir(uri.Filename()) == dirPath {
relevantPkg = true
break
}
@@ -268,13 +268,13 @@
// Add a found package used in current directory as a high relevance
// suggestion and the test package for it as a medium relevance
// suggestion.
- if score := float64(matcher.Score(string(pkg.Name()))); score > 0 {
- packages = append(packages, toCandidate(string(pkg.Name()), score*highScore))
+ if score := float64(matcher.Score(string(m.Name))); score > 0 {
+ packages = append(packages, toCandidate(string(m.Name), score*highScore))
}
- seenPkgs[pkg.Name()] = struct{}{}
+ seenPkgs[m.Name] = struct{}{}
- testPkgName := pkg.Name() + "_test"
- if _, ok := seenPkgs[testPkgName]; ok || strings.HasSuffix(string(pkg.Name()), "_test") {
+ testPkgName := m.Name + "_test"
+ if _, ok := seenPkgs[testPkgName]; ok || strings.HasSuffix(string(m.Name), "_test") {
continue
}
if score := float64(matcher.Score(string(testPkgName))); score > 0 {
diff --git a/gopls/internal/lsp/source/diagnostics.go b/gopls/internal/lsp/source/diagnostics.go
index 7780f47..db4fc90 100644
--- a/gopls/internal/lsp/source/diagnostics.go
+++ b/gopls/internal/lsp/source/diagnostics.go
@@ -6,6 +6,7 @@
import (
"context"
+ "fmt"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/span"
@@ -24,7 +25,7 @@
Message string
}
-func Analyze(ctx context.Context, snapshot Snapshot, pkg Package, includeConvenience bool) (map[span.URI][]*Diagnostic, error) {
+func Analyze(ctx context.Context, snapshot Snapshot, pkgid PackageID, includeConvenience bool) (map[span.URI][]*Diagnostic, error) {
// Exit early if the context has been canceled. This also protects us
// from a race on Options, see golang/go#36699.
if ctx.Err() != nil {
@@ -39,6 +40,7 @@
if includeConvenience { // e.g. for codeAction
categories = append(categories, options.ConvenienceAnalyzers) // e.g. fillstruct
}
+
var analyzers []*Analyzer
for _, cat := range categories {
for _, a := range cat {
@@ -46,13 +48,13 @@
}
}
- analysisDiagnostics, err := snapshot.Analyze(ctx, pkg.ID(), analyzers)
+ analysisDiagnostics, err := snapshot.Analyze(ctx, pkgid, analyzers)
if err != nil {
return nil, err
}
- reports := map[span.URI][]*Diagnostic{}
// Report diagnostics and errors from root analyzers.
+ reports := map[span.URI][]*Diagnostic{}
for _, diag := range analysisDiagnostics {
reports[diag.URI] = append(reports[diag.URI], diag)
}
@@ -64,15 +66,19 @@
if err != nil {
return VersionedFileIdentity{}, nil, err
}
- pkg, _, err := GetTypedFile(ctx, snapshot, fh, NarrowestPackage)
+ metas, err := snapshot.MetadataForFile(ctx, uri)
if err != nil {
return VersionedFileIdentity{}, nil, err
}
- diagnostics, err := snapshot.DiagnosePackage(ctx, pkg)
+ if len(metas) == 0 {
+ return VersionedFileIdentity{}, nil, fmt.Errorf("no package containing file %q", uri)
+ }
+ id := metas[0].ID // 0 => narrowest package
+ diagnostics, err := snapshot.DiagnosePackage(ctx, id)
if err != nil {
return VersionedFileIdentity{}, nil, err
}
- analysisDiags, err := Analyze(ctx, snapshot, pkg, false)
+ analysisDiags, err := Analyze(ctx, snapshot, id, false)
if err != nil {
return VersionedFileIdentity{}, nil, err
}
diff --git a/gopls/internal/lsp/source/gc_annotations.go b/gopls/internal/lsp/source/gc_annotations.go
index ab0fd60..d5245c8 100644
--- a/gopls/internal/lsp/source/gc_annotations.go
+++ b/gopls/internal/lsp/source/gc_annotations.go
@@ -35,11 +35,11 @@
Bounds Annotation = "bounds"
)
-func GCOptimizationDetails(ctx context.Context, snapshot Snapshot, pkg Package) (map[VersionedFileIdentity][]*Diagnostic, error) {
- if len(pkg.CompiledGoFiles()) == 0 {
+func GCOptimizationDetails(ctx context.Context, snapshot Snapshot, m *Metadata) (map[VersionedFileIdentity][]*Diagnostic, error) {
+ if len(m.CompiledGoFiles) == 0 {
return nil, nil
}
- pkgDir := filepath.Dir(pkg.CompiledGoFiles()[0].URI.Filename())
+ pkgDir := filepath.Dir(m.CompiledGoFiles[0].Filename())
outDir := filepath.Join(os.TempDir(), fmt.Sprintf("gopls-%d.details", os.Getpid()))
if err := os.MkdirAll(outDir, 0700); err != nil {
diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go
index 1168370..493553b 100644
--- a/gopls/internal/lsp/source/view.go
+++ b/gopls/internal/lsp/source/view.go
@@ -107,12 +107,43 @@
// Position information is added to FileSet().
ParseGo(ctx context.Context, fh FileHandle, mode ParseMode) (*ParsedGoFile, error)
- // DiagnosePackage returns basic diagnostics, including list, parse, and type errors
- // for pkg, grouped by file.
- DiagnosePackage(ctx context.Context, pkg Package) (map[span.URI][]*Diagnostic, error)
+ // DiagnosePackage returns basic diagnostics, including list,
+ // parse, and type errors for pkg, grouped by file.
+ //
+ // It may include suggested fixes for type errors, created by
+ // running the analysis framework.
+ //
+ // TODO(adonovan): this operation does a mix of checks that
+ // range from cheap (list errors, parse errors) to expensive
+ // (type errors, type-error-analyzer results). In particular,
+ // type-error analyzers (as currently implemented) depend on
+ // the full analyzer machinery, and in the near future that
+ // will be completely separate from regular type checking.
+ // So, we must choose between:
+ //
+ // (1) rewriting them as ad-hoc functions that operate on
+ // type-checked packages. That's a fair amount of work
+ // since they are fairly deeply enmeshed in the framework,
+ // (some have horizontal dependencies such as Inspector),
+ // and quite reasonably so. So we reject this in favor of:
+ //
+ // (2) separating the generation of type errors (which happens
+ // at the lowest latency) from full analysis, which is
+ // slower, although hopefully eventually only on the order
+ // of seconds. In this case, type error analyzers are
+ // basically not special; the only special part would be
+ // the postprocessing step to merge the
+ // type-error-analyzer fixes into the ordinary diagnostics
+ // produced by type checking. (Not yet sure how to do that.)
+ //
+ // So then the role of this function is "report fast
+ // diagnostics, up to type-checking", and the role of
+ // Analyze() is "run the analysis framework, included
+ // suggested fixes for type errors"
+ DiagnosePackage(ctx context.Context, id PackageID) (map[span.URI][]*Diagnostic, error)
- // Analyze runs the analyses for the given package at this snapshot.
- Analyze(ctx context.Context, pkgID PackageID, analyzers []*Analyzer) ([]*Diagnostic, error)
+ // Analyze runs the specified analyzers on the given package at this snapshot.
+ Analyze(ctx context.Context, id PackageID, analyzers []*Analyzer) ([]*Diagnostic, error)
// RunGoCommandPiped runs the given `go` command, writing its output
// to stdout and stderr. Verb, Args, and WorkingDir must be specified.
@@ -198,11 +229,12 @@
// need ever to "load everything at once" using this function.
KnownPackages(ctx context.Context) ([]Package, error)
- // ActivePackages returns the packages considered 'active' in the workspace.
+ // ActiveMetadata returns a new, unordered slice containing
+ // metadata for all packages considered 'active' in the workspace.
//
// In normal memory mode, this is all workspace packages. In degraded memory
// mode, this is just the reverse transitive closure of open packages.
- ActivePackages(ctx context.Context) ([]Package, error)
+ ActiveMetadata(ctx context.Context) ([]*Metadata, error)
// AllValidMetadata returns all valid metadata loaded for the snapshot.
AllValidMetadata(ctx context.Context) ([]*Metadata, error)
@@ -217,9 +249,15 @@
// MetadataForFile returns a new slice containing metadata for each
// package containing the Go file identified by uri, ordered by the
// number of CompiledGoFiles (i.e. "narrowest" to "widest" package).
+ // The result may include tests and intermediate test variants of
+ // importable packages.
// It returns an error if the context was cancelled.
MetadataForFile(ctx context.Context, uri span.URI) ([]*Metadata, error)
+ // TypeCheck parses and type-checks the specified packages,
+ // and returns them in the same order as the ids.
+ TypeCheck(ctx context.Context, mode TypecheckMode, ids ...PackageID) ([]Package, error)
+
// GetCriticalError returns any critical errors in the workspace.
//
// A nil result may mean success, or context cancellation.
