From c0b7c36d435441788b5a1f2330a66613656a3e47 Mon Sep 17 00:00:00 2001 From: David Salvisberg Date: Wed, 18 Dec 2024 08:50:49 +0100 Subject: [PATCH] [`ruff`] Avoid false positives for RUF027 for typing context bindings. (#15037) Closes #14000 ## Summary For typing context bindings we know that they won't be available at runtime. We shouldn't recommend a fix, that will result in name errors at runtime. ## Test Plan `cargo nextest run` --- .../resources/test/fixtures/ruff/RUF027_1.py | 6 +++ .../ruff/rules/missing_fstring_syntax.rs | 9 ++++- crates/ruff_python_semantic/src/model.rs | 39 +++++++++++++++++-- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py index 6f3b7cf9afc36..b91ec7b69e8f3 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py @@ -68,3 +68,9 @@ def negative_cases(): @app.get("/items/{item_id}") async def read_item(item_id): return {"item_id": item_id} + + from typing import TYPE_CHECKING + if TYPE_CHECKING: + from datetime import date + + t = "foo/{date}" diff --git a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs index a7cee9f3b85cd..ca19c06346d23 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs @@ -209,7 +209,14 @@ fn should_be_fstring( return false; } if semantic - .lookup_symbol(id) + // the parsed expression nodes have incorrect ranges + // so we need to use the range of the literal for the + // lookup in order to get reasonable results. + .simulate_runtime_load_at_location_in_scope( + id, + literal.range(), + semantic.scope_id, + ) .map_or(true, |id| semantic.binding(id).kind.is_builtin()) { return false; diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index f602c763046fd..f43cf24de1ec5 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -711,12 +711,43 @@ impl<'a> SemanticModel<'a> { /// References from within an [`ast::Comprehension`] can produce incorrect /// results when referring to a [`BindingKind::NamedExprAssignment`]. pub fn simulate_runtime_load(&self, name: &ast::ExprName) -> Option { - let symbol = name.id.as_str(); - let range = name.range; + self.simulate_runtime_load_at_location_in_scope(name.id.as_str(), name.range, self.scope_id) + } + + /// Simulates a runtime load of the given symbol. + /// + /// This should not be run until after all the bindings have been visited. + /// + /// The main purpose of this method and what makes this different from + /// [`SemanticModel::lookup_symbol_in_scope`] is that it may be used to + /// perform speculative name lookups. + /// + /// In most cases a load can be accurately modeled simply by calling + /// [`SemanticModel::lookup_symbol`] at the right time during semantic + /// analysis, however for speculative lookups this is not the case, + /// since we're aiming to change the semantic meaning of our load. + /// E.g. we want to check what would happen if we changed a forward + /// reference to an immediate load or vice versa. + /// + /// Use caution when utilizing this method, since it was primarily designed + /// to work for speculative lookups from within type definitions, which + /// happen to share some nice properties, where attaching each binding + /// to a range in the source code and ordering those bindings based on + /// that range is a good enough approximation of which bindings are + /// available at runtime for which reference. + /// + /// References from within an [`ast::Comprehension`] can produce incorrect + /// results when referring to a [`BindingKind::NamedExprAssignment`]. + pub fn simulate_runtime_load_at_location_in_scope( + &self, + symbol: &str, + symbol_range: TextRange, + scope_id: ScopeId, + ) -> Option { let mut seen_function = false; let mut class_variables_visible = true; let mut source_order_sensitive_lookup = true; - for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() { + for (index, scope_id) in self.scopes.ancestor_ids(scope_id).enumerate() { let scope = &self.scopes[scope_id]; // Only once we leave a function scope and its enclosing type scope should @@ -776,7 +807,7 @@ impl<'a> SemanticModel<'a> { _ => binding.range, }; - if binding_range.ordering(range).is_lt() { + if binding_range.ordering(symbol_range).is_lt() { return Some(shadowed_id); } }