Skip to content

Commit 031e6e6

Browse files
committed
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 <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]>
1 parent f2cd9ef commit 031e6e6

File tree

4 files changed

+61
-46
lines changed

4 files changed

+61
-46
lines changed

gopls/internal/lsp/source/hover.go

+27-16
Original file line numberDiff line numberDiff line change
@@ -534,27 +534,38 @@ func FindHoverContext(ctx context.Context, s Snapshot, pkg Package, obj types.Ob
534534
}
535535
case *ast.ImportSpec:
536536
// Try to find the package documentation for an imported package.
537-
importPath, err := strconv.Unquote(node.Path.Value)
538-
if err != nil {
539-
return nil, err
537+
importPath := UnquoteImportPath(node)
538+
impID := pkg.Metadata().DepsByImpPath[importPath]
539+
if impID == "" {
540+
return nil, fmt.Errorf("failed to resolve import %q", importPath)
540541
}
541-
// TODO(rfindley): avoid type-checking here, by re-parsing the package with
542-
// ParseHeader.
543-
imp, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, ImportPath(importPath))
544-
if err != nil {
545-
return nil, err
542+
impMetadata := s.Metadata(impID)
543+
if impMetadata == nil {
544+
return nil, fmt.Errorf("failed to resolve import ID %q", impID)
546545
}
547-
// Assume that only one file will contain package documentation,
548-
// so pick the first file that has a doc comment.
549-
for _, file := range imp.GetSyntax() {
550-
if file.Doc != nil {
551-
info = &HoverContext{Comment: file.Doc}
552-
if file.Name != nil {
553-
info.signatureSource = "package " + file.Name.Name
546+
for _, f := range impMetadata.CompiledGoFiles {
547+
fh, err := s.GetFile(ctx, f)
548+
if err != nil {
549+
if ctx.Err() != nil {
550+
return nil, ctx.Err()
554551
}
555-
break
552+
continue
553+
}
554+
pgf, err := s.ParseGo(ctx, fh, ParseHeader)
555+
if err != nil {
556+
if ctx.Err() != nil {
557+
return nil, ctx.Err()
558+
}
559+
continue
560+
}
561+
if pgf.File.Doc != nil {
562+
return &HoverContext{
563+
Comment: pgf.File.Doc,
564+
signatureSource: "package " + impMetadata.Name,
565+
}, nil
556566
}
557567
}
568+
558569
case *ast.GenDecl:
559570
switch obj := obj.(type) {
560571
case *types.TypeName, *types.Var, *types.Const, *types.Func:

gopls/internal/lsp/source/identifier.go

+26-8
Original file line numberDiff line numberDiff line change
@@ -463,10 +463,7 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Parsed
463463
if err != nil {
464464
return nil, fmt.Errorf("import path not quoted: %s (%v)", imp.Path.Value, err)
465465
}
466-
imported, err := ResolveImportPath(ctx, snapshot, pkg.Metadata().ID, ImportPath(importPath))
467-
if err != nil {
468-
return nil, err
469-
}
466+
470467
result := &IdentifierInfo{
471468
Snapshot: snapshot,
472469
Name: importPath, // should this perhaps be imported.PkgPath()?
@@ -475,10 +472,31 @@ func importSpec(ctx context.Context, snapshot Snapshot, pkg Package, pgf *Parsed
475472
if result.MappedRange, err = posToMappedRange(ctx, snapshot, pkg, imp.Path.Pos(), imp.Path.End()); err != nil {
476473
return nil, err
477474
}
478-
// Consider the "declaration" of an import spec to be the imported package.
479-
// Return all of the files in the package as the definition of the import spec.
480-
for _, dst := range imported.GetSyntax() {
481-
rng, err := posToMappedRange(ctx, snapshot, pkg, dst.Pos(), dst.End())
475+
476+
impID := pkg.Metadata().DepsByImpPath[ImportPath(importPath)]
477+
if impID == "" {
478+
return nil, fmt.Errorf("failed to resolve import %q", importPath)
479+
}
480+
impMetadata := snapshot.Metadata(impID)
481+
if impMetadata == nil {
482+
return nil, fmt.Errorf("failed to resolve import ID %q", impID)
483+
}
484+
for _, f := range impMetadata.CompiledGoFiles {
485+
fh, err := snapshot.GetFile(ctx, f)
486+
if err != nil {
487+
if ctx.Err() != nil {
488+
return nil, ctx.Err()
489+
}
490+
continue
491+
}
492+
pgf, err := snapshot.ParseGo(ctx, fh, ParseHeader)
493+
if err != nil {
494+
if ctx.Err() != nil {
495+
return nil, ctx.Err()
496+
}
497+
continue
498+
}
499+
rng, err := pgf.PosMappedRange(pgf.File.Pos(), pgf.File.End())
482500
if err != nil {
483501
return nil, err
484502
}

gopls/internal/lsp/source/rename_check.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -840,11 +840,15 @@ func pathEnclosingInterval(ctx context.Context, s Snapshot, pkg Package, start,
840840
if importPath == "" {
841841
continue
842842
}
843-
imported, err := ResolveImportPath(ctx, s, pkg.Metadata().ID, importPath)
843+
depID, ok := pkg.Metadata().DepsByImpPath[importPath]
844+
if !ok {
845+
return nil, nil, nil, false
846+
}
847+
depPkgs, err := s.TypeCheck(ctx, TypecheckWorkspace, depID)
844848
if err != nil {
845849
return nil, nil, nil, false
846850
}
847-
pkgs = append(pkgs, imported)
851+
pkgs = append(pkgs, depPkgs[0])
848852
}
849853
}
850854
for _, p := range pkgs {

gopls/internal/lsp/source/util.go

+2-20
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ func CompareDiagnostic(a, b *Diagnostic) int {
223223
}
224224

225225
// findFileInDeps finds uri in pkg or its dependencies.
226+
//
227+
// TODO(rfindley): eliminate this function.
226228
func findFileInDeps(ctx context.Context, snapshot Snapshot, pkg Package, uri span.URI) (*ParsedGoFile, Package, error) {
227229
pkgs := []Package{pkg}
228230
deps := recursiveDeps(snapshot, pkg.Metadata())[1:]
@@ -490,23 +492,3 @@ func embeddedIdent(x ast.Expr) *ast.Ident {
490492
}
491493
return nil
492494
}
493-
494-
// ResolveImportPath returns the directly imported dependency of the package with id fromID,
495-
// given its ImportPath, type-checked in its workspace parse mode.
496-
//
497-
// TODO(rfindley): eliminate this function, in favor of inlining where it is used.
498-
func ResolveImportPath(ctx context.Context, snapshot Snapshot, fromID PackageID, importPath ImportPath) (Package, error) {
499-
meta := snapshot.Metadata(fromID)
500-
if meta == nil {
501-
return nil, fmt.Errorf("unknown package %s", fromID)
502-
}
503-
depID, ok := meta.DepsByImpPath[importPath]
504-
if !ok {
505-
return nil, fmt.Errorf("package does not import %s", importPath)
506-
}
507-
pkgs, err := snapshot.TypeCheck(ctx, TypecheckWorkspace, depID)
508-
if err != nil {
509-
return nil, fmt.Errorf("type checking dep: %v", err)
510-
}
511-
return pkgs[0], nil
512-
}

0 commit comments

Comments
 (0)