From 364bd82aee3d15733bec381f6359b64e976fcaba Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 13 Jun 2023 14:47:29 -0400 Subject: [PATCH] Don't treat annotations as resolved in forward references (#5060) ## 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/)). --- crates/ruff/src/rules/pyflakes/mod.rs | 24 ++++++++++++++++++++ crates/ruff_python_semantic/src/model.rs | 29 ++++++++++++------------ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index e29f54393c..80e6fd868d 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/crates/ruff/src/rules/pyflakes/mod.rs @@ -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 "#, &[], ); @@ -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 "#, &[], ); diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index e7bd442985..e347dfbe8a 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -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); + } } } @@ -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 @@ -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; }