go/analysis/passes/stringintconv: make first fix behavior-preserving
Go 1.28 rejects string(x) conversions where x is a non-byte,
non-rune integer (go.dev/issue/3939). To let "go fix" migrate code
before a module's go directive is raised to go1.28, this CL makes
the analyzer's first suggested fix preserve the existing behavior,
and adds the analyzer to the go fix suite.
The old "Convert a single rune to a string" fix, string(rune(x)),
was not behavior-preserving for types wider than 32 bits due to
truncation. For such types the fix now uses fmt.Sprintf("%c", x),
which is exactly equivalent to string(x). The plain rune(x) form is
still used for constants that fit in 32 bits and for types of at
most 32 bits.
The safe fix is now offered first, since that's what go fix applies.
The decimal-formatting fix, which changes behavior, is second; it now
uses fmt.Sprintf("%d", x), for symmetry, instead of fmt.Sprint(x).
If no behavior-preserving fix is available no fixes are offered at
all, so that go fix never applies the second fix.
Also:
- simplify the edits of both Sprintf fixes by choosing the range to
rewrite once: string(x) -> fmt.Sprintf(...), and
mystring(x) -> mystring(fmt.Sprintf(...)).
This fixes a bug in the old fmt.Sprint fix, which dropped the
conversion S(x) when S was a type parameter whose sole term
is string, yielding an ill-typed result.
- update the doc, which recommended strconv.Itoa.
Updates golang/go#81760
Change-Id: If445f650d045125475b2051e4c50e28f630a11ae
Reviewed-on: https://go-review.googlesource.com/c/tools/+/840805
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alex Putman <aputman@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/passes/stringintconv/doc.go b/go/analysis/passes/stringintconv/doc.go
index 205cd64..0807afa 100644
--- a/go/analysis/passes/stringintconv/doc.go
+++ b/go/analysis/passes/stringintconv/doc.go
@@ -15,7 +15,22 @@
// 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.
+// As of Go 1.28, such conversions are rejected by the compiler in
+// files whose Go version is go1.28 or later.
+//
+// The checker offers two fixes. The first, which is applied by "go
+// fix", preserves the existing behavior by converting x to a rune
+// first: string(rune(x)). If x may be wider than 32 bits, truncation
+// to a rune could turn an invalid code point into a valid one, so
+// the fix instead uses fmt.Sprintf("%c", x), which, like string(x),
+// yields "\uFFFD" for all values outside the range of valid code
+// points.
+//
+// The second fix formats the number as a decimal using
+// fmt.Sprintf("%d", x), which is usually what was intended,
+// but changes the behavior.
+//
+// To migrate a module to Go 1.28, run "go fix" before updating the
+// go directive in its go.mod file, since after that, the package no
+// longer compiles and cannot be analyzed.
package stringintconv
diff --git a/go/analysis/passes/stringintconv/preserve_test.go b/go/analysis/passes/stringintconv/preserve_test.go
new file mode 100644
index 0000000..7c4c9d0
--- /dev/null
+++ b/go/analysis/passes/stringintconv/preserve_test.go
@@ -0,0 +1,63 @@
+// Copyright 2026 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 (
+ "fmt"
+ "math"
+ "testing"
+ "unicode/utf8"
+)
+
+// TestFixPreservesBehavior checks that the expressions produced by
+// the analyzer's "Convert [a] single rune to [a] string" fixes are
+// equivalent to the original string(x) conversion.
+//
+// We can't write string(x) here (vet would reject it, as would
+// go1.28), so we use the spec's definition as an oracle:
+// "Converting a signed or unsigned integer value to a string type
+// yields a string containing the UTF-8 representation of the
+// integer. Values outside the range of valid Unicode code points
+// are converted to "\uFFFD"."
+func TestFixPreservesBehavior(t *testing.T) {
+ oracle := func(x int64, inRange bool) string {
+ if !inRange || x < 0 || x > utf8.MaxRune {
+ return "\uFFFD"
+ }
+ return string(rune(x)) // (surrogates also yield "\uFFFD")
+ }
+
+ // Types wider than 32 bits use fmt.Sprintf("%c", x).
+ signed := []int64{
+ math.MinInt64, math.MinInt32 - 1, math.MinInt32, -1, 0, 'x',
+ 0xD800, utf8.MaxRune, utf8.MaxRune + 1, math.MaxInt32, math.MaxInt32 + 1,
+ 1<<32 + 'x', // truncates to 'x'
+ math.MaxInt64,
+ }
+ for _, x := range signed {
+ if got, want := fmt.Sprintf("%c", x), oracle(x, true); got != want {
+ t.Errorf("int64 %#x: got %q, want %q", x, got, want)
+ }
+ }
+
+ unsigned := []uint64{
+ 0, 'x', 0xD800, utf8.MaxRune, utf8.MaxRune + 1,
+ math.MaxInt32, math.MaxInt32 + 1, math.MaxUint32,
+ 1<<32 + 'x', // truncates to 'x'
+ math.MaxUint64,
+ }
+ for _, x := range unsigned {
+ if got, want := fmt.Sprintf("%c", x), oracle(int64(x), x <= math.MaxInt64); got != want {
+ t.Errorf("uint64 %#x: got %q, want %q", x, got, want)
+ }
+ }
+
+ // uint32 and narrower use string(rune(x)).
+ for _, x := range []uint32{0, 'x', utf8.MaxRune + 1, math.MaxInt32 + 1, 1<<31 + 'x', math.MaxUint32} {
+ if got, want := string(rune(x)), oracle(int64(x), true); got != want {
+ t.Errorf("uint32 %#x: got %q, want %q", x, got, want)
+ }
+ }
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden
index c3f9ce5..3a2e8af 100644
--- a/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fix.go.golden
@@ -1,4 +1,4 @@
--- Format the number as a decimal --
+-- Format number as decimal (changes behavior) --
// This code relies on pre-go1.28 string(integer) conversion rules;
// the build tag downgrades the file to go1.21 (the oldest version
// to which a build tag can downgrade).
@@ -9,10 +9,10 @@
import "fmt"
func _(x uint64) {
- println(fmt.Sprint(x)) // want `conversion from uint64 to string yields...`
+ println(fmt.Sprintf("%d", x)) // want `conversion from uint64 to string yields...`
}
--- Convert a single rune to a string --
+-- Convert single rune to string (preserves behavior) --
// This code relies on pre-go1.28 string(integer) conversion rules;
// the build tag downgrades the file to go1.21 (the oldest version
// to which a build tag can downgrade).
@@ -20,6 +20,8 @@
package fix
+import "fmt"
+
func _(x uint64) {
- println(string(rune(x))) // want `conversion from uint64 to string yields...`
+ println(fmt.Sprintf("%c", x)) // want `conversion from uint64 to string yields...`
}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden
index 49d616b..68e7341 100644
--- a/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixdot.go.golden
@@ -1,4 +1,4 @@
--- Format the number as a decimal --
+-- Format number as decimal (changes behavior) --
// This code relies on pre-go1.28 string(integer) conversion rules;
// the build tag downgrades the file to go1.21 (the oldest version
// to which a build tag can downgrade).
@@ -9,10 +9,10 @@
import . "fmt"
func _(x uint64) {
- Println(Sprint(x)) // want `conversion from uint64 to string yields...`
+ Println(Sprintf("%d", x)) // want `conversion from uint64 to string yields...`
}
--- Convert a single rune to a string --
+-- Convert single rune to string (preserves behavior) --
// This code relies on pre-go1.28 string(integer) conversion rules;
// the build tag downgrades the file to go1.21 (the oldest version
// to which a build tag can downgrade).
@@ -23,5 +23,5 @@
import . "fmt"
func _(x uint64) {
- Println(string(rune(x))) // want `conversion from uint64 to string yields...`
+ Println(Sprintf("%c", x)) // want `conversion from uint64 to string yields...`
}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden
index 8a01e97..d9044a2 100644
--- a/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixnamed.go.golden
@@ -1,4 +1,4 @@
--- Format the number as a decimal --
+-- Format number as decimal (changes behavior) --
// This code relies on pre-go1.28 string(integer) conversion rules;
// the build tag downgrades the file to go1.21 (the oldest version
// to which a build tag can downgrade).
@@ -11,7 +11,7 @@
type mystring string
func _(x int16) mystring {
- return mystring(fmt.Sprint(x)) // want `conversion from int16 to mystring \(string\)...`
+ return mystring(fmt.Sprintf("%d", x)) // want `conversion from int16 to mystring \(string\)...`
}
-- Convert a single rune to a string --
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go b/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go
new file mode 100644
index 0000000..1a01c7d
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go
@@ -0,0 +1,18 @@
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+type str = string
+
+// The conversion is dropped when T is string, even via an alias...
+func _(x int16) str {
+ return str(x) // want `conversion from int16 to str \(string\) yields...`
+}
+
+// ...but not when T is a type parameter whose only term is string.
+func _[S ~string](x int16) S {
+ return S(x) // want `conversion from int16 to string \(in S\) yields...`
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go.golden
new file mode 100644
index 0000000..aa9544e
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixtarget.go.golden
@@ -0,0 +1,41 @@
+-- Convert a single rune to a string --
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+type str = string
+
+// The conversion is dropped when T is string, even via an alias...
+func _(x int16) str {
+ return str(rune(x)) // want `conversion from int16 to str \(string\) yields...`
+}
+
+// ...but not when T is a type parameter whose only term is string.
+func _[S ~string](x int16) S {
+ return S(rune(x)) // want `conversion from int16 to string \(in S\) yields...`
+}
+
+-- Format number as decimal (changes behavior) --
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+import "fmt"
+
+type str = string
+
+// The conversion is dropped when T is string, even via an alias...
+func _(x int16) str {
+ return fmt.Sprintf("%d", x) // want `conversion from int16 to str \(string\) yields...`
+}
+
+// ...but not when T is a type parameter whose only term is string.
+func _[S ~string](x int16) S {
+ return S(fmt.Sprintf("%d", x)) // want `conversion from int16 to string \(in S\) yields...`
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go b/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go
new file mode 100644
index 0000000..b8be7c3
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go
@@ -0,0 +1,34 @@
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+const (
+ small int64 = 'x'
+ large int64 = 1 << 40
+)
+
+func _(i int, i64 int64, u uint, u32 uint32, i8 int8, up uintptr) {
+ println(string(i)) // want `conversion from int to string yields...`
+ println(string(i64)) // want `conversion from int64 to string yields...`
+ println(string(u)) // want `conversion from uint to string yields...`
+ println(string(up)) // want `conversion from uintptr to string yields...`
+ println(string(u32)) // want `conversion from uint32 to string yields...`
+ println(string(i8)) // want `conversion from int8 to string yields...`
+ println(string(small)) // want `conversion from int64 to string yields...`
+ println(string(large)) // want `conversion from int64 to string yields...`
+}
+
+type myint int64
+
+func (myint) String() string { return "" }
+
+func _(m myint) {
+ // No fix is offered, since Sprintf might call a method (such as Format).
+ println(string(m)) // want `conversion from myint \(int64\) to string yields...`
+
+ // No fix is offered, since the constant overflows int32 and has no type.
+ println(string(1 << 40)) // want `conversion from untyped int to string yields...`
+}
diff --git a/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go.golden b/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go.golden
new file mode 100644
index 0000000..587b3ea
--- /dev/null
+++ b/go/analysis/passes/stringintconv/testdata/src/fix/fixwide.go.golden
@@ -0,0 +1,111 @@
+-- Convert a single rune to a string --
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+const (
+ small int64 = 'x'
+ large int64 = 1 << 40
+)
+
+func _(i int, i64 int64, u uint, u32 uint32, i8 int8, up uintptr) {
+ println(string(i)) // want `conversion from int to string yields...`
+ println(string(i64)) // want `conversion from int64 to string yields...`
+ println(string(u)) // want `conversion from uint to string yields...`
+ println(string(up)) // want `conversion from uintptr to string yields...`
+ println(string(rune(u32))) // want `conversion from uint32 to string yields...`
+ println(string(rune(i8))) // want `conversion from int8 to string yields...`
+ println(string(rune(small))) // want `conversion from int64 to string yields...`
+ println(string(large)) // want `conversion from int64 to string yields...`
+}
+
+type myint int64
+
+func (myint) String() string { return "" }
+
+func _(m myint) {
+ // No fix is offered, since Sprintf might call a method (such as Format).
+ println(string(m)) // want `conversion from myint \(int64\) to string yields...`
+
+ // No fix is offered, since the constant overflows int32 and has no type.
+ println(string(1 << 40)) // want `conversion from untyped int to string yields...`
+}
+
+-- Convert single rune to string (preserves behavior) --
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+import "fmt"
+
+const (
+ small int64 = 'x'
+ large int64 = 1 << 40
+)
+
+func _(i int, i64 int64, u uint, u32 uint32, i8 int8, up uintptr) {
+ println(fmt.Sprintf("%c", i)) // want `conversion from int to string yields...`
+ println(fmt.Sprintf("%c", i64)) // want `conversion from int64 to string yields...`
+ println(fmt.Sprintf("%c", u)) // want `conversion from uint to string yields...`
+ println(fmt.Sprintf("%c", up)) // want `conversion from uintptr to string yields...`
+ println(string(u32)) // want `conversion from uint32 to string yields...`
+ println(string(i8)) // want `conversion from int8 to string yields...`
+ println(string(small)) // want `conversion from int64 to string yields...`
+ println(fmt.Sprintf("%c", large)) // want `conversion from int64 to string yields...`
+}
+
+type myint int64
+
+func (myint) String() string { return "" }
+
+func _(m myint) {
+ // No fix is offered, since Sprintf might call a method (such as Format).
+ println(string(m)) // want `conversion from myint \(int64\) to string yields...`
+
+ // No fix is offered, since the constant overflows int32 and has no type.
+ println(string(1 << 40)) // want `conversion from untyped int to string yields...`
+}
+
+-- Format number as decimal (changes behavior) --
+// This code relies on pre-go1.28 string(integer) conversion rules;
+// the build tag downgrades the file to go1.21 (the oldest version
+// to which a build tag can downgrade).
+//go:build go1.21
+
+package fix
+
+import "fmt"
+
+const (
+ small int64 = 'x'
+ large int64 = 1 << 40
+)
+
+func _(i int, i64 int64, u uint, u32 uint32, i8 int8, up uintptr) {
+ println(fmt.Sprintf("%d", i)) // want `conversion from int to string yields...`
+ println(fmt.Sprintf("%d", i64)) // want `conversion from int64 to string yields...`
+ println(fmt.Sprintf("%d", u)) // want `conversion from uint to string yields...`
+ println(fmt.Sprintf("%d", up)) // want `conversion from uintptr to string yields...`
+ println(fmt.Sprintf("%d", u32)) // want `conversion from uint32 to string yields...`
+ println(fmt.Sprintf("%d", i8)) // want `conversion from int8 to string yields...`
+ println(fmt.Sprintf("%d", small)) // want `conversion from int64 to string yields...`
+ println(fmt.Sprintf("%d", large)) // want `conversion from int64 to string yields...`
+}
+
+type myint int64
+
+func (myint) String() string { return "" }
+
+func _(m myint) {
+ // No fix is offered, since Sprintf might call a method (such as Format).
+ println(string(m)) // want `conversion from myint \(int64\) to string yields...`
+
+ // No fix is offered, since the constant overflows int32 and has no type.
+ println(string(1 << 40)) // want `conversion from untyped int to string yields...`
+}
diff --git a/go/analysis/suite/fix/fix.go b/go/analysis/suite/fix/fix.go
index aa63b45..08f9228 100644
--- a/go/analysis/suite/fix/fix.go
+++ b/go/analysis/suite/fix/fix.go
@@ -20,6 +20,7 @@
"golang.org/x/tools/go/analysis/passes/hostport"
"golang.org/x/tools/go/analysis/passes/inline"
"golang.org/x/tools/go/analysis/passes/modernize"
+ "golang.org/x/tools/go/analysis/passes/stringintconv"
)
// Suite is the suite of analyzers run by cmd/fix.
@@ -31,6 +32,10 @@
buildtag.Analyzer,
hostport.Analyzer,
inline.Analyzer,
+ // stringintconv's first fix, string(int) -> string(rune(int)),
+ // preserves behavior, and prepares for go1.28, which rejects
+ // string(int) conversions (go.dev/issue/3939).
+ stringintconv.Analyzer,
},
modernize.Suite,
// TODO(adonovan): add any other vet analyzers whose fixes are always safe.
@@ -41,6 +46,5 @@
// - composites: some types (e.g. PointXY{1,2}) don't want field names.
// - timeformat: flipping MM/DD is a behavior change, but the code
// could potentially be a workaround for another bug.
- // - stringintconv: offers two fixes, user input required to choose.
// - fieldalignment: poor signal/noise; fix could be a regression.
)
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index 865559f..2db1147 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -4259,7 +4259,13 @@
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.
+As of Go 1.28, such conversions are rejected by the compiler in files whose Go version is go1.28 or later.
+
+The checker offers two fixes. The first, which is applied by "go fix", preserves the existing behavior by converting x to a rune first: string(rune(x)). If x may be wider than 32 bits, truncation to a rune could turn an invalid code point into a valid one, so the fix instead uses fmt.Sprintf("%c", x), which, like string(x), yields "\\uFFFD" for all values outside the range of valid code points.
+
+The second fix formats the number as a decimal using fmt.Sprintf("%d", x), which is usually what was intended, but changes the behavior.
+
+To migrate a module to Go 1.28, run "go fix" before updating the go directive in its go.mod file, since after that, the package no longer compiles and cannot be analyzed.
Default: on.
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index ebbb44b..e9bc6cf 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -1762,7 +1762,7 @@
},
{
"Name": "\"stringintconv\"",
- "Doc": "check for string(int) conversions\n\nThis checker flags conversions of the form string(x) where x is an integer\n(but not byte or rune) type. Such conversions are discouraged because they\nreturn the UTF-8 representation of the Unicode code point x, and not a decimal\nstring representation of x as one might expect. Furthermore, if x denotes an\ninvalid code point, the conversion cannot be statically rejected.\n\nFor conversions that intend on using the code point, consider replacing them\nwith string(rune(x)). Otherwise, strconv.Itoa and its equivalents return the\nstring representation of the value in the desired base.",
+ "Doc": "check for string(int) conversions\n\nThis checker flags conversions of the form string(x) where x is an integer\n(but not byte or rune) type. Such conversions are discouraged because they\nreturn the UTF-8 representation of the Unicode code point x, and not a decimal\nstring representation of x as one might expect. Furthermore, if x denotes an\ninvalid code point, the conversion cannot be statically rejected.\n\nAs of Go 1.28, such conversions are rejected by the compiler in\nfiles whose Go version is go1.28 or later.\n\nThe checker offers two fixes. The first, which is applied by \"go\nfix\", preserves the existing behavior by converting x to a rune\nfirst: string(rune(x)). If x may be wider than 32 bits, truncation\nto a rune could turn an invalid code point into a valid one, so\nthe fix instead uses fmt.Sprintf(\"%c\", x), which, like string(x),\nyields \"\\uFFFD\" for all values outside the range of valid code\npoints.\n\nThe second fix formats the number as a decimal using\nfmt.Sprintf(\"%d\", x), which is usually what was intended,\nbut changes the behavior.\n\nTo migrate a module to Go 1.28, run \"go fix\" before updating the\ngo directive in its go.mod file, since after that, the package no\nlonger compiles and cannot be analyzed.",
"Default": "true",
"Status": ""
},
@@ -3829,7 +3829,7 @@
},
{
"Name": "stringintconv",
- "Doc": "check for string(int) conversions\n\nThis checker flags conversions of the form string(x) where x is an integer\n(but not byte or rune) type. Such conversions are discouraged because they\nreturn the UTF-8 representation of the Unicode code point x, and not a decimal\nstring representation of x as one might expect. Furthermore, if x denotes an\ninvalid code point, the conversion cannot be statically rejected.\n\nFor conversions that intend on using the code point, consider replacing them\nwith string(rune(x)). Otherwise, strconv.Itoa and its equivalents return the\nstring representation of the value in the desired base.",
+ "Doc": "check for string(int) conversions\n\nThis checker flags conversions of the form string(x) where x is an integer\n(but not byte or rune) type. Such conversions are discouraged because they\nreturn the UTF-8 representation of the Unicode code point x, and not a decimal\nstring representation of x as one might expect. Furthermore, if x denotes an\ninvalid code point, the conversion cannot be statically rejected.\n\nAs of Go 1.28, such conversions are rejected by the compiler in\nfiles whose Go version is go1.28 or later.\n\nThe checker offers two fixes. The first, which is applied by \"go\nfix\", preserves the existing behavior by converting x to a rune\nfirst: string(rune(x)). If x may be wider than 32 bits, truncation\nto a rune could turn an invalid code point into a valid one, so\nthe fix instead uses fmt.Sprintf(\"%c\", x), which, like string(x),\nyields \"\\uFFFD\" for all values outside the range of valid code\npoints.\n\nThe second fix formats the number as a decimal using\nfmt.Sprintf(\"%d\", x), which is usually what was intended,\nbut changes the behavior.\n\nTo migrate a module to Go 1.28, run \"go fix\" before updating the\ngo directive in its go.mod file, since after that, the package no\nlonger compiles and cannot be analyzed.",
"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/stringintconv",
"Default": true
},
diff --git a/internal/analysis/intconv/intconv.go b/internal/analysis/intconv/intconv.go
index ed1716c..4e9e74f 100644
--- a/internal/analysis/intconv/intconv.go
+++ b/internal/analysis/intconv/intconv.go
@@ -11,7 +11,10 @@
import (
"fmt"
"go/ast"
+ "go/constant"
+ "go/token"
"go/types"
+ "math"
"strings"
"golang.org/x/tools/go/analysis"
@@ -64,6 +67,12 @@
// Check reports whether the call is a conversion string(x) from a
// non-byte, non-rune integer x, and if so, returns a diagnostic
// with suggested fixes.
+//
+// Such conversions are rejected by the type checker in files
+// using go1.28 or later, so in practice the analyzer only sees
+// them in older files. (An exception is a tool built with a
+// pre-go1.28 go/types analyzing a go1.28 file, in which case
+// the diagnostic and its fix are just as useful.)
func Check(info *types.Info, curCall inspector.Cursor) (_ analysis.Diagnostic, ok bool) {
call := curCall.Node().(*ast.CallExpr)
@@ -163,75 +172,141 @@
Pos: call.Pos(),
Message: fmt.Sprintf("conversion from %s to %s yields a string of one rune, not a string of digits", source, target),
}
- addFix := func(message string, edits []analysis.TextEdit) {
+ if !convertibleToRune {
+ return diag, true // no fixes
+ }
+ addFix := func(message string, edits ...analysis.TextEdit) {
diag.SuggestedFixes = append(diag.SuggestedFixes, analysis.SuggestedFix{
Message: message,
TextEdits: edits,
})
}
+ edit := func(start, end token.Pos, text string) analysis.TextEdit {
+ return analysis.TextEdit{Pos: start, End: end, NewText: []byte(text)}
+ }
- // Fix 1: use fmt.Sprint(x)
+ // sprintf returns edits that replace x by fmt.Sprintf("%verb", x),
+ // adding an import of "fmt" as needed.
+ // The conversion is retained unless T is exactly string:
//
- // Prefer fmt.Sprint over strconv.Itoa, FormatInt,
- // or FormatUint, as it works for any type.
- // Add an import of "fmt" as needed.
+ // string(x) -> fmt.Sprintf("%verb", x)
+ // mystring(x) -> mystring(fmt.Sprintf("%verb", x))
+ sprintf := func(verb string) []analysis.TextEdit {
+ start, end := arg.Pos(), arg.End()
+ if types.Identical(T, types.Typ[types.String]) {
+ start, end = call.Pos(), call.End()
+ }
+ file := astutil.EnclosingFile(curCall)
+ prefix, importEdits := refactor.AddImport(info, file, "fmt", "fmt", "Sprintf", arg.Pos())
+ return append(importEdits,
+ edit(start, arg.Pos(), prefix+`Sprintf("%`+verb+`", `),
+ edit(arg.End(), end, ")"))
+ }
+
+ // sprintfOK reports whether fmt.Sprintf may be used.
//
- // Unless the type is exactly string, we must retain the conversion.
- //
- // Do not offer this fix if type parameters are involved,
+ // Do not use it if type parameters are involved,
// as there are too many combinations and subtleties.
// Consider x = rune | int16 | []byte: in all cases,
// string(x) is legal, but the appropriate diagnostic
- // and fix differs. Similarly, don't offer the fix if
- // the type has methods, as some {String,GoString,Format}
- // may change the behavior of fmt.Sprint.
- if len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0 {
- file := astutil.EnclosingFile(curCall)
- prefix, importEdits := refactor.AddImport(info, file, "fmt", "fmt", "Sprint", arg.Pos())
- if types.Identical(T0, types.Typ[types.String]) {
- // string(x) -> fmt.Sprint(x)
- addFix("Format the number as a decimal", append(importEdits,
- analysis.TextEdit{
- Pos: call.Fun.Pos(),
- End: call.Fun.End(),
- NewText: []byte(prefix + "Sprint"),
- }),
- )
- } else {
- // mystring(x) -> mystring(fmt.Sprint(x))
- addFix("Format the number as a decimal", append(importEdits,
- analysis.TextEdit{
- Pos: call.Lparen + 1,
- End: call.Lparen + 1,
- NewText: []byte(prefix + "Sprint("),
- },
- analysis.TextEdit{
- Pos: call.Rparen,
- End: call.Rparen,
- NewText: []byte(")"),
- }),
- )
- }
+ // and fix differs. Similarly, don't use it if the type
+ // has methods, as a Format method may change the behavior
+ // of fmt.Sprintf.
+ sprintfOK := len(ttypes) == 1 && len(vtypes) == 1 && types.NewMethodSet(V0).Len() == 0
+
+ // The first fix must preserve the existing behavior,
+ // since it is the one applied by "go fix". (This lets
+ // "go fix" migrate code before its go.mod file is
+ // upgraded to go1.28, at which point string(int)
+ // conversions become a compile error.)
+ //
+ // If no behavior-preserving fix is available,
+ // we offer no fixes at all.
+
+ // Fix 1: use string(rune(x)) or fmt.Sprintf("%c", x),
+ // preserving behavior.
+ //
+ // string(x) yields "\uFFFD" for any x outside the range of valid
+ // code points, whereas rune(x) truncates x to 32 bits, which may
+ // turn an invalid code point into a valid one. So rune(x) is
+ // exact only if x is a constant that fits in 32 bits, or if every
+ // type in the type set of V is at most 32 bits wide.
+ // (For uint32, values ≥ 1<<31 become negative runes, which are
+ // just as invalid.)
+ //
+ // For a wider type we use fmt.Sprintf("%c", x), which is exactly
+ // equivalent: fmt converts x to uint64 and treats any value
+ // above MaxRune (including negative values) as "\uFFFD".
+ // In addition to the sprintfOK conditions, we avoid a
+ // single-term type parameter (e.g. ~int64), since it
+ // could be instantiated by a type with a Format method,
+ // and untyped constants (whose default type int might
+ // overflow).
+ if runeIsExact(info, arg, vtypes) {
+ addFix("Convert a single rune to a string",
+ edit(arg.Pos(), arg.Pos(), "rune("),
+ edit(arg.End(), arg.End(), ")"))
+
+ } else if sprintfOK &&
+ !is[*types.TypeParam](types.Unalias(V)) &&
+ !isUntypedConst(info, arg) {
+ addFix("Convert single rune to string (preserves behavior)", sprintf("c")...)
+
+ } else {
+ return diag, true // no behavior-preserving fix, so no fixes
}
- // Fix 2: use string(rune(x))
- if convertibleToRune {
- addFix("Convert a single rune to a string", []analysis.TextEdit{
- {
- Pos: arg.Pos(),
- End: arg.Pos(),
- NewText: []byte("rune("),
- },
- {
- Pos: arg.End(),
- End: arg.End(),
- NewText: []byte(")"),
- },
- })
+ // Fix 2: use fmt.Sprintf("%d", x), which changes behavior.
+ //
+ // Prefer fmt.Sprintf over strconv.Itoa, FormatInt,
+ // or FormatUint, as it works for any integer type.
+ if sprintfOK {
+ addFix("Format number as decimal (changes behavior)", sprintf("d")...)
}
+
return diag, true
}
+// runeIsExact reports whether string(rune(x)) is equivalent to
+// string(x) for the argument x, whose type set is vtypes.
+func runeIsExact(info *types.Info, x ast.Expr, vtypes []types.Type) bool {
+ // Constant that fits in 32 bits?
+ if tv := info.Types[x]; tv.Value != nil {
+ v, exact := constant.Int64Val(constant.ToInt(tv.Value))
+ return exact && math.MinInt32 <= v && v <= math.MaxInt32
+ }
+
+ // All types in the type set at most 32 bits wide?
+ for _, t := range vtypes {
+ u, ok := t.Underlying().(*types.Basic)
+ if !ok {
+ return false
+ }
+ switch u.Kind() {
+ case types.Int8, types.Int16, types.Int32,
+ types.Uint8, types.Uint16, types.Uint32:
+ default:
+ return false // int, int64, uint, uint64, uintptr, etc
+ }
+ }
+ return true
+}
+
+// isUntypedConst reports whether x is an untyped constant.
+func isUntypedConst(info *types.Info, x ast.Expr) bool {
+ tv := info.Types[x]
+ if tv.Value == nil {
+ return false
+ }
+ u, ok := tv.Type.(*types.Basic)
+ return ok && u.Info()&types.IsUntyped != 0
+}
+
+func is[T any](x any) bool {
+ _, ok := x.(T)
+ return ok
+}
+
func structuralTypes(t types.Type) ([]types.Type, error) {
var structuralTypes []types.Type
if tp, ok := types.Unalias(t).(*types.TypeParam); ok {
