From caa14508959781b9220f5c9f83c4e0a4c586ce0c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 21 Mar 2024 12:22:50 -0400 Subject: [PATCH] Don't treat annotations as redefinitions in `.pyi` files (#10512) ## Summary In https://github.com/astral-sh/ruff/pull/10341, we fixed some false positives in `.pyi` files, but introduced others. This PR effectively reverts the change in #10341 and fixes it in a slightly different way. Instead of changing the _bindings_ we generate in the semantic model in `.pyi` files, we instead change how we _resolve_ them. Closes https://github.com/astral-sh/ruff/issues/10509. --- .../test/fixtures/pyflakes/F811_29.pyi | 8 ++++++ crates/ruff_linter/src/checkers/ast/mod.rs | 2 +- crates/ruff_linter/src/rules/pyflakes/mod.rs | 1 + ...es__pyflakes__tests__F811_F811_29.pyi.snap | 11 ++++++++ crates/ruff_python_semantic/src/model.rs | 26 ++++++++++++++----- 5 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyflakes/F811_29.pyi create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_29.pyi.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_29.pyi b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_29.pyi new file mode 100644 index 0000000000000..f4204eab3ca38 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_29.pyi @@ -0,0 +1,8 @@ +"""Regression test for: https://github.com/astral-sh/ruff/issues/10509""" + +from foo import Bar as Bar + +class Eggs: + Bar: int # OK + +Bar = 1 # F811 diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 562cb4e37c7d2..75b934d5ee46d 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -1836,7 +1836,7 @@ impl<'a> Checker<'a> { if matches!( parent, Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. }) - ) && !(self.semantic.in_annotation() || self.source_type.is_stub()) + ) && !self.semantic.in_annotation() { self.add_binding(id, expr.range(), BindingKind::Annotation, flags); return; diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 563c48422c138..f8d8b5fd9eb5e 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -125,6 +125,7 @@ mod tests { #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_28.py"))] + #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_29.pyi"))] #[test_case(Rule::UndefinedName, Path::new("F821_0.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_1.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_29.pyi.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_29.pyi.snap new file mode 100644 index 0000000000000..0a7ee144e4415 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_29.pyi.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F811_29.pyi:8:1: F811 Redefinition of unused `Bar` from line 3 + | +6 | Bar: int # OK +7 | +8 | Bar = 1 # F811 + | ^^^ F811 + | + = help: Remove definition: `Bar` diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index f9f4fdc471602..93ec108911bd5 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -397,7 +397,10 @@ 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. - BindingKind::Annotation => continue, + // + // Stub files are an exception. In a stub file, it _is_ considered valid to + // resolve to a type annotation. + BindingKind::Annotation if !self.in_stub_file() => continue, // If it's a deletion, don't treat it as resolved, since the name is now // unbound. For example, given: @@ -1570,6 +1573,11 @@ impl<'a> SemanticModel<'a> { .intersects(SemanticModelFlags::FUTURE_ANNOTATIONS) } + /// Return `true` if the model is in a stub file (i.e., a file with a `.pyi` extension). + pub const fn in_stub_file(&self) -> bool { + self.flags.intersects(SemanticModelFlags::STUB_FILE) + } + /// Return `true` if the model is in a named expression assignment (e.g., `x := 1`). pub const fn in_named_expression_assignment(&self) -> bool { self.flags @@ -1675,7 +1683,7 @@ bitflags! { /// Flags indicating the current model state. #[derive(Debug, Default, Copy, Clone, Eq, PartialEq)] pub struct SemanticModelFlags: u32 { - /// The model is in a type annotation that will only be evaluated when running a type + /// The model is in a type annotation that will only be evaluated when running a type /// checker. /// /// For example, the model could be visiting `int` in: @@ -1875,6 +1883,9 @@ bitflags! { /// ``` const FUTURE_ANNOTATIONS = 1 << 15; + /// The model is in a Python stub file (i.e., a `.pyi` file). + const STUB_FILE = 1 << 16; + /// The model has traversed past the module docstring. /// /// For example, the model could be visiting `x` in: @@ -1883,7 +1894,7 @@ bitflags! { /// /// x: int = 1 /// ``` - const MODULE_DOCSTRING_BOUNDARY = 1 << 16; + const MODULE_DOCSTRING_BOUNDARY = 1 << 17; /// The model is in a type parameter definition. /// @@ -1893,7 +1904,7 @@ bitflags! { /// /// Record = TypeVar("Record") /// - const TYPE_PARAM_DEFINITION = 1 << 17; + const TYPE_PARAM_DEFINITION = 1 << 18; /// The model is in a named expression assignment. /// @@ -1901,7 +1912,7 @@ bitflags! { /// ```python /// if (x := 1): ... /// ``` - const NAMED_EXPRESSION_ASSIGNMENT = 1 << 18; + const NAMED_EXPRESSION_ASSIGNMENT = 1 << 19; /// The model is in a comprehension variable assignment. /// @@ -1909,7 +1920,7 @@ bitflags! { /// ```python /// [_ for x in range(10)] /// ``` - const COMPREHENSION_ASSIGNMENT = 1 << 19; + const COMPREHENSION_ASSIGNMENT = 1 << 20; /// The model is in a module / class / function docstring. /// @@ -1928,7 +1939,7 @@ bitflags! { /// """Function docstring.""" /// pass /// ``` - const DOCSTRING = 1 << 20; + const DOCSTRING = 1 << 21; /// The context is in any type annotation. const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_EVALUATED_ANNOTATION.bits() | Self::RUNTIME_REQUIRED_ANNOTATION.bits(); @@ -1953,6 +1964,7 @@ impl SemanticModelFlags { pub fn new(path: &Path) -> Self { let mut flags = Self::default(); if is_python_stub_file(path) { + flags |= Self::STUB_FILE; flags |= Self::FUTURE_ANNOTATIONS; } flags