-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Reject method/function object used as a condition #2073
Comments
Is that really common? It sounds like more of a linter feature. |
The example was overly simplistic. This is more realistic, and here we need type information:
I've been bitten by this and I've heard similar bugs mentioned by other people. It's probably not a very big thing, but it should be pretty easy to catch these. |
@JukkaL @gvanrossum It seems to be that this is related to 9a8a8c0 ... The boolean value of Where this will be super useful is with the new async def is_foo_active() -> bool:
return await some_database_call_about_foo()
async def do_a_thing() -> None:
if is_foo_active(): # BUG! Engineer forgot to await so this is always truthy!!!!!
do_foo()
else:
do_bar() # Dead code Hack now throws an error if you branch on something always truth-y to solve this problem. |
More generally, we could warn about dead code? |
This doesn't necessarily result in dead code, but sometimes it does. An always-true condition in an if/while statement could be flagged as a potential error, but not always. I think that a reasonable goal would be to reject code that looks to the reader like it might not be truthy, when we actually know that it will always be truthy. |
I think that such errors are very common, especially when turning a method into a property, and it would be most useful. |
I recently typed Btw before anyone says this should have been a property, there are other similar methods that take parameters so it's trying to be consistent. |
OK, I am raising priority to normal. |
A possible more general approach would be to give an error if an object is used in a boolean context, but doesn't override |
You mean if the type does not have any possible instance that overrides object's I think handling (unions of) functions, specifically, is the way to go. |
An important corner case to consider is: def run(cb: Optional[Callable[[], None]]) -> None:
...
if cb:
cb() I think we should always allow such code. |
I think that it would be okay to flag an error if a condition is a direct reference to a module-level function or method. Anything else we may want to allow. In particular, variables with an explicit |
It seems like this may be resolved now, via the from typing import Optional, Callable
def required(cb: Callable[[], None]) -> None:
if cb: # error: Function "cb" could always be true in boolean context [truthy-function]
pass
def optional(cb: Optional[Callable[[], None]]) -> None:
if cb: # no error
pass The missing- Maybe this one can be closed? |
Mypy could reject this code, which is almost always bad and a reasonably common sort of bug:
(A related issue is
if name == 'foo'
but there is a separate issue for that already.)The text was updated successfully, but these errors were encountered: