internal/lsp: return error when renaming within an import spec

Since renaming an identifier within an import spec is not yet supported,
return an error when this is encountered. These idents from the import
spec have a nil declaration object.

Import paths that contain '.' or '/' are caught by the valid identifier check
avoiding the crash, but import paths such as "fmt" are not as fmt is a
valid identifier. This change checks if i.decl.obj is nil and returns an error
if it is to avoid the crash.

Fixes golang/go#33768

Change-Id: I4e757b42bedffd648fc821590e4a383826200dc3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191163
Run-TryBot: Suzy Mueller <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>

Author: suzmue

internal/lsp/source/rename.go

@@ -43,6 +43,11 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U
 	if i.Name == newName {
 		return nil, errors.Errorf("old and new names are the same: %s", newName)
 	}
+	// If the object declaration is nil, assume it is an import spec and return an error.
+	// TODO(suzmue): support renaming of identifiers in an import spec.
+	if i.decl.obj == nil {
+		return nil, errors.Errorf("renaming import %q not supported", i.Name)
+	}
 	if !isValidIdentifier(i.Name) {
 		return nil, errors.Errorf("invalid identifier to rename: %q", i.Name)
 	}

internal/lsp/testdata/rename/a/random.go.golden

@@ -4,7 +4,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -49,7 +49,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	fmt2 "fmt"
 )
 
@@ -88,13 +88,15 @@ func sw() {
 	}
 }
 
+-- fmty-rename --
+renaming import "fmt" not supported
 -- format-rename --
 random.go:
 package a
 
 import (
 	lg "log"
-	format "fmt"
+	format "fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -139,7 +141,7 @@ package a
 
 import (
 	"log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -184,7 +186,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -229,7 +231,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -274,7 +276,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -319,7 +321,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -364,7 +366,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -409,7 +411,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
@@ -454,7 +456,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 

internal/lsp/testdata/rename/a/random.go.in

@@ -2,7 +2,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 

internal/lsp/tests/tests.go

@@ -38,7 +38,7 @@ const (
 	ExpectedTypeDefinitionsCount   = 2
 	ExpectedHighlightsCount        = 2
 	ExpectedReferencesCount        = 5
-	ExpectedRenamesCount           = 17
+	ExpectedRenamesCount           = 18
 	ExpectedSymbolsCount           = 1
 	ExpectedSignaturesCount        = 21
 	ExpectedLinksCount             = 4