Skip to content

Commit

Permalink
F821, F822: fix false positive for .pyi files; add more test covera…
Browse files Browse the repository at this point in the history
…ge for `.pyi` files (#10341)

This PR fixes the following false positive in a `.pyi` stub file:

```py
x: int
y = x  # F821 currently emitted here, but shouldn't be in a stub file
```

In a `.py` file, this is invalid regardless of whether `from __future__ import annotations` is enabled or not. In a `.pyi` stub file, however, it's always valid, as an annotation counts as a binding in a stub file even if no value is assigned to the variable.

I also added more test coverage for `.pyi` stub files in various edge cases where ruff's behaviour is currently correct, but where `.pyi` stub files do slightly different things to `.py` files.
  • Loading branch information
AlexWaygood committed Mar 11, 2024
1 parent 06284c3 commit 4b06669
Show file tree
Hide file tree
Showing 14 changed files with 298 additions and 2 deletions.
16 changes: 16 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_11.pyi
@@ -0,0 +1,16 @@
"""Test case: strings used within calls within type annotations."""

from typing import Callable

import bpy
from mypy_extensions import VarArg

class LightShow(bpy.types.Operator):
label = "Create Character"
name = "lightshow.letter_creation"

filepath: bpy.props.StringProperty(subtype="FILE_PATH") # OK


def f(x: Callable[[VarArg("os")], None]): # F821
pass
44 changes: 44 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.py
@@ -0,0 +1,44 @@
"""Tests for constructs allowed in `.pyi` stub files but not at runtime"""

from typing import Optional, TypeAlias, Union

__version__: str
__author__: str

# Forward references:
MaybeCStr: TypeAlias = Optional[CStr] # valid in a `.pyi` stub file, not in a `.py` runtime file
MaybeCStr2: TypeAlias = Optional["CStr"] # always okay
CStr: TypeAlias = Union[C, str] # valid in a `.pyi` stub file, not in a `.py` runtime file
CStr2: TypeAlias = Union["C", str] # always okay

# References to a class from inside the class:
class C:
other: C = ... # valid in a `.pyi` stub file, not in a `.py` runtime file
other2: "C" = ... # always okay
def from_str(self, s: str) -> C: ... # valid in a `.pyi` stub file, not in a `.py` runtime file
def from_str2(self, s: str) -> "C": ... # always okay

# Circular references:
class A:
foo: B # valid in a `.pyi` stub file, not in a `.py` runtime file
foo2: "B" # always okay
bar: dict[str, B] # valid in a `.pyi` stub file, not in a `.py` runtime file
bar2: dict[str, "A"] # always okay

class B:
foo: A # always okay
bar: dict[str, A] # always okay

class Leaf: ...
class Tree(list[Tree | Leaf]): ... # valid in a `.pyi` stub file, not in a `.py` runtime file
class Tree2(list["Tree | Leaf"]): ... # always okay

# Annotations are treated as assignments in .pyi files, but not in .py files
class MyClass:
foo: int
bar = foo # valid in a `.pyi` stub file, not in a `.py` runtime file
bar = "foo" # always okay

baz: MyClass
eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file
eggs = "baz" # always okay
44 changes: 44 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_26.pyi
@@ -0,0 +1,44 @@
"""Tests for constructs allowed in `.pyi` stub files but not at runtime"""

from typing import Optional, TypeAlias, Union

__version__: str
__author__: str

# Forward references:
MaybeCStr: TypeAlias = Optional[CStr] # valid in a `.pyi` stub file, not in a `.py` runtime file
MaybeCStr2: TypeAlias = Optional["CStr"] # always okay
CStr: TypeAlias = Union[C, str] # valid in a `.pyi` stub file, not in a `.py` runtime file
CStr2: TypeAlias = Union["C", str] # always okay

# References to a class from inside the class:
class C:
other: C = ... # valid in a `.pyi` stub file, not in a `.py` runtime file
other2: "C" = ... # always okay
def from_str(self, s: str) -> C: ... # valid in a `.pyi` stub file, not in a `.py` runtime file
def from_str2(self, s: str) -> "C": ... # always okay

# Circular references:
class A:
foo: B # valid in a `.pyi` stub file, not in a `.py` runtime file
foo2: "B" # always okay
bar: dict[str, B] # valid in a `.pyi` stub file, not in a `.py` runtime file
bar2: dict[str, "A"] # always okay

class B:
foo: A # always okay
bar: dict[str, A] # always okay

class Leaf: ...
class Tree(list[Tree | Leaf]): ... # valid in a `.pyi` stub file, not in a `.py` runtime file
class Tree2(list["Tree | Leaf"]): ... # always okay

# Annotations are treated as assignments in .pyi files, but not in .py files
class MyClass:
foo: int
bar = foo # valid in a `.pyi` stub file, not in a `.py` runtime file
bar = "foo" # always okay

baz: MyClass
eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file
eggs = "baz" # always okay
35 changes: 35 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_27.py
@@ -0,0 +1,35 @@
"""Tests for constructs allowed when `__future__` annotations are enabled but not otherwise"""
from __future__ import annotations

from typing import Optional, TypeAlias, Union

__version__: str
__author__: str

# References to a class from inside the class:
class C:
other: C = ... # valid when `__future__.annotations are enabled
other2: "C" = ... # always okay
def from_str(self, s: str) -> C: ... # valid when `__future__.annotations are enabled
def from_str2(self, s: str) -> "C": ... # always okay

# Circular references:
class A:
foo: B # valid when `__future__.annotations are enabled
foo2: "B" # always okay
bar: dict[str, B] # valid when `__future__.annotations are enabled
bar2: dict[str, "A"] # always okay

class B:
foo: A # always okay
bar: dict[str, A] # always okay

# Annotations are treated as assignments in .pyi files, but not in .py files
class MyClass:
foo: int
bar = foo # Still invalid even when `__future__.annotations` are enabled
bar = "foo" # always okay

baz: MyClass
eggs = baz # Still invalid even when `__future__.annotations` are enabled
eggs = "baz" # always okay
10 changes: 10 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pyflakes/F821_5.pyi
@@ -0,0 +1,10 @@
"""Test: inner class annotation."""

class RandomClass:
def bad_func(self) -> InnerClass: ... # F821
def good_func(self) -> OuterClass.InnerClass: ... # Okay

class OuterClass:
class InnerClass: ...

def good_func(self) -> InnerClass: ... # Okay
@@ -0,0 +1,4 @@
a = 1
b: int # Considered a binding in a `.pyi` stub file, not in a `.py` runtime file

__all__ = ["a", "b", "c"] # c is flagged as missing; b is not
6 changes: 4 additions & 2 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Expand Up @@ -1839,11 +1839,13 @@ impl<'a> Checker<'a> {
flags.insert(BindingFlags::UNPACKED_ASSIGNMENT);
}

// Match the left-hand side of an annotated assignment, like `x` in `x: int`.
// Match the left-hand side of an annotated assignment without a value,
// like `x` in `x: int`. N.B. In stub files, these should be viewed
// as assignments on par with statements such as `x: int = 5`.
if matches!(
parent,
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
) && !self.semantic.in_annotation()
) && !(self.semantic.in_annotation() || self.source_type.is_stub())
{
self.add_binding(id, expr.range(), BindingKind::Annotation, flags);
return;
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_linter/src/rules/pyflakes/mod.rs
Expand Up @@ -130,12 +130,14 @@ mod tests {
#[test_case(Rule::UndefinedName, Path::new("F821_3.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_4.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_5.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_5.pyi"))]
#[test_case(Rule::UndefinedName, Path::new("F821_6.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_7.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_8.pyi"))]
#[test_case(Rule::UndefinedName, Path::new("F821_9.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_10.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_11.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_11.pyi"))]
#[test_case(Rule::UndefinedName, Path::new("F821_12.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_13.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_14.py"))]
Expand All @@ -150,7 +152,11 @@ mod tests {
#[test_case(Rule::UndefinedName, Path::new("F821_23.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_24.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_25.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_26.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_26.pyi"))]
#[test_case(Rule::UndefinedName, Path::new("F821_27.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_0.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_0.pyi"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_1.py"))]
#[test_case(Rule::UndefinedExport, Path::new("F822_2.py"))]
#[test_case(Rule::UndefinedLocal, Path::new("F823.py"))]
Expand Down
@@ -0,0 +1,9 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F821_11.pyi:15:28: F821 Undefined name `os`
|
15 | def f(x: Callable[[VarArg("os")], None]): # F821
| ^^ F821
16 | pass
|
@@ -0,0 +1,83 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F821_26.py:9:33: F821 Undefined name `CStr`
|
8 | # Forward references:
9 | MaybeCStr: TypeAlias = Optional[CStr] # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^^^^ F821
10 | MaybeCStr2: TypeAlias = Optional["CStr"] # always okay
11 | CStr: TypeAlias = Union[C, str] # valid in a `.pyi` stub file, not in a `.py` runtime file
|

F821_26.py:11:25: F821 Undefined name `C`
|
9 | MaybeCStr: TypeAlias = Optional[CStr] # valid in a `.pyi` stub file, not in a `.py` runtime file
10 | MaybeCStr2: TypeAlias = Optional["CStr"] # always okay
11 | CStr: TypeAlias = Union[C, str] # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^ F821
12 | CStr2: TypeAlias = Union["C", str] # always okay
|

F821_26.py:16:12: F821 Undefined name `C`
|
14 | # References to a class from inside the class:
15 | class C:
16 | other: C = ... # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^ F821
17 | other2: "C" = ... # always okay
18 | def from_str(self, s: str) -> C: ... # valid in a `.pyi` stub file, not in a `.py` runtime file
|

F821_26.py:18:35: F821 Undefined name `C`
|
16 | other: C = ... # valid in a `.pyi` stub file, not in a `.py` runtime file
17 | other2: "C" = ... # always okay
18 | def from_str(self, s: str) -> C: ... # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^ F821
19 | def from_str2(self, s: str) -> "C": ... # always okay
|

F821_26.py:23:10: F821 Undefined name `B`
|
21 | # Circular references:
22 | class A:
23 | foo: B # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^ F821
24 | foo2: "B" # always okay
25 | bar: dict[str, B] # valid in a `.pyi` stub file, not in a `.py` runtime file
|

F821_26.py:25:20: F821 Undefined name `B`
|
23 | foo: B # valid in a `.pyi` stub file, not in a `.py` runtime file
24 | foo2: "B" # always okay
25 | bar: dict[str, B] # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^ F821
26 | bar2: dict[str, "A"] # always okay
|

F821_26.py:33:17: F821 Undefined name `Tree`
|
32 | class Leaf: ...
33 | class Tree(list[Tree | Leaf]): ... # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^^^^ F821
34 | class Tree2(list["Tree | Leaf"]): ... # always okay
|

F821_26.py:39:11: F821 Undefined name `foo`
|
37 | class MyClass:
38 | foo: int
39 | bar = foo # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^^^ F821
40 | bar = "foo" # always okay
|

F821_26.py:43:8: F821 Undefined name `baz`
|
42 | baz: MyClass
43 | eggs = baz # valid in a `.pyi` stub file, not in a `.py` runtime file
| ^^^ F821
44 | eggs = "baz" # always okay
|
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---

@@ -0,0 +1,19 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F821_27.py:30:11: F821 Undefined name `foo`
|
28 | class MyClass:
29 | foo: int
30 | bar = foo # Still invalid even when `__future__.annotations` are enabled
| ^^^ F821
31 | bar = "foo" # always okay
|

F821_27.py:34:8: F821 Undefined name `baz`
|
33 | baz: MyClass
34 | eggs = baz # Still invalid even when `__future__.annotations` are enabled
| ^^^ F821
35 | eggs = "baz" # always okay
|
@@ -0,0 +1,10 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F821_5.pyi:4:27: F821 Undefined name `InnerClass`
|
3 | class RandomClass:
4 | def bad_func(self) -> InnerClass: ... # F821
| ^^^^^^^^^^ F821
5 | def good_func(self) -> OuterClass.InnerClass: ... # Okay
|
@@ -0,0 +1,10 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F822_0.pyi:4:1: F822 Undefined name `c` in `__all__`
|
2 | b: int # Considered a binding in a `.pyi` stub file, not in a `.py` runtime file
3 |
4 | __all__ = ["a", "b", "c"] # c is flagged as missing; b is not
| ^^^^^^^ F822
|

0 comments on commit 4b06669

Please sign in to comment.