gopls/internal/lsp/source: eliminate ResolveImportPath
Following up on CL 461944, eliminate uses of ResolveImportPath.
At two of the three callsites, we avoid type-checking. The one that
remains is in renaming.
For golang/go#57987
Change-Id: Ia974d39f2db72a1fe1373cff5faeb07ecb54effb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463376
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/lsp/source/hover.go b/gopls/internal/lsp/source/hover.go
index c270656..e67e4d8 100644
--- a/gopls/internal/lsp/source/hover.go
+++ b/gopls/internal/lsp/source/hover.go
@@ -534,27 +534,38 @@
}
case *ast.ImportSpec:
// Try to find the package documentation for an imported package.
- importPath, err := strconv.Unquote(node.Path.Value)
- if err != nil {
- return nil, err
+ importPath := UnquoteImportPath(node)
+ impID := pkg.Metadata().DepsByImpPath[importPath]
+ if impID == "" {
+ return nil, fmt.Errorf("failed to resolve import %q", importPath)
}
- // TODO(rfindley): avoid type-checking here, by re-parsing the package with
- // ParseHeader.
- imp, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, ImportPath(importPath))
- if err != nil {
- return nil, err
+ impMetadata := s.Metadata(impID)
+ if impMetadata == nil {
+ return nil, fmt.Errorf("failed to resolve import ID %q", impID)
}
- // Assume that only one file will contain package documentation,
- // so pick the first file that has a doc comment.
- for _, file := range imp.GetSyntax() {
- if file.Doc != nil {
- info = &HoverContext{Comment: file.Doc}
- if file.Name != nil {
- info.signatureSource = "package " + file.Name.Name
+ for _, f := range impMetadata.CompiledGoFiles {
+ fh, err := s.GetFile(ctx, f)
+ if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
}
- break
+ continue
+ }
+ pgf, err := s.ParseGo(ctx, fh, ParseHeader)
+ if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
+ continue
+ }
+ if pgf.File.Doc != nil {
+ return &HoverContext{
+ Comment: pgf.File.Doc,
+ signatureSource: "package " + impMetadata.Name,
+ }, nil
}
}
+
case *ast.GenDecl:
switch obj := obj.(type) {
case *types.TypeName, *types.Var, *types.Const, *types.Func:
diff --git a/gopls/internal/lsp/source/identifier.go b/gopls/internal/lsp/source/identifier.go
index 0be26b5..d34fdf1 100644
--- a/gopls/internal/lsp/source/identifier.go
+++ b/gopls/internal/lsp/source/identifier.go
@@ -463,10 +463,7 @@
if err != nil {
return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
}
- imported, err := ResolveImportPath(ctx, snapshot, pkg.Metadata().ID, ImportPath(importPath))
- if err != nil {
- return nil, err
- }
+
result := &IdentifierInfo{
Snapshot: snapshot,
Name: importPath, // should this perhaps be imported.PkgPath()?
@@ -475,10 +472,31 @@
if result.MappedRange, err = posToMappedRange(ctx, snapshot, pkg, imp.Path.Pos(), imp.Path.End()); err != nil {
return nil, err
}
- // Consider the "declaration" of an import spec to be the imported package.
- // Return all of the files in the package as the definition of the import spec.
- for _, dst := range imported.GetSyntax() {
- rng, err := posToMappedRange(ctx, snapshot, pkg, dst.Pos(), dst.End())
+
+ impID := pkg.Metadata().DepsByImpPath[ImportPath(importPath)]
+ if impID == "" {
+ return nil, fmt.Errorf("failed to resolve import %q", importPath)
+ }
+ impMetadata := snapshot.Metadata(impID)
+ if impMetadata == nil {
+ return nil, fmt.Errorf("failed to resolve import ID %q", impID)
+ }
+ for _, f := range impMetadata.CompiledGoFiles {
+ fh, err := snapshot.GetFile(ctx, f)
+ if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
+ continue
+ }
+ pgf, err := snapshot.ParseGo(ctx, fh, ParseHeader)
+ if err != nil {
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
+ }
+ continue
+ }
+ rng, err := pgf.PosMappedRange(pgf.File.Pos(), pgf.File.End())
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/source/rename_check.go b/gopls/internal/lsp/source/rename_check.go
index e442dae..5495e6a 100644
--- a/gopls/internal/lsp/source/rename_check.go
+++ b/gopls/internal/lsp/source/rename_check.go
@@ -840,11 +840,15 @@
if importPath == "" {
continue
}
- imported, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, importPath)
+ depID, ok := pkg.Metadata().DepsByImpPath[importPath]
+ if !ok {
+ return nil, nil, nil, false
+ }
+ depPkgs, err := s.TypeCheck(ctx, TypecheckWorkspace, depID)
if err != nil {
return nil, nil, nil, false
}
- pkgs = append(pkgs, imported)
+ pkgs = append(pkgs, depPkgs[0])
}
}
for _, p := range pkgs {
diff --git a/gopls/internal/lsp/source/util.go b/gopls/internal/lsp/source/util.go
index eacd9ce..7dab47b 100644
--- a/gopls/internal/lsp/source/util.go
+++ b/gopls/internal/lsp/source/util.go
@@ -223,6 +223,8 @@
}
// findFileInDeps finds uri in pkg or its dependencies.
+//
+// TODO(rfindley): eliminate this function.
func findFileInDeps(ctx context.Context, snapshot Snapshot, pkg Package, uri span.URI) (*ParsedGoFile, Package, error) {
pkgs := []Package{pkg}
deps := recursiveDeps(snapshot, pkg.Metadata())[1:]
@@ -490,23 +492,3 @@
}
return nil
}
-
-// ResolveImportPath returns the directly imported dependency of the package with id fromID,
-// given its ImportPath, type-checked in its workspace parse mode.
-//
-// TODO(rfindley): eliminate this function, in favor of inlining where it is used.
-func ResolveImportPath(ctx context.Context, snapshot Snapshot, fromID PackageID, importPath ImportPath) (Package, error) {
- meta := snapshot.Metadata(fromID)
- if meta == nil {
- return nil, fmt.Errorf("unknown package %s", fromID)
- }
- depID, ok := meta.DepsByImpPath[importPath]
- if !ok {
- return nil, fmt.Errorf("package does not import %s", importPath)
- }
- pkgs, err := snapshot.TypeCheck(ctx, TypecheckWorkspace, depID)
- if err != nil {
- return nil, fmt.Errorf("type checking dep: %v", err)
- }
- return pkgs[0], nil
-}