diff --git a/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go b/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go index 47d411eeb16..fc4f4a2200d 100644 --- a/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go +++ b/pkg/linters/stringbytesroundtrip/stringbytesroundtrip.go @@ -135,13 +135,23 @@ func reportWastefulCloneRoundTrip(pass *analysis.Pass, outer, inner *ast.CallExp ) } +// isStringType reports whether t is a string basic type. Callers pass an +// already-.Underlying()-resolved type, so this also matches named string types. func isStringType(t types.Type) bool { basic, ok := t.(*types.Basic) return ok && basic.Kind() == types.String } +// isExactString reports whether t denotes the predeclared string type, not a +// named type whose underlying type is string. Unlike isStringType, which +// expects an already-.Underlying()-resolved type, isExactString must be given +// the raw type so it can tell string from `type MyString string`. Aliases are +// resolved first, because an alias may denote either the predeclared string +// (`type A = string`) or a named string type (`type A = MyString`). That +// distinction matters because only the predeclared string can have both +// conversions removed; a named string type still needs an outer conversion. func isExactString(t types.Type) bool { - return isStringType(t) + return isStringType(types.Unalias(t)) } func isByteSliceType(t types.Type) bool { diff --git a/pkg/linters/stringbytesroundtrip/testdata/src/stringbytesroundtrip/stringbytesroundtrip.go b/pkg/linters/stringbytesroundtrip/testdata/src/stringbytesroundtrip/stringbytesroundtrip.go index 5ec67cd4c58..980b0331b1b 100644 --- a/pkg/linters/stringbytesroundtrip/testdata/src/stringbytesroundtrip/stringbytesroundtrip.go +++ b/pkg/linters/stringbytesroundtrip/testdata/src/stringbytesroundtrip/stringbytesroundtrip.go @@ -37,6 +37,30 @@ func badNamedTypes() string { return string([]byte(ms)) // want `string\(\[\]byte\(ms\)\) is a redundant round-trip; replace it with string\(ms\)` } +// aliasString is an alias for the predeclared string type, so round-trips +// through it are fully redundant. +type aliasString = string + +// namedAlias is an alias for a named string type, so an outer conversion is +// still required even though its underlying type is string. +type namedAlias = myString + +func badMixedNamedTypes() { + s := "hello" + var ms myString = "hello" + var as aliasString = "hello" + var na namedAlias = "hello" + + // Outer conversion is a named string type: the outer conversion must stay. + _ = myString([]byte(s)) // want `myString\(\[\]byte\(s\)\) is a redundant round-trip; replace it with myString\(s\)` + // Argument is a named string type: an outer conversion is still needed. + _ = string([]byte(ms)) // want `string\(\[\]byte\(ms\)\) is a redundant round-trip; replace it with string\(ms\)` + // Alias of the predeclared string type: both conversions can be removed. + _ = aliasString([]byte(as)) // want `aliasString\(\[\]byte\(as\)\) is a redundant round-trip; both conversions can be removed` + // Alias of a named string type: the outer conversion must stay. + _ = namedAlias([]byte(na)) // want `namedAlias\(\[\]byte\(na\)\) is a redundant round-trip; replace it with namedAlias\(na\)` +} + // helperString is a regular function, not a type conversion — must not be flagged. func helperString(b []byte) string { return string(b) }