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: flag likely incorrect %T usages on reflect.Type #41590

Open
dsnet opened this issue Sep 23, 2020 · 3 comments
Open

cmd/vet: flag likely incorrect %T usages on reflect.Type #41590

dsnet opened this issue Sep 23, 2020 · 3 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Sep 23, 2020

In code that uses Go reflection, it is common that the user prints the type. However, it easy for people (out of habit) to use %T with a reflect.Type when they should actually be using %v instead.

For example:

t := reflect.TypeOf(...)
if checkValid(t) {
    panic(fmt.Sprintf("invalid type: %T", t)) // incorrect: should be using %v
}

It is unlikely that the wrong verb is detected during code review.

In the instances similar to this, I don't think I saw a single case where a person actually wanted to print the type of reflect.Type itself (which by the way is *reflect.rtype and not particularly interesting for users).

\cc @dominikh in case this is a check better suited for staticcheck than vet.

@mvdan
Copy link
Member

mvdan commented Sep 23, 2020

I think this would be fine for precision and correctness. I'm less sure about frequency; how often does this bug come up in practice? I think I've seen this bug maybe once or twice before, which makes me think it would be significantly less frequent than a lot of the other vet checks, but perhaps my experience is biased.

@dmitshur dmitshur added FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 23, 2020
@dmitshur dmitshur added this to the Backlog milestone Sep 23, 2020
@timothy-king timothy-king added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Jan 26, 2021
@dolmen
Copy link
Contributor

dolmen commented Jul 9, 2021

@dsnet There is a typo in the example code: missing closing double quote.

@timothy-king
Copy link
Contributor

timothy-king commented Jul 19, 2021

@dolmen Thanks for pointing out the typo. I've updated the example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants