Skip to content

Commit c0b7c36

Browse files
authored
[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`
1 parent e8e461d commit c0b7c36

File tree

3 files changed

+49
-5
lines changed

3 files changed

+49
-5
lines changed

crates/ruff_linter/resources/test/fixtures/ruff/RUF027_1.py

+6
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,9 @@ def negative_cases():
6868
@app.get("/items/{item_id}")
6969
async def read_item(item_id):
7070
return {"item_id": item_id}
71+
72+
from typing import TYPE_CHECKING
73+
if TYPE_CHECKING:
74+
from datetime import date
75+
76+
t = "foo/{date}"

crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,14 @@ fn should_be_fstring(
209209
return false;
210210
}
211211
if semantic
212-
.lookup_symbol(id)
212+
// the parsed expression nodes have incorrect ranges
213+
// so we need to use the range of the literal for the
214+
// lookup in order to get reasonable results.
215+
.simulate_runtime_load_at_location_in_scope(
216+
id,
217+
literal.range(),
218+
semantic.scope_id,
219+
)
213220
.map_or(true, |id| semantic.binding(id).kind.is_builtin())
214221
{
215222
return false;

crates/ruff_python_semantic/src/model.rs

+35-4
Original file line numberDiff line numberDiff line change
@@ -711,12 +711,43 @@ impl<'a> SemanticModel<'a> {
711711
/// References from within an [`ast::Comprehension`] can produce incorrect
712712
/// results when referring to a [`BindingKind::NamedExprAssignment`].
713713
pub fn simulate_runtime_load(&self, name: &ast::ExprName) -> Option<BindingId> {
714-
let symbol = name.id.as_str();
715-
let range = name.range;
714+
self.simulate_runtime_load_at_location_in_scope(name.id.as_str(), name.range, self.scope_id)
715+
}
716+
717+
/// Simulates a runtime load of the given symbol.
718+
///
719+
/// This should not be run until after all the bindings have been visited.
720+
///
721+
/// The main purpose of this method and what makes this different from
722+
/// [`SemanticModel::lookup_symbol_in_scope`] is that it may be used to
723+
/// perform speculative name lookups.
724+
///
725+
/// In most cases a load can be accurately modeled simply by calling
726+
/// [`SemanticModel::lookup_symbol`] at the right time during semantic
727+
/// analysis, however for speculative lookups this is not the case,
728+
/// since we're aiming to change the semantic meaning of our load.
729+
/// E.g. we want to check what would happen if we changed a forward
730+
/// reference to an immediate load or vice versa.
731+
///
732+
/// Use caution when utilizing this method, since it was primarily designed
733+
/// to work for speculative lookups from within type definitions, which
734+
/// happen to share some nice properties, where attaching each binding
735+
/// to a range in the source code and ordering those bindings based on
736+
/// that range is a good enough approximation of which bindings are
737+
/// available at runtime for which reference.
738+
///
739+
/// References from within an [`ast::Comprehension`] can produce incorrect
740+
/// results when referring to a [`BindingKind::NamedExprAssignment`].
741+
pub fn simulate_runtime_load_at_location_in_scope(
742+
&self,
743+
symbol: &str,
744+
symbol_range: TextRange,
745+
scope_id: ScopeId,
746+
) -> Option<BindingId> {
716747
let mut seen_function = false;
717748
let mut class_variables_visible = true;
718749
let mut source_order_sensitive_lookup = true;
719-
for (index, scope_id) in self.scopes.ancestor_ids(self.scope_id).enumerate() {
750+
for (index, scope_id) in self.scopes.ancestor_ids(scope_id).enumerate() {
720751
let scope = &self.scopes[scope_id];
721752

722753
// Only once we leave a function scope and its enclosing type scope should
@@ -776,7 +807,7 @@ impl<'a> SemanticModel<'a> {
776807
_ => binding.range,
777808
};
778809

779-
if binding_range.ordering(range).is_lt() {
810+
if binding_range.ordering(symbol_range).is_lt() {
780811
return Some(shadowed_id);
781812
}
782813
}

0 commit comments

Comments
 (0)