Skip to content

Commit 198a0a8

Browse files
committed
imports: prefer math/rand/v2 over math/rand
When a program refers to 'rand.Float64()' without an import statement, import math/rand/v2 is the better answer. Fixes: golang/go#66407 Change-Id: I92709c335b61aa10dd49ce2e5336ea233d8f9af8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/573075 Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 79df971 commit 198a0a8

File tree

3 files changed

+81
-6
lines changed

3 files changed

+81
-6
lines changed

gopls/internal/test/integration/misc/imports_test.go

+46-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func a() {
7272
Run(t, stuff, func(t *testing.T, env *Env) {
7373
env.OpenFile("a.go")
7474
was := env.BufferText("a.go")
75-
env.Await(NoDiagnostics())
75+
env.AfterChange(NoDiagnostics())
7676
env.OrganizeImports("a.go")
7777
is := env.BufferText("a.go")
7878
if diff := compare.Text(was, is); diff != "" {
@@ -81,6 +81,51 @@ func a() {
8181
})
8282
}
8383

84+
func TestIssue66407(t *testing.T) {
85+
const files = `
86+
-- go.mod --
87+
module foo
88+
go 1.21
89+
-- a.go --
90+
package foo
91+
92+
func f(x float64) float64 {
93+
return x + rand.Float64()
94+
}
95+
-- b.go --
96+
package foo
97+
98+
func g() {
99+
_ = rand.Int63()
100+
}
101+
`
102+
WithOptions(Modes(Default)).
103+
Run(t, files, func(t *testing.T, env *Env) {
104+
env.OpenFile("a.go")
105+
was := env.BufferText("a.go")
106+
env.OrganizeImports("a.go")
107+
is := env.BufferText("a.go")
108+
// expect complaint that module is before 1.22
109+
env.AfterChange(Diagnostics(ForFile("a.go")))
110+
diff := compare.Text(was, is)
111+
// check that it found the 'right' rand
112+
if !strings.Contains(diff, `import "math/rand/v2"`) {
113+
t.Errorf("expected rand/v2, got %q", diff)
114+
}
115+
env.OpenFile("b.go")
116+
was = env.BufferText("b.go")
117+
env.OrganizeImports("b.go")
118+
// a.go still has its module problem but b.go is fine
119+
env.AfterChange(Diagnostics(ForFile("a.go")),
120+
NoDiagnostics(ForFile("b.go")))
121+
is = env.BufferText("b.go")
122+
diff = compare.Text(was, is)
123+
if !strings.Contains(diff, `import "math/rand"`) {
124+
t.Errorf("expected math/rand, got %q", diff)
125+
}
126+
})
127+
}
128+
84129
func TestVim1(t *testing.T) {
85130
const vim1 = `package main
86131

internal/imports/fix.go

+34-4
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,20 @@ func (p *pass) loadPackageNames(imports []*ImportInfo) error {
301301
return nil
302302
}
303303

304+
// if there is a trailing major version, remove it
305+
func withoutVersion(nm string) string {
306+
if v := path.Base(nm); len(v) > 0 && v[0] == 'v' {
307+
if _, err := strconv.Atoi(v[1:]); err == nil {
308+
// this is, for instance, called with rand/v2 and returns rand
309+
if len(v) < len(nm) {
310+
xnm := nm[:len(nm)-len(v)-1]
311+
return path.Base(xnm)
312+
}
313+
}
314+
}
315+
return nm
316+
}
317+
304318
// importIdentifier returns the identifier that imp will introduce. It will
305319
// guess if the package name has not been loaded, e.g. because the source
306320
// is not available.
@@ -310,7 +324,7 @@ func (p *pass) importIdentifier(imp *ImportInfo) string {
310324
}
311325
known := p.knownPackages[imp.ImportPath]
312326
if known != nil && known.name != "" {
313-
return known.name
327+
return withoutVersion(known.name)
314328
}
315329
return ImportPathToAssumedName(imp.ImportPath)
316330
}
@@ -1059,6 +1073,18 @@ func addStdlibCandidates(pass *pass, refs references) error {
10591073
if err != nil {
10601074
return err
10611075
}
1076+
localbase := func(nm string) string {
1077+
ans := path.Base(nm)
1078+
if ans[0] == 'v' {
1079+
// this is called, for instance, with math/rand/v2 and returns rand/v2
1080+
if _, err := strconv.Atoi(ans[1:]); err == nil {
1081+
ix := strings.LastIndex(nm, ans)
1082+
more := path.Base(nm[:ix])
1083+
ans = path.Join(more, ans)
1084+
}
1085+
}
1086+
return ans
1087+
}
10621088
add := func(pkg string) {
10631089
// Prevent self-imports.
10641090
if path.Base(pkg) == pass.f.Name.Name && filepath.Join(goenv["GOROOT"], "src", pkg) == pass.srcDir {
@@ -1067,13 +1093,17 @@ func addStdlibCandidates(pass *pass, refs references) error {
10671093
exports := symbolNameSet(stdlib.PackageSymbols[pkg])
10681094
pass.addCandidate(
10691095
&ImportInfo{ImportPath: pkg},
1070-
&packageInfo{name: path.Base(pkg), exports: exports})
1096+
&packageInfo{name: localbase(pkg), exports: exports})
10711097
}
10721098
for left := range refs {
10731099
if left == "rand" {
1074-
// Make sure we try crypto/rand before math/rand.
1100+
// Make sure we try crypto/rand before any version of math/rand as both have Int()
1101+
// and our policy is to recommend crypto
10751102
add("crypto/rand")
1076-
add("math/rand")
1103+
// if the user's no later than go1.21, this should be "math/rand"
1104+
// but we have no way of figuring out what the user is using
1105+
// TODO: investigate using the toolchain version to disambiguate in the stdlib
1106+
add("math/rand/v2")
10771107
continue
10781108
}
10791109
for importPath := range stdlib.PackageSymbols {

internal/imports/fix_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ var _ = rand.NewZipf
11511151
`,
11521152
out: `package p
11531153
1154-
import "math/rand"
1154+
import "math/rand/v2"
11551155
11561156
var _ = rand.NewZipf
11571157
`,

0 commit comments

Comments
 (0)