Reject method/function object used as a condition

Mypy could reject this code, which is almost always bad and a reasonably common sort of bug:

def name() -> str: ...

if name:  # Probably meant name()?
    do_something()

(A related issue is if name == 'foo' but there is a separate issue for that already.)

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:

class A:
    def name(self) -> Optional[str]: ...

def f(a: A) -> None:
    if a.name:  # Probably meant a.name()?
        do_something(a.name())

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

@JukkaL @gvanrossum It seems to be that this is related to 9a8a8c0 ... The boolean value of a.name in @JukkaL's example above is true_only

Where this will be super useful is with the new async/await support. In Hack-land, we'd always see bugs of the form:

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. while True: and if 1: should likely be okay, as it's quite clear that this is what the programmer intended.

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 if user.is_admin: instead of if user.is_admin(): and was pretty disappointed none of the linters was able to catch this. From where I'm standing this looks like a pretty important feature.

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 object's __bool__ method, which means it will always appear as True. I don't know if that would cause a lot of false positives though.

You mean if the type does not have any possible instance that overrides object's __bool__(). There aren't many final types in Python.

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 Callable[...] type might have a custom __bool__ (they could be instances or user-defined classes), so perhaps we should always allow them to be used in conditions.

It seems like this may be resolved now, via the truthy-function error code: https://mypy-play.net/?mypy=latest&python=3.12&gist=067efa3ee1e7617950d94c90cc60116e

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-await issue above seems better covered by the (much newer, but more specific) #16069

Maybe this one can be closed?

Navigation Menu

Repository navigation

Reject method/function object used as a condition #2073

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Reject method/function object used as a condition #2073

Description

Activity

gvanrossum commented on Sep 1, 2016

JukkaL commented on Sep 2, 2016

rowillia commented on Sep 2, 2016

ddfisher commented on Sep 2, 2016

JukkaL commented on Sep 5, 2016

elazarg commented on Sep 11, 2016

rvolgers commented on Aug 17, 2018

ilevkivskyi commented on Aug 17, 2018

JelleZijlstra commented on Aug 17, 2018

elazarg commented on Aug 17, 2018

ilevkivskyi commented on Feb 10, 2019

JukkaL commented on Feb 12, 2019

huonw commented on Oct 7, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions