diff --git a/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py index 2bda6ff983..7b082da74e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py +++ b/crates/ruff/resources/test/fixtures/flake8_type_checking/TCH002.py @@ -146,3 +146,7 @@ def f(): import pandas as pd x = dict[pd.DataFrame, pd.DataFrame] + + +def f(): + import pandas as pd diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index f48cded720..fcff49190f 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -30,6 +30,7 @@ use ruff_python_semantic::binding::{ use ruff_python_semantic::definition::{ContextualizedDefinition, Module, ModuleKind}; use ruff_python_semantic::model::{ResolvedReference, SemanticModel, SemanticModelFlags}; use ruff_python_semantic::node::NodeId; +use ruff_python_semantic::reference::ReferenceContext; use ruff_python_semantic::scope::{ClassDef, FunctionDef, Lambda, Scope, ScopeId, ScopeKind}; use ruff_python_stdlib::builtins::{BUILTINS, MAGIC_GLOBALS}; use ruff_python_stdlib::path::is_python_stub_file; @@ -234,20 +235,26 @@ where // Add the binding to the current scope. let context = self.semantic_model.execution_context(); let exceptions = self.semantic_model.exceptions(); - let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; - let usage = Some((self.semantic_model.scope_id, stmt.range())); for (name, range) in names.iter().zip(ranges.iter()) { - let id = self.semantic_model.bindings.push(Binding { + // Add a binding to the current scope. + let binding_id = self.semantic_model.bindings.push(Binding { kind: BindingKind::Global, - runtime_usage: None, - synthetic_usage: usage, - typing_usage: None, range: *range, + references: Vec::new(), source: self.semantic_model.stmt_id, context, exceptions, }); - scope.add(name, id); + + // Add a reference to the global binding. + self.semantic_model.add_local_reference( + binding_id, + stmt.range(), + ReferenceContext::Synthetic, + ); + + let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; + scope.add(name, binding_id); } } @@ -263,21 +270,26 @@ where if !self.semantic_model.scope_id.is_global() { let context = self.semantic_model.execution_context(); let exceptions = self.semantic_model.exceptions(); - let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; - let usage = Some((self.semantic_model.scope_id, stmt.range())); for (name, range) in names.iter().zip(ranges.iter()) { // Add a binding to the current scope. - let id = self.semantic_model.bindings.push(Binding { + let binding_id = self.semantic_model.bindings.push(Binding { kind: BindingKind::Nonlocal, - runtime_usage: None, - synthetic_usage: usage, - typing_usage: None, range: *range, + references: Vec::new(), source: self.semantic_model.stmt_id, context, exceptions, }); - scope.add(name, id); + + // Add a reference to the global binding. + self.semantic_model.add_local_reference( + binding_id, + stmt.range(), + ReferenceContext::Synthetic, + ); + + let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; + scope.add(name, binding_id); } // Mark the binding in the defining scopes as used too. (Skip the global scope @@ -292,7 +304,11 @@ where .find_map(|scope| scope.get(name.as_str())); if let Some(binding_id) = binding_id { - self.semantic_model.bindings[*binding_id].runtime_usage = usage; + self.semantic_model.add_local_reference( + *binding_id, + stmt.range(), + ReferenceContext::Runtime, + ); } else { // Ensure that every nonlocal has an existing binding from a parent scope. if self.enabled(Rule::NonlocalWithoutBinding) { @@ -797,24 +813,26 @@ where for alias in names { if &alias.name == "__future__" { let name = alias.asname.as_ref().unwrap_or(&alias.name); - self.add_binding( + + let binding_id = self.add_binding( name, Binding { kind: BindingKind::FutureImportation, - runtime_usage: None, - // Always mark `__future__` imports as used. - synthetic_usage: Some(( - self.semantic_model.scope_id, - alias.range(), - )), - typing_usage: None, range: alias.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), }, ); + // Always mark `__future__` imports as used. + self.semantic_model.add_local_reference( + binding_id, + alias.range(), + ReferenceContext::Synthetic, + ); + if self.enabled(Rule::LateFutureImport) { if self.semantic_model.seen_futures_boundary() { self.diagnostics.push(Diagnostic::new( @@ -835,42 +853,41 @@ where name, full_name, }), - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: alias.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), }, ); } else { + let name = alias.asname.as_ref().unwrap_or(&alias.name); + let full_name = &alias.name; + let binding_id = self.add_binding( + name, + Binding { + kind: BindingKind::Importation(Importation { name, full_name }), + range: alias.range(), + references: Vec::new(), + source: self.semantic_model.stmt_id, + context: self.semantic_model.execution_context(), + exceptions: self.semantic_model.exceptions(), + }, + ); + // Treat explicit re-export as usage (e.g., `from .applications // import FastAPI as FastAPI`). let is_explicit_reexport = alias .asname .as_ref() .map_or(false, |asname| asname == &alias.name); - - let name = alias.asname.as_ref().unwrap_or(&alias.name); - let full_name = &alias.name; - self.add_binding( - name, - Binding { - kind: BindingKind::Importation(Importation { name, full_name }), - runtime_usage: None, - synthetic_usage: if is_explicit_reexport { - Some((self.semantic_model.scope_id, alias.range())) - } else { - None - }, - typing_usage: None, - range: alias.range(), - source: self.semantic_model.stmt_id, - context: self.semantic_model.execution_context(), - exceptions: self.semantic_model.exceptions(), - }, - ); + if is_explicit_reexport { + self.semantic_model.add_local_reference( + binding_id, + alias.range(), + ReferenceContext::Synthetic, + ); + } if let Some(asname) = &alias.asname { if self.enabled(Rule::BuiltinVariableShadowing) { @@ -1085,24 +1102,26 @@ where for alias in names { if let Some("__future__") = module { let name = alias.asname.as_ref().unwrap_or(&alias.name); - self.add_binding( + + let binding_id = self.add_binding( name, Binding { kind: BindingKind::FutureImportation, - runtime_usage: None, - // Always mark `__future__` imports as used. - synthetic_usage: Some(( - self.semantic_model.scope_id, - alias.range(), - )), - typing_usage: None, range: alias.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), }, ); + // Always mark `__future__` imports as used. + self.semantic_model.add_local_reference( + binding_id, + alias.range(), + ReferenceContext::Synthetic, + ); + if self.enabled(Rule::FutureFeatureNotDefined) { pyflakes::rules::future_feature_not_defined(self, alias); } @@ -1151,39 +1170,40 @@ where } } - // Treat explicit re-export as usage (e.g., `from .applications - // import FastAPI as FastAPI`). - let is_explicit_reexport = alias - .asname - .as_ref() - .map_or(false, |asname| asname == &alias.name); - // Given `from foo import bar`, `name` would be "bar" and `full_name` would // be "foo.bar". Given `from foo import bar as baz`, `name` would be "baz" // and `full_name` would be "foo.bar". let name = alias.asname.as_ref().unwrap_or(&alias.name); let full_name = helpers::format_import_from_member(level, module, &alias.name); - self.add_binding( + let binding_id = self.add_binding( name, Binding { kind: BindingKind::FromImportation(FromImportation { name, full_name, }), - runtime_usage: None, - synthetic_usage: if is_explicit_reexport { - Some((self.semantic_model.scope_id, alias.range())) - } else { - None - }, - typing_usage: None, range: alias.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), }, ); + + // Treat explicit re-export as usage (e.g., `from .applications + // import FastAPI as FastAPI`). + let is_explicit_reexport = alias + .asname + .as_ref() + .map_or(false, |asname| asname == &alias.name); + if is_explicit_reexport { + self.semantic_model.add_local_reference( + binding_id, + alias.range(), + ReferenceContext::Synthetic, + ); + } } if self.enabled(Rule::RelativeImports) { @@ -1889,10 +1909,8 @@ where name, Binding { kind: BindingKind::FunctionDefinition, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: stmt.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -1914,10 +1932,8 @@ where { let id = self.semantic_model.bindings.push(Binding { kind: BindingKind::Assignment, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: stmt.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -1979,10 +1995,8 @@ where { let id = self.semantic_model.bindings.push(Binding { kind: BindingKind::Assignment, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: stmt.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -2167,10 +2181,8 @@ where name, Binding { kind: BindingKind::ClassDefinition, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: stmt.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4043,7 +4055,7 @@ where let scope = self.semantic_model.scope_mut(); &scope.remove(name) } { - if !self.semantic_model.bindings[*binding_id].used() { + if !self.semantic_model.is_used(*binding_id) { if self.enabled(Rule::UnusedVariable) { let mut diagnostic = Diagnostic::new( pyflakes::rules::UnusedVariable { name: name.into() }, @@ -4130,10 +4142,8 @@ where &arg.arg, Binding { kind: BindingKind::Argument, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: arg.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4179,10 +4189,8 @@ where name, Binding { kind: BindingKind::Assignment, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: pattern.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4310,7 +4318,7 @@ impl<'a> Checker<'a> { } /// Add a [`Binding`] to the current scope, bound to the given name. - fn add_binding(&mut self, name: &'a str, binding: Binding<'a>) { + fn add_binding(&mut self, name: &'a str, binding: Binding<'a>) -> BindingId { let binding_id = self.semantic_model.bindings.next_id(); if let Some((stack_index, existing_binding_id)) = self .semantic_model @@ -4351,7 +4359,7 @@ impl<'a> Checker<'a> { )); } } else if in_current_scope { - if !existing.used() + if !existing.is_used() && binding.redefines(existing) && (!self.settings.dummy_variable_rgx.is_match(name) || existing_is_import) && !(existing.kind.is_function_definition() @@ -4366,7 +4374,7 @@ impl<'a> Checker<'a> { #[allow(deprecated)] let line = self.locator.compute_line_index( existing - .trimmed_range(self.semantic_model(), self.locator) + .trimmed_range(&self.semantic_model, self.locator) .start(), ); @@ -4375,7 +4383,7 @@ impl<'a> Checker<'a> { name: name.to_string(), line, }, - binding.trimmed_range(self.semantic_model(), self.locator), + binding.trimmed_range(&self.semantic_model, self.locator), ); if let Some(parent) = binding.source { let parent = self.semantic_model.stmts[parent]; @@ -4423,17 +4431,13 @@ impl<'a> Checker<'a> { // If the original binding was a global or nonlocal, and the new binding conflicts within // the current scope, then the new binding is also as the same. Binding { - runtime_usage: existing.runtime_usage, - synthetic_usage: existing.synthetic_usage, - typing_usage: existing.typing_usage, + references: existing.references.clone(), kind: kind.clone(), ..binding } } _ => Binding { - runtime_usage: existing.runtime_usage, - synthetic_usage: existing.synthetic_usage, - typing_usage: existing.typing_usage, + references: existing.references.clone(), ..binding }, } @@ -4444,37 +4448,42 @@ impl<'a> Checker<'a> { // Don't treat annotations as assignments if there is an existing value // in scope. if binding.kind.is_annotation() && scope.defines(name) { - self.semantic_model.bindings.push(binding); - return; + return self.semantic_model.bindings.push(binding); } // Add the binding to the scope. scope.add(name, binding_id); // Add the binding to the arena. - self.semantic_model.bindings.push(binding); + self.semantic_model.bindings.push(binding) } fn bind_builtins(&mut self) { - let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; - for builtin in BUILTINS .iter() .chain(MAGIC_GLOBALS.iter()) .copied() .chain(self.settings.builtins.iter().map(String::as_str)) { - let id = self.semantic_model.bindings.push(Binding { + // Add the builtin to the scope. + let binding_id = self.semantic_model.bindings.push(Binding { kind: BindingKind::Builtin, range: TextRange::default(), - runtime_usage: None, - synthetic_usage: Some((ScopeId::global(), TextRange::default())), - typing_usage: None, source: None, + references: Vec::new(), context: ExecutionContext::Runtime, exceptions: Exceptions::empty(), }); - scope.add(builtin, id); + + // Add a reference to the builtin. + self.semantic_model.add_global_reference( + binding_id, + TextRange::default(), + ReferenceContext::Synthetic, + ); + + let scope = &mut self.semantic_model.scopes[self.semantic_model.scope_id]; + scope.add(builtin, binding_id); } } @@ -4587,10 +4596,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::Annotation, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4604,10 +4611,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::LoopVar, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4621,10 +4626,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::Binding, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4702,10 +4705,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::Export(Export { names: all_names }), - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4724,10 +4725,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::NamedExprAssignment, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4740,10 +4739,8 @@ impl<'a> Checker<'a> { id, Binding { kind: BindingKind::Assignment, - runtime_usage: None, - synthetic_usage: None, - typing_usage: None, range: expr.range(), + references: Vec::new(), source: self.semantic_model.stmt_id, context: self.semantic_model.execution_context(), exceptions: self.semantic_model.exceptions(), @@ -4985,10 +4982,10 @@ impl<'a> Checker<'a> { if let Some((bindings, range)) = all_bindings { for binding_id in bindings { - self.semantic_model.bindings[binding_id].mark_used( - ScopeId::global(), + self.semantic_model.add_global_reference( + binding_id, range, - ExecutionContext::Runtime, + ReferenceContext::Runtime, ); } } @@ -5019,7 +5016,10 @@ impl<'a> Checker<'a> { .binding_ids() .map(|binding_id| &self.semantic_model.bindings[*binding_id]) .filter(|binding| { - flake8_type_checking::helpers::is_valid_runtime_import(binding) + flake8_type_checking::helpers::is_valid_runtime_import( + &self.semantic_model, + binding, + ) }) .collect() }) @@ -5111,7 +5111,7 @@ impl<'a> Checker<'a> { | BindingKind::SubmoduleImportation(..) | BindingKind::FutureImportation ) { - if binding.used() { + if binding.is_used() { continue; } @@ -5123,7 +5123,7 @@ impl<'a> Checker<'a> { #[allow(deprecated)] let line = self.locator.compute_line_index( binding - .trimmed_range(self.semantic_model(), self.locator) + .trimmed_range(&self.semantic_model, self.locator) .start(), ); @@ -5132,7 +5132,7 @@ impl<'a> Checker<'a> { name: (*name).to_string(), line, }, - rebound.trimmed_range(self.semantic_model(), self.locator), + rebound.trimmed_range(&self.semantic_model, self.locator), ); if let Some(source) = rebound.source { let parent = &self.semantic_model.stmts[source]; @@ -5164,7 +5164,10 @@ impl<'a> Checker<'a> { let binding = &self.semantic_model.bindings[*binding_id]; if let Some(diagnostic) = - flake8_type_checking::rules::runtime_import_in_type_checking_block(binding) + flake8_type_checking::rules::runtime_import_in_type_checking_block( + binding, + &self.semantic_model, + ) { if self.enabled(diagnostic.kind.rule()) { diagnostics.push(diagnostic); @@ -5174,6 +5177,7 @@ impl<'a> Checker<'a> { flake8_type_checking::rules::typing_only_runtime_import( binding, &runtime_imports, + &self.semantic_model, self.package, self.settings, ) @@ -5198,6 +5202,10 @@ impl<'a> Checker<'a> { for binding_id in scope.binding_ids() { let binding = &self.semantic_model.bindings[*binding_id]; + if binding.is_used() { + continue; + } + let full_name = match &binding.kind { BindingKind::Importation(Importation { full_name, .. }) => full_name, BindingKind::FromImportation(FromImportation { full_name, .. }) => { @@ -5210,10 +5218,6 @@ impl<'a> Checker<'a> { _ => continue, }; - if binding.used() { - continue; - } - let child_id = binding.source.unwrap(); let parent_id = self.semantic_model.stmts.parent_id(child_id); diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index d365b8cf57..bbd6058181 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -158,7 +158,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, if scope .bindings_for_name(name) .map(|binding_id| &checker.semantic_model().bindings[*binding_id]) - .all(|binding| !binding.used()) + .all(|binding| !binding.is_used()) { #[allow(deprecated)] diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( diff --git a/crates/ruff/src/rules/flake8_type_checking/helpers.rs b/crates/ruff/src/rules/flake8_type_checking/helpers.rs index 2fda263bab..c9b54ed3c5 100644 --- a/crates/ruff/src/rules/flake8_type_checking/helpers.rs +++ b/crates/ruff/src/rules/flake8_type_checking/helpers.rs @@ -8,7 +8,7 @@ use ruff_python_semantic::model::SemanticModel; use ruff_python_semantic::scope::ScopeKind; /// Return `true` if [`Expr`] is a guard for a type-checking block. -pub(crate) fn is_type_checking_block(model: &SemanticModel, test: &Expr) -> bool { +pub(crate) fn is_type_checking_block(semantic_model: &SemanticModel, test: &Expr) -> bool { // Ex) `if False:` if matches!( test, @@ -32,50 +32,60 @@ pub(crate) fn is_type_checking_block(model: &SemanticModel, test: &Expr) -> bool } // Ex) `if typing.TYPE_CHECKING:` - if model.resolve_call_path(test).map_or(false, |call_path| { - call_path.as_slice() == ["typing", "TYPE_CHECKING"] - }) { + if semantic_model + .resolve_call_path(test) + .map_or(false, |call_path| { + call_path.as_slice() == ["typing", "TYPE_CHECKING"] + }) + { return true; } false } -pub(crate) const fn is_valid_runtime_import(binding: &Binding) -> bool { +pub(crate) fn is_valid_runtime_import(semantic_model: &SemanticModel, binding: &Binding) -> bool { if matches!( binding.kind, BindingKind::Importation(..) | BindingKind::FromImportation(..) | BindingKind::SubmoduleImportation(..) ) { - binding.runtime_usage.is_some() && matches!(binding.context, ExecutionContext::Runtime) + matches!(binding.context, ExecutionContext::Runtime) + && binding.references().any(|reference_id| { + semantic_model + .references + .resolve(reference_id) + .context() + .is_runtime() + }) } else { false } } pub(crate) fn runtime_evaluated( - model: &SemanticModel, + semantic_model: &SemanticModel, base_classes: &[String], decorators: &[String], ) -> bool { if !base_classes.is_empty() { - if runtime_evaluated_base_class(model, base_classes) { + if runtime_evaluated_base_class(semantic_model, base_classes) { return true; } } if !decorators.is_empty() { - if runtime_evaluated_decorators(model, decorators) { + if runtime_evaluated_decorators(semantic_model, decorators) { return true; } } false } -fn runtime_evaluated_base_class(model: &SemanticModel, base_classes: &[String]) -> bool { - if let ScopeKind::Class(class_def) = &model.scope().kind { +fn runtime_evaluated_base_class(semantic_model: &SemanticModel, base_classes: &[String]) -> bool { + if let ScopeKind::Class(class_def) = &semantic_model.scope().kind { for base in class_def.bases.iter() { - if let Some(call_path) = model.resolve_call_path(base) { + if let Some(call_path) = semantic_model.resolve_call_path(base) { if base_classes .iter() .any(|base_class| from_qualified_name(base_class) == call_path) @@ -88,10 +98,10 @@ fn runtime_evaluated_base_class(model: &SemanticModel, base_classes: &[String]) false } -fn runtime_evaluated_decorators(model: &SemanticModel, decorators: &[String]) -> bool { - if let ScopeKind::Class(class_def) = &model.scope().kind { +fn runtime_evaluated_decorators(semantic_model: &SemanticModel, decorators: &[String]) -> bool { + if let ScopeKind::Class(class_def) = &semantic_model.scope().kind { for decorator in class_def.decorator_list.iter() { - if let Some(call_path) = model.resolve_call_path(map_callable(decorator)) { + if let Some(call_path) = semantic_model.resolve_call_path(map_callable(decorator)) { if decorators .iter() .any(|decorator| from_qualified_name(decorator) == call_path) diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index 81eb309dc0..95a2b6af95 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -1,8 +1,9 @@ use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_semantic::binding::{ - Binding, BindingKind, ExecutionContext, FromImportation, Importation, SubmoduleImportation, + Binding, BindingKind, FromImportation, Importation, SubmoduleImportation, }; +use ruff_python_semantic::model::SemanticModel; /// ## What it does /// Checks for runtime imports defined in a type-checking block. @@ -51,7 +52,10 @@ impl Violation for RuntimeImportInTypeCheckingBlock { } /// TCH004 -pub(crate) fn runtime_import_in_type_checking_block(binding: &Binding) -> Option { +pub(crate) fn runtime_import_in_type_checking_block( + binding: &Binding, + semantic_model: &SemanticModel, +) -> Option { let full_name = match &binding.kind { BindingKind::Importation(Importation { full_name, .. }) => full_name, BindingKind::FromImportation(FromImportation { full_name, .. }) => full_name.as_str(), @@ -59,7 +63,15 @@ pub(crate) fn runtime_import_in_type_checking_block(binding: &Binding) -> Option _ => return None, }; - if matches!(binding.context, ExecutionContext::Typing) && binding.runtime_usage.is_some() { + if binding.context.is_typing() + && binding.references().any(|reference_id| { + semantic_model + .references + .resolve(reference_id) + .context() + .is_runtime() + }) + { Some(Diagnostic::new( RuntimeImportInTypeCheckingBlock { full_name: full_name.to_string(), diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 64b01fe628..df32a2dcbb 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -3,8 +3,9 @@ use std::path::Path; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_semantic::binding::{ - Binding, BindingKind, ExecutionContext, FromImportation, Importation, SubmoduleImportation, + Binding, BindingKind, FromImportation, Importation, SubmoduleImportation, }; +use ruff_python_semantic::model::SemanticModel; use crate::rules::isort::{categorize, ImportSection, ImportType}; use crate::settings::Settings; @@ -243,6 +244,7 @@ fn is_exempt(name: &str, exempt_modules: &[&str]) -> bool { pub(crate) fn typing_only_runtime_import( binding: &Binding, runtime_imports: &[&Binding], + semantic_model: &SemanticModel, package: Option<&Path>, settings: &Settings, ) -> Option { @@ -275,10 +277,15 @@ pub(crate) fn typing_only_runtime_import( return None; } - if matches!(binding.context, ExecutionContext::Runtime) - && binding.typing_usage.is_some() - && binding.runtime_usage.is_none() - && binding.synthetic_usage.is_none() + if binding.context.is_runtime() + && binding.is_used() + && binding.references().all(|reference_id| { + semantic_model + .references + .resolve(reference_id) + .context() + .is_typing() + }) { // Extract the module base and level from the full name. // Ex) `foo.bar.baz` -> `foo`, `0` diff --git a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs b/crates/ruff/src/rules/flake8_unused_arguments/rules.rs index 246763abf8..4188f8d39f 100644 --- a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs +++ b/crates/ruff/src/rules/flake8_unused_arguments/rules.rs @@ -262,8 +262,8 @@ fn call<'a>( .get(arg.arg.as_str()) .map(|binding_id| &bindings[*binding_id]) { - if !binding.used() - && binding.kind.is_argument() + if binding.kind.is_argument() + && !binding.is_used() && !dummy_variable_rgx.is_match(arg.arg.as_str()) { diagnostics.push(Diagnostic::new( diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs index 2df6f58f6a..dea25fd715 100644 --- a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs +++ b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs @@ -46,11 +46,19 @@ pub(crate) fn undefined_local(checker: &mut Checker, name: &str) { .map(|binding_id| &checker.semantic_model().bindings[*binding_id]) { // And has already been accessed in the current scope... - if let Some((scope_id, location)) = binding.runtime_usage { - if scope_id == checker.semantic_model().scope_id { - // Then it's probably an error. - return Some(location); + if let Some(range) = binding.references().find_map(|reference_id| { + let reference = checker.semantic_model().references.resolve(reference_id); + if checker + .semantic_model() + .is_current_scope(reference.scope_id()) + { + Some(reference.range()) + } else { + None } + }) { + // Then it's probably an error. + return Some(range); } } diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs b/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs index 62b2093bc8..5db7a2f5aa 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs @@ -27,8 +27,8 @@ pub(crate) fn unused_annotation(checker: &mut Checker, scope: ScopeId) { let name = *name; let binding = &checker.semantic_model().bindings[*binding_id]; - if !binding.used() - && binding.kind.is_annotation() + if binding.kind.is_annotation() + && !binding.is_used() && !checker.settings.dummy_variable_rgx.is_match(name) { Some((name.to_string(), binding.range)) diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs index 932c318ebc..59b6fd1f5f 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs +++ b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs @@ -322,8 +322,8 @@ pub(crate) fn unused_variable(checker: &mut Checker, scope: ScopeId) { .bindings() .map(|(name, binding_id)| (*name, &checker.semantic_model().bindings[*binding_id])) .filter_map(|(name, binding)| { - if !binding.used() - && (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment()) + if (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment()) + && !binding.is_used() && !checker.settings.dummy_variable_rgx.is_match(name) && name != "__tracebackhide__" && name != "__traceback_info__" diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index 2e20ef7a8e..23802f4212 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -1,14 +1,15 @@ use std::ops::{Deref, DerefMut}; -use crate::model::SemanticModel; use bitflags::bitflags; +use ruff_text_size::TextRange; + use ruff_index::{newtype_index, IndexSlice, IndexVec}; use ruff_python_ast::helpers; use ruff_python_ast::source_code::Locator; -use ruff_text_size::TextRange; +use crate::model::SemanticModel; use crate::node::NodeId; -use crate::scope::ScopeId; +use crate::reference::ReferenceId; #[derive(Debug, Clone)] pub struct Binding<'a> { @@ -18,33 +19,20 @@ pub struct Binding<'a> { pub context: ExecutionContext, /// The statement in which the [`Binding`] was defined. pub source: Option, - /// Tuple of (scope index, range) indicating the scope and range at which - /// the binding was last used in a runtime context. - pub runtime_usage: Option<(ScopeId, TextRange)>, - /// Tuple of (scope index, range) indicating the scope and range at which - /// the binding was last used in a typing-time context. - pub typing_usage: Option<(ScopeId, TextRange)>, - /// Tuple of (scope index, range) indicating the scope and range at which - /// the binding was last used in a synthetic context. This is used for - /// (e.g.) `__future__` imports, explicit re-exports, and other bindings - /// that should be considered used even if they're never referenced. - pub synthetic_usage: Option<(ScopeId, TextRange)>, + /// The references to the binding. + pub references: Vec, /// The exceptions that were handled when the binding was defined. pub exceptions: Exceptions, } impl<'a> Binding<'a> { - pub fn mark_used(&mut self, scope: ScopeId, range: TextRange, context: ExecutionContext) { - match context { - ExecutionContext::Runtime => self.runtime_usage = Some((scope, range)), - ExecutionContext::Typing => self.typing_usage = Some((scope, range)), - } + pub fn is_used(&self) -> bool { + !self.references.is_empty() } - pub const fn used(&self) -> bool { - self.runtime_usage.is_some() - || self.synthetic_usage.is_some() - || self.typing_usage.is_some() + /// Returns an iterator over all references for the current [`Binding`]. + pub fn references(&self) -> impl Iterator + '_ { + self.references.iter().copied() } pub const fn is_definition(&self) -> bool { @@ -266,7 +254,7 @@ bitflags! { } } -#[derive(Copy, Debug, Clone)] +#[derive(Copy, Debug, Clone, is_macro::Is)] pub enum ExecutionContext { Runtime, Typing, diff --git a/crates/ruff_python_semantic/src/lib.rs b/crates/ruff_python_semantic/src/lib.rs index 5270ccd5d1..60fc39f4a8 100644 --- a/crates/ruff_python_semantic/src/lib.rs +++ b/crates/ruff_python_semantic/src/lib.rs @@ -3,4 +3,5 @@ pub mod binding; pub mod definition; pub mod model; pub mod node; +pub mod reference; pub mod scope; diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 93c9c992a3..e70dd9bc37 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -18,6 +18,7 @@ use crate::binding::{ }; use crate::definition::{Definition, DefinitionId, Definitions, Member, Module}; use crate::node::{NodeId, Nodes}; +use crate::reference::{ReferenceContext, References}; use crate::scope::{Scope, ScopeId, ScopeKind, Scopes}; /// A semantic model for a Python module, to enable querying the module's semantic information. @@ -39,6 +40,8 @@ pub struct SemanticModel<'a> { pub definition_id: DefinitionId, // A stack of all bindings created in any scope, at any point in execution. pub bindings: Bindings<'a>, + // Stack of all references created in any scope, at any point in execution. + pub references: References, // Map from binding index to indexes of bindings that shadow it in other scopes. pub shadowed_bindings: HashMap, BuildNoHashHasher>, // Body iteration; used to peek at siblings. @@ -63,6 +66,7 @@ impl<'a> SemanticModel<'a> { definitions: Definitions::for_module(module), definition_id: DefinitionId::module(), bindings: Bindings::default(), + references: References::default(), shadowed_bindings: IntMap::default(), body: &[], body_index: 0, @@ -119,17 +123,33 @@ impl<'a> SemanticModel<'a> { // PEP 563 indicates that if a forward reference can be resolved in the module scope, we // should prefer it over local resolutions. if self.in_deferred_type_definition() { - if let Some(binding_id) = self.scopes.global().get(symbol) { + if let Some(binding_id) = self.scopes.global().get(symbol).copied() { // Mark the binding as used. let context = self.execution_context(); - self.bindings[*binding_id].mark_used(ScopeId::global(), range, context); + let reference_id = self.references.push( + ScopeId::global(), + range, + match context { + ExecutionContext::Runtime => ReferenceContext::Runtime, + ExecutionContext::Typing => ReferenceContext::Typing, + }, + ); + self.bindings[binding_id].references.push(reference_id); // Mark any submodule aliases as used. - if let Some(binding_id) = self.resolve_submodule(ScopeId::global(), *binding_id) { - self.bindings[binding_id].mark_used(ScopeId::global(), range, context); + if let Some(binding_id) = self.resolve_submodule(ScopeId::global(), binding_id) { + let reference_id = self.references.push( + ScopeId::global(), + range, + match context { + ExecutionContext::Runtime => ReferenceContext::Runtime, + ExecutionContext::Typing => ReferenceContext::Typing, + }, + ); + self.bindings[binding_id].references.push(reference_id); } - return ResolvedReference::Resolved(*binding_id); + return ResolvedReference::Resolved(binding_id); } } @@ -153,14 +173,30 @@ impl<'a> SemanticModel<'a> { } } - if let Some(binding_id) = scope.get(symbol) { + if let Some(binding_id) = scope.get(symbol).copied() { // Mark the binding as used. let context = self.execution_context(); - self.bindings[*binding_id].mark_used(self.scope_id, range, context); + let reference_id = self.references.push( + self.scope_id, + range, + match context { + ExecutionContext::Runtime => ReferenceContext::Runtime, + ExecutionContext::Typing => ReferenceContext::Typing, + }, + ); + self.bindings[binding_id].references.push(reference_id); // Mark any submodule aliases as used. - if let Some(binding_id) = self.resolve_submodule(scope_id, *binding_id) { - self.bindings[binding_id].mark_used(ScopeId::global(), range, context); + if let Some(binding_id) = self.resolve_submodule(scope_id, binding_id) { + let reference_id = self.references.push( + self.scope_id, + range, + match context { + ExecutionContext::Runtime => ReferenceContext::Runtime, + ExecutionContext::Typing => ReferenceContext::Typing, + }, + ); + 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 @@ -174,12 +210,12 @@ 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_deferred_type_definition() - && self.bindings[*binding_id].kind.is_annotation() + && self.bindings[binding_id].kind.is_annotation() { continue; } - return ResolvedReference::Resolved(*binding_id); + return ResolvedReference::Resolved(binding_id); } // Allow usages of `__module__` and `__qualname__` within class scopes, e.g.: @@ -343,8 +379,8 @@ impl<'a> SemanticModel<'a> { member: &str, ) -> Option<(&Stmt, String)> { self.scopes().enumerate().find_map(|(scope_index, scope)| { - scope.binding_ids().find_map(|binding_index| { - let binding = &self.bindings[*binding_index]; + scope.binding_ids().copied().find_map(|binding_id| { + let binding = &self.bindings[binding_id]; match &binding.kind { // Ex) Given `module="sys"` and `object="exit"`: // `import sys` -> `sys.exit` @@ -519,11 +555,17 @@ impl<'a> SemanticModel<'a> { self.scopes.ancestors(self.scope_id) } + /// Returns an iterator over all parent statements. pub fn parents(&self) -> impl Iterator + '_ { let node_id = self.stmt_id.expect("No current statement"); self.stmts.ancestor_ids(node_id).map(|id| self.stmts[id]) } + /// Return `true` if the given [`ScopeId`] matches that of the current scope. + pub fn is_current_scope(&self, scope_id: ScopeId) -> bool { + self.scope_id == scope_id + } + /// Return `true` if the context is at the top level of the module (i.e., in the module scope, /// and not nested within any statements). pub fn at_top_level(&self) -> bool { @@ -533,6 +575,33 @@ impl<'a> SemanticModel<'a> { .map_or(true, |stmt_id| self.stmts.parent_id(stmt_id).is_none()) } + /// Returns `true` if the given [`BindingId`] is used. + pub fn is_used(&self, binding_id: BindingId) -> bool { + self.bindings[binding_id].is_used() + } + + /// Add a reference to the given [`BindingId`] in the local scope. + pub fn add_local_reference( + &mut self, + binding_id: BindingId, + range: TextRange, + context: ReferenceContext, + ) { + let reference_id = self.references.push(self.scope_id, range, context); + self.bindings[binding_id].references.push(reference_id); + } + + /// Add a reference to the given [`BindingId`] in the global scope. + pub fn add_global_reference( + &mut self, + binding_id: BindingId, + range: TextRange, + context: ReferenceContext, + ) { + let reference_id = self.references.push(ScopeId::global(), range, context); + self.bindings[binding_id].references.push(reference_id); + } + /// Return the [`ExecutionContext`] of the current scope. pub const fn execution_context(&self) -> ExecutionContext { if self.in_type_checking_block() diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs new file mode 100644 index 0000000000..2ffa51cfe4 --- /dev/null +++ b/crates/ruff_python_semantic/src/reference.rs @@ -0,0 +1,70 @@ +use ruff_text_size::TextRange; + +use ruff_index::{newtype_index, IndexVec}; + +use crate::scope::ScopeId; + +#[derive(Debug, Clone)] +pub struct Reference { + /// The scope in which the reference is defined. + scope_id: ScopeId, + /// The range of the reference in the source code. + range: TextRange, + /// The context in which the reference occurs. + context: ReferenceContext, +} + +impl Reference { + pub const fn scope_id(&self) -> ScopeId { + self.scope_id + } + + pub const fn range(&self) -> TextRange { + self.range + } + + pub const fn context(&self) -> &ReferenceContext { + &self.context + } +} + +#[derive(Debug, Clone, is_macro::Is)] +pub enum ReferenceContext { + /// The reference occurs in a runtime context. + Runtime, + /// The reference occurs in a typing-only context. + Typing, + /// The reference occurs in a synthetic context, used for `__future__` imports, explicit + /// re-exports, and other bindings that should be considered used even if they're never + /// "referenced". + Synthetic, +} + +/// Id uniquely identifying a read reference in a program. +#[newtype_index] +pub struct ReferenceId; + +/// The references of a program indexed by [`ReferenceId`]. +#[derive(Debug, Default)] +pub struct References(IndexVec); + +impl References { + /// Pushes a new read reference and returns its unique id. + pub fn push( + &mut self, + scope_id: ScopeId, + range: TextRange, + context: ReferenceContext, + ) -> ReferenceId { + self.0.push(Reference { + scope_id, + range, + context, + }) + } + + /// Returns the [`Reference`] with the given id. + pub fn resolve(&self, id: ReferenceId) -> &Reference { + &self.0[id] + } +}