diff --git a/gopls/internal/lsp/cache/analysis.go b/gopls/internal/lsp/cache/analysis.go index e2c56a53c0f..69bb39313ab 100644 --- a/gopls/internal/lsp/cache/analysis.go +++ b/gopls/internal/lsp/cache/analysis.go @@ -462,8 +462,12 @@ func factType(fact analysis.Fact) reflect.Type { 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 9495f3fa3cf..fa30df18e36 100644 --- a/gopls/internal/lsp/cache/mod_tidy.go +++ b/gopls/internal/lsp/cache/mod_tidy.go @@ -192,12 +192,7 @@ func modTidyDiagnostics(ctx context.Context, snapshot *snapshot, pm *source.Pars // 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 ac92e8cd79d..b439e9e2908 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -299,6 +299,7 @@ func (s *snapshot) ValidBuildConfiguration() bool { } // 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 @@ func (s *snapshot) PackagesForFile(ctx context.Context, uri span.URI, mode sourc 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 @@ func (s *snapshot) PackageForFile(ctx context.Context, uri span.URI, mode source 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 @@ func (s *snapshot) containingPackages(ctx context.Context, uri span.URI) ([]*sou 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 @@ func (s *snapshot) getImportedBy(id PackageID) []PackageID { 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 @@ func (s *snapshot) knownFilesInDir(ctx context.Context, dir span.URI) []span.URI return files } -func (s *snapshot) ActivePackages(ctx context.Context) ([]source.Package, error) { - phs, err := s.activePackageHandles(ctx) - if err != nil { +func (s *snapshot) ActiveMetadata(ctx context.Context) ([]*source.Metadata, error) { + if err := s.awaitLoaded(ctx); 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) { - if err := s.awaitLoaded(ctx); err != nil { - return nil, err + if s.view.Options().MemoryMode == source.ModeNormal { + return s.workspaceMetadata(), nil } - var phs []*packageHandle - for _, pkgID := range s.activePackageIDs() { - ph, err := s.buildPackageHandle(ctx, pkgID, s.workspaceParseMode(pkgID)) - if err != nil { - return nil, err + + // 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)) } - phs = append(phs, ph) } - return phs, nil + return active, nil } // Symbols extracts and returns the symbols for each file in all the snapshot's views. @@ -1394,8 +1369,8 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError { // 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 @@ func (s *snapshot) GetCriticalError(ctx context.Context) *source.CriticalError { // 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 @@ const adHocPackagesWarning = `You are outside of a module and outside of $GOPATH 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 @@ func shouldShowAdHocPackagesWarning(snapshot source.Snapshot, pkgs []source.Pack 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 1cf58f9ddf2..63c0d67c6a5 100644 --- a/gopls/internal/lsp/code_action.go +++ b/gopls/internal/lsp/code_action.go @@ -155,16 +155,19 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara 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 dec452b2454..addfa96aca8 100644 --- a/gopls/internal/lsp/diagnostics.go +++ b/gopls/internal/lsp/diagnostics.go @@ -15,6 +15,7 @@ import ( "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 @@ func (s *Server) diagnoseChangedFiles(ctx context.Context, snapshot source.Snaps 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 @@ func (s *Server) diagnoseChangedFiles(ctx context.Context, snapshot source.Snaps 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 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn } 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 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn } store(modVulncheckSource, "diagnosing vulnerabilities", vulnReports, vulnErr, false) + activeMetas, activeErr := snapshot.ActiveMetadata(ctx) if s.shouldIgnoreError(ctx, snapshot, activeErr) { return } @@ -313,7 +313,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn // 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 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn 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) diagnose(ctx context.Context, snapshot source.Snapshot, forceAn } } -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 77a7e024266..a0e42720a8f 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -11,9 +11,11 @@ import ( "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 @@ func ModDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.Fil // 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 19c52491cef..abfa8b8e8a5 100644 --- a/gopls/internal/lsp/source/completion/package.go +++ b/gopls/internal/lsp/source/completion/package.go @@ -215,7 +215,7 @@ func (c *completer) packageNameCompletions(ctx context.Context, fileURI span.URI // file. This also includes test packages for these packages (_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 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s // 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 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s // 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 7780f471f11..db4fc900a4e 100644 --- a/gopls/internal/lsp/source/diagnostics.go +++ b/gopls/internal/lsp/source/diagnostics.go @@ -6,6 +6,7 @@ package source import ( "context" + "fmt" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/span" @@ -24,7 +25,7 @@ type RelatedInformation struct { 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 @@ func Analyze(ctx context.Context, snapshot Snapshot, pkg Package, includeConveni 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 @@ func Analyze(ctx context.Context, snapshot Snapshot, pkg Package, includeConveni } } - 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 @@ func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (Vers 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 ab0fd6035e6..d5245c8cdd1 100644 --- a/gopls/internal/lsp/source/gc_annotations.go +++ b/gopls/internal/lsp/source/gc_annotations.go @@ -35,11 +35,11 @@ const ( 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 1168370c6f8..493553bc86d 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -107,12 +107,43 @@ type Snapshot interface { // 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 @@ type Snapshot interface { // 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 @@ type Snapshot interface { // 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.