Skip to content

Commit 6f27922

Browse files
adonovangopherbot
authored andcommitted
[gopls-release-branch.0.18] gopls/internal/analysis/modernize: rangeint: non-integer untyped constants
This CL fixes a bug in rangeint that caused it to replace const limit = 1e3 for i := 0; i < limit; i++ {} with for range limit {} // error: limit is not an integer Now, we check that the type of limit is assignable to int, and if not insert an explicit int(limit) conversion. Updates golang/go#71847 (item d) Change-Id: Icfaa96e5506fcb5a3e6f3ed8f911bf4bda9cf32f Reviewed-on: https://go-review.googlesource.com/c/tools/+/653616 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> (cherry picked from commit 0efa5e5) Reviewed-on: https://go-review.googlesource.com/c/tools/+/655955 Reviewed-by: Robert Findley <rfindley@google.com>
1 parent c632512 commit 6f27922

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed

gopls/internal/analysis/modernize/rangeint.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131
// - The ':=' may be replaced by '='.
3232
// - The fix may remove "i :=" if it would become unused.
3333
//
34+
// TODO(adonovan): permit variants such as "i := int64(0)".
35+
//
3436
// Restrictions:
3537
// - The variable i must not be assigned or address-taken within the
3638
// loop, because a "for range int" loop does not respect assignments
@@ -108,6 +110,31 @@ func rangeint(pass *analysis.Pass) {
108110
limit = call.Args[0]
109111
}
110112

113+
// If the limit is a untyped constant of non-integer type,
114+
// such as "const limit = 1e3", its effective type may
115+
// differ between the two forms.
116+
// In a for loop, it must be comparable with int i,
117+
// for i := 0; i < limit; i++
118+
// but in a range loop it would become a float,
119+
// for i := range limit {}
120+
// which is a type error. We need to convert it to int
121+
// in this case.
122+
//
123+
// Unfortunately go/types discards the untyped type
124+
// (but see Untyped in golang/go#70638) so we must
125+
// re-type check the expression to detect this case.
126+
var beforeLimit, afterLimit string
127+
if v := info.Types[limit].Value; v != nil {
128+
beforeLimit, afterLimit = "int(", ")"
129+
info2 := &types.Info{Types: make(map[ast.Expr]types.TypeAndValue)}
130+
if types.CheckExpr(pass.Fset, pass.Pkg, limit.Pos(), limit, info2) == nil {
131+
tLimit := types.Default(info2.TypeOf(limit))
132+
if types.AssignableTo(tLimit, types.Typ[types.Int]) {
133+
beforeLimit, afterLimit = "", ""
134+
}
135+
}
136+
}
137+
111138
pass.Report(analysis.Diagnostic{
112139
Pos: init.Pos(),
113140
End: inc.End(),
@@ -121,15 +148,30 @@ func rangeint(pass *analysis.Pass) {
121148
// ----- ---
122149
// -------
123150
// for i := range limit {}
151+
152+
// Delete init.
124153
{
125154
Pos: init.Rhs[0].Pos(),
126155
End: limit.Pos(),
127156
NewText: []byte("range "),
128157
},
158+
// Add "int(" before limit, if needed.
159+
{
160+
Pos: limit.Pos(),
161+
End: limit.Pos(),
162+
NewText: []byte(beforeLimit),
163+
},
164+
// Delete inc.
129165
{
130166
Pos: limit.End(),
131167
End: inc.End(),
132168
},
169+
// Add ")" after limit, if needed.
170+
{
171+
Pos: limit.End(),
172+
End: limit.End(),
173+
NewText: []byte(afterLimit),
174+
},
133175
}...),
134176
}},
135177
})

gopls/internal/analysis/modernize/testdata/src/rangeint/rangeint.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,14 @@ func _(s string) {
5151
}
5252
}
5353
}
54+
55+
// Non-integer untyped constants need to be explicitly converted to int.
56+
func issue71847d() {
57+
const limit = 1e3 // float
58+
for i := 0; i < limit; i++ { // want "for loop can be modernized using range over int"
59+
}
60+
61+
const limit2 = 1 + 0i // complex
62+
for i := 0; i < limit2; i++ { // want "for loop can be modernized using range over int"
63+
}
64+
}

gopls/internal/analysis/modernize/testdata/src/rangeint/rangeint.go.golden

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,14 @@ func _(s string) {
5151
}
5252
}
5353
}
54+
55+
// Non-integer untyped constants need to be explicitly converted to int.
56+
func issue71847d() {
57+
const limit = 1e3 // float
58+
for range int(limit) { // want "for loop can be modernized using range over int"
59+
}
60+
61+
const limit2 = 1 + 0i // complex
62+
for range int(limit2) { // want "for loop can be modernized using range over int"
63+
}
64+
}

0 commit comments

Comments
 (0)