Skip to content

Commit

Permalink
Don't treat annotations as resolved in forward references (#5060)
Browse files Browse the repository at this point in the history
## Summary

This behavior dates back to a Pyflakes commit (5fc37cbd), which was used
to allow this test to pass:

```py
from __future__ import annotations
T: object
def f(t: T): pass
def g(t: 'T'): pass
```

But, I think this is an error. Mypy and Pyright don't accept it -- you
can only use variables as type annotations if they're type aliases
(i.e., annotated with `TypeAlias`), in which case, there has to be an
assignment on the right-hand side (see: [PEP
613](https://peps.python.org/pep-0613/)).
  • Loading branch information
charliermarsh committed Jun 13, 2023
1 parent f9f08d6 commit 364bd82
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 14 deletions.
24 changes: 24 additions & 0 deletions crates/ruff/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3190,6 +3190,20 @@ mod tests {
r#"
T: object
def g(t: 'T'): pass
"#,
&[Rule::UndefinedName],
);
flakes(
r#"
T = object
def f(t: T): pass
"#,
&[],
);
flakes(
r#"
T = object
def g(t: 'T'): pass
"#,
&[],
);
Expand Down Expand Up @@ -3381,6 +3395,16 @@ mod tests {
T: object
def f(t: T): pass
def g(t: 'T'): pass
"#,
&[Rule::UndefinedName, Rule::UndefinedName],
);

flakes(
r#"
from __future__ import annotations
T = object
def f(t: T): pass
def g(t: 'T'): pass
"#,
&[],
);
Expand Down
29 changes: 15 additions & 14 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,22 @@ impl<'a> SemanticModel<'a> {
// should prefer it over local resolutions.
if self.in_forward_reference() {
if let Some(binding_id) = self.scopes.global().get(symbol) {
// Mark the binding as used.
let context = self.execution_context();
let reference_id = self.references.push(ScopeId::global(), range, context);
self.bindings[binding_id].references.push(reference_id);

// Mark any submodule aliases as used.
if let Some(binding_id) =
self.resolve_submodule(symbol, ScopeId::global(), binding_id)
{
if !self.bindings[binding_id].kind.is_annotation() {
// Mark the binding as used.
let context = self.execution_context();
let reference_id = self.references.push(ScopeId::global(), range, context);
self.bindings[binding_id].references.push(reference_id);
}

return ResolvedRead::Resolved(binding_id);
// Mark any submodule aliases as used.
if let Some(binding_id) =
self.resolve_submodule(symbol, ScopeId::global(), binding_id)
{
let reference_id = self.references.push(ScopeId::global(), range, context);
self.bindings[binding_id].references.push(reference_id);
}

return ResolvedRead::Resolved(binding_id);
}
}
}

Expand Down Expand Up @@ -226,8 +228,7 @@ impl<'a> SemanticModel<'a> {
self.bindings[binding_id].references.push(reference_id);
}

// But if it's a type annotation, don't treat it as resolved, unless we're in a
// forward reference. For example, given:
// But if it's a type annotation, don't treat it as resolved. For example, given:
//
// ```python
// name: str
Expand All @@ -236,7 +237,7 @@ impl<'a> SemanticModel<'a> {
//
// The `name` in `print(name)` should be treated as unresolved, but the `name` in
// `name: str` should be treated as used.
if !self.in_forward_reference() && self.bindings[binding_id].kind.is_annotation() {
if self.bindings[binding_id].kind.is_annotation() {
continue;
}

Expand Down

0 comments on commit 364bd82

Please sign in to comment.