go/analysis: add pass to check string(int) conversions
Issue golang/go#3939 proposes to remove string(int) from the language on the
grounds that it produces non-obvious results that can't be statically checked.
An intermediate step is to have go vet check for these sorts of conversions.
This change adds an analysis pass to check for string(int) conversions. It
suggests a fix to replace string(int) with string(rune(int)).
Updates golang/go#32479.
Change-Id: Ifafd6d74f9bd4a903ce6b29ac3a3c7a15f8a1ad9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/212919
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/go/analysis/passes/stringintconv/cmd/stringintconv/main.go b/go/analysis/passes/stringintconv/cmd/stringintconv/main.go
new file mode 100644
index 0000000..118b957
--- /dev/null
+++ b/go/analysis/passes/stringintconv/cmd/stringintconv/main.go
@@ -0,0 +1,13 @@
+// Copyright 2020 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.
+
+// The stringintconv command runs the stringintconv analyzer.
+package main
+
+import (
+ "golang.org/x/tools/go/analysis/passes/stringintconv"
+ "golang.org/x/tools/go/analysis/singlechecker"
+)
+
+func main() { singlechecker.Main(stringintconv.Analyzer) }
diff --git a/go/analysis/passes/stringintconv/string.go b/go/analysis/passes/stringintconv/string.go
new file mode 100644
index 0000000..ac2cd84
--- /dev/null
+++ b/go/analysis/passes/stringintconv/string.go
@@ -0,0 +1,126 @@
+// Copyright 2020 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 stringintconv defines an Analyzer that flags type conversions
+// from integers to strings.
+package stringintconv
+
+import (
+ "fmt"
+ "go/ast"
+ "go/types"
+
+ "golang.org/x/tools/go/analysis"
+ "golang.org/x/tools/go/analysis/passes/inspect"
+ "golang.org/x/tools/go/ast/inspector"
+)
+
+const Doc = `check for string(int) conversions
+
+This checker flags conversions of the form string(x) where x is an integer
+(but not byte or rune) type. Such conversions are discouraged because they
+return the UTF-8 representation of the Unicode code point x, and not a decimal
+string representation of x as one might expect. Furthermore, if x denotes an
+invalid code point, the conversion cannot be statically rejected.
+
+For conversions that intend on using the code point, consider replacing them
+with string(rune(x)). Otherwise, strconv.Itoa and its equivalents return the
+string representation of the value in the desired base.
+`
+
+var Analyzer = &analysis.Analyzer{
+ Name: "stringintconv",
+ Doc: Doc,
+ Requires: []*analysis.Analyzer{inspect.Analyzer},
+ Run: run,
+}
+
+func typeName(typ types.Type) string {
+ if v, _ := typ.(interface{ Name() string }); v != nil {
+ return v.Name()
+ }
+ if v, _ := typ.(interface{ Obj() *types.TypeName }); v != nil {
+ return v.Obj().Name()
+ }
+ return ""
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+ inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+ nodeFilter := []ast.Node{
+ (*ast.CallExpr)(nil),
+ }
+ inspect.Preorder(nodeFilter, func(n ast.Node) {
+ call := n.(*ast.CallExpr)
+
+ // Retrieve target type name.
+ var tname *types.TypeName
+ switch fun := call.Fun.(type) {
+ case *ast.Ident:
+ tname, _ = pass.TypesInfo.Uses[fun].(*types.TypeName)
+ case *ast.SelectorExpr:
+ tname, _ = pass.TypesInfo.Uses[fun.Sel].(*types.TypeName)
+ }
+ 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
+ }
+ if s := T.Name(); target != s {
+ target += " (" + s + ")"
+ }
+
+ // Check that type V of v has an underlying integral type that is not byte or rune.
+ if len(call.Args) != 1 {
+ return
+ }
+ v := call.Args[0]
+ vtyp := pass.TypesInfo.TypeOf(v)
+ V, _ := vtyp.Underlying().(*types.Basic)
+ if V == nil || V.Info()&types.IsInteger == 0 {
+ return
+ }
+ switch V.Kind() {
+ case types.Byte, types.Rune, types.UntypedRune:
+ return
+ }
+
+ // Retrieve source type name.
+ source := typeName(vtyp)
+ if source == "" {
+ return
+ }
+ if s := V.Name(); source != s {
+ source += " (" + s + ")"
+ }
+ diag := analysis.Diagnostic{
+ Pos: n.Pos(),
+ Message: fmt.Sprintf("conversion from %s to %s yields a string of one rune", source, target),
+ SuggestedFixes: []analysis.SuggestedFix{
+ {
+ Message: "Did you mean to convert a rune to a string?",
+ TextEdits: []analysis.TextEdit{
+ {
+ Pos: v.Pos(),
+ End: v.Pos(),
+ NewText: []byte("rune("),
+ },
+ {
+ Pos: v.End(),
+ End: v.End(),
+ NewText: []byte(")"),
+ },
+ },
+ },
+ },
+ }
+ pass.Report(diag)
+ })
+ return nil, nil
+}
diff --git a/go/analysis/passes/stringintconv/string_test.go b/go/analysis/passes/stringintconv/string_test.go
new file mode 100644
index 0000000..ed06332
--- /dev/null
+++ b/go/analysis/passes/stringintconv/string_test.go
@@ -0,0 +1,17 @@
+// Copyright 2020 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 stringintconv_test
+
+import (
+ "testing"
+
+ "golang.org/x/tools/go/analysis/analysistest"
+ "golang.org/x/tools/go/analysis/passes/stringintconv"
+)
+
+func Test(t *testing.T) {
+ testdata := analysistest.TestData()
+ analysistest.Run(t, testdata, stringintconv.Analyzer, "a")
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/a/a.go b/go/analysis/passes/stringintconv/testdata/src/a/a.go
new file mode 100644
index 0000000..72ceb97
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/a/a.go
@@ -0,0 +1,36 @@
+// Copyright 2020 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.
+
+// This file contains tests for the stringintconv checker.
+
+package a
+
+type A string
+
+type B = string
+
+type C int
+
+type D = uintptr
+
+func StringTest() {
+ var (
+ i int
+ j rune
+ k byte
+ l C
+ m D
+ n = []int{0, 1, 2}
+ o struct{ x int }
+ )
+ const p = 0
+ _ = string(i) // want `^conversion from int to string yields a string of one rune$`
+ _ = string(j)
+ _ = string(k)
+ _ = string(p) // want `^conversion from untyped int to string yields a string of one rune$`
+ _ = A(l) // want `^conversion from C \(int\) to A \(string\) yields a string of one rune$`
+ _ = B(m) // want `^conversion from uintptr to B \(string\) yields a string of one rune$`
+ _ = string(n[1]) // want `^conversion from int to string yields a string of one rune$`
+ _ = string(o.x) // want `^conversion from int to string yields a string of one rune$`
+}
