diff --git a/crates/ruff_linter/resources/test/fixtures/airflow/AIR201.py b/crates/ruff_linter/resources/test/fixtures/airflow/AIR201.py index c386125034b23..233252ad25cd4 100644 --- a/crates/ruff_linter/resources/test/fixtures/airflow/AIR201.py +++ b/crates/ruff_linter/resources/test/fixtures/airflow/AIR201.py @@ -114,6 +114,26 @@ def extract_data(): bash_command="{{ ti.xcom_pull(task_ids='unknown_task') }}", # AIR201 (no fix) ) +# Referencing a builtin does not imply that a task variable is visible (no fix) +task_26 = PythonOperator( + task_id="task_26", + op_args="{{ ti.xcom_pull(task_ids='len') }}", # AIR201 (no fix) +) + +# The same is true if the builtin was materialized by an earlier load +print(max) +task_27 = PythonOperator( + task_id="task_27", + op_args="{{ ti.xcom_pull(task_ids='max') }}", # AIR201 (no fix) +) + +# A variable that shadows a builtin is still eligible for a fix +len = PythonOperator(task_id="len", python_callable=my_callable) +task_28 = PythonOperator( + task_id="task_28", + op_args="{{ ti.xcom_pull(task_ids='len') }}", # AIR201 (fix: len.output) +) + # Cases that should NOT trigger AIR201: diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py index 83ee814b6e124..156d18675e934 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE808.py @@ -17,3 +17,11 @@ # regression test for https://github.com/astral-sh/ruff/pull/18805 range((0), 42) + + +# A pre-scanned global declaration should not shadow the builtin. +def f(): + global range + + +range(0, 3) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 7b8119f12dcb8..cdfa6c1bc4a78 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -59,7 +59,6 @@ use ruff_python_semantic::{ Import, Module, ModuleKind, ModuleSource, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, StarImport, SubmoduleImport, }; -use ruff_python_stdlib::builtins::{python_builtins, python_magic_globals}; use ruff_python_trivia::CommentRanges; use ruff_source_file::{OneIndexed, SourceFile, SourceFileBuilder, SourceRow}; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -272,7 +271,14 @@ impl<'a> Checker<'a> { target_version: TargetVersion, context: &'a LintContext<'a>, ) -> Self { - let semantic = SemanticModel::new(&settings.typing_modules, path, module); + let semantic = SemanticModel::new( + &settings.typing_modules, + &settings.builtins, + target_version.linter_version(), + source_type, + path, + module, + ); Self { parsed, parsed_type_annotation: None, @@ -1184,7 +1190,7 @@ impl<'a> Visitor<'a> for Checker<'a> { node_index: _, }) if !self.semantic.scope_id.is_global() => { for name in names { - let binding_id = self.semantic.global_scope().get(name); + let binding_id = self.semantic.global_binding(name); // Mark the binding in the global scope as "rebound" in the current scope. if let Some(binding_id) = binding_id { @@ -1194,6 +1200,7 @@ impl<'a> Visitor<'a> for Checker<'a> { // Add a binding to the current scope. let binding_id = self.semantic.push_binding( + name, name.range(), BindingKind::Global(binding_id), BindingFlags::GLOBAL, @@ -1224,6 +1231,7 @@ impl<'a> Visitor<'a> for Checker<'a> { // Add a binding to the current scope. let binding_id = self.semantic.push_binding( + name, name.range(), BindingKind::Nonlocal(binding_id, scope_id), BindingFlags::NONLOCAL, @@ -1292,6 +1300,7 @@ impl<'a> Visitor<'a> for Checker<'a> { let added_dunder_class_scope = if self.semantic.current_scope().kind.is_class() { self.semantic.push_scope(ScopeKind::DunderClassCell); let binding_id = self.semantic.push_binding( + "__class__", TextRange::default(), BindingKind::DunderClassCell, BindingFlags::empty(), @@ -2285,7 +2294,7 @@ impl<'a> Visitor<'a> for Checker<'a> { }) => { if let Some(name) = name { // Store the existing binding, if any. - let binding_id = self.semantic.lookup_symbol(name.as_str()); + let binding_id = self.semantic.lookup_binding(name.as_str()); // Add the bound exception name to the scope. self.add_binding( @@ -2694,7 +2703,7 @@ impl<'a> Checker<'a> { } // Create the `Binding`. - let binding_id = self.semantic.push_binding(range, kind, flags); + let binding_id = self.semantic.push_binding(name, range, kind, flags); // If the name is private, mark is as such. if name.starts_with('_') { @@ -2754,35 +2763,7 @@ impl<'a> Checker<'a> { binding_id } - fn bind_builtins(&mut self) { - let target_version = self.target_version(); - let settings = self.settings(); - let builtin_count = python_builtins(target_version.minor, self.source_type.is_ipynb()) - .count() - + python_magic_globals(target_version.minor).count() - + settings.builtins.len(); - - self.semantic.reserve_builtin_bindings(builtin_count); - - let mut bind_builtin = |builtin| { - // Add the builtin to the scope. - let binding_id = self.semantic.push_builtin(); - let scope = self.semantic.global_scope_mut(); - scope.add(builtin, binding_id); - }; - let standard_builtins = python_builtins(target_version.minor, self.source_type.is_ipynb()); - for builtin in standard_builtins { - bind_builtin(builtin); - } - for builtin in python_magic_globals(target_version.minor) { - bind_builtin(builtin); - } - for builtin in &settings.builtins { - bind_builtin(builtin); - } - } - - fn handle_node_load(&mut self, expr: &Expr) { + fn handle_node_load(&mut self, expr: &'a Expr) { let Expr::Name(expr) = expr else { return; }; @@ -2920,9 +2901,12 @@ impl<'a> Checker<'a> { } // Create a binding to model the deletion. - let binding_id = - self.semantic - .push_binding(expr.range(), BindingKind::Deletion, BindingFlags::empty()); + let binding_id = self.semantic.push_binding( + id, + expr.range(), + BindingKind::Deletion, + BindingFlags::empty(), + ); let scope = self.semantic.current_scope_mut(); scope.add(id, binding_id); } @@ -3203,6 +3187,7 @@ impl<'a> Checker<'a> { { self.semantic.push_scope(ScopeKind::DunderClassCell); let binding_id = self.semantic.push_binding( + "__class__", TextRange::default(), BindingKind::DunderClassCell, BindingFlags::empty(), @@ -3260,7 +3245,7 @@ impl<'a> Checker<'a> { for definition in definitions { for export in definition.names() { let (name, range) = (export.name(), export.range()); - if let Some(binding_id) = self.semantic.global_scope().get(name) { + if let Some(binding_id) = self.semantic.global_binding(name) { self.semantic.flags |= SemanticModelFlags::DUNDER_ALL_DEFINITION; // Mark anything referenced in `__all__` as used. self.semantic @@ -3394,8 +3379,6 @@ pub(crate) fn check_ast( target_version, context, ); - checker.bind_builtins(); - // Iterate over the AST. checker.visit_module(parsed.suite()); checker.visit_body(parsed.suite()); diff --git a/crates/ruff_linter/src/rules/airflow/helpers.rs b/crates/ruff_linter/src/rules/airflow/helpers.rs index 73af54872c7ab..d16587c374671 100644 --- a/crates/ruff_linter/src/rules/airflow/helpers.rs +++ b/crates/ruff_linter/src/rules/airflow/helpers.rs @@ -101,7 +101,7 @@ pub(crate) fn is_guarded_by_try_except( try_block_contains_undeprecated_attribute(try_node, module, name, semantic) } Expr::Name(ExprName { id, .. }) => { - let Some(binding_id) = semantic.lookup_symbol(id.as_str()) else { + let Some(binding_id) = semantic.lookup_symbol(id.as_str()).binding_id() else { return false; }; let binding = semantic.binding(binding_id); @@ -247,7 +247,7 @@ pub(crate) fn generate_remove_and_runtime_import_edit( let semantic = checker.semantic(); let binding = semantic .resolve_name(head) - .or_else(|| checker.semantic().lookup_symbol(&head.id)) + .or_else(|| checker.semantic().lookup_symbol(&head.id).binding_id()) .map(|id| checker.semantic().binding(id))?; let stmt = binding.statement(semantic)?; let remove_edit = remove_unused_imports( diff --git a/crates/ruff_linter/src/rules/airflow/rules/task_implicit_multiple_outputs.rs b/crates/ruff_linter/src/rules/airflow/rules/task_implicit_multiple_outputs.rs index 7f18047e52aa7..e718d3647190c 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/task_implicit_multiple_outputs.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/task_implicit_multiple_outputs.rs @@ -221,7 +221,7 @@ fn annotation_is_typed_dict_subclass(annotation: &Expr, semantic: &SemanticModel }; let Some(binding_id) = semantic .resolve_name(name) - .or_else(|| semantic.lookup_symbol(&name.id)) + .or_else(|| semantic.lookup_symbol(&name.id).binding_id()) else { return false; }; diff --git a/crates/ruff_linter/src/rules/airflow/rules/xcom_pull_in_template_string.rs b/crates/ruff_linter/src/rules/airflow/rules/xcom_pull_in_template_string.rs index c85d7319c227d..bc2d72ffb5ef7 100644 --- a/crates/ruff_linter/src/rules/airflow/rules/xcom_pull_in_template_string.rs +++ b/crates/ruff_linter/src/rules/airflow/rules/xcom_pull_in_template_string.rs @@ -114,7 +114,12 @@ pub(crate) fn xcom_pull_in_template_string(checker: &Checker, call: &ast::ExprCa // If the task_id matches a variable in scope, provide an unsafe fix // replacing the template string with `.output`. - if checker.semantic().lookup_symbol(&task_id).is_some() { + if checker + .semantic() + .lookup_symbol(&task_id) + .binding_id() + .is_some_and(|binding_id| !checker.semantic().binding(binding_id).kind.is_builtin()) + { diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( format!("{task_id}.output"), arg_value.range(), diff --git a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR201_AIR201.py.snap b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR201_AIR201.py.snap index f9febbd582256..d30b8dcab2360 100644 --- a/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR201_AIR201.py.snap +++ b/crates/ruff_linter/src/rules/airflow/snapshots/ruff_linter__rules__airflow__tests__AIR201_AIR201.py.snap @@ -281,3 +281,43 @@ AIR201 Use the `.output` attribute on the task object for "unknown_task" instead 115 | ) | help: Replace with `unknown_task.output` + +AIR201 Use the `.output` attribute on the task object for "len" instead of `xcom_pull` in a template string + --> AIR201.py:120:13 + | +118 | task_26 = PythonOperator( +119 | task_id="task_26", +120 | op_args="{{ ti.xcom_pull(task_ids='len') }}", # AIR201 (no fix) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +121 | ) + | +help: Replace with `len.output` + +AIR201 Use the `.output` attribute on the task object for "max" instead of `xcom_pull` in a template string + --> AIR201.py:127:13 + | +125 | task_27 = PythonOperator( +126 | task_id="task_27", +127 | op_args="{{ ti.xcom_pull(task_ids='max') }}", # AIR201 (no fix) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +128 | ) + | +help: Replace with `max.output` + +AIR201 [*] Use the `.output` attribute on the task object for "len" instead of `xcom_pull` in a template string + --> AIR201.py:134:13 + | +132 | task_28 = PythonOperator( +133 | task_id="task_28", +134 | op_args="{{ ti.xcom_pull(task_ids='len') }}", # AIR201 (fix: len.output) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +135 | ) + | +help: Replace with `len.output` + | +133 | task_id="task_28", + - op_args="{{ ti.xcom_pull(task_ids='len') }}", # AIR201 (fix: len.output) +134 + op_args=len.output, # AIR201 (fix: len.output) +135 | ) + | +note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs index 334ead958b7b2..6ae68e4abfbd7 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs @@ -241,7 +241,7 @@ fn is_bound_to_tuple(arg: &Expr, semantic: &SemanticModel) -> bool { return false; }; - let Some(binding_id) = semantic.lookup_symbol(id.as_str()) else { + let Some(binding_id) = semantic.lookup_symbol(id.as_str()).binding_id() else { return false; }; diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap index 0656f612c13a3..9f784595426c5 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE808_PIE808.py.snap @@ -47,4 +47,18 @@ help: Remove `start` argument 18 | # regression test for https://github.com/astral-sh/ruff/pull/18805 - range((0), 42) 19 + range(42) +20 | + | + +PIE808 [*] Unnecessary `start` argument in `range` + --> PIE808.py:27:7 + | +27 | range(0, 3) + | ^ + | +help: Remove `start` argument + | +26 | + - range(0, 3) +27 + range(3) | diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index 05bb13ccb7c0b..e695c55fa1ba4 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs @@ -109,7 +109,7 @@ pub(crate) fn runtime_import_in_type_checking_block(checker: &Checker, scope: &S let ignore_dunder_all_references = checker .semantic() .lookup_symbol_in_scope("__getattr__", ScopeId::global(), false) - .is_some(); + .is_bound(); for binding_id in scope.binding_ids() { let binding = checker.semantic().binding(binding_id); diff --git a/crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs b/crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs index ed519e343632f..efe8576b5184e 100644 --- a/crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs +++ b/crates/ruff_linter/src/rules/numpy/rules/numpy_2_0_deprecation.rs @@ -759,7 +759,7 @@ fn is_guarded_by_try_except( try_block_contains_undeprecated_attribute(try_node, &replacement.details, semantic) } Expr::Name(ast::ExprName { id, .. }) => { - let Some(binding_id) = semantic.lookup_symbol(id.as_str()) else { + let Some(binding_id) = semantic.lookup_symbol(id.as_str()).binding_id() else { return false; }; let binding = semantic.binding(binding_id); diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/non_lowercase_variable_in_function.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/non_lowercase_variable_in_function.rs index 31daccab06862..c171f23ea9b2c 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/non_lowercase_variable_in_function.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/non_lowercase_variable_in_function.rs @@ -60,6 +60,7 @@ pub(crate) fn non_lowercase_variable_in_function(checker: &Checker, range: TextR if checker .semantic() .lookup_symbol(name) + .binding_id() .is_some_and(|id| checker.semantic().binding(id).is_global()) { return; diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 13ff7e7d72967..3cfdb088fff30 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -1231,6 +1231,21 @@ mod tests { flakes("__builtins__", &[]); } + #[test] + fn builtin_after_exception_target_cleanup() { + flakes( + r" + try: + pass + except Exception as len: + pass + + print(len) + ", + &[Rule::UnusedVariable], + ); + } + #[test] fn magic_globals_name() { // Use of the C{__name__} magic global should not emit an undefined name diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index 1a89a337220bd..b64bcc0899727 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -966,6 +966,7 @@ fn has_simple_shadowed_bindings(scope: &Scope, id: BindingId, semantic: &Semanti fn symbol_used_in_dunder_all(semantic: &SemanticModel<'_>, binding: &ImportBinding) -> bool { semantic .lookup_symbol_in_scope(binding.symbol_stored_in_outer_scope(), binding.scope, false) + .binding_id() .is_some_and(|bdg| { semantic .binding(bdg) diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs index 8aee02b6ae084..51811924195c3 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs @@ -224,7 +224,7 @@ pub(crate) fn unnecessary_lambda(checker: &Checker, lambda: &ExprLambda) { // Suppress the fix if the assignment expression target shadows one of the lambda's parameters. // This is necessary to avoid introducing a change in the behavior of the program. for name in names { - if let Some(binding_id) = checker.semantic().lookup_symbol(name.id()) { + if let Some(binding_id) = checker.semantic().lookup_symbol(name.id()).binding_id() { let binding = checker.semantic().binding(binding_id); if checker .semantic() diff --git a/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs b/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs index 00aeb71ed83dc..674f0b0cfbdce 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/useless_exception_statement.rs @@ -100,7 +100,7 @@ fn is_custom_exception( let Some(symbol) = qualified_name.segments().last() else { return false; }; - let Some(binding_id) = semantic.lookup_symbol(symbol) else { + let Some(binding_id) = semantic.lookup_symbol(symbol).binding_id() else { return false; }; let binding = semantic.binding(binding_id); diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs index 645bf51a6f0d2..c2c6f00fdeb84 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/pep695/mod.rs @@ -284,6 +284,7 @@ pub(crate) fn expr_name_to_type_var<'a>( ) -> Option> { let StmtAssign { value, .. } = semantic .lookup_symbol(name.id.as_str()) + .binding_id() .and_then(|binding_id| semantic.binding(binding_id).source) .map(|node_id| semantic.statement(node_id))? .as_assign_stmt()?; diff --git a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs index 1d1e55d448de2..a53d878d8ab90 100644 --- a/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs +++ b/crates/ruff_linter/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs @@ -137,7 +137,7 @@ pub(crate) fn unnecessary_builtin_import( if &alias.name == "*" { return true; } - let Some(binding_id) = semantic.lookup_symbol(alias.name.as_str()) else { + let Some(binding_id) = semantic.lookup_symbol(alias.name.as_str()).binding_id() else { return false; }; let binding = semantic.binding(binding_id); diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index d76a1d85de099..916bba9bf2f52 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -1207,7 +1207,7 @@ pub fn resolve_assignment<'a>( /// This function will return a `NumberLiteral` with value `Int(42)` when called with `foo` and a /// `StringLiteral` with value `"str"` when called with `bla`. pub fn find_assigned_value<'a>(symbol: &str, semantic: &'a SemanticModel<'a>) -> Option<&'a Expr> { - let binding_id = semantic.lookup_symbol(symbol)?; + let binding_id = semantic.lookup_symbol(symbol).binding_id()?; let binding = semantic.binding(binding_id); find_binding_value(binding, semantic) } diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 0f4db749203e3..21e10500b1b35 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -5,7 +5,8 @@ use rustc_hash::FxHashMap; use ruff_python_ast::helpers::{from_relative_import, map_subscript}; use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; -use ruff_python_ast::{self as ast, Expr, ExprContext, PySourceType, Stmt}; +use ruff_python_ast::{self as ast, Expr, ExprContext, PySourceType, PythonVersion, Stmt}; +use ruff_python_stdlib::builtins::{is_python_builtin, python_builtins, python_magic_globals}; use ruff_text_size::{Ranged, TextRange, TextSize}; use crate::Imported; @@ -27,9 +28,38 @@ use crate::scope::{Scope, ScopeId, ScopeKind, Scopes}; pub mod all; +/// The result of looking up a symbol in a [`SemanticModel`]. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum Symbol { + /// The symbol resolves to a concrete binding. + Binding(BindingId), + /// The symbol resolves to a builtin that has not been materialized yet. + Builtin, + /// The symbol does not resolve to any binding. + Unbound, +} + +impl Symbol { + /// Returns the [`BindingId`] if this symbol resolves to a concrete binding. + pub const fn binding_id(self) -> Option { + match self { + Self::Binding(binding_id) => Some(binding_id), + Self::Builtin | Self::Unbound => None, + } + } + + /// Returns `true` if this symbol resolves to any binding, including a builtin. + pub const fn is_bound(self) -> bool { + !matches!(self, Self::Unbound) + } +} + /// A semantic model for a Python module, to enable querying the module's semantic information. pub struct SemanticModel<'a> { typing_modules: &'a [String], + custom_builtins: &'a [String], + target_version: PythonVersion, + source_type: PySourceType, module: Module<'a>, /// Stack of all AST nodes in the program. @@ -147,9 +177,19 @@ pub struct SemanticModel<'a> { } impl<'a> SemanticModel<'a> { - pub fn new(typing_modules: &'a [String], path: &Path, module: Module<'a>) -> Self { - Self { + pub fn new( + typing_modules: &'a [String], + custom_builtins: &'a [String], + target_version: PythonVersion, + source_type: PySourceType, + path: &Path, + module: Module<'a>, + ) -> Self { + let mut semantic = Self { typing_modules, + custom_builtins, + target_version, + source_type, module, nodes: Nodes::default(), node_id: None, @@ -170,7 +210,19 @@ impl<'a> SemanticModel<'a> { seen: Modules::empty(), handled_exceptions: Vec::default(), resolved_names: FxHashMap::default(), - } + }; + + let builtin_count = python_builtins(target_version.minor, source_type.is_ipynb()).count() + + python_magic_globals(target_version.minor).count() + + custom_builtins.len(); + // Match the capacity that repeated `push` calls would reach while avoiding the + // intermediate allocations. + semantic + .bindings + .reserve_exact(builtin_count.next_power_of_two()); + semantic.global_scope_mut().reserve_bindings(builtin_count); + + semantic } /// Return the [`Binding`] for the given [`BindingId`]. @@ -226,12 +278,32 @@ impl<'a> SemanticModel<'a> { .chain(self.typing_modules.iter().map(String::as_str)) } - /// Reserves capacity for builtin bindings. - pub fn reserve_builtin_bindings(&mut self, additional: usize) { - // Match the capacity that repeated `push` calls would reach while avoiding the - // intermediate allocations. - self.bindings.reserve_exact(additional.next_power_of_two()); - self.global_scope_mut().reserve_bindings(additional); + /// Returns `true` if `name` is provided by the configured Python runtime. + /// + /// This includes version- and source-type-specific builtins, magic globals, and custom + /// builtins. It does not account for bindings that shadow the name in a scope. + fn is_builtin_name(&self, name: &str) -> bool { + is_python_builtin(name, self.target_version.minor, self.source_type.is_ipynb()) + || python_magic_globals(self.target_version.minor).any(|builtin| builtin == name) + || self.custom_builtins.iter().any(|builtin| builtin == name) + } + + /// Creates and returns a concrete binding for an unmaterialized builtin. + fn materialize_builtin_binding(&mut self, name: &'a str) -> Option { + if let Some(binding_id) = self.global_scope().get(name) { + return self.bindings[binding_id] + .kind + .is_builtin() + .then_some(binding_id); + } + + if !self.is_builtin_name(name) { + return None; + } + + let binding_id = self.push_builtin(); + self.global_scope_mut().add(name, binding_id); + Some(binding_id) } /// Create a new [`Binding`] for a builtin. @@ -251,10 +323,13 @@ impl<'a> SemanticModel<'a> { /// Create a new [`Binding`] for the given `name` and `range`. pub fn push_binding( &mut self, + name: &'a str, range: TextRange, kind: BindingKind<'a>, flags: BindingFlags, ) -> BindingId { + self.materialize_builtin_binding(name); + self.bindings.push(Binding { range, kind, @@ -289,9 +364,11 @@ impl<'a> SemanticModel<'a> { /// module, e.g. `import builtins; builtins.open`. It *only* includes the bindings /// that are pre-populated in Python's global scope before any imports have taken place. pub fn has_builtin_binding_in_scope(&self, member: &str, scope: ScopeId) -> bool { - self.lookup_symbol_in_scope(member, scope, false) - .map(|binding_id| &self.bindings[binding_id]) - .is_some_and(|binding| binding.kind.is_builtin()) + match self.lookup_symbol_in_scope(member, scope, false) { + Symbol::Binding(binding_id) => self.bindings[binding_id].kind.is_builtin(), + Symbol::Builtin => true, + Symbol::Unbound => false, + } } /// If `expr` is a reference to a builtins symbol, @@ -351,13 +428,16 @@ impl<'a> SemanticModel<'a> { /// Return `true` if `member` is an "available" symbol in a given scope, i.e., /// a symbol that has not been bound in that current scope, or in any containing scope. pub fn is_available_in_scope(&self, member: &str, scope_id: ScopeId) -> bool { - self.lookup_symbol_in_scope(member, scope_id, false) - .map(|binding_id| &self.bindings[binding_id]) - .is_none_or(|binding| binding.kind.is_builtin()) + match self.lookup_symbol_in_scope(member, scope_id, false) { + Symbol::Binding(binding_id) => self.bindings[binding_id].kind.is_builtin(), + Symbol::Builtin | Symbol::Unbound => true, + } } /// Resolve a `del` reference to `symbol` at `range`. - pub fn resolve_del(&mut self, symbol: &str, range: TextRange) { + pub fn resolve_del(&mut self, symbol: &'a str, range: TextRange) { + self.materialize_builtin_binding(symbol); + let is_unbound = self.scopes[self.scope_id] .get(symbol) .is_none_or(|binding_id| { @@ -378,7 +458,7 @@ impl<'a> SemanticModel<'a> { } /// Resolve a `load` reference to an [`ast::ExprName`]. - pub fn resolve_load(&mut self, name: &ast::ExprName) -> ReadResult { + pub fn resolve_load(&mut self, name: &'a ast::ExprName) -> ReadResult { // 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_forward_reference() { @@ -679,6 +759,19 @@ impl<'a> SemanticModel<'a> { import_starred = import_starred || scope.uses_star_imports(); } + if let Some(binding_id) = self.materialize_builtin_binding(name.id.as_str()) { + let reference_id = self.resolved_references.push( + self.scope_id, + self.node_id, + ExprContext::Load, + self.flags, + name.range, + ); + self.bindings[binding_id].references.push(reference_id); + self.resolved_names.insert(name.into(), binding_id); + return ReadResult::Resolved(binding_id); + } + if import_starred { self.unresolved_references.push( name.range, @@ -698,12 +791,43 @@ impl<'a> SemanticModel<'a> { } } - /// Lookup a symbol in the current scope. - pub fn lookup_symbol(&self, symbol: &str) -> Option { + /// Lookup a symbol in the current scope without materializing lazy builtins. + pub fn lookup_symbol(&self, symbol: &str) -> Symbol { self.lookup_symbol_in_scope(symbol, self.scope_id, self.in_forward_reference()) } - /// Lookup a symbol in a certain scope + /// Lookup a concrete binding in the current scope. + /// + /// If `symbol` resolves to an unmaterialized builtin, this creates its binding before + /// returning the [`BindingId`]. + pub fn lookup_binding(&mut self, symbol: &'a str) -> Option { + self.lookup_binding_in_scope(symbol, self.scope_id, self.in_forward_reference()) + } + + /// Return a binding from the global scope, materializing it first if it is a builtin. + pub fn global_binding(&mut self, symbol: &'a str) -> Option { + self.materialize_builtin_binding(symbol); + self.global_scope().get(symbol) + } + + /// Lookup a concrete binding in a certain scope. + /// + /// If `symbol` resolves to an unmaterialized builtin, this creates its binding before + /// returning the [`BindingId`]. + fn lookup_binding_in_scope( + &mut self, + symbol: &'a str, + scope_id: ScopeId, + in_forward_reference: bool, + ) -> Option { + match self.lookup_symbol_in_scope(symbol, scope_id, in_forward_reference) { + Symbol::Binding(binding_id) => Some(binding_id), + Symbol::Builtin => self.materialize_builtin_binding(symbol), + Symbol::Unbound => None, + } + } + + /// Lookup a symbol in a certain scope without materializing lazy builtins. /// /// This is a carbon copy of [`Self::resolve_load`], but /// doesn't add any read references to the resolved symbol. @@ -712,11 +836,11 @@ impl<'a> SemanticModel<'a> { symbol: &str, scope_id: ScopeId, in_forward_reference: bool, - ) -> Option { + ) -> Symbol { if in_forward_reference { if let Some(binding_id) = self.scopes.global().get(symbol) { if !self.bindings[binding_id].is_unbound() { - return Some(binding_id); + return Symbol::Binding(binding_id); } } } @@ -738,20 +862,28 @@ impl<'a> SemanticModel<'a> { if let Some(binding_id) = scope.get(symbol) { match self.bindings[binding_id].kind { BindingKind::Annotation => continue, - BindingKind::Deletion | BindingKind::UnboundException(None) => return None, - BindingKind::UnboundException(Some(binding_id)) => return Some(binding_id), - _ => return Some(binding_id), + BindingKind::Deletion | BindingKind::UnboundException(None) => { + return Symbol::Unbound; + } + BindingKind::UnboundException(Some(binding_id)) => { + return Symbol::Binding(binding_id); + } + _ => return Symbol::Binding(binding_id), } } if index == 0 && scope.kind.is_class() { if matches!(symbol, "__module__" | "__qualname__") { - return None; + return Symbol::Unbound; } } } - None + if self.is_builtin_name(symbol) { + Symbol::Builtin + } else { + Symbol::Unbound + } } /// Simulates a runtime load of a given [`ast::ExprName`]. @@ -934,8 +1066,9 @@ impl<'a> SemanticModel<'a> { // Find the symbol in the current scope. let (symbol, attribute) = unqualified_name.segments().split_first()?; - let mut binding_id = - self.lookup_symbol_in_scope(symbol, scope_id, self.in_forward_reference())?; + let mut binding_id = self + .lookup_symbol_in_scope(symbol, scope_id, self.in_forward_reference()) + .binding_id()?; // Recursively resolve class attributes, e.g., `foo.bar.baz` in. let mut tail = attribute; @@ -1040,10 +1173,25 @@ impl<'a> SemanticModel<'a> { // If the name was already resolved, look it up; otherwise, search for the symbol. let head = match_head(value)?; - let binding = self + let symbol = self .resolve_name(head) - .or_else(|| self.lookup_symbol(&head.id)) - .map(|id| self.binding(id))?; + .map_or_else(|| self.lookup_symbol(&head.id), Symbol::Binding); + let binding = match symbol { + Symbol::Binding(binding_id) => self.binding(binding_id), + Symbol::Builtin => { + if value.is_name_expr() { + return Some(QualifiedName::builtin(head.id.as_str())); + } + + let value_name = UnqualifiedName::from_expr(value)?; + return Some( + std::iter::once("") + .chain(value_name.segments().iter().copied()) + .collect(), + ); + } + Symbol::Unbound => return None, + }; match &binding.kind { BindingKind::Import(Import { qualified_name }) => { @@ -1597,6 +1745,10 @@ impl<'a> SemanticModel<'a> { // ``` if !self.at_top_level() { for (name, range) in globals.iter() { + // Global pre-scanning synthesizes an assignment when no global binding exists. + // Materialize builtins first so `global range` doesn't replace the builtin with + // that synthetic assignment. + self.materialize_builtin_binding(name); if self .global_scope() .get(name) diff --git a/crates/ruff_python_semantic/src/model/all.rs b/crates/ruff_python_semantic/src/model/all.rs index fa8100c399d47..17ff47540cea6 100644 --- a/crates/ruff_python_semantic/src/model/all.rs +++ b/crates/ruff_python_semantic/src/model/all.rs @@ -30,8 +30,8 @@ pub struct DunderAllName<'a> { range: TextRange, } -impl DunderAllName<'_> { - pub fn name(&self) -> &str { +impl<'a> DunderAllName<'a> { + pub fn name(&self) -> &'a str { self.name } }