Skip to content

Commit a886a1c

Browse files
jbagopherbot
jba
authored and
gopherbot
committed
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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Jonathan Amsterdam <jba@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
1 parent 94c3c49 commit a886a1c

File tree

16 files changed

+220
-40
lines changed

16 files changed

+220
-40
lines changed

‎go/analysis/passes/stringintconv/string.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,14 @@ func run(pass *analysis.Pass) (interface{}, error) {
198198
// the type has methods, as some {String,GoString,Format}
199199
// may change the behavior of fmt.Sprint.
200200
if len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0 {
201-
fmtName, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, arg.Pos(), "fmt", "fmt")
201+
_, prefix, importEdits := analysisinternal.AddImport(pass.TypesInfo, file, "fmt", "fmt", "Sprint", arg.Pos())
202202
if types.Identical(T0, types.Typ[types.String]) {
203203
// string(x) -> fmt.Sprint(x)
204204
addFix("Format the number as a decimal", append(importEdits,
205205
analysis.TextEdit{
206206
Pos: call.Fun.Pos(),
207207
End: call.Fun.End(),
208-
NewText: []byte(fmtName + ".Sprint"),
208+
NewText: []byte(prefix + "Sprint"),
209209
}),
210210
)
211211
} else {
@@ -214,7 +214,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
214214
analysis.TextEdit{
215215
Pos: call.Lparen + 1,
216216
End: call.Lparen + 1,
217-
NewText: []byte(fmtName + ".Sprint("),
217+
NewText: []byte(prefix + "Sprint("),
218218
},
219219
analysis.TextEdit{
220220
Pos: call.Rparen,

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package fix
2+
3+
import . "fmt"
4+
5+
func _(x uint64) {
6+
Println(string(x)) // want `conversion from uint64 to string yields...`
7+
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- Format the number as a decimal --
2+
package fix
3+
4+
import . "fmt"
5+
6+
func _(x uint64) {
7+
Println(Sprint(x)) // want `conversion from uint64 to string yields...`
8+
}
9+
10+
-- Convert a single rune to a string --
11+
package fix
12+
13+
import . "fmt"
14+
15+
func _(x uint64) {
16+
Println(string(rune(x))) // want `conversion from uint64 to string yields...`
17+
}
18+

‎gopls/internal/analysis/gofix/gofix.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,13 @@ func run(pass *analysis.Pass) (any, error) {
262262
continue
263263
}
264264
}
265-
importPrefix := ""
266-
var edits []analysis.TextEdit
265+
var (
266+
importPrefix string
267+
edits []analysis.TextEdit
268+
)
267269
if fcon.RHSPkgPath != pass.Pkg.Path() {
268-
// TODO(jba): fix AddImport so that it returns "." if an existing dot import will work.
269-
// We will need to tell AddImport the name of the identifier we want to qualify (fcon.RHSName here).
270-
importID, eds := analysisinternal.AddImport(
271-
pass.TypesInfo, curFile, n.Pos(), fcon.RHSPkgPath, fcon.RHSPkgName)
272-
importPrefix = importID + "."
273-
edits = eds
270+
_, importPrefix, edits = analysisinternal.AddImport(
271+
pass.TypesInfo, curFile, fcon.RHSPkgName, fcon.RHSPkgPath, fcon.RHSName, n.Pos())
274272
}
275273
var (
276274
pos = n.Pos()

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package b
22

33
import a0 "a"
44

5-
import "c"
6-
75
import (
86
"a"
97
. "c"
@@ -29,7 +27,7 @@ func g() {
2927
// a second import of "a" will be added with a new package identifer.
3028
x = a0.Uno // want `Constant in2 should be inlined`
3129

32-
x = c.C // want `Constant in3 should be inlined`
30+
x = C // want `Constant in3 should be inlined`
3331

3432
_ = a
3533
_ = x

‎gopls/internal/analysis/modernize/maps.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,28 +126,33 @@ func mapsloop(pass *analysis.Pass) {
126126
}
127127
}
128128

129-
// Choose function, report diagnostic, and suggest fix.
129+
// Choose function.
130+
var funcName string
131+
if mrhs != nil {
132+
funcName = cond(xmap, "Clone", "Collect")
133+
} else {
134+
funcName = cond(xmap, "Copy", "Insert")
135+
}
136+
137+
// Report diagnostic, and suggest fix.
130138
rng := curRange.Node()
131-
mapsName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "maps", "maps")
139+
_, prefix, importEdits := analysisinternal.AddImport(info, file, "maps", "maps", funcName, rng.Pos())
132140
var (
133-
funcName string
134141
newText []byte
135142
start, end token.Pos
136143
)
137144
if mrhs != nil {
138145
// Replace RHS of preceding m=... assignment (and loop) with expression.
139146
start, end = mrhs.Pos(), rng.End()
140-
funcName = cond(xmap, "Clone", "Collect")
141-
newText = fmt.Appendf(nil, "%s.%s(%s)",
142-
mapsName,
147+
newText = fmt.Appendf(nil, "%s%s(%s)",
148+
prefix,
143149
funcName,
144150
analysisinternal.Format(pass.Fset, x))
145151
} else {
146152
// Replace loop with call statement.
147153
start, end = rng.Pos(), rng.End()
148-
funcName = cond(xmap, "Copy", "Insert")
149-
newText = fmt.Appendf(nil, "%s.%s(%s, %s)",
150-
mapsName,
154+
newText = fmt.Appendf(nil, "%s%s(%s, %s)",
155+
prefix,
151156
funcName,
152157
analysisinternal.Format(pass.Fset, m),
153158
analysisinternal.Format(pass.Fset, x))

‎gopls/internal/analysis/modernize/slices.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func appendclipped(pass *analysis.Pass) {
9292
}
9393

9494
// append(zerocap, s...) -> slices.Clone(s)
95-
slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
95+
_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Clone", call.Pos())
9696
pass.Report(analysis.Diagnostic{
9797
Pos: call.Pos(),
9898
End: call.End(),
@@ -103,7 +103,7 @@ func appendclipped(pass *analysis.Pass) {
103103
TextEdits: append(importEdits, []analysis.TextEdit{{
104104
Pos: call.Pos(),
105105
End: call.End(),
106-
NewText: fmt.Appendf(nil, "%s.Clone(%s)", slicesName, analysisinternal.Format(pass.Fset, s)),
106+
NewText: fmt.Appendf(nil, "%sClone(%s)", prefix, analysisinternal.Format(pass.Fset, s)),
107107
}}...),
108108
}},
109109
})
@@ -116,7 +116,7 @@ func appendclipped(pass *analysis.Pass) {
116116
// - slices.Clone(s) -> s
117117
// - s[:len(s):len(s)] -> s
118118
// - slices.Clip(s) -> s
119-
slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
119+
_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", "Concat", call.Pos())
120120
pass.Report(analysis.Diagnostic{
121121
Pos: call.Pos(),
122122
End: call.End(),
@@ -127,7 +127,7 @@ func appendclipped(pass *analysis.Pass) {
127127
TextEdits: append(importEdits, []analysis.TextEdit{{
128128
Pos: call.Pos(),
129129
End: call.End(),
130-
NewText: fmt.Appendf(nil, "%s.Concat(%s)", slicesName, formatExprs(pass.Fset, sliceArgs)),
130+
NewText: fmt.Appendf(nil, "%sConcat(%s)", prefix, formatExprs(pass.Fset, sliceArgs)),
131131
}}...),
132132
}},
133133
})

‎gopls/internal/analysis/modernize/slicescontains.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,9 @@ func slicescontains(pass *analysis.Pass) {
158158
}
159159

160160
// Prepare slices.Contains{,Func} call.
161-
slicesName, importEdits := analysisinternal.AddImport(info, file, rng.Pos(), "slices", "slices")
162-
contains := fmt.Sprintf("%s.%s(%s, %s)",
163-
slicesName,
161+
_, prefix, importEdits := analysisinternal.AddImport(info, file, "slices", "slices", funcName, rng.Pos())
162+
contains := fmt.Sprintf("%s%s(%s, %s)",
163+
prefix,
164164
funcName,
165165
analysisinternal.Format(pass.Fset, rng.X),
166166
analysisinternal.Format(pass.Fset, arg2))

‎gopls/internal/analysis/modernize/slicesdelete.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func slicesdelete(pass *analysis.Pass) {
2424
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
2525
info := pass.TypesInfo
2626
report := func(file *ast.File, call *ast.CallExpr, slice1, slice2 *ast.SliceExpr) {
27-
slicesName, edits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
27+
_, prefix, edits := analysisinternal.AddImport(info, file, "slices", "slices", "Delete", call.Pos())
2828
pass.Report(analysis.Diagnostic{
2929
Pos: call.Pos(),
3030
End: call.End(),
@@ -37,7 +37,7 @@ func slicesdelete(pass *analysis.Pass) {
3737
{
3838
Pos: call.Fun.Pos(),
3939
End: call.Fun.End(),
40-
NewText: []byte(slicesName + ".Delete"),
40+
NewText: []byte(prefix + "Delete"),
4141
},
4242
// Delete ellipsis.
4343
{

‎gopls/internal/analysis/modernize/sortslice.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ func sortslice(pass *analysis.Pass) {
7070
if isIndex(compare.X, i) && isIndex(compare.Y, j) {
7171
// Have: sort.Slice(s, func(i, j int) bool { return s[i] < s[j] })
7272

73-
slicesName, importEdits := analysisinternal.AddImport(info, file, call.Pos(), "slices", "slices")
73+
_, prefix, importEdits := analysisinternal.AddImport(
74+
info, file, "slices", "slices", "Sort", call.Pos())
7475

7576
pass.Report(analysis.Diagnostic{
7677
// Highlight "sort.Slice".
@@ -85,7 +86,7 @@ func sortslice(pass *analysis.Pass) {
8586
// Replace sort.Slice with slices.Sort.
8687
Pos: call.Fun.Pos(),
8788
End: call.Fun.End(),
88-
NewText: []byte(slicesName + ".Sort"),
89+
NewText: []byte(prefix + "Sort"),
8990
},
9091
{
9192
// Eliminate FuncLit.

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//go:build go1.23
2+
3+
package mapsloop
4+
5+
import . "maps"
6+
7+
var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
8+
9+
func useCopyDot(dst, src map[int]string) {
10+
// Replace loop by maps.Copy.
11+
for key, value := range src {
12+
dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Copy"
13+
}
14+
}
15+
16+
func useCloneDot(src map[int]string) {
17+
// Replace make(...) by maps.Clone.
18+
dst := make(map[int]string, len(src))
19+
for key, value := range src {
20+
dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Clone"
21+
}
22+
println(dst)
23+
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//go:build go1.23
2+
3+
package mapsloop
4+
5+
import . "maps"
6+
7+
var _ = Clone[M] // force "maps" import so that each diagnostic doesn't add one
8+
9+
func useCopyDot(dst, src map[int]string) {
10+
// Replace loop by maps.Copy.
11+
Copy(dst, src)
12+
}
13+
14+
func useCloneDot(src map[int]string) {
15+
// Replace make(...) by maps.Clone.
16+
dst := Clone(src)
17+
println(dst)
18+
}
19+

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package sortslice
2+
3+
import . "slices"
4+
import "sort"
5+
6+
func _(s []myint) {
7+
sort.Slice(s, func(i, j int) bool { return s[i] < s[j] }) // want "sort.Slice can be modernized using slices.Sort"
8+
}
9+
10+
func _(x *struct{ s []int }) {
11+
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"
12+
}
13+
14+
func _(s []int) {
15+
sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
16+
}
17+
18+
func _(s []int) {
19+
sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
20+
}
21+
22+
func _(s2 []struct{ x int }) {
23+
sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
24+
}
25+
26+
func _() { Clip([]int{}) }

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package sortslice
2+
3+
import . "slices"
4+
import "sort"
5+
6+
func _(s []myint) {
7+
Sort(s) // want "sort.Slice can be modernized using slices.Sort"
8+
}
9+
10+
func _(x *struct{ s []int }) {
11+
Sort(x.s) // want "sort.Slice can be modernized using slices.Sort"
12+
}
13+
14+
func _(s []int) {
15+
sort.Slice(s, func(i, j int) bool { return s[i] > s[j] }) // nope: wrong comparison operator
16+
}
17+
18+
func _(s []int) {
19+
sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
20+
}
21+
22+
func _(s2 []struct{ x int }) {
23+
sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
24+
}
25+
26+
func _() { Clip([]int{}) }

‎internal/analysisinternal/addimport_test.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,42 @@ import foo "encoding/json"
183183
184184
func _() {
185185
foo
186+
}`,
187+
},
188+
{
189+
descr: descr("dot import unshadowed"),
190+
src: `package a
191+
192+
import . "fmt"
193+
194+
func _() {
195+
«. fmt»
196+
}`,
197+
want: `package a
198+
199+
import . "fmt"
200+
201+
func _() {
202+
.
203+
}`,
204+
},
205+
{
206+
descr: descr("dot import shadowed"),
207+
src: `package a
208+
209+
import . "fmt"
210+
211+
func _(Print fmt.Stringer) {
212+
«fmt fmt»
213+
}`,
214+
want: `package a
215+
216+
import "fmt"
217+
218+
import . "fmt"
219+
220+
func _(Print fmt.Stringer) {
221+
fmt
186222
}`,
187223
},
188224
} {
@@ -218,7 +254,8 @@ func _() {
218254
conf.Check(f.Name.Name, fset, []*ast.File{f}, info)
219255

220256
// add import
221-
name, edits := analysisinternal.AddImport(info, f, pos, path, name)
257+
// The "Print" argument is only relevant for dot-import tests.
258+
name, prefix, edits := analysisinternal.AddImport(info, f, name, path, "Print", pos)
222259

223260
var edit analysis.TextEdit
224261
switch len(edits) {
@@ -229,6 +266,15 @@ func _() {
229266
t.Fatalf("expected at most one edit, got %d", len(edits))
230267
}
231268

269+
// prefix is a simple function of name.
270+
wantPrefix := name + "."
271+
if name == "." {
272+
wantPrefix = ""
273+
}
274+
if prefix != wantPrefix {
275+
t.Errorf("got prefix %q, want %q", prefix, wantPrefix)
276+
}
277+
232278
// apply patch
233279
start := fset.Position(edit.Pos)
234280
end := fset.Position(edit.End)

0 commit comments

Comments
 (0)