-
-
Notifications
You must be signed in to change notification settings - Fork 378
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
staticcheck: flag defer foo()
where foo returns a closure
#466
Comments
In what situation would you create a closure and not call it. Note that with no assignment to the closure you've lost access to the return value, so it would need to have modified state somewhere. In that case, the code is horrible and should be flagged anyway :) |
Maybe a function does some work but also returns a closure that is optional to call to do some other work. I'm not saying that it's great code or that it should exist, but given enough programmers with typewriters, all code exists :) |
A function changing a global state and returning a "cancel" has a right to exist. I would still like to see a warning. If the static analyses can deduct that the function does not modify global variables I would probably mark the code as an error. |
https://godoc.org/github.com/fortytw2/leaktest#Check is a good example of a very easy to make error that this check would catch. This form of code is generally useful when you want to do something at the start and end of a function without worrying about it... like |
Add a new checker to flag cases where a deferred function returns exactly one function, and that function isn't called. For example: func f() func() { // Do stuff. return func() { // Do stuff } } func main() { defer f() // Error: should be f()() } As mentioned in dominikh#466, there's a change for false positives. I ran the check on Go stdlib, all my own code, and about 200 popular Go packages, and haven't found any false positives (and one match: grpc/grpc-go#7270) Fixes dominikh#466
Add a new checker to flag cases where a deferred function returns exactly one function, and that function isn't called. For example: func f() func() { // Do stuff. return func() { // Do stuff } } func main() { defer f() // Error: should be f()() } As mentioned in dominikh#466, there's a change for false positives. I ran the check on Go stdlib, all my own code, and about 200 popular Go packages, and haven't found any false positives (and one match: grpc/grpc-go#7270) Fixes dominikh#466
Add a new checker to flag cases where a deferred function returns exactly one function, and that function isn't called. For example: func f() func() { // Do stuff. return func() { // Do stuff } } func main() { defer f() // Error: should be f()() } As mentioned in dominikh#466, there's a change for false positives. I ran the check on Go stdlib, all my own code, and about 200 popular Go packages, and haven't found any false positives (and one match: grpc/grpc-go#7270) Fixes dominikh#466
Inspired by golang/lint#451
Obviously concerns about false positives exist. This check will need a thorough test run on the Go corpus.
The text was updated successfully, but these errors were encountered: