internal/imports: don't add imports that shadow predeclared names

Also, a test.

Fixes golang/go#63592

Change-Id: I36cab69cb224f90cc8459c3fced2408486193a1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/557216
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/test/marker/testdata/codeaction/import-shadows-builtin.txt

@@ -0,0 +1,55 @@
+This is a regression test for bug #63592 in "organize imports" whereby
+the new imports would shadow predeclared names.
+
+In the original example, the conflict was between predeclared error
+type and the unfortunately named package github.com/coreos/etcd/error,
+but this example uses a package with the ludicrous name of complex128.
+
+The new behavior is that we will not attempt to import packages
+that shadow predeclared names. (Ideally we would do that only if
+the predeclared name is actually referenced in the file, which
+complex128 happens to be in this example, but that's a trickier
+analysis than the internal/imports package is game for.)
+
+The name complex127 works as usual.
+
+-- go.mod --
+module example.com
+go 1.18
+
+-- complex128/a.go --
+package complex128
+
+var V int
+
+-- complex127/a.go --
+package complex127
+
+var V int
+
+-- main.go --
+package main
+
+import () //@codeaction("import", "", "source.organizeImports", out)
+
+func main() {
+	complex128.V() //@diag("V", re"type complex128 has no field")
+	complex127.V() //@diag("complex127", re"(undeclared|undefined)")
+}
+
+func _() {
+	var _ complex128 = 1 + 2i
+}
+-- @out/main.go --
+package main
+
+import "example.com/complex127" //@codeaction("import", "", "source.organizeImports", out)
+
+func main() {
+	complex128.V() //@diag("V", re"type complex128 has no field")
+	complex127.V() //@diag("complex127", re"(undeclared|undefined)")
+}
+
+func _() {
+	var _ complex128 = 1 + 2i
+}

internal/imports/fix.go

@@ -13,6 +13,7 @@ import (
 	"go/build"
 	"go/parser"
 	"go/token"
+	"go/types"
 	"io/fs"
 	"io/ioutil"
 	"os"
@@ -1151,6 +1152,17 @@ func addExternalCandidates(ctx context.Context, pass *pass, refs references, fil
 	}()
 
 	for result := range results {
+		// Don't offer completions that would shadow predeclared
+		// names, such as github.com/coreos/etcd/error.
+		if types.Universe.Lookup(result.pkg.name) != nil { // predeclared
+			// Ideally we would skip this candidate only
+			// if the predeclared name is actually
+			// referenced by the file, but that's a lot
+			// trickier to compute and would still create
+			// an import that is likely to surprise the
+			// user before long.
+			continue
+		}
 		pass.addCandidate(result.imp, result.pkg)
 	}
 	return firstErr