internal/lsp: use LookupParent for finding scope

The lexicalLookup function is used to determine if
a use of an object would be shadowed by a different
definition after a rename. Switch to using LookupParent
which is more careful about the positions of the
identifiers.

Fixes golang/go#47583

Change-Id: I3dbdf79e537ce637d1276ddbecb094db21f1c26d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340551
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/lsp/source/rename_check.go

@@ -195,7 +195,7 @@ func (r *renamer) checkInLexicalScope(from types.Object, pkg Package) {
 			// The name r.to is defined in a superblock.
 			// Is that name referenced from within this block?
 			forEachLexicalRef(pkg, to, func(id *ast.Ident, block *types.Scope) bool {
-				_, obj := lexicalLookup(block, from.Name(), id.Pos())
+				_, obj := block.LookupParent(from.Name(), id.Pos())
 				if obj == from {
 					// super-block conflict
 					r.errorf(from.Pos(), "renaming this %s %q to %q",
@@ -215,10 +215,9 @@ func (r *renamer) checkInLexicalScope(from types.Object, pkg Package) {
 	forEachLexicalRef(pkg, from, func(id *ast.Ident, block *types.Scope) bool {
 		// Find the block that defines the found reference.
 		// It may be an ancestor.
-		fromBlock, _ := lexicalLookup(block, from.Name(), id.Pos())
-
+		fromBlock, _ := block.LookupParent(from.Name(), id.Pos())
 		// See what r.to would resolve to in the same scope.
-		toBlock, to := lexicalLookup(block, r.to, id.Pos())
+		toBlock, to := block.LookupParent(r.to, id.Pos())
 		if to != nil {
 			// sub-block conflict
 			if deeper(toBlock, fromBlock) {
@@ -249,26 +248,6 @@ func (r *renamer) checkInLexicalScope(from types.Object, pkg Package) {
 	}
 }
 
-// lexicalLookup is like (*types.Scope).LookupParent but respects the
-// environment visible at pos.  It assumes the relative position
-// information is correct with each file.
-func lexicalLookup(block *types.Scope, name string, pos token.Pos) (*types.Scope, types.Object) {
-	for b := block; b != nil; b = b.Parent() {
-		obj := b.Lookup(name)
-		// The scope of a package-level object is the entire package,
-		// so ignore pos in that case.
-		// No analogous clause is needed for file-level objects
-		// since no reference can appear before an import decl.
-		if obj == nil || obj.Pkg() == nil {
-			continue
-		}
-		if b == obj.Pkg().Scope() || obj.Pos() < pos {
-			return b, obj
-		}
-	}
-	return nil, nil
-}
-
 // deeper reports whether block x is lexically deeper than y.
 func deeper(x, y *types.Scope) bool {
 	if x == y || x == nil {

internal/lsp/testdata/rename/shadow/shadow.go

@@ -0,0 +1,20 @@
+package shadow
+
+func _() {
+	a := true
+	b, c, _ := A(), B(), D() //@rename("A", "a"),rename("B", "b"),rename("b", "c"),rename("D", "d")
+	d := false
+	_, _, _, _ = a, b, c, d
+}
+
+func A() int {
+	return 0
+}
+
+func B() int {
+	return 0
+}
+
+func D() int {
+	return 0
+}

internal/lsp/testdata/rename/shadow/shadow.go.golden

@@ -0,0 +1,48 @@
+-- a-rename --
+renaming this func "A" to "a"	would cause this reference to become shadowed	by this intervening var definition
+-- b-rename --
+package shadow
+
+func _() {
+	a := true
+	b, c, _ := A(), b(), D() //@rename("A", "a"),rename("B", "b"),rename("b", "c"),rename("D", "d")
+	d := false
+	_, _, _, _ = a, b, c, d
+}
+
+func A() int {
+	return 0
+}
+
+func b() int {
+	return 0
+}
+
+func D() int {
+	return 0
+}
+
+-- c-rename --
+renaming this var "b" to "c"	conflicts with var in same block
+-- d-rename --
+package shadow
+
+func _() {
+	a := true
+	b, c, _ := A(), B(), d() //@rename("A", "a"),rename("B", "b"),rename("b", "c"),rename("D", "d")
+	d := false
+	_, _, _, _ = a, b, c, d
+}
+
+func A() int {
+	return 0
+}
+
+func B() int {
+	return 0
+}
+
+func d() int {
+	return 0
+}
+

internal/lsp/testdata/summary.txt.golden

@@ -20,7 +20,7 @@ DefinitionsCount = 95
 TypeDefinitionsCount = 18
 HighlightsCount = 69
 ReferencesCount = 27
-RenamesCount = 37
+RenamesCount = 41
 PrepareRenamesCount = 7
 SymbolsCount = 5
 WorkspaceSymbolsCount = 20