internal/analysisinternal: AddImport handles dot imports

If AddImport finds that an existing dot import suffices to refer to an
name, it returns that information by means of a first return value of
".", and does not add a new import.

For this to work, AddImport must know the name for which an import
is needed, so it can determine whether it is shadowed.

Change-Id: Ie4c9edf78fb89fc1b64f344517627173a253b999
Reviewed-on: https://go-review.googlesource.com/c/tools/+/647437
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Jonathan Amsterdam <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

go/analysis/passes/stringintconv/string.go

@@ -198,14 +198,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		// the type has methods, as some {String,GoString,Format}
 		// may change the behavior of fmt.Sprint.
 		if len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0 {
-			fmtName, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, arg.Pos(), "fmt", "fmt")
+			_, prefix, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, "fmt", "fmt", "Sprint", arg.Pos())
 			if types.Identical(T0, types.Typ[types.String]) {
 				// string(x) -> fmt.Sprint(x)
 				addFix("Format the number as a decimal", append(importEdits,
 					analysis.TextEdit{
 						Pos:     call.Fun.Pos(),
 						End:     call.Fun.End(),
-						NewText: []byte(fmtName + ".Sprint"),
+						NewText: []byte(prefix + "Sprint"),
 					}),
 				)
 			} else {
@@ -214,7 +214,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 					analysis.TextEdit{
 						Pos:     call.Lparen + 1,
 						End:     call.Lparen + 1,
-						NewText: []byte(fmtName + ".Sprint("),
+						NewText: []byte(prefix + "Sprint("),
 					},
 					analysis.TextEdit{
 						Pos:     call.Rparen,

go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go

@@ -0,0 +1,7 @@
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+	Println(string(x)) // want `conversion from uint64 to string yields...`
+}

go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden

@@ -0,0 +1,18 @@
+-- Format the number as a decimal --
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+     Println(Sprint(x)) // want `conversion from uint64 to string yields...`
+}
+
+-- Convert a single rune to a string --
+package fix
+
+import . "fmt"
+
+func _(x uint64) {
+     Println(string(rune(x))) // want `conversion from uint64 to string yields...`
+}
+

gopls/internal/analysis/gofix/gofix.go

@@ -262,15 +262,13 @@ func run(pass *analysis.Pass) (any, error) {
 						continue
 					}
 				}
-				importPrefix := ""
-				var edits []analysis.TextEdit
+				var (
+					importPrefix string
+					edits        []analysis.TextEdit
+				)
 				if fcon.RHSPkgPath != pass.Pkg.Path() {
-					// TODO(jba): fix AddImport so that it returns "." if an existing dot import will work.
-					// We will need to tell AddImport the name of the identifier we want to qualify (fcon.RHSName here).
-					importID, eds := analysisinternal.AddImport(
-						pass.TypesInfo, curFile, n.Pos(), fcon.RHSPkgPath, fcon.RHSPkgName)
-					importPrefix = importID + "."
-					edits = eds
+					_, importPrefix, edits = analysisinternal.AddImport(
+						pass.TypesInfo, curFile, fcon.RHSPkgName, fcon.RHSPkgPath, fcon.RHSName, n.Pos())
 				}
 				var (
 					pos  = n.Pos()

gopls/internal/analysis/gofix/testdata/src/b/b.go.golden

@@ -2,8 +2,6 @@ package b
 
 import a0 "a"
 
-import "c"
-
 import (
 	"a"
 	. "c"
@@ -29,7 +27,7 @@ func g() {
 	// a second import of "a" will be added with a new package identifer.
 	x = a0.Uno // want `Constant in2 should be inlined`
 
-	x = c.C // want `Constant in3 should be inlined`
+	x = C // want `Constant in3 should be inlined`
 
 	_ = a
 	_ = x

gopls/internal/analysis/modernize/maps.go

@@ -126,28 +126,33 @@ func mapsloop(pass *analysis.Pass) {
 			}
 		}
 
-		// Choose function, report diagnostic, and suggest fix.
+		// Choose function.
+		var funcName string
+		if mrhs != nil {
+			funcName = cond(xmap, "Clone", "Collect")
+		} else {
+			funcName = cond(xmap, "Copy", "Insert")
+		}
+
+		// Report diagnostic, and suggest fix.
 		rng := curRange.Node()
-		mapsName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "maps", "maps")
+		_, prefix, importEdits := analysisinternal.AddImport(info, file, "maps", "maps", funcName, rng.Pos())
 		var (
-			funcName   string
 			newText    []byte
 			start, end token.Pos
 		)
 		if mrhs != nil {
 			// Replace RHS of preceding m=... assignment (and loop) with expression.
 			start, end = mrhs.Pos(), rng.End()
-			funcName = cond(xmap, "Clone", "Collect")
-			newText = fmt.Appendf(nil, "%s.%s(%s)",
-				mapsName,
+			newText = fmt.Appendf(nil, "%s%s(%s)",
+				prefix,
 				funcName,
 				analysisinternal.Format(pass.Fset, x))
 		} else {
 			// Replace loop with call statement.
 			start, end = rng.Pos(), rng.End()
-			funcName = cond(xmap, "Copy", "Insert")
-			newText = fmt.Appendf(nil, "%s.%s(%s, %s)",
-				mapsName,
+			newText = fmt.Appendf(nil, "%s%s(%s, %s)",
+				prefix,
 				funcName,
 				analysisinternal.Format(pass.Fset, m),
 				analysisinternal.Format(pass.Fset, x))

gopls/internal/analysis/modernize/slices.go

@@ -92,7 +92,7 @@ func appendclipped(pass *analysis.Pass) {
 			}
 
 			// append(zerocap, s...) -> slices.Clone(s)
-			slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+			_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Clone", call.Pos())
 			pass.Report(analysis.Diagnostic{
 				Pos:      call.Pos(),
 				End:      call.End(),
@@ -103,7 +103,7 @@ func appendclipped(pass *analysis.Pass) {
 					TextEdits: append(importEdits, []analysis.TextEdit{{
 						Pos:     call.Pos(),
 						End:     call.End(),
-						NewText: fmt.Appendf(nil, "%s.Clone(%s)", slicesName, analysisinternal.Format(pass.Fset, s)),
+						NewText: fmt.Appendf(nil, "%sClone(%s)", prefix, analysisinternal.Format(pass.Fset, s)),
 					}}...),
 				}},
 			})
@@ -116,7 +116,7 @@ func appendclipped(pass *analysis.Pass) {
 		// - slices.Clone(s)   -> s
 		// - s[:len(s):len(s)] -> s
 		// - slices.Clip(s)    -> s
-		slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+		_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Concat", call.Pos())
 		pass.Report(analysis.Diagnostic{
 			Pos:      call.Pos(),
 			End:      call.End(),
@@ -127,7 +127,7 @@ func appendclipped(pass *analysis.Pass) {
 				TextEdits: append(importEdits, []analysis.TextEdit{{
 					Pos:     call.Pos(),
 					End:     call.End(),
-					NewText: fmt.Appendf(nil, "%s.Concat(%s)", slicesName, formatExprs(pass.Fset, sliceArgs)),
+					NewText: fmt.Appendf(nil, "%sConcat(%s)", prefix, formatExprs(pass.Fset, sliceArgs)),
 				}}...),
 			}},
 		})

gopls/internal/analysis/modernize/slicescontains.go

@@ -158,9 +158,9 @@ func slicescontains(pass *analysis.Pass) {
 		}
 
 		// Prepare slices.Contains{,Func} call.
-		slicesName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "slices", "slices")
-		contains := fmt.Sprintf("%s.%s(%s, %s)",
-			slicesName,
+		_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", funcName, rng.Pos())
+		contains := fmt.Sprintf("%s%s(%s, %s)",
+			prefix,
 			funcName,
 			analysisinternal.Format(pass.Fset, rng.X),
 			analysisinternal.Format(pass.Fset, arg2))

gopls/internal/analysis/modernize/slicesdelete.go

@@ -24,7 +24,7 @@ func slicesdelete(pass *analysis.Pass) {
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	info := pass.TypesInfo
 	report := func(file *ast.File, call *ast.CallExpr, slice1, slice2 *ast.SliceExpr) {
-		slicesName, edits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+		_, prefix, edits := analysisinternal.AddImport(info, file, "slices", "slices", "Delete", call.Pos())
 		pass.Report(analysis.Diagnostic{
 			Pos:      call.Pos(),
 			End:      call.End(),
@@ -37,7 +37,7 @@ func slicesdelete(pass *analysis.Pass) {
 					{
 						Pos:     call.Fun.Pos(),
 						End:     call.Fun.End(),
-						NewText: []byte(slicesName + ".Delete"),
+						NewText: []byte(prefix + "Delete"),
 					},
 					// Delete ellipsis.
 					{

gopls/internal/analysis/modernize/sortslice.go

@@ -70,7 +70,8 @@ func sortslice(pass *analysis.Pass) {
 				if isIndex(compare.X, i) && isIndex(compare.Y, j) {
 					// Have: sort.Slice(s, func(i, j int) bool { return s[i] < s[j] })
 
-					slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
+					_, prefix, importEdits := analysisinternal.AddImport(
+						info, file, "slices", "slices", "Sort", call.Pos())
 
 					pass.Report(analysis.Diagnostic{
 						// Highlight "sort.Slice".
@@ -85,7 +86,7 @@ func sortslice(pass *analysis.Pass) {
 									// Replace sort.Slice with slices.Sort.
 									Pos:     call.Fun.Pos(),
 									End:     call.Fun.End(),
-									NewText: []byte(slicesName + ".Sort"),
+									NewText: []byte(prefix + "Sort"),
 								},
 								{
 									// Eliminate FuncLit.

gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go

@@ -0,0 +1,23 @@
+//go:build go1.23
+
+package mapsloop
+
+import . "maps"
+
+var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
+
+func useCopyDot(dst, src map[int]string) {
+	// Replace loop by maps.Copy.
+	for key, value := range src {
+		dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Copy"
+	}
+}
+
+func useCloneDot(src map[int]string) {
+	// Replace make(...) by maps.Clone.
+	dst := make(map[int]string, len(src))
+	for key, value := range src {
+		dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Clone"
+	}
+	println(dst)
+}

gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop_dot.go.golden

@@ -0,0 +1,19 @@
+//go:build go1.23
+
+package mapsloop
+
+import . "maps"
+
+var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
+
+func useCopyDot(dst, src map[int]string) {
+	// Replace loop by maps.Copy.
+	Copy(dst, src)
+}
+
+func useCloneDot(src map[int]string) {
+	// Replace make(...) by maps.Clone.
+	dst := Clone(src)
+	println(dst)
+}
+

gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go

@@ -0,0 +1,26 @@
+package sortslice
+
+import . "slices"
+import "sort"
+
+func _(s []myint) {
+	sort.Slice(s, func(i, j int) bool { return s[i] < s[j] }) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(x *struct{ s []int }) {
+	sort.Slice(x.s, func(first, second int) bool { return x.s[first] < x.s[second] }) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(s []int) {
+	sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
+}
+
+func _(s []int) {
+	sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
+}
+
+func _(s2 []struct{ x int }) {
+	sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+}
+
+func _() { Clip([]int{}) }

gopls/internal/analysis/modernize/testdata/src/sortslice/sortslice_dot.go.golden

@@ -0,0 +1,26 @@
+package sortslice
+
+import . "slices"
+import "sort"
+
+func _(s []myint) {
+	Sort(s) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(x *struct{ s []int }) {
+	Sort(x.s) // want "sort.Slice can be modernized using slices.Sort"
+}
+
+func _(s []int) {
+	sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
+}
+
+func _(s []int) {
+	sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
+}
+
+func _(s2 []struct{ x int }) {
+	sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+}
+
+func _() { Clip([]int{}) }

internal/analysisinternal/addimport_test.go

@@ -183,6 +183,42 @@ import foo "encoding/json"
 
 func _() {
 	foo
+}`,
+		},
+		{
+			descr: descr("dot import unshadowed"),
+			src: `package a
+
+import . "fmt"
+
+func _() {
+	«. fmt»
+}`,
+			want: `package a
+
+import . "fmt"
+
+func _() {
+	.
+}`,
+		},
+		{
+			descr: descr("dot import shadowed"),
+			src: `package a
+
+import . "fmt"
+
+func _(Print fmt.Stringer) {
+	«fmt fmt»
+}`,
+			want: `package a
+
+import "fmt"
+
+import . "fmt"
+
+func _(Print fmt.Stringer) {
+	fmt
 }`,
 		},
 	} {
@@ -218,7 +254,8 @@ func _() {
 			conf.Check(f.Name.Name, fset, []*ast.File{f}, info)
 
 			// add import
-			name, edits := analysisinternal.AddImport(info, f, pos, path, name)
+			// The "Print" argument is only relevant for dot-import tests.
+			name, prefix, edits := analysisinternal.AddImport(info, f, name, path, "Print", pos)
 
 			var edit analysis.TextEdit
 			switch len(edits) {
@@ -229,6 +266,15 @@ func _() {
 				t.Fatalf("expected at most one edit, got %d", len(edits))
 			}
 
+			// prefix is a simple function of name.
+			wantPrefix := name + "."
+			if name == "." {
+				wantPrefix = ""
+			}
+			if prefix != wantPrefix {
+				t.Errorf("got prefix %q, want %q", prefix, wantPrefix)
+			}
+
 			// apply patch
 			start := fset.Position(edit.Pos)
 			end := fset.Position(edit.End)