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

PEP 695 type annotations with lambdas produce false F401 #9159

Closed
rijenkii opened this issue Dec 16, 2023 · 5 comments · Fixed by #9175
Closed

PEP 695 type annotations with lambdas produce false F401 #9159

rijenkii opened this issue Dec 16, 2023 · 5 comments · Fixed by #9175
Assignees
Labels
bug Something isn't working

Comments

@rijenkii
Copy link

Minimal reproduction:

from typing import Annotated
import re

type X = Annotated[int, lambda: re.compile("x")]

Result:

$ ruff repro.py --isolated
repro.py:2:8: F401 [*] `re` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

Settings:

[tool.ruff.lint]
select = ["F", "E"]

Version 0.1.8

@charliermarsh charliermarsh added the bug Something isn't working label Dec 16, 2023
@charliermarsh
Copy link
Member

charliermarsh commented Dec 16, 2023

Thanks! Agree that we should probably fix this, although I admittedly don't understand how Annotated is supposed to work here. For example:

from typing import Annotated

type X = Annotated[int, lambda: re.compile("x")]
print(X.__metadata__)

Yields:

Traceback (most recent call last):
  File "/Users/crmarsh/workspace/foo.py", line 10, in <module>
    print(X.__metadata__)
          ^^^^^^^^^^^^^^
AttributeError: 'typing.TypeAliasType' object has no attribute '__metadata__'. Did you mean: '__getstate__'?

@charliermarsh
Copy link
Member

On the other hand, Ruff correctly does not flag F401 here:

from typing import Annotated

X = Annotated[int, lambda: re.compile("x")]
print(X.__metadata__)

So I suspect our treatment right be correct.

@charliermarsh charliermarsh added question Asking for support or clarification and removed bug Something isn't working labels Dec 16, 2023
@rijenkii
Copy link
Author

rijenkii commented Dec 16, 2023

Thanks! Agree that we should probably fix this, although I admittedly don't understand how Annotated is supposed to work here

Here:

>>> from typing import Annotated
>>> import re
>>> type X = Annotated[int, lambda: re.compile("x")]
>>> print(X.__value__.__metadata__)
(<function X.<locals>.<lambda> at 0x7f4df0747f60>,)

@charliermarsh charliermarsh added bug Something isn't working and removed question Asking for support or clarification labels Dec 16, 2023
@charliermarsh
Copy link
Member

Huh, ok. I will admit that I find Annotated very strange but I agree that this is a false positive. Thanks.

@rijenkii
Copy link
Author

rijenkii commented Dec 16, 2023

This example also produces an F401 and has no Annotation. I have not used it in the OP because according to pyright it is just wrong, but it works at runtime:

import re
type X = lambda: re.compile("x")
$ ruff repro.py --isolated
repro.py:1:8: F401 [*] `re` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.
>>> import re
>>> type X = lambda: re.compile("x")
>>> X.__value__
<function X.<locals>.<lambda> at 0x7f6d4cb30040>
>>> X.__value__()
re.compile('x')

@charliermarsh charliermarsh self-assigned this Dec 18, 2023
charliermarsh added a commit that referenced this issue Jan 16, 2024
## Summary

This PR is a more holistic fix for
#9534 and
#9159.

When we visit the AST, we track nodes that we need to visit _later_
(deferred nodes). For example, when visiting a function, we defer the
function body, since we don't want to visit the body until we've visited
the rest of the statements in the containing scope.

However, deferred nodes can themselves contain deferred nodes... For
example, a function body can contain a lambda (which contains a deferred
body). And then there are rarer cases, like a lambda inside of a type
annotation.

The aforementioned issues were fixed by reordering the deferral visits
to catch common cases. But even with those fixes, we still fail on cases
like:

```python
from __future__ import annotations

import re
from typing import cast

cast(lambda: re.match, 1)
```

Since we don't expect lambdas to appear inside of type definitions.

This PR modifies the `Checker` to keep visiting until all the deferred
stacks are empty. We _already_ do this for any one kind of deferred
node; now, we do it for _all_ of them at a level above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants