diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go index 634afdff71..281c5d9b08 100644 --- a/gopls/internal/lsp/cache/snapshot.go +++ b/gopls/internal/lsp/cache/snapshot.go @@ -1150,19 +1150,14 @@ func (s *snapshot) KnownPackages(ctx context.Context) ([]source.Package, error) return nil, err } - // The WorkspaceSymbols implementation relies on this function returning - // workspace packages first. - ids := s.workspacePackageIDs() + ids := make([]source.PackageID, 0, len(s.meta.metadata)) s.mu.Lock() for id := range s.meta.metadata { - if _, ok := s.workspacePackages[id]; ok { - continue - } ids = append(ids, id) } s.mu.Unlock() - var pkgs []source.Package + pkgs := make([]source.Package, 0, len(ids)) for _, id := range ids { pkg, err := s.checkedPackage(ctx, id, s.workspaceParseMode(id)) if err != nil { diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index a3d93e265c..28cf3471a7 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -735,7 +735,7 @@ func (c *commandHandler) ListKnownPackages(ctx context.Context, args command.URI progress: "Listing packages", forURI: args.URI, }, func(ctx context.Context, deps commandDeps) error { - pkgs, err := source.KnownPackages(ctx, deps.snapshot, deps.fh) + pkgs, err := source.KnownPackagePaths(ctx, deps.snapshot, deps.fh) for _, pkg := range pkgs { result.Packages = append(result.Packages, string(pkg)) } diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go index 1260e0e7c3..144d6587da 100644 --- a/gopls/internal/lsp/source/completion/completion.go +++ b/gopls/internal/lsp/source/completion/completion.go @@ -1475,6 +1475,9 @@ func (c *completer) unimportedPackages(ctx context.Context, seen map[string]stru count := 0 + // TODO(adonovan): strength-reduce to a metadata query. + // All that's needed below is Package.{Name,Path}. + // Presumably that can be answered more thoroughly more quickly. known, err := c.snapshot.CachedImportPaths(ctx) if err != nil { return err diff --git a/gopls/internal/lsp/source/known_packages.go b/gopls/internal/lsp/source/known_packages.go index efaaee57ae..6f3bf8c902 100644 --- a/gopls/internal/lsp/source/known_packages.go +++ b/gopls/internal/lsp/source/known_packages.go @@ -16,21 +16,24 @@ import ( "golang.org/x/tools/internal/imports" ) -// KnownPackages returns a list of all known packages -// in the package graph that could potentially be imported -// by the given file. -func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle) ([]PackagePath, error) { +// KnownPackagePaths returns a new list of package paths of all known +// packages in the package graph that could potentially be imported by +// the given file. The list is ordered lexicographically, except that +// all dot-free paths (standard packages) appear before dotful ones. +func KnownPackagePaths(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle) ([]PackagePath, error) { + // TODO(adonovan): this whole algorithm could be more + // simply expressed in terms of Metadata, not Packages. + // All we need below is: + // - for fh: Metadata.{DepsByPkgPath,Path,Name} + // - for all cached packages: Metadata.{Path,Name,ForTest,DepsByPkgPath}. pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage) if err != nil { return nil, fmt.Errorf("GetParsedFile: %w", err) } alreadyImported := map[PackagePath]struct{}{} - for _, imp := range pgf.File.Imports { - // TODO(adonovan): the correct PackagePath might need a "vendor/" prefix. - alreadyImported[PackagePath(imp.Path.Value)] = struct{}{} + for _, imp := range pkg.Imports() { + alreadyImported[imp.PkgPath()] = struct{}{} } - // TODO(adonovan): this whole algorithm could be more - // simply expressed in terms of Metadata, not Packages. pkgs, err := snapshot.CachedImportPaths(ctx) if err != nil { return nil, err @@ -40,13 +43,8 @@ func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandl paths []PackagePath ) for path, knownPkg := range pkgs { - gofiles := knownPkg.CompiledGoFiles() - if len(gofiles) == 0 || gofiles[0].File.Name == nil { - continue - } - pkgName := gofiles[0].File.Name.Name // package main cannot be imported - if pkgName == "main" { + if knownPkg.Name() == "main" { continue } // test packages cannot be imported @@ -57,7 +55,7 @@ func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandl if _, ok := alreadyImported[path]; ok { continue } - // snapshot.KnownPackages could have multiple versions of a pkg + // snapshot.CachedImportPaths could have multiple versions of a pkg if _, ok := seen[path]; ok { continue } @@ -86,7 +84,8 @@ func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandl return } paths = append(paths, path) - }, "", pgf.URI.Filename(), pkg.GetTypes().Name(), o.Env) + seen[path] = struct{}{} + }, "", pgf.URI.Filename(), string(pkg.Name()), o.Env) }) if err != nil { // if an error occurred, we still have a decent list we can @@ -97,11 +96,8 @@ func KnownPackages(ctx context.Context, snapshot Snapshot, fh VersionedFileHandl importI, importJ := paths[i], paths[j] iHasDot := strings.Contains(string(importI), ".") jHasDot := strings.Contains(string(importJ), ".") - if iHasDot && !jHasDot { - return false - } - if jHasDot && !iHasDot { - return true + if iHasDot != jHasDot { + return jHasDot // dot-free paths (standard packages) compare less } return importI < importJ }) diff --git a/gopls/internal/lsp/source/stub.go b/gopls/internal/lsp/source/stub.go index 3dcdb0ec6b..9e4b704c22 100644 --- a/gopls/internal/lsp/source/stub.go +++ b/gopls/internal/lsp/source/stub.go @@ -98,13 +98,8 @@ func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFi // stubMethods returns the Go code of all methods // that implement the given interface func stubMethods(ctx context.Context, concreteFile *ast.File, si *stubmethods.StubInfo, snapshot Snapshot) ([]byte, []*stubImport, error) { - ifacePkg, err := deducePkgFromTypes(ctx, snapshot, si.Interface) - if err != nil { - return nil, nil, err - } - si.Concrete.Obj().Type() concMS := types.NewMethodSet(types.NewPointer(si.Concrete.Obj().Type())) - missing, err := missingMethods(ctx, snapshot, concMS, si.Concrete.Obj().Pkg(), si.Interface, ifacePkg, map[string]struct{}{}) + missing, err := missingMethods(ctx, snapshot, concMS, si.Concrete.Obj().Pkg(), si.Interface, map[string]struct{}{}) if err != nil { return nil, nil, fmt.Errorf("missingMethods: %w", err) } @@ -188,19 +183,6 @@ func printStubMethod(md methodData) []byte { return b.Bytes() } -func deducePkgFromTypes(ctx context.Context, snapshot Snapshot, ifaceObj types.Object) (Package, error) { - pkgs, err := snapshot.KnownPackages(ctx) - if err != nil { - return nil, err - } - for _, p := range pkgs { - if p.PkgPath() == PackagePath(ifaceObj.Pkg().Path()) { - return p, nil - } - } - return nil, fmt.Errorf("pkg %q not found", ifaceObj.Pkg().Path()) -} - func deduceIfaceName(concretePkg, ifacePkg *types.Package, ifaceObj types.Object) string { if concretePkg.Path() == ifacePkg.Path() { return ifaceObj.Name() @@ -245,7 +227,7 @@ returns }, } */ -func missingMethods(ctx context.Context, snapshot Snapshot, concMS *types.MethodSet, concPkg *types.Package, ifaceObj types.Object, ifacePkg Package, visited map[string]struct{}) ([]*missingInterface, error) { +func missingMethods(ctx context.Context, snapshot Snapshot, concMS *types.MethodSet, concPkg *types.Package, ifaceObj types.Object, visited map[string]struct{}) ([]*missingInterface, error) { iface, ok := ifaceObj.Type().Underlying().(*types.Interface) if !ok { return nil, fmt.Errorf("expected %v to be an interface but got %T", iface, ifaceObj.Type().Underlying()) @@ -253,17 +235,7 @@ func missingMethods(ctx context.Context, snapshot Snapshot, concMS *types.Method missing := []*missingInterface{} for i := 0; i < iface.NumEmbeddeds(); i++ { eiface := iface.Embedded(i).Obj() - depPkg := ifacePkg - if path := PackagePath(eiface.Pkg().Path()); path != ifacePkg.PkgPath() { - // TODO(adonovan): I'm not sure what this is trying to do, but it - // looks wrong the in case of type aliases. - var err error - depPkg, err = ifacePkg.DirectDep(path) - if err != nil { - return nil, err - } - } - em, err := missingMethods(ctx, snapshot, concMS, concPkg, eiface, depPkg, visited) + em, err := missingMethods(ctx, snapshot, concMS, concPkg, eiface, visited) if err != nil { return nil, err } @@ -274,7 +246,6 @@ func missingMethods(ctx context.Context, snapshot Snapshot, concMS *types.Method return nil, fmt.Errorf("error getting iface file: %w", err) } mi := &missingInterface{ - pkg: ifacePkg, iface: iface, file: parsedFile.File, } @@ -322,7 +293,6 @@ func getStubFile(ctx context.Context, obj types.Object, snapshot Snapshot) (*Par type missingInterface struct { iface *types.Interface file *ast.File - pkg Package missing []*types.Func } diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index 47d642d95f..4178197e48 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -182,8 +182,11 @@ type Snapshot interface { // and checked in TypecheckWorkspace mode. CachedImportPaths(ctx context.Context) (map[PackagePath]Package, error) - // KnownPackages returns all the packages loaded in this snapshot, checked - // in TypecheckWorkspace mode. + // KnownPackages returns a new unordered list of all packages + // loaded in this snapshot, checked in TypecheckWorkspace mode. + // + // TODO(adonovan): opt: rewrite 'implementations' to avoid the + // 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. diff --git a/internal/imports/fix.go b/internal/imports/fix.go index 9b7b106fde..646455802b 100644 --- a/internal/imports/fix.go +++ b/internal/imports/fix.go @@ -697,6 +697,9 @@ func candidateImportName(pkg *pkg) string { // GetAllCandidates calls wrapped for each package whose name starts with // searchPrefix, and can be imported from filename with the package name filePkg. +// +// Beware that the wrapped function may be called multiple times concurrently. +// TODO(adonovan): encapsulate the concurrency. func GetAllCandidates(ctx context.Context, wrapped func(ImportFix), searchPrefix, filename, filePkg string, env *ProcessEnv) error { callback := &scanCallback{ rootFound: func(gopathwalk.Root) bool {