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

Fix F821 false negatives when from __future__ import annotations is active (attempt 2) #10524

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Fix F821 false negatives when from __future__ import annotations is active (attempt 2) #10524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_27.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,16 @@ class MyClass:
baz: MyClass
eggs = baz # Still invalid even when `__future__.annotations` are enabled
eggs = "baz" # always okay

# Forward references:
MaybeDStr: TypeAlias = Optional[DStr] # Still invalid even when `__future__.annotations` are enabled
MaybeDStr2: TypeAlias = Optional["DStr"] # always okay
DStr: TypeAlias = Union[D, str] # Still invalid even when `__future__.annotations` are enabled
DStr2: TypeAlias = Union["D", str] # always okay

class D: ...

# More circular references
class Leaf: ...
class Tree(list[Tree | Leaf]): ... # Still invalid even when `__future__.annotations` are enabled
class Tree2(list["Tree | Leaf"]): ... # always okay
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite testing what was intended and ought to be:

Suggested change
class Tree2(list["Tree | Leaf"]): ... # always okay
class Tree2(list["Tree2 | Leaf"]): ... # always okay

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Luckily the corrected test still passes, however.

1 change: 1 addition & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
&& !self.semantic.in_deferred_type_definition()
&& self.semantic.in_type_definition()
&& self.semantic.future_annotations()
&& (self.semantic.in_annotation() || self.source_type.is_stub())
{
if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = expr {
self.visit.string_type_definitions.push((
Expand Down
27 changes: 27 additions & 0 deletions ...er/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_27.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,30 @@ F821_27.py:34:8: F821 Undefined name `baz`
| ^^^ F821
35 | eggs = "baz" # always okay
|

F821_27.py:38:33: F821 Undefined name `DStr`
|
37 | # Forward references:
38 | MaybeDStr: TypeAlias = Optional[DStr] # Still invalid even when `__future__.annotations` are enabled
| ^^^^ F821
39 | MaybeDStr2: TypeAlias = Optional["DStr"] # always okay
40 | DStr: TypeAlias = Union[D, str] # Still invalid even when `__future__.annotations` are enabled
|

F821_27.py:40:25: F821 Undefined name `D`
|
38 | MaybeDStr: TypeAlias = Optional[DStr] # Still invalid even when `__future__.annotations` are enabled
39 | MaybeDStr2: TypeAlias = Optional["DStr"] # always okay
40 | DStr: TypeAlias = Union[D, str] # Still invalid even when `__future__.annotations` are enabled
| ^ F821
41 | DStr2: TypeAlias = Union["D", str] # always okay
|

F821_27.py:47:17: F821 Undefined name `Tree`
|
45 | # More circular references
46 | class Leaf: ...
47 | class Tree(list[Tree | Leaf]): ... # Still invalid even when `__future__.annotations` are enabled
| ^^^^ F821
48 | class Tree2(list["Tree | Leaf"]): ... # always okay
|
Loading