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

F401 false positive with PEP-695 upper bound specification #8017

Closed
bryanforbes opened this issue Oct 17, 2023 · 2 comments · Fixed by #8033
Closed

F401 false positive with PEP-695 upper bound specification #8017

bryanforbes opened this issue Oct 17, 2023 · 2 comments · Fixed by #8033
Assignees
Labels
bug Something isn't working

Comments

@bryanforbes
Copy link

bryanforbes commented Oct 17, 2023

Running the following command with Ruff 0.1.0:

ruff --target-version py312 --select F401 --isolated --no-cache blah.py

Given the following code:

from __future__ import annotations

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from collections.abc import Callable

    from .foo import Record

type RecordCallback[R: Record] = Callable[[R], None]


def process_record[R: Record](record: R) -> None:
    ...


class RecordContainer[R: Record]:
    def add_record(self, record: R) -> None:
        ...

F401 is reported on line 8, but Record is being used in the upper bound specifications.

@charliermarsh charliermarsh self-assigned this Oct 17, 2023
@charliermarsh charliermarsh added the bug Something isn't working label Oct 17, 2023
@charliermarsh
Copy link
Member

Strange, will take a look.

@charliermarsh
Copy link
Member

This is a regression in the most recent release due to #7968.

charliermarsh added a commit that referenced this issue Oct 18, 2023
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

In #7968, I introduced a
regression whereby we started to treat imports used _only_ in type
annotation bounds (with `__future__` annotations) as unused.

The root of the issue is that I started using `visit_annotation` for
these bounds. So we'd queue up the bound in the list of deferred type
parameters, then when visiting, we'd further queue it up in the list of
deferred type annotations... Which we'd then never visit, since deferred
type annotations are visited _before_ deferred type parameters.

Anyway, the better solution here is to use a dedicated flag for these,
since they have slightly different behavior than type annotations.

I've also fixed what I _think_ is a bug whereby we previously failed to
resolve `Callable` in:

```python
type RecordCallback[R: Record] = Callable[[R], None]

from collections.abc import Callable
```

IIUC, the values in type aliases should be evaluated lazily, like type
parameters.

Closes #8017.

## Test Plan

`cargo test`
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