-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add linter wastedassign #1651
Add linter wastedassign #1651
Conversation
Hey, thank you for opening your first Pull Request ! |
Hello @sanposhiho, could you explain why you created a fork of I see your change: mode := ssa.BuilderMode(ssa.NaiveForm) // Some Analyzers may need GlobalDebug, in which case we'll have
// to set it globally, but let's wait till we need it.
mode := ssa.BuilderMode(0) It seems, when I read the comment, that a PR on that point on the repo https://github.com/golang/tools will be welcome. I pay attention to this point because it can impact golangci-lint's performance because this lib is at the heart of golangci-lint's performance optimizations. |
Hi @ldez!
I use the mode
OK, I'll create PR on https://github.com/golang/tools |
69f18ee
to
7aba93f
Compare
7aba93f
to
b406ebd
Compare
Hi team. I removed the dependency of wastedassign on sanposhiho/tools, because it seemed to take a while for the proposal to go/tools to be supported.
So, This concern has been addressed now. |
it would be great if you can use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you replace the panic by errors?
https://github.com/sanposhiho/wastedassign/blob/master/wastedassign.go#L75
https://github.com/sanposhiho/wastedassign/blob/master/wastedassign.go#L80
change an order Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey, @sanposhiho — we just merged your PR to
By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests. Thanks again! |
closes #1492
Hi :D
Add wastedassign
wastedassign
finds wasted assignment statementsfound the value ...