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 <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tim King <taking@google.com>

go/analysis/passes/stringintconv/string.go

@@ -10,10 +10,12 @@ import (
 	"fmt"
 	"go/ast"
 	"go/types"
+	"strings"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 const Doc = `check for string(int) conversions
@@ -36,6 +38,35 @@ var Analyzer = &analysis.Analyzer{
 	Run:      run,
 }
 
+// describe returns a string describing the type typ contained within the type
+// set of inType. If non-empty, inName is used as the name of inType (this is
+// necessary so that we can use alias type names that may not be reachable from
+// inType itself).
+func describe(typ, inType types.Type, inName string) string {
+	name := inName
+	if typ != inType {
+		name = typeName(typ)
+	}
+	if name == "" {
+		return ""
+	}
+
+	var parentheticals []string
+	if underName := typeName(typ.Underlying()); underName != "" && underName != name {
+		parentheticals = append(parentheticals, underName)
+	}
+
+	if typ != inType && inName != "" && inName != name {
+		parentheticals = append(parentheticals, "in "+inName)
+	}
+
+	if len(parentheticals) > 0 {
+		name += " (" + strings.Join(parentheticals, ", ") + ")"
+	}
+
+	return name
+}
+
 func typeName(typ types.Type) string {
 	if v, _ := typ.(interface{ Name() string }); v != nil {
 		return v.Name()
@@ -54,6 +85,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	inspect.Preorder(nodeFilter, func(n ast.Node) {
 		call := n.(*ast.CallExpr)
 
+		if len(call.Args) != 1 {
+			return
+		}
+		arg := call.Args[0]
+
 		// Retrieve target type name.
 		var tname *types.TypeName
 		switch fun := call.Fun.(type) {
@@ -65,60 +101,100 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		if tname == nil {
 			return
 		}
-		target := tname.Name()
 
-		// Check that target type T in T(v) has an underlying type of string.
-		T, _ := tname.Type().Underlying().(*types.Basic)
-		if T == nil || T.Kind() != types.String {
-			return
+		// In the conversion T(v) of a value v of type V to a target type T, we
+		// look for types T0 in the type set of T and V0 in the type set of V, such
+		// that V0->T0 is a problematic conversion. If T and V are not type
+		// parameters, this amounts to just checking if V->T is a problematic
+		// conversion.
+
+		// First, find a type T0 in T that has an underlying type of string.
+		T := tname.Type()
+		tterms, err := typeparams.StructuralTerms(T)
+		if err != nil {
+			return // invalid type
 		}
-		if s := T.Name(); target != s {
-			target += " (" + s + ")"
+
+		var T0 types.Type // string type in the type set of T
+
+		for _, term := range tterms {
+			u, _ := term.Type().Underlying().(*types.Basic)
+			if u != nil && u.Kind() == types.String {
+				T0 = term.Type()
+				break
+			}
 		}
 
-		// Check that type V of v has an underlying integral type that is not byte or rune.
-		if len(call.Args) != 1 {
+		if T0 == nil {
+			// No target types have an underlying type of string.
 			return
 		}
-		v := call.Args[0]
-		vtyp := pass.TypesInfo.TypeOf(v)
-		V, _ := vtyp.Underlying().(*types.Basic)
-		if V == nil || V.Info()&types.IsInteger == 0 {
-			return
+
+		// Next, find a type V0 in V that has an underlying integral type that is
+		// not byte or rune.
+		V := pass.TypesInfo.TypeOf(arg)
+		vterms, err := typeparams.StructuralTerms(V)
+		if err != nil {
+			return // invalid type
 		}
-		switch V.Kind() {
-		case types.Byte, types.Rune, types.UntypedRune:
-			return
+
+		var V0 types.Type // integral type in the type set of V
+
+		for _, term := range vterms {
+			u, _ := term.Type().Underlying().(*types.Basic)
+			if u != nil && u.Info()&types.IsInteger != 0 {
+				switch u.Kind() {
+				case types.Byte, types.Rune, types.UntypedRune:
+					continue
+				}
+				V0 = term.Type()
+				break
+			}
 		}
 
-		// Retrieve source type name.
-		source := typeName(vtyp)
-		if source == "" {
+		if V0 == nil {
+			// No source types are non-byte or rune integer types.
 			return
 		}
-		if s := V.Name(); source != s {
-			source += " (" + s + ")"
+
+		convertibleToRune := true // if true, we can suggest a fix
+		for _, term := range vterms {
+			if !types.ConvertibleTo(term.Type(), types.Typ[types.Rune]) {
+				convertibleToRune = false
+				break
+			}
+		}
+
+		target := describe(T0, T, tname.Name())
+		source := describe(V0, V, typeName(V))
+
+		if target == "" || source == "" {
+			return // something went wrong
 		}
+
 		diag := analysis.Diagnostic{
 			Pos:     n.Pos(),
 			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),
-			SuggestedFixes: []analysis.SuggestedFix{
+		}
+
+		if convertibleToRune {
+			diag.SuggestedFixes = []analysis.SuggestedFix{
 				{
 					Message: "Did you mean to convert a rune to a string?",
 					TextEdits: []analysis.TextEdit{
 						{
-							Pos:     v.Pos(),
-							End:     v.Pos(),
+							Pos:     arg.Pos(),
+							End:     arg.Pos(),
 							NewText: []byte("rune("),
 						},
 						{
-							Pos:     v.End(),
-							End:     v.End(),
+							Pos:     arg.End(),
+							End:     arg.End(),
 							NewText: []byte(")"),
 						},
 					},
 				},
-			},
+			}
 		}
 		pass.Report(diag)
 	})

go/analysis/passes/stringintconv/string_test.go

@@ -9,9 +9,14 @@ import (
 
 	"golang.org/x/tools/go/analysis/analysistest"
 	"golang.org/x/tools/go/analysis/passes/stringintconv"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
-	analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, "a")
+	pkgs := []string{"a"}
+	if typeparams.Enabled {
+		pkgs = append(pkgs, "typeparams")
+	}
+	analysistest.RunWithSuggestedFixes(t, testdata, stringintconv.Analyzer, pkgs...)
 }

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

@@ -0,0 +1,49 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package typeparams
+
+type (
+	Int     int
+	Uintptr = uintptr
+	String  string
+)
+
+func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, NamedString String | Int]() {
+	var (
+		i int
+		r rune
+		b byte
+		I Int
+		U uintptr
+		M MaybeString
+		N NotString
+	)
+	const p = 0
+
+	_ = 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.\?.`
+	_ = MaybeString(r)
+	_ = MaybeString(b)
+	_ = 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.\?.`
+	_ = 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.\?.`
+	// Type parameters are never constant types, so arguments are always
+	// converted to their default type (int versus untyped int, in this case)
+	_ = 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.\?.`
+	// ...even if the type parameter is only strings.
+	_ = 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.\?.`
+
+	_ = NotString(i)
+	_ = NotString(r)
+	_ = NotString(b)
+	_ = NotString(I)
+	_ = NotString(U)
+	_ = NotString(p)
+
+	_ = 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.\?.`
+	_ = 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.\?.`
+
+	// Note that M is not convertible to rune.
+	_ = 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.\?.`
+	_ = NotString(N)   // ok
+}

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

@@ -0,0 +1,49 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package typeparams
+
+type (
+	Int     int
+	Uintptr = uintptr
+	String  string
+)
+
+func _[AllString ~string, MaybeString ~string | ~int, NotString ~int | byte, NamedString String | Int]() {
+	var (
+		i int
+		r rune
+		b byte
+		I Int
+		U uintptr
+		M MaybeString
+		N NotString
+	)
+	const p = 0
+
+	_ = 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.\?.`
+	_ = MaybeString(r)
+	_ = MaybeString(b)
+	_ = 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.\?.`
+	_ = 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.\?.`
+	// Type parameters are never constant types, so arguments are always
+	// converted to their default type (int versus untyped int, in this case)
+	_ = 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.\?.`
+	// ...even if the type parameter is only strings.
+	_ = 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.\?.`
+
+	_ = NotString(i)
+	_ = NotString(r)
+	_ = NotString(b)
+	_ = NotString(I)
+	_ = NotString(U)
+	_ = NotString(p)
+
+	_ = 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.\?.`
+	_ = 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.\?.`
+
+	// Note that M is not convertible to rune.
+	_ = 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.\?.`
+	_ = NotString(N)   // ok
+}