Skip to content
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

Regression for truthy bool with getattr #14480

Closed
ilinum opened this issue Jan 20, 2023 · 2 comments · Fixed by #14510
Closed

Regression for truthy bool with getattr #14480

ilinum opened this issue Jan 20, 2023 · 2 comments · Fixed by #14510
Labels
bug mypy got something wrong

Comments

@ilinum
Copy link
Collaborator

ilinum commented Jan 20, 2023

The following code generates an error on latest master but no on latest release (with --enable-error-codes=truthy-bool):

def f(obj: object) -> None:
    print("{}".format(
        getattr(type(obj), "__foo__", "") or "unknown"  # E: "builtins.getattr" returns "object" which does not implement __bool__ or __len__ so it could always be true in boolean context  [truthy-bool]
    ))

f("")

It's clearly a false positive, if your run the code you see that it prints unknown.

@ilinum ilinum added the bug mypy got something wrong label Jan 20, 2023
@ilinum ilinum changed the title Regression for truthy bool with getattr. Regression for truthy bool with getattr Jan 20, 2023
@ilinum
Copy link
Collaborator Author

ilinum commented Jan 20, 2023

Using @hauntsaninja's mypy_primer, it pointed to #14178.

$ cat test.py
# flags: --enable-error-code truthy-bool
def f(obj: object) -> None:
    print("{}".format(
        getattr(type(obj), "__foo__", "") or "unknown"  # E: "builtins.getattr" returns "object" which does not implement __bool__ or __len__ so it could always be true in boolean context  [truthy-bool]
    ))
$ python3 mypy_primer.py -p test.py --bisect --debug --old v0.991
...
4471c7e76f27ee51eb8d47a4803097ec15c62128 is the first bad commit
...

This is likely the same underlying issue as #14481.

@ilevkivskyi

@ilinum ilinum mentioned this issue Jan 21, 2023
17 tasks
@JukkaL
Copy link
Collaborator

JukkaL commented Jan 23, 2023

Here's a bit simpler example that reproduces the issue:

def f(t: type[object]) -> None:
    o: object = getattr(t, "x", "") or ""

This looks different from #14481, as there are no TypedDicts involved here.

JukkaL added a commit that referenced this issue Jan 23, 2023
There are two reasons I'm proposing this change. First, we know that
many subclasses of 'object' can be falsy. Second, mypy sometimes
simplifies `object | Any` into just `object`. The latter was
considered always truthy, while the prior one wasn't. Now both of them
are treated consistently. An alternative fix would be to not simplify
unions like `object | Any`, but this seems a bit ad hoc.

Fixes #14480. This doesn't just fix the regression but fixes a more
general issue.
JukkaL added a commit that referenced this issue Jan 23, 2023
There are two reasons I'm proposing this change. First, we know that
many subclasses of 'object' can be falsy. Second, mypy sometimes
simplifies `object | Any` into just `object`. The latter was considered
always truthy, while the prior one wasn't. Now both of them are treated
consistently. An alternative fix would be to not simplify unions like
`object | Any`, but this seems a bit ad hoc.

This only has an effect when the `truthy-bool` error code is explicitly
enabled.

Fixes #14480. This doesn't just fix the regression but fixes a more
general issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants