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.

Sign up for GitHub

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

Jump to bottom

FA100 and FA102 do not trigger with function annotations #5574

Closed
aazuspan opened this issue Jul 7, 2023 · 3 comments · Fixed by #5575
Closed

FA100 and FA102 do not trigger with function annotations #5574

aazuspan opened this issue Jul 7, 2023 · 3 comments · Fixed by #5575
Assignees
@charliermarsh
Labels
bug Something isn't working

Comments

@aazuspan
Copy link

aazuspan commented Jul 7, 2023 •
edited
Loading

I'm migrating a project to ruff (huge fan so far, thanks maintainers!), and getting inconsistent behavior from the flake8-future-annotations rules, FA100 and FA102. They trigger as expected for variable type annotations but do not trigger for function parameter or return type annotations.

Reproducing

pyproject.toml config.

[tool.ruff]
target-version = "py38"
select = ["FA"]

main.py using the example code from the ruff docs.

from typing import List


# Doesn't trigger FA100
def foo(a: List[str]):
    ...

# Doesn't trigger FA102
def bar(a: int | None):
    ...

And the commands I'm running:

$ ruff --version
0.0.277

$ ruff check ./main.py -v
[2023-07-06][17:10:49][ruff_cli::commands::run][DEBUG] Identified files to lint in: 2.948576ms
[2023-07-06][17:10:49][ruff_cli::diagnostics][DEBUG] Checking: /home/az/ruff_test/main.py
[2023-07-06][17:10:49][ruff_cli::commands::run][DEBUG] Checked 1 files in: 299.315µs

No rules triggered.

Exceptions

Both rules will trigger with variable type annotations. For example:

main.py

from typing import List


# Does trigger FA100
def foo():
    a: List[str]

# Does trigger FA102
def bar():
    a: int | None
$ ruff check ./main.py -v
[2023-07-06][17:10:02][ruff_cli::commands::run][DEBUG] Identified files to lint in: 1.427045ms
[2023-07-06][17:10:02][ruff_cli::diagnostics][DEBUG] Checking: /home/az/ruff_test/main.py
[2023-07-06][17:10:02][ruff_cli::commands::run][DEBUG] Checked 1 files in: 416.495µs
main.py:19:8: FA100 Missing `from __future__ import annotations`, but uses `typing.List`
main.py:23:8: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
Found 2 errors.

This seems to be inconsistent with flake8-future-annotations, the example code in the ruff docs, and I believe the intended behavior. Let me know if you need any more details, thanks!
@aazuspan aazuspan mentioned this issue Jul 7, 2023
Merged
@charliermarsh
Copy link
Member

Thanks for the clear issue. I need to look back to refresh myself on this.

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

Yeah this is a bug. Thanks! I'll make sure it's fixed for the next release.

@aazuspan
Copy link
Author

aazuspan commented Jul 7, 2023

Awesome, thanks for such a quick response!

@charliermarsh charliermarsh mentioned this issue Jul 7, 2023
Merged
charliermarsh added a commit that referenced this issue Jul 7, 2023
@charliermarsh
Differentiate between runtime and typing-time annotations (#5575)
bf4b96c 
## Summary

In Python, the annotations on `x` and `y` here have very different
treatment:

```python
def foo(x: int):
  y: int
```

The `int` in `x: int` is a runtime-required annotation, because `x` gets
added to the function's `__annotations__`. You'll notice, for example,
that this fails:

```python
from typing import TYPE_CHECKING

if TYPE_CHECKING:
  from foo import Bar

def f(x: Bar):
  ...
```

Because `Bar` is required to be available at runtime, not just at typing
time. Meanwhile, this succeeds:

```python
from typing import TYPE_CHECKING

if TYPE_CHECKING:
  from foo import Bar

def f():
  x: Bar = 1

f()
```

(Both cases are fine if you use `from __future__ import annotations`.)

Historically, we've tracked those annotations that are _not_
runtime-required via the semantic model's `ANNOTATION` flag. But
annotations that _are_ runtime-required have been treated as "type
definitions" that aren't annotations.

This causes problems for the flake8-future-annotations rules, which try
to detect whether adding `from __future__ import annotations` would
_allow_ you to rewrite a type annotation. We need to know whether we're
in _any_ type annotation, runtime-required or not, since adding `from
__future__ import annotations` will convert any runtime-required
annotation to a typing-only annotation.

This PR adds separate state to track these runtime-required annotations.
The changes in the test fixtures are correct -- these were false
negatives before.

Closes #5574.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Differentiate between runtime and typing-time annotations astral-sh/ruff
2 participants
@charliermarsh @aazuspan