From fc82fb2afd64396b05ea9aa0bccd6e8f2257b154 Mon Sep 17 00:00:00 2001
From: Suzy Mueller <suzmue@golang.org>
Date: Wed, 21 Aug 2019 18:13:42 -0400
Subject: [PATCH] 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 <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
---
 internal/lsp/source/rename.go                 |  5 ++++
 .../lsp/testdata/rename/a/random.go.golden    | 24 ++++++++++---------
 internal/lsp/testdata/rename/a/random.go.in   |  2 +-
 internal/lsp/tests/tests.go                   |  2 +-
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go
index 56e6dfab8d..5ac65fbd6e 100644
--- a/internal/lsp/source/rename.go
+++ b/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)
 	}
diff --git a/internal/lsp/testdata/rename/a/random.go.golden b/internal/lsp/testdata/rename/a/random.go.golden
index 53bbc29d3e..518757f14f 100644
--- a/internal/lsp/testdata/rename/a/random.go.golden
+++ b/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"
 )
 
diff --git a/internal/lsp/testdata/rename/a/random.go.in b/internal/lsp/testdata/rename/a/random.go.in
index c2cf0202c0..12026d0503 100644
--- a/internal/lsp/testdata/rename/a/random.go.in
+++ b/internal/lsp/testdata/rename/a/random.go.in
@@ -2,7 +2,7 @@ package a
 
 import (
 	lg "log"
-	"fmt"
+	"fmt" //@rename("fmt", "fmty")
 	f2 "fmt"
 )
 
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index e83c695feb..01b7d1169f 100644
--- a/internal/lsp/tests/tests.go
+++ b/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