Skip to content

Commit 42daa65

Browse files
findleyrfindleyr
committed
go/analysis/passes/stringintconv: add support for type parameters
Check for potentially invalid string int conversions via type parameters. Updates golang/go#48704 Change-Id: I0269857f3245909cf60c78c6dd624c1c0090c22d Reviewed-on: https://go-review.googlesource.com/c/tools/+/359294 Trust: Robert Findley <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Tim King <[email protected]>
1 parent 513e3fb commit 42daa65

File tree

4 files changed

+208
-29
lines changed

4 files changed

+208
-29
lines changed

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

+104-28
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import (
1010
"fmt"
1111
"go/ast"
1212
"go/types"
13+
"strings"
1314

1415
"golang.org/x/tools/go/analysis"
1516
"golang.org/x/tools/go/analysis/passes/inspect"
1617
"golang.org/x/tools/go/ast/inspector"
18+
"golang.org/x/tools/internal/typeparams"
1719
)
1820

1921
const Doc = `check for string(int) conversions
@@ -36,6 +38,35 @@ var Analyzer = &analysis.Analyzer{
3638
Run: run,
3739
}
3840

41+
// describe returns a string describing the type typ contained within the type
42+
// set of inType. If non-empty, inName is used as the name of inType (this is
43+
// necessary so that we can use alias type names that may not be reachable from
44+
// inType itself).
45+
func describe(typ, inType types.Type, inName string) string {
46+
name := inName
47+
if typ != inType {
48+
name = typeName(typ)
49+
}
50+
if name == "" {
51+
return ""
52+
}
53+
54+
var parentheticals []string
55+
if underName := typeName(typ.Underlying()); underName != "" && underName != name {
56+
parentheticals = append(parentheticals, underName)
57+
}
58+
59+
if typ != inType && inName != "" && inName != name {
60+
parentheticals = append(parentheticals, "in "+inName)
61+
}
62+
63+
if len(parentheticals) > 0 {
64+
name += " (" + strings.Join(parentheticals, ", ") + ")"
65+
}
66+
67+
return name
68+
}
69+
3970
func typeName(typ types.Type) string {
4071
if v, _ := typ.(interface{ Name() string }); v != nil {
4172
return v.Name()
@@ -54,6 +85,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
5485
inspect.Preorder(nodeFilter, func(n ast.Node) {
5586
call := n.(*ast.CallExpr)
5687

88+
if len(call.Args) != 1 {
89+
return
90+
}
91+
arg := call.Args[0]
92+
5793
// Retrieve target type name.
5894
var tname *types.TypeName
5995
switch fun := call.Fun.(type) {
@@ -65,60 +101,100 @@ func run(pass *analysis.Pass) (interface{}, error) {
65101
if tname == nil {
66102
return
67103
}
68-
target := tname.Name()
69104

70-
// Check that target type T in T(v) has an underlying type of string.
71-
T, _ := tname.Type().Underlying().(*types.Basic)
72-
if T == nil || T.Kind() != types.String {
73-
return
105+
// In the conversion T(v) of a value v of type V to a target type T, we
106+
// look for types T0 in the type set of T and V0 in the type set of V, such
107+
// that V0->T0 is a problematic conversion. If T and V are not type
108+
// parameters, this amounts to just checking if V->T is a problematic
109+
// conversion.
110+
111+
// First, find a type T0 in T that has an underlying type of string.
112+
T := tname.Type()
113+
tterms, err := typeparams.StructuralTerms(T)
114+
if err != nil {
115+
return // invalid type
74116
}
75-
if s := T.Name(); target != s {
76-
target += " (" + s + ")"
117+
118+
var T0 types.Type // string type in the type set of T
119+
120+
for _, term := range tterms {
121+
u, _ := term.Type().Underlying().(*types.Basic)
122+
if u != nil && u.Kind() == types.String {
123+
T0 = term.Type()
124+
break
125+
}
77126
}
78127

79-
// Check that type V of v has an underlying integral type that is not byte or rune.
80-
if len(call.Args) != 1 {
128+
if T0 == nil {
129+
// No target types have an underlying type of string.
81130
return
82131
}
83-
v := call.Args[0]
84-
vtyp := pass.TypesInfo.TypeOf(v)
85-
V, _ := vtyp.Underlying().(*types.Basic)
86-
if V == nil || V.Info()&types.IsInteger == 0 {
87-
return
132+
133+
// Next, find a type V0 in V that has an underlying integral type that is
134+
// not byte or rune.
135+
V := pass.TypesInfo.TypeOf(arg)
136+
vterms, err := typeparams.StructuralTerms(V)
137+
if err != nil {
138+
return // invalid type
88139
}
89-
switch V.Kind() {
90-
case types.Byte, types.Rune, types.UntypedRune:
91-
return
140+
141+
var V0 types.Type // integral type in the type set of V
142+
143+
for _, term := range vterms {
144+
u, _ := term.Type().Underlying().(*types.Basic)
145+
if u != nil && u.Info()&types.IsInteger != 0 {
146+
switch u.Kind() {
147+
case types.Byte, types.Rune, types.UntypedRune:
148+
continue
149+
}
150+
V0 = term.Type()
151+
break
152+
}
92153
}
93154

94-
// Retrieve source type name.
95-
source := typeName(vtyp)
96-
if source == "" {
155+
if V0 == nil {
156+
// No source types are non-byte or rune integer types.
97157
return
98158
}
99-
if s := V.Name(); source != s {
100-
source += " (" + s + ")"
159+
160+
convertibleToRune := true // if true, we can suggest a fix
161+
for _, term := range vterms {
162+
if !types.ConvertibleTo(term.Type(), types.Typ[types.Rune]) {
163+
convertibleToRune = false
164+
break
165+
}
166+
}
167+
168+
target := describe(T0, T, tname.Name())
169+
source := describe(V0, V, typeName(V))
170+
171+
if target == "" || source == "" {
172+
return // something went wrong
101173
}
174+
102175
diag := analysis.Diagnostic{
103176
Pos: n.Pos(),
104177
Message: fmt.Sprintf("conversion from %s to %s yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)", source, target),
105-
SuggestedFixes: []analysis.SuggestedFix{
178+
}
179+
180+
if convertibleToRune {
181+
diag.SuggestedFixes = []analysis.SuggestedFix{
106182
{
107183
Message: "Did you mean to convert a rune to a string?",
108184
TextEdits: []analysis.TextEdit{
109185
{
110-
Pos: v.Pos(),
111-
End: v.Pos(),
186+
Pos: arg.Pos(),
187+
End: arg.Pos(),
112188
NewText: []byte("rune("),
113189
},
114190
{
115-
Pos: v.End(),
116-
End: v.End(),
191+
Pos: arg.End(),
192+
End: arg.End(),
117193
NewText: []byte(")"),
118194
},
119195
},
120196
},
121-
},
197+
}
122198
}
123199
pass.Report(diag)
124200
})

‎go/analysis/passes/stringintconv/string_test.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,14 @@ import (
99

1010
"golang.org/x/tools/go/analysis/analysistest"
1111
"golang.org/x/tools/go/analysis/passes/stringintconv"
12+
"golang.org/x/tools/internal/typeparams"
1213
)
1314

1415
func Test(t *testing.T) {
1516
testdata := analysistest.TestData()
16-
analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, "a")
17+
pkgs := []string{"a"}
18+
if typeparams.Enabled {
19+
pkgs = append(pkgs, "typeparams")
20+
}
21+
analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, pkgs...)
1722
}

‎go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2021 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package typeparams
6+
7+
type (
8+
Int int
9+
Uintptr = uintptr
10+
String string
11+
)
12+
13+
func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, NamedString String | Int]() {
14+
var (
15+
i int
16+
r rune
17+
b byte
18+
I Int
19+
U uintptr
20+
M MaybeString
21+
N NotString
22+
)
23+
const p = 0
24+
25+
_ = MaybeString(i) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
26+
_ = MaybeString(r)
27+
_ = MaybeString(b)
28+
_ = MaybeString(I) // want `conversion from Int .int. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
29+
_ = MaybeString(U) // want `conversion from uintptr to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
30+
// Type parameters are never constant types, so arguments are always
31+
// converted to their default type (int versus untyped int, in this case)
32+
_ = MaybeString(p) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
33+
// ...even if the type parameter is only strings.
34+
_ = AllString(p) // want `conversion from int to string .in AllString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
35+
36+
_ = NotString(i)
37+
_ = NotString(r)
38+
_ = NotString(b)
39+
_ = NotString(I)
40+
_ = NotString(U)
41+
_ = NotString(p)
42+
43+
_ = NamedString(i) // want `conversion from int to String .string, in NamedString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
44+
_ = string(M) // want `conversion from int .in MaybeString. to string yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
45+
46+
// Note that M is not convertible to rune.
47+
_ = MaybeString(M) // want `conversion from int .in MaybeString. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
48+
_ = NotString(N) // ok
49+
}

‎go/analysis/passes/stringintconv/testdata/src/typeparams/typeparams.go.golden

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2021 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package typeparams
6+
7+
type (
8+
Int int
9+
Uintptr = uintptr
10+
String string
11+
)
12+
13+
func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, NamedString String | Int]() {
14+
var (
15+
i int
16+
r rune
17+
b byte
18+
I Int
19+
U uintptr
20+
M MaybeString
21+
N NotString
22+
)
23+
const p = 0
24+
25+
_ = MaybeString(rune(i)) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
26+
_ = MaybeString(r)
27+
_ = MaybeString(b)
28+
_ = MaybeString(rune(I)) // want `conversion from Int .int. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
29+
_ = MaybeString(rune(U)) // want `conversion from uintptr to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
30+
// Type parameters are never constant types, so arguments are always
31+
// converted to their default type (int versus untyped int, in this case)
32+
_ = MaybeString(rune(p)) // want `conversion from int to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
33+
// ...even if the type parameter is only strings.
34+
_ = AllString(rune(p)) // want `conversion from int to string .in AllString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
35+
36+
_ = NotString(i)
37+
_ = NotString(r)
38+
_ = NotString(b)
39+
_ = NotString(I)
40+
_ = NotString(U)
41+
_ = NotString(p)
42+
43+
_ = NamedString(rune(i)) // want `conversion from int to String .string, in NamedString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
44+
_ = string(M) // want `conversion from int .in MaybeString. to string yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
45+
46+
// Note that M is not convertible to rune.
47+
_ = MaybeString(M) // want `conversion from int .in MaybeString. to string .in MaybeString. yields a string of one rune, not a string of digits .did you mean fmt\.Sprint.x.\?.`
48+
_ = NotString(N) // ok
49+
}

0 commit comments

Comments
 (0)