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

False positive F401 "imported but unused" when use is cast() inside a lambda #9534

Closed
jriddy opened this issue Jan 15, 2024 · 2 comments · Fixed by #9540
Closed

False positive F401 "imported but unused" when use is cast() inside a lambda #9534

jriddy opened this issue Jan 15, 2024 · 2 comments · Fixed by #9540
Assignees
Labels
bug Something isn't working

Comments

@jriddy
Copy link

jriddy commented Jan 15, 2024

I found a fun edgy edge case running this one my code base. Ruff cannot detect use of classes in calls to cast() inside a lambda with from __future__ import annotations on.

Minimal reproducer

from __future__ import annotations

from collections import Counter
from typing import cast


def get_most_common_fn(k):
    return lambda c: cast(Counter, c).most_common(k)

Running ruff results in an error:

❯ ruff check --isolated repro.py
repro.py:3:25: F401 [*] `collections.Counter` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

Expected behavior

I expected Ruff to detect the use of the imported class in the call to cast(), but it looks like the combination of from __future__ import annotations and having the cast call in a lambda expression obscures that use from Ruff. Note that with the annotations future feature off this does not occur, nor does using a cast in non-lambda code seem to cause any issue.

Info & versions

Ruff: 0.1.13
Python: 3.11.5
OS: Fedora 38

@charliermarsh charliermarsh added the bug Something isn't working label Jan 15, 2024
@charliermarsh
Copy link
Member

Thank you, very much a bug!

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

This is effectively the same problem as
#9175. And this just papers over
it again, though I'm gonna try a more holistic fix in a follow-up PR.
The _real_ fix here is that we need to continue to visit deferred items
until they're exhausted since, e.g., we still get this case wrong
(flagging `re` as unused):

```python
import re

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

Closes #9534.
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.
@jriddy
Copy link
Author

jriddy commented Jan 16, 2024

Thanks for the quick turnaround on this! It gives me a lot of confidence in the project

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