cmd/vet: warn about string(int) type conversions #32479

ianlancetaylor · 2019-06-07T18:18:36Z

#3939 proposes removing the explicit type conversion string(i) where i has an integer type other than rune. That is a backward incompatible change, so we cannot make it until we can count on having modules everywhere, with the support modules provide for determining the version of the language to use to compile code.

There is a step we can take today, which is to change go vet to report conversions of integer types other than rune to string. If we think that removing this feature from the language is a good idea, then a vet check for it is a good idea, and it is one that we can implement today.

The text was updated successfully, but these errors were encountered:

agnivade · 2019-06-08T03:53:04Z

@alandonovan @mvdan

networkimprov · 2019-06-08T05:28:50Z

Pls also permit string(b) where b is a byte or uint8.

mvdan · 2019-06-24T22:53:36Z

This seems fine to me. The warning should probably point the user in the right direction.

alanfo · 2019-06-27T18:19:59Z

Pls also permit string(b) where b is a byte or uint8.

I think this is essential if this proposal (and eventually #3939) is to be adopted.

The string(byte) conversion, when you're dealing with ASCII characters only, is probably used at least as often as string(rune)in my experience.

dolmen · 2019-07-02T16:09:24Z

@networkimprov @alanfo You will still be able to use string([]byte{b}).

networkimprov · 2019-07-02T16:38:59Z

That would be a surprising requirement. And allocates a byte slice for a single statement?

alanfo · 2019-07-02T23:15:40Z

@dolmen

You will still be able to use string([]byte{b}).

Well string(rune(b)) would be a better alternative because you wouldn't then have to allocate a byte slice but it would still be a nuisance to have to update all my code to use this workaround :(

smasher164 · 2019-11-06T05:48:24Z

I will take a crack at adding an analysis pass and vet check for this.

alandonovan · 2019-11-06T14:39:05Z

string([]byte{b}) is not equivalent to string(b) for a byte value b: the former yields the string whose bytes are just b, the latter returns the string that UTF-8-encodes the single rune b. If b is greater than 127, then the former will be a single-byte string but the latter will be a two-byte encoding.

it would still be a nuisance to have to update all my code to use this workaround :(

Michael Matloob recently presented at a conference the "suggested fixes" feature of the analysis Diagnostics API. The diagnostic for the new analyzer could replace (or offer to replace) the deprecated expressions by the preferred version.

networkimprov · 2019-11-06T20:29:54Z

string([]byte{b}) ... yields the string whose bytes are just b

That could be invalid UTF-8. So string(buf[x:y]) may be a bug.

To extract strings from buffers, should we generally use string(bytes.ToValidUTF8(buf, nil))?

alandonovan · 2019-11-06T20:39:09Z

That could be invalid UTF-8. So string(buf[x:y]) may be a bug.

It could be, but it very much depends on the situation.

To extract strings from buffers, should we generally use string(bytes.ToValidUTF8(buf, nil))?

I had never even heard of that function until now, and I'm more deliberate than most about string encodings. Slicing a string or byte slice may be perfectly fine, again depending on the situation.

alanfo · 2019-11-07T09:13:26Z

The diagnostic for the new analyzer could replace (or offer to replace) the deprecated expressions by the preferred version.

Be that as it may, it still seems anomalous to allow string(rune) but deprecate string(byte).

After all, string([]rune] and string([]byte) are both allowed but not conversions from slices of other integer types. It may therefore seem strange to people that both corresponding 'scalar' conversions are not allowed as well.

Indeed, @ianlancetaylor made a similar point in #3939 itself.

smasher164 · 2019-12-30T05:19:24Z

The diagnostic for the new analyzer could replace (or offer to replace) the deprecated expressions by the preferred version.

What would be the suggested fix for an integer that overflows the size of the rune? Although this shouldn't occur in practice, simply changing it to rune(i) would break pre-existing code.

package main

import "fmt"

func main() {
	i := int64(1 << 33)
	fmt.Printf("%q %q\n", string(i), string(rune(i))) // prints "�" "\x00"
}

gopherbot · 2019-12-31T04:36:55Z

Change https://golang.org/cl/212919 mentions this issue: go/analysis: add pass to check string(int) conversions

ianlancetaylor · 2019-12-31T18:11:28Z

@smasher164 Do you have any examples of correct code that has this problem?

The correct fix is going to be to add a condition comparing the value to utf8.MaxRune, and when it is larger using utf8.RuneError.

smasher164 · 2019-12-31T23:03:05Z

@ianlancetaylor Running the tool over my GOPATH revealed 16 cases where an int, uint, int64, or uint64 were converted to a string, and did not do bounds checking to ensure that the converted value doesn't overflow MaxRune. However, most of these cases depend on these functions' call sites, which could potentially be passing only valid input.

github.com/antlr/antlr4/runtime/Go/antlr/dfa_serializer.go:115:15: conversion from int to string
func (l *LexerDFASerializer) getEdgeLabel(i int) string {
    return "'" + string(i) + "'"
}

github.com/antlr/antlr4/runtime/Go/antlr/interval_set.go:235:34: conversion from int to string
github.com/antlr/antlr4/runtime/Go/antlr/interval_set.go:238:30: conversion from int to string
github.com/antlr/antlr4/runtime/Go/antlr/interval_set.go:238:53: conversion from int to string
type Interval struct {
    start int
    stop  int
}
type IntervalSet struct {
    intervals []*Interval
    readOnly  bool
}
for j := 0; j < len(i.intervals); j++ {
    v := i.intervals[j]
    if v.stop == v.start+1 {
        if v.start == TokenEOF {
            names = append(names, "<EOF>")
        } else {
            names = append(names, ("'" + string(v.start) + "'"))
        }
    } else {
        names = append(names, "'"+string(v.start)+"'..'"+string(v.stop-1)+"'")
    }
}

github.com/antlr/antlr4/runtime/Go/antlr/lexer_atn_simulator.go:633:15: conversion from int to string
func (l *LexerATNSimulator) GetTokenName(tt int) string {
    if tt == -1 {
        return "EOF"
    }

    return "'" + string(tt) + "'"
}

github.com/antlr/antlr4/runtime/Go/antlr/transition.go:239:15: conversion from int to string
github.com/antlr/antlr4/runtime/Go/antlr/transition.go:239:42: conversion from int to string
type RangeTransition struct {
    *BaseTransition

    start, stop int
}
func (t *RangeTransition) String() string {
    return "'" + string(t.start) + "'..'" + string(t.stop) + "'"
}

github.com/cznic/ql/etc.go:464:10: conversion from int to string
switch i := int(o); i {
case andand:
    return "&&"
case andnot:
    return "&^"
case lsh:
    return "<<"
case le:
    return "<="
case eq:
    return "=="
case ge:
    return ">="
case neq:
    return "!="
case oror:
    return "||"
case rsh:
    return ">>"
default:
    return string(i)
}

github.com/cznic/ql/etc.go:901:11: conversion from idealInt to string
github.com/cznic/ql/etc.go:905:11: conversion from idealUint to string
github.com/cznic/ql/etc.go:918:11: conversion from int64 to string
github.com/cznic/ql/etc.go:928:11: conversion from uint64 to string
case qString:
switch x := val.(type) {
//case nil:
//case idealComplex:
//case idealFloat:
case idealInt:
    return string(x), nil
case idealRune:
    return string(x), nil
case idealUint:
    return string(x), nil
//case bool:
//case complex64:
//case complex128:
//case float32:
//case float64:
case int8:
    return string(x), nil
case int16:
    return string(x), nil
case int32:
    return string(x), nil
case int64:
    return string(x), nil
case string:
    return x, nil
case uint8:
    return string(x), nil
case uint16:
    return string(x), nil
case uint32:
    return string(x), nil
case uint64:
    return string(x), nil
case []byte:
    return string(x), nil
case *big.Int:
    return x.String(), nil
case time.Time:
    return x.String(), nil
case time.Duration:
    return x.String(), nil
default:
    return invConv(val, typ)
}

github.com/cznic/ql/etc.go:1698:15: conversion from uint64 to string
rec[i] = string(y)

github.com/cznic/ql/ql.go:820:17: conversion from int to string
for _, f := range f {
    a = append(a, string(f.typ)+f.name)
}

github.com/go-delve/delve/pkg/proc/eval.go:535:9: conversion from int64 to string
b, _ := constant.Int64Val(argv.Value)
s := string(b)

github.com/unidoc/unidoc/pdf/internal/cmap/cmap.go:396:36: conversion from uint64 to string
target := hexToUint64(v)
i := uint64(0)
for sc := srcCodeFrom; sc <= srcCodeTo; sc++ {
    r := target + i
    cmap.codeMap[numBytes-1][sc] = string(r)
    i++
}

ianlancetaylor · 2020-01-01T18:00:23Z

Thanks for the list. The antlr and ql cases all look like incorrect uses of string(int), where they really meant to use strconv.Itoa(int). If I'm right, the warning is serving its purpose for those cases. The unicode case probably really does mean to use string(r), so that code would have to change. I'm not sure about the go-delve case.

smasher164 · 2020-01-01T18:50:14Z

I'm not sure about the go-delve case.

The go-delve case is implementing eval for type conversion, so I would ignore that case.

FWIW, if the goal of backwards compatibility in the suggested fixes is to ensure that the fix results in the same string as string(int), then comparing against utf8.MaxRune isn't the only option:

func conv(v uint64) uint64 {
    if v > utf8.MaxRune {
        return utf8.RuneError
    }
    return v
}

When a rune is converted to a string, if bit 22 is set, then the string will print "�". Using this fact, the conversion can be made branchless:

func conv(v uint64) uint64 {
    return v | (1-((v>>22|v>>21&1)-1)>>63)<<21
}

This checks if any of the top 43 bits are nonzero, and uses that truth to set bit 22.

ianlancetaylor · 2020-01-01T19:40:21Z

The goal of the warning is to tell people when their code is not acting as they expect. As far as I can see the fix for cases like antlr is not to add a condition; it's to change the code to use strconv.Itoa instead of string(int), so that the right thing happens.

smasher164 · 2020-01-01T19:54:07Z

That's fair. But we'd have to detect and distinguish those cases from cases where the conversion is correctly made and stays within the bounds of a rune. For instance:

/Users/akhil/work/src/golang.org/x/net/webdav/internal/xml/xml.go:1043:14: conversion from uint64 to string
if err == nil && n <= unicode.MaxRune {
    text = string(n)
    haveText = true
}

/Users/akhil/work/src/golang.org/x/net/webdav/internal/xml/xml.go:1066:15: conversion from int to string
var entity = map[string]int{
    "lt":   '<',
    "gt":   '>',
    "amp":  '&',
    "apos": '\'',
    "quot": '"',
}
if r, ok := entity[s]; ok {
    text = string(r)
    haveText = true
} else if d.Entity != nil {
    text, haveText = d.Entity[s]
}

/Users/akhil/work/src/golang.org/x/text/unicode/cldr/base.go:101:10: conversion from int64 to string
if s[1] == '#' {
    r, _ := strconv.ParseInt(s[3:len(s)-1], 16, 32)
    return string(r)
}

/Users/akhil/work/src/golang.org/x/text/message/fmt_test.go:1454:9: conversion from int to string
for i := 0; i < 128; i++ {
    if f.Flag(i) {
        s += string(i)
    }
}

/Users/akhil/work/src/golang.org/x/exp/errors/fmt/scan.go:607:50: conversion from int64 to string
r := int64(s.getRune())
n := uint(bitSize)
x := (r << (64 - n)) >> (64 - n)
if x != r {
    s.errorString("overflow on character value " + string(r))
}

/Users/akhil/work/src/github.com/docker/docker/vendor/github.com/golang/protobuf/proto/text_parser.go:315:10: conversion from uint64 to string
if i > utf8.MaxRune {
    return "", "", fmt.Errorf(`\%c%s is not a valid Unicode code point`, r, ss)
}
return string(i), s, nil

/Users/akhil/work/src/github.com/docker/docker/pkg/chrootarchive/archive_test.go:103:32: conversion from int to string
for i := 0; i < 65534; i++ {
    excludes[i] = strings.Repeat(string(i), 64)
}

/Users/akhil/work/src/github.com/rogpeppe/godef/vendor/9fans.net/go/plan9/dir.go:173:9: conversion from int to string
/Users/akhil/work/src/github.com/rogpeppe/godef/vendor/9fans.net/go/plan9/dir.go:177:10: conversion from int to string
var permChars = []permChar{
    permChar{DMDIR, 'd'},
    permChar{DMAPPEND, 'a'},
    permChar{DMAUTH, 'A'},
    permChar{DMDEVICE, 'D'},
    permChar{DMSOCKET, 'S'},
    permChar{DMNAMEDPIPE, 'P'},
    permChar{0, '-'},
    permChar{DMEXCL, 'l'},
    permChar{DMSYMLINK, 'L'},
    permChar{0, '-'},
    permChar{0400, 'r'},
    permChar{0, '-'},
    permChar{0200, 'w'},
    permChar{0, '-'},
    permChar{0100, 'x'},
    permChar{0, '-'},
    permChar{0040, 'r'},
    permChar{0, '-'},
    permChar{0020, 'w'},
    permChar{0, '-'},
    permChar{0010, 'x'},
    permChar{0, '-'},
    permChar{0004, 'r'},
    permChar{0, '-'},
    permChar{0002, 'w'},
    permChar{0, '-'},
    permChar{0001, 'x'},
    permChar{0, '-'},
}
for _, pc := range permChars {
    if p&pc.bit != 0 {
        did = true
        s += string(pc.c)
    }
    if pc.bit == 0 {
        if !did {
            s += string(pc.c)
        }
        did = false
    }
}

/Users/akhil/work/src/github.com/miekg/dns/msg_test.go:70:13: conversion from int64 to string
n, _ := strconv.ParseInt(escape[1:], 10, 8)
return string(n)

/Users/akhil/work/src/github.com/google/skylark/library.go:406:16: conversion from int to string
i, err := AsInt32(args[0])
if err != nil {
    return nil, fmt.Errorf("chr: got %s, want int", args[0].Type())
}
if i < 0 {
    return nil, fmt.Errorf("chr: Unicode code point %d out of range (<0)", i)
}
if i > unicode.MaxRune {
    return nil, fmt.Errorf("chr: Unicode code point U+%X out of range (>0x10FFFF)", i)
}
return String(string(i)), nil

/Users/akhil/work/src/github.com/cznic/ql/all_test.go:2949:9: conversion from int to string
for _, v := range rand.Perm(32) {
    s += string('0' + v)
}

/Users/akhil/work/src/upspin.io/client/clientutil/all_test.go:129:40: conversion from int to string
for i := 0; i < 10; i++ {
    loc := upspin.Location{
        Endpoint:  inProcess,
        Reference: upspin.Reference("ref" + string(i)),
    }
    tLocs = append(tLocs, loc)
}

…1.15: golang/go#32479

See golang/go#32479 Fix cosmos72#101. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

Symptoms: pkg/gadgets/networkpolicy/networkpolicy.go:143:21: conversion from uint16 to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?) See: golang/go#32479 Signed-off-by: Alban Crequy <alban@kinvolk.io>

golang/go#32479 removed the explicit type conversion string(i) where i has an integer type other than rune in go 1.15. That is a **backward incompatible** change. Fix talent-plan#70.

Symptoms: pkg/gadgets/networkpolicy/networkpolicy.go:143:21: conversion from uint16 to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?) See: golang/go#32479 Signed-off-by: Alban Crequy <alban@kinvolk.io>

See golang/go#32479 Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

See golang/go#32479 Fix duosecurity#21. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

testtools/genblocks/main.go:91:74: conversion from int to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?) golang/go#32479

See golang/go#32479 Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

#403) Summary: refer to [golang issues](golang/go#32479) Pull Request resolved: #403 Reviewed By: nemith Differential Revision: D29911116 Pulled By: vitaut fbshipit-source-id: 5828d101edb4647834c684453303e27302f1c38a

Symptoms: pkg/gadgets/networkpolicy/networkpolicy.go:143:21: conversion from uint16 to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?) See: golang/go#32479 Signed-off-by: Alban Crequy <alban@kinvolk.io>

Convert int to string using rune() See golang/go#32479 Fix #384. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 7, 2019

ianlancetaylor added this to the Go1.14 milestone Jun 7, 2019

ianlancetaylor mentioned this issue Jun 24, 2019

proposal: new vet check complaining about string(int) and string(rune) #32758

Closed

rsc modified the milestones: Go1.14, Backlog Oct 9, 2019

ianlancetaylor modified the milestones: Backlog, Go1.15 Nov 5, 2019

bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 5, 2019

gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 5, 2019

alanfo mentioned this issue Dec 31, 2019

proposal: spec: remove string(int) #3939

Open

aro5000 added a commit to aro5000/sharkie that referenced this issue Dec 31, 2020

adding rune() to the string map for emojis per new go vet changes in …

09a1b42

…1.15: golang/go#32479

eclipseo added a commit to eclipseo/gomacro that referenced this issue Jan 9, 2021

Convert to string using rune()

df1a2ac

See golang/go#32479 Fix cosmos72#101. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

alban mentioned this issue Jan 19, 2021

Fix string conversion error inspektor-gadget/inspektor-gadget#157

Merged

An-DJ mentioned this issue Jan 25, 2021

fix(parser): fix go test error on parser (#70) talent-plan/tinysql#71

Closed

An-DJ mentioned this issue Jan 26, 2021

fix(parser): fix go test error on parser (#70) talent-plan/tinysql#72

Closed

sykesm added a commit to sykesm/fabric that referenced this issue Feb 10, 2021

Change string cast of int value to rune cast

0adb544

See golang/go#32479 Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

batterystaples mentioned this issue Feb 11, 2021

Fix conversions to work with Go 1.15 flashmob/go-guerrilla#223

Open

jyellick pushed a commit to hyperledger/fabric that referenced this issue Feb 15, 2021

Change string cast of int value to rune cast

6cb530b

See golang/go#32479 Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

kaperys mentioned this issue Feb 26, 2021

async: message ordering guarantee doc uw-labs/substrate-tools#9

Merged

bensallen pushed a commit to bensallen/duo_api_golang that referenced this issue Mar 3, 2021

Convert id to string using strconv.Itoa (duosecurity#22)

a0509a4

See golang/go#32479 Fix duosecurity#21. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

chipaca mentioned this issue Mar 22, 2021

With Go 1.15, tests fail beme/abide#55

Open

LarryRuane mentioned this issue Apr 6, 2021

eliminate genblocks compile warning zcash/lightwalletd#341

Merged

renovate bot mentioned this issue May 23, 2021

fix(deps): update module github.com/jedib0t/go-pretty to v6 - autoclosed aevea/commitsar#359

Closed

1 task

dongbiao830415 pushed a commit to dongbiao830415/golex that referenced this issue Jun 6, 2021

Convert int to string using rune()

49cf065

See golang/go#32479 Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

cpuguy83 pushed a commit to cpuguy83/go-ansiterm that referenced this issue Jun 17, 2021

Convert int to string using rune()

f1956e5

See golang/go#32479 Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

cpuguy83 pushed a commit to cpuguy83/go-ansiterm that referenced this issue Jun 17, 2021

Convert int to string using rune()

fcb45a0

See golang/go#32479 Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

cpuguy83 pushed a commit to cpuguy83/go-ansiterm that referenced this issue Jun 17, 2021

Convert int to string using rune()

65725d2

See golang/go#32479 Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

renovate bot mentioned this issue Jul 14, 2021

chore(deps): Update module github.com/jedib0t/go-pretty to v6 newrelic/newrelic-cli#978

Merged

1 task

stephen-fox mentioned this issue Aug 23, 2021

Fix attrcategory type chrismarget/cisco-l2t#23

Merged

smasher164 mentioned this issue Sep 20, 2021

x/tools/analysis/passes/tests: report examples with misplaced Output comments #48362

Closed

stevenh pushed a commit to robertkrimen/otto that referenced this issue Sep 27, 2021

fix: go vet 1.15 warnings (#385)

dee429e

Convert int to string using rune() See golang/go#32479 Fix #384. Signed-off-by: Robert-André Mauchin <zebob.m@gmail.com>

golang locked and limited conversation to collaborators Oct 21, 2021

gopherbot added the FrozenDueToAge label Oct 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/vet: warn about string(int) type conversions #32479

cmd/vet: warn about string(int) type conversions #32479

ianlancetaylor commented Jun 7, 2019

agnivade commented Jun 8, 2019

networkimprov commented Jun 8, 2019

mvdan commented Jun 24, 2019

alanfo commented Jun 27, 2019

dolmen commented Jul 2, 2019 •

edited

networkimprov commented Jul 2, 2019

alanfo commented Jul 2, 2019

smasher164 commented Nov 6, 2019

alandonovan commented Nov 6, 2019 •

edited

networkimprov commented Nov 6, 2019

alandonovan commented Nov 6, 2019

alanfo commented Nov 7, 2019

smasher164 commented Dec 30, 2019

gopherbot commented Dec 31, 2019

ianlancetaylor commented Dec 31, 2019

smasher164 commented Dec 31, 2019

ianlancetaylor commented Jan 1, 2020

smasher164 commented Jan 1, 2020

ianlancetaylor commented Jan 1, 2020

smasher164 commented Jan 1, 2020

cmd/vet: warn about string(int) type conversions #32479

cmd/vet: warn about string(int) type conversions #32479

Comments

ianlancetaylor commented Jun 7, 2019

agnivade commented Jun 8, 2019

networkimprov commented Jun 8, 2019

mvdan commented Jun 24, 2019

alanfo commented Jun 27, 2019

dolmen commented Jul 2, 2019 • edited

networkimprov commented Jul 2, 2019

alanfo commented Jul 2, 2019

smasher164 commented Nov 6, 2019

alandonovan commented Nov 6, 2019 • edited

networkimprov commented Nov 6, 2019

alandonovan commented Nov 6, 2019

alanfo commented Nov 7, 2019

smasher164 commented Dec 30, 2019

gopherbot commented Dec 31, 2019

ianlancetaylor commented Dec 31, 2019

smasher164 commented Dec 31, 2019

ianlancetaylor commented Jan 1, 2020

smasher164 commented Jan 1, 2020

ianlancetaylor commented Jan 1, 2020

smasher164 commented Jan 1, 2020

dolmen commented Jul 2, 2019 •

edited

alandonovan commented Nov 6, 2019 •

edited