gopls/internal/lsp/source/rename: use incremental algorithm

This change is a reimplementation of the renaming operation so
that it no longer mixes types.Objects from different packages.
The basic approach is as follows.

First, the referenced object is found and classified. If it is
nonexported (e.g. lowercase, or inherently local such as an
import or label, or within a function body), the operation is
local to that package and is carried out essentially as before.

However, if it is exported, then the scope of the global search
is deduced (direct for package-level var/func/const/type,
transitive for fields and methods). The object is converted to an
objectpath, and all the reverse dependencies are analyzed, using
the objectpath to identify the target in each package.

The renameObject function (the entry point for the fiddly renamer
algorithm) is now called once per package, and the results of all
calls are combined.

Because we process variants, duplicate edits are likely. We sort
and de-dup at the very end under the assumption that edits are
well behaved. The "seen edit" tracking in package renaming is no
longer needed.

Also:
- Package.importMap maps PackagePath to Package for all dependencies,
  so that we can resolve targets identified by (PackagePath,
  objectpath) to their objects in a different types.Importer "realm".
  It is materialized as a DAG of k/v pairs and exposed as
  Package.DependencyTypes.
- The rename_check algorithm (renamer) is now applied once to each
  package instead of once to all packages.
- The set of references to update is derived from types.Info, not the
  references operation.

Still to do in follow-ups:
- Method renaming needs to expand the set of renamed types (based on
  'satisfy') and recompute the dependencies of their declarations,
  until a fixed point is reached. (Not supporting this case is a
  functional regression since v0.11.0, but we plan to submit this
  change to unblock foundational progress and then fix it before the
  next release. See golang/go#58461)
- Lots of generics cases to consider (see golang/go#58462).
- Lots more tests required. For golang/go#57987.

Change-Id: I5fd8538ab35d61744d765d8bd101cd4efa41bd33
Reviewed-on: https://go-review.googlesource.com/c/tools/+/464902
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

gopls/internal/lsp/cache/check.go

@@ -447,10 +447,11 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, inputs typeCheckInputs
 	defer done()
 
 	pkg := &syntaxPackage{
-		id:    inputs.id,
-		mode:  inputs.mode,
-		fset:  snapshot.FileSet(), // must match parse call below (snapshot.ParseGo for now)
-		types: types.NewPackage(string(inputs.pkgPath), string(inputs.name)),
+		id:        inputs.id,
+		mode:      inputs.mode,
+		fset:      snapshot.FileSet(), // must match parse call below (snapshot.ParseGo for now)
+		types:     types.NewPackage(string(inputs.pkgPath), string(inputs.name)),
+		importMap: new(importMap),
 		typesInfo: &types.Info{
 			Types:      make(map[ast.Expr]types.TypeAndValue),
 			Defs:       make(map[*ast.Ident]types.Object),
@@ -461,6 +462,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, inputs typeCheckInputs
 		},
 	}
 	typeparams.InitInstanceInfo(pkg.typesInfo)
+	defer func() { pkg.importMap.types = pkg.types }() // simplifies early return in "unsafe"
 
 	// Parse the non-compiled GoFiles. (These aren't presented to
 	// the type checker but are part of the returned pkg.)
@@ -533,6 +535,7 @@ func doTypeCheck(ctx context.Context, snapshot *snapshot, inputs typeCheckInputs
 			if err != nil {
 				return nil, err
 			}
+			pkg.importMap.union(depPkg.importMap)
 			return depPkg.types, nil
 		}),
 	}

gopls/internal/lsp/cache/pkg.go

@@ -104,6 +104,7 @@ type syntaxPackage struct {
 	typeErrors      []types.Error
 	types           *types.Package
 	typesInfo       *types.Info
+	importMap       *importMap        // required for DependencyTypes (until we have shallow export data)
 	hasFixedFiles   bool              // if true, AST was sufficiently mangled that we should hide type errors
 	xrefs           []byte            // serializable index of outbound cross-references
 	analyses        memoize.Store     // maps analyzer.Name to Promise[actionResult]
@@ -112,8 +113,42 @@ type syntaxPackage struct {
 
 func (p *Package) String() string { return string(p.m.ID) }
 
-func (p *Package) Metadata() *source.Metadata {
-	return p.m
+func (p *Package) Metadata() *source.Metadata { return p.m }
+
+// An importMap is an mapping from source.PackagePath to types.Package
+// of the dependencies of a syntaxPackage. Once constructed (by calls
+// to union) it is never subsequently modified.
+type importMap struct {
+	// Concretely, it is a node that contains one element of the
+	// mapping and whose deps are edges in DAG that comprises the
+	// rest of the mapping. This design optimizes union over get.
+	//
+	// TODO(adonovan): simplify when we use shallow export data.
+	// At that point it becomes a simple lookup in the importers
+	// map, which should be retained in syntaxPackage.
+	// (Otherwise this implementation could expose types.Packages
+	// that represent an old state that has since changed, but
+	// not in a way that is consequential to a downstream package.)
+
+	types *types.Package // map entry for types.Path => types
+	deps  []*importMap   // all other entries
+}
+
+func (m *importMap) union(dep *importMap) { m.deps = append(m.deps, dep) }
+
+func (m *importMap) get(path source.PackagePath, seen map[*importMap]bool) *types.Package {
+	if !seen[m] {
+		seen[m] = true
+		if source.PackagePath(m.types.Path()) == path {
+			return m.types
+		}
+		for _, dep := range m.deps {
+			if pkg := dep.get(path, seen); pkg != nil {
+				return pkg
+			}
+		}
+	}
+	return nil
 }
 
 // A loadScope defines a package loading scope for use with go/packages.
@@ -182,6 +217,14 @@ func (p *Package) GetTypesInfo() *types.Info {
 	return p.pkg.typesInfo
 }
 
+// DependencyTypes returns the type checker's symbol for the specified
+// package. It returns nil if path is not among the transitive
+// dependencies of p, or if no symbols from that package were
+// referenced during the type-checking of p.
+func (p *Package) DependencyTypes(path source.PackagePath) *types.Package {
+	return p.pkg.importMap.get(path, make(map[*importMap]bool))
+}
+
 func (p *Package) HasParseErrors() bool {
 	return len(p.pkg.parseErrors) != 0
 }

gopls/internal/lsp/cmd/test/integration_test.go

@@ -592,7 +592,7 @@ func oldname() {}
 	{
 		res := gopls(t, tree, "rename", "a.go:1:3", "newname")
 		res.checkExit(false)
-		res.checkStderr("no object found")
+		res.checkStderr("no identifier found")
 	}
 	// in 'package' identifier
 	{

gopls/internal/lsp/lsp_test.go

@@ -965,11 +965,9 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
 	}
 
 	wedit, err := r.server.Rename(r.ctx, &protocol.RenameParams{
-		TextDocument: protocol.TextDocumentIdentifier{
-			URI: protocol.URIFromSpanURI(uri),
-		},
-		Position: loc.Range.Start,
-		NewName:  newText,
+		TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},
+		Position:     loc.Range.Start,
+		NewName:      newText,
 	})
 	if err != nil {
 		renamed := string(r.data.Golden(t, tag, filename, func() ([]byte, error) {
@@ -990,6 +988,8 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) {
 	}
 	sort.Strings(orderedURIs)
 
+	// Print the name and content of each modified file,
+	// concatenated, and compare against the golden.
 	var got string
 	for i := 0; i < len(res); i++ {
 		if i != 0 {

gopls/internal/lsp/rename.go

@@ -10,7 +10,6 @@ import (
 
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 	"golang.org/x/tools/gopls/internal/lsp/source"
-	"golang.org/x/tools/gopls/internal/span"
 )
 
 func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*protocol.WorkspaceEdit, error) {
@@ -36,8 +35,8 @@ func (s *Server) rename(ctx context.Context, params *protocol.RenameParams) (*pr
 		docChanges = append(docChanges, documentChanges(fh, e)...)
 	}
 	if isPkgRenaming {
-		uri := params.TextDocument.URI.SpanURI()
-		oldBase := filepath.Dir(span.URI.Filename(uri))
+		// Update the last component of the file's enclosing directory.
+		oldBase := filepath.Dir(fh.URI().Filename())
 		newURI := filepath.Join(filepath.Dir(oldBase), params.NewName)
 		docChanges = append(docChanges, protocol.DocumentChanges{
 			RenameFile: &protocol.RenameFile{

gopls/internal/lsp/source/references2.go

@@ -523,6 +523,8 @@ func effectiveReceiver(obj types.Object) types.Type {
 // Each object is mapped to the syntax node that was treated as an
 // identifier, which is not always an ast.Ident. The second component
 // of the result is the innermost node enclosing pos.
+//
+// TODO(adonovan): factor in common with referencedObject.
 func objectsAt(info *types.Info, file *ast.File, pos token.Pos) (map[types.Object]ast.Node, ast.Node, error) {
 	path := pathEnclosingObjNode(file, pos)
 	if path == nil {