Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/vet: json.Number is easy to misuse (vet check?) #24001

Closed
bsiegert opened this issue Feb 21, 2018 · 2 comments

Comments

Projects
None yet
4 participants
@bsiegert
Copy link
Contributor

commented Feb 21, 2018

What version of Go are you using (go version)?

1.10

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

linux-amd64, but any architecture really

What did you do?

I reviewed some code that contained an erroneous instantiation of a json.Number. See https://play.golang.org/p/w6cy7I1A--Q.

What did you expect to see?

The author of that code was trying to store the value 1 in a json.Number by using json.Number(1).

What did you see instead?

The type is defined as

type Number string

Thus, the following happens: the 1 is interpreted as a rune, and the string is initialized from the rune. Thus, instead of storing a 1, it stores an ASCII 0x1 character. The correct way of initializing this constant would be json.Number("1").

This seems like a good fit for a vet check, as it probably is never what was intended: catch casting a number (constant? int? rune?) into a json.Number and flag it.

I looked through our internal repo and found no uses of Number instances being initialized from runes or numbers.

Would such a vet check be accepted?

@bradfitz bradfitz changed the title json.Number is easy to misuse (vet check?) cmd/vet: json.Number is easy to misuse (vet check?) Feb 21, 2018

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 21, 2018

New vet checks are usually denied for a variety of reasons.

It might be accepted in @dominikh's https://github.com/dominikh/go-tools/tree/master/cmd/staticcheck though.

@bradfitz bradfitz added this to the Go1.11 milestone Feb 21, 2018

@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2018

Really a dup of #3939.

@rsc rsc closed this Mar 26, 2018

@dominikh dominikh referenced this issue Sep 18, 2018

Open

staticcheck: incorporate go vet checks #149

5 of 22 tasks complete

@golang golang locked and limited conversation to collaborators Mar 26, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.