Skip to content

vet: fmt.Errorf("ordinary string") should be replaced by errors.New("ordinary string") #17173

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

Closed
kevinburke opened this issue Sep 21, 2016 · 7 comments

Comments

@kevinburke
Copy link
Contributor

kevinburke commented Sep 21, 2016

Please answer these questions before submitting your issue. Thanks!

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

1.7.1

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

All

What did you do?

Vet this program: https://play.golang.org/p/krp8cxL_Lv

package main

import "fmt"

func main() {
    err := fmt.Errorf("ordinary error")
    fmt.Printf("%#v\n", err)
}

What did you expect to see?

fmt.Errorf("an ordinary string") - with no formatting arguments - should be replaced by errors.New("an ordinary string"), to avoid the overhead of calling errors.New(Sprintf(...)).

What did you see instead?

No warning.

I'm happy to try to provide a patch here, if this is of interest.

@minux
Copy link
Member

minux commented Sep 21, 2016 via email

@dsnet
Copy link
Member

dsnet commented Sep 21, 2016

vet is intended to check issues of correctness. The difference between using fmt.Errorf or errors.New here is clearly one of style since they are semantically equivalent. This should be an issue to file at golang/lint, which is intended to check issues of style. I just checked and it doesn't seem like golint complains about this case.

I'm going to close this, but feel free to open the issue on the lint tracker.

@dsnet dsnet closed this as completed Sep 21, 2016
@minux
Copy link
Member

minux commented Sep 21, 2016 via email

@jimmyfrasche
Copy link
Member

Perhaps you could try https://github.com/dominikh/go-simple

@dominikh
Copy link
Member

It's probably not appropriate for gosimple, either, for the reason minux has given. One could of course check if fmt is being used for anything else already, but I don't think the slight benefit is worth the added complexity.

@jimmyfrasche
Copy link
Member

Well thought I'd mention it as it was the most likely place it would find a home that I know of.

Personally I'd consider it a simplification even if it meant importing errors and fmt. A Printf/Errorf/Anythingf with just a string looks wrong.

Understandable if it's out of scope, though.

@kevinburke
Copy link
Contributor Author

OK

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants