Move binding accesses into SemanticModel method (#5084)

crates/ruff/src/checkers/ast/mod.rs

@@ -4208,7 +4208,7 @@ impl<'a> Checker<'a> {
 
         // Create the `Binding`.
         let binding_id = self.semantic_model.push_binding(range, kind, flags);
-        let binding = &self.semantic_model.bindings[binding_id];
+        let binding = self.semantic_model.binding(binding_id);
 
         // Determine whether the binding shadows any existing bindings.
         if let Some((stack_index, shadowed_id)) = self
@@ -4218,7 +4218,7 @@ impl<'a> Checker<'a> {
             .enumerate()
             .find_map(|(stack_index, scope)| {
                 scope.get(name).and_then(|binding_id| {
-                    let binding = &self.semantic_model.bindings[binding_id];
+                    let binding = self.semantic_model.binding(binding_id);
                     if binding.is_unbound() {
                         None
                     } else {
@@ -4227,7 +4227,7 @@ impl<'a> Checker<'a> {
                 })
             })
         {
-            let shadowed = &self.semantic_model.bindings[shadowed_id];
+            let shadowed = self.semantic_model.binding(shadowed_id);
             let in_current_scope = stack_index == 0;
             if !shadowed.kind.is_builtin()
                 && shadowed.source.map_or(true, |left| {
@@ -4321,10 +4321,7 @@ impl<'a> Checker<'a> {
 
             // If this is an annotation, and we already have an existing value in the same scope,
             // don't treat it as an assignment (i.e., avoid adding it to the scope).
-            if self.semantic_model.bindings[binding_id]
+            if self.semantic_model.binding(binding_id).kind.is_annotation() {
-                .kind
-                .is_annotation()
-            {
                 return binding_id;
             }
         }
@@ -4424,7 +4421,7 @@ impl<'a> Checker<'a> {
                     .scope()
                     .get(id)
                     .map_or(false, |binding_id| {
-                        self.semantic_model.bindings[binding_id].kind.is_global()
+                        self.semantic_model.binding(binding_id).kind.is_global()
                     })
                 {
                     pep8_naming::rules::non_lowercase_variable_in_function(self, expr, parent, id);
@@ -4570,7 +4567,7 @@ impl<'a> Checker<'a> {
 
             // If the name is unbound, then it's an error.
             if self.enabled(Rule::UndefinedName) {
-                let binding = &self.semantic_model.bindings[binding_id];
+                let binding = self.semantic_model.binding(binding_id);
                 if binding.is_unbound() {
                     self.diagnostics.push(Diagnostic::new(
                         pyflakes::rules::UndefinedName {
@@ -4734,10 +4731,7 @@ impl<'a> Checker<'a> {
                         let parent = &self.semantic_model.scopes[scope.parent.unwrap()];
                         self.diagnostics
                             .extend(flake8_unused_arguments::rules::unused_arguments(
-                                self,
+                                self, parent, scope,
-                                parent,
-                                scope,
-                                &self.semantic_model.bindings,
                             ));
                     }
                 }
@@ -4826,7 +4820,7 @@ impl<'a> Checker<'a> {
                     .map(|scope| {
                         scope
                             .binding_ids()
-                            .map(|binding_id| &self.semantic_model.bindings[binding_id])
+                            .map(|binding_id| self.semantic_model.binding(binding_id))
                             .filter(|binding| {
                                 flake8_type_checking::helpers::is_valid_runtime_import(
                                     &self.semantic_model,
@@ -4885,7 +4879,7 @@ impl<'a> Checker<'a> {
             // PLW0602
             if self.enabled(Rule::GlobalVariableNotAssigned) {
                 for (name, binding_id) in scope.bindings() {
-                    let binding = &self.semantic_model.bindings[binding_id];
+                    let binding = self.semantic_model.binding(binding_id);
                     if binding.kind.is_global() {
                         if let Some(source) = binding.source {
                             let stmt = &self.semantic_model.stmts[source];
@@ -4913,7 +4907,7 @@ impl<'a> Checker<'a> {
             if self.enabled(Rule::RedefinedWhileUnused) {
                 for (name, binding_id) in scope.bindings() {
                     if let Some(shadowed_id) = self.semantic_model.shadowed_binding(binding_id) {
-                        let shadowed = &self.semantic_model.bindings[shadowed_id];
+                        let shadowed = self.semantic_model.binding(shadowed_id);
                         if shadowed.is_used() {
                             continue;
                         }
@@ -4925,7 +4919,7 @@ impl<'a> Checker<'a> {
                                 .start(),
                         );
 
-                        let binding = &self.semantic_model.bindings[binding_id];
+                        let binding = self.semantic_model.binding(binding_id);
                         let mut diagnostic = Diagnostic::new(
                             pyflakes::rules::RedefinedWhileUnused {
                                 name: (*name).to_string(),

crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs

@@ -157,7 +157,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, target: &Expr,
                 let scope = checker.semantic_model().scope();
                 if scope
                     .bindings_for_name(name)
-                    .map(|binding_id| &checker.semantic_model().bindings[binding_id])
+                    .map(|binding_id| checker.semantic_model().binding(binding_id))
                     .all(|binding| !binding.is_used())
                 {
                     diagnostic.set_fix(Fix::suggested(Edit::range_replacement(

crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs

@@ -75,7 +75,7 @@ pub(crate) fn runtime_import_in_type_checking_block(
     let mut ignores_by_statement: FxHashMap<NodeId, Vec<Import>> = FxHashMap::default();
 
     for binding_id in scope.binding_ids() {
-        let binding = &checker.semantic_model().bindings[binding_id];
+        let binding = checker.semantic_model().binding(binding_id);
 
         let Some(qualified_name) = binding.qualified_name() else {
             continue;

crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

@@ -197,7 +197,7 @@ pub(crate) fn typing_only_runtime_import(
         FxHashMap::default();
 
     for binding_id in scope.binding_ids() {
-        let binding = &checker.semantic_model().bindings[binding_id];
+        let binding = checker.semantic_model().binding(binding_id);
 
         // If we're in un-strict mode, don't flag typing-only imports that are
         // implicitly loaded by way of a valid runtime import.

crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs

@@ -10,7 +10,7 @@ use ruff_macros::{derive_message_formats, violation};
 use ruff_python_semantic::analyze::function_type;
 use ruff_python_semantic::analyze::function_type::FunctionType;
 use ruff_python_semantic::analyze::visibility;
-use ruff_python_semantic::binding::Bindings;
+use ruff_python_semantic::model::SemanticModel;
 use ruff_python_semantic::scope::{Scope, ScopeKind};
 
 use crate::checkers::ast::Checker;
@@ -221,7 +221,7 @@ fn function(
     argumentable: Argumentable,
     args: &Arguments,
     values: &Scope,
-    bindings: &Bindings,
+    model: &SemanticModel,
     dummy_variable_rgx: &Regex,
     ignore_variadic_names: bool,
 ) -> Vec<Diagnostic> {
@@ -240,7 +240,7 @@ fn function(
                 .flatten()
                 .skip(usize::from(ignore_variadic_names)),
         );
-    call(argumentable, args, values, bindings, dummy_variable_rgx)
+    call(argumentable, args, values, model, dummy_variable_rgx)
 }
 
 /// Check a method for unused arguments.
@@ -248,7 +248,7 @@ fn method(
     argumentable: Argumentable,
     args: &Arguments,
     values: &Scope,
-    bindings: &Bindings,
+    model: &SemanticModel,
     dummy_variable_rgx: &Regex,
     ignore_variadic_names: bool,
 ) -> Vec<Diagnostic> {
@@ -268,21 +268,21 @@ fn method(
                 .flatten()
                 .skip(usize::from(ignore_variadic_names)),
         );
-    call(argumentable, args, values, bindings, dummy_variable_rgx)
+    call(argumentable, args, values, model, dummy_variable_rgx)
 }
 
 fn call<'a>(
     argumentable: Argumentable,
     args: impl Iterator<Item = &'a Arg>,
     values: &Scope,
-    bindings: &Bindings,
+    model: &SemanticModel,
     dummy_variable_rgx: &Regex,
 ) -> Vec<Diagnostic> {
     let mut diagnostics: Vec<Diagnostic> = vec![];
     for arg in args {
         if let Some(binding) = values
             .get(arg.arg.as_str())
-            .map(|binding_id| &bindings[binding_id])
+            .map(|binding_id| model.binding(binding_id))
         {
             if binding.kind.is_argument()
                 && !binding.is_used()
@@ -303,7 +303,6 @@ pub(crate) fn unused_arguments(
     checker: &Checker,
     parent: &Scope,
     scope: &Scope,
-    bindings: &Bindings,
 ) -> Vec<Diagnostic> {
     match &scope.kind {
         ScopeKind::Function(ast::StmtFunctionDef {
@@ -336,7 +335,7 @@ pub(crate) fn unused_arguments(
                             Argumentable::Function,
                             args,
                             scope,
-                            bindings,
+                            checker.semantic_model(),
                             &checker.settings.dummy_variable_rgx,
                             checker
                                 .settings
@@ -362,7 +361,7 @@ pub(crate) fn unused_arguments(
                             Argumentable::Method,
                             args,
                             scope,
-                            bindings,
+                            checker.semantic_model(),
                             &checker.settings.dummy_variable_rgx,
                             checker
                                 .settings
@@ -388,7 +387,7 @@ pub(crate) fn unused_arguments(
                             Argumentable::ClassMethod,
                             args,
                             scope,
-                            bindings,
+                            checker.semantic_model(),
                             &checker.settings.dummy_variable_rgx,
                             checker
                                 .settings
@@ -414,7 +413,7 @@ pub(crate) fn unused_arguments(
                             Argumentable::StaticMethod,
                             args,
                             scope,
-                            bindings,
+                            checker.semantic_model(),
                             &checker.settings.dummy_variable_rgx,
                             checker
                                 .settings
@@ -433,7 +432,7 @@ pub(crate) fn unused_arguments(
                     Argumentable::Lambda,
                     args,
                     scope,
-                    bindings,
+                    checker.semantic_model(),
                     &checker.settings.dummy_variable_rgx,
                     checker
                         .settings

crates/ruff/src/rules/pyflakes/rules/undefined_local.rs

@@ -68,7 +68,7 @@ pub(crate) fn undefined_local(checker: &mut Checker, name: &str) {
             // If the name was defined in that scope...
             if let Some(binding) = scope
                 .get(name)
-                .map(|binding_id| &checker.semantic_model().bindings[binding_id])
+                .map(|binding_id| checker.semantic_model().binding(binding_id))
             {
                 // And has already been accessed in the current scope...
                 if let Some(range) = binding.references().find_map(|reference_id| {

crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs

@@ -39,7 +39,7 @@ pub(crate) fn unused_annotation(checker: &mut Checker, scope: ScopeId) {
     let bindings: Vec<_> = scope
         .bindings()
         .filter_map(|(name, binding_id)| {
-            let binding = &checker.semantic_model().bindings[binding_id];
+            let binding = checker.semantic_model().binding(binding_id);
             if binding.kind.is_annotation()
                 && !binding.is_used()
                 && !checker.settings.dummy_variable_rgx.is_match(name)

crates/ruff/src/rules/pyflakes/rules/unused_import.rs

@@ -104,7 +104,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
     let mut ignored: FxHashMap<(NodeId, Exceptions), Vec<Import>> = FxHashMap::default();
 
     for binding_id in scope.binding_ids() {
-        let binding = &checker.semantic_model().bindings[binding_id];
+        let binding = checker.semantic_model().binding(binding_id);
 
         if binding.is_used() || binding.is_explicit_export() {
             continue;

crates/ruff/src/rules/pyflakes/rules/unused_variable.rs

@@ -292,7 +292,7 @@ pub(crate) fn unused_variable(checker: &mut Checker, scope: ScopeId) {
 
     let bindings: Vec<_> = scope
         .bindings()
-        .map(|(name, binding_id)| (name, &checker.semantic_model().bindings[binding_id]))
+        .map(|(name, binding_id)| (name, checker.semantic_model().binding(binding_id)))
         .filter_map(|(name, binding)| {
             if (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment())
                 && !binding.is_used()

crates/ruff/src/rules/pylint/rules/global_statement.rs

@@ -57,7 +57,7 @@ impl Violation for GlobalStatement {
 pub(crate) fn global_statement(checker: &mut Checker, name: &str) {
     let scope = checker.semantic_model().scope();
     if let Some(binding_id) = scope.get(name) {
-        let binding = &checker.semantic_model().bindings[binding_id];
+        let binding = checker.semantic_model().binding(binding_id);
         if binding.kind.is_global() {
             let source = checker.semantic_model().stmts[binding
                 .source

crates/ruff_python_semantic/src/binding.rs

@@ -198,15 +198,10 @@ impl nohash_hasher::IsEnabled for BindingId {}
 pub struct Bindings<'a>(IndexVec<BindingId, Binding<'a>>);
 
 impl<'a> Bindings<'a> {
-    /// Pushes a new binding and returns its id
+    /// Pushes a new [`Binding`] and returns its [`BindingId`].
     pub fn push(&mut self, binding: Binding<'a>) -> BindingId {
         self.0.push(binding)
     }
-
-    /// Returns the id that will be assigned when pushing the next binding
-    pub fn next_id(&self) -> BindingId {
-        self.0.next_index()
-    }
 }
 
 impl<'a> Deref for Bindings<'a> {

crates/ruff_python_semantic/src/model.rs

@@ -106,6 +106,18 @@ impl<'a> SemanticModel<'a> {
         }
     }
 
+    /// Return the [`Binding`] for the given [`BindingId`].
+    #[inline]
+    pub fn binding(&self, id: BindingId) -> &Binding {
+        &self.bindings[id]
+    }
+
+    /// Resolve the [`Reference`] for the given [`ReferenceId`].
+    #[inline]
+    pub fn reference(&self, id: ReferenceId) -> &Reference {
+        &self.references[id]
+    }
+
     /// Return `true` if the `Expr` is a reference to `typing.${target}`.
     pub fn match_typing_expr(&self, expr: &Expr, target: &str) -> bool {
         self.resolve_call_path(expr).map_or(false, |call_path| {
@@ -717,11 +729,6 @@ impl<'a> SemanticModel<'a> {
         self.bindings[binding_id].references.push(reference_id);
     }
 
-    /// Resolve a [`ReferenceId`].
-    pub fn reference(&self, reference_id: ReferenceId) -> &Reference {
-        self.references.resolve(reference_id)
-    }
-
     /// Return the [`ExecutionContext`] of the current scope.
     pub const fn execution_context(&self) -> ExecutionContext {
         if self.in_type_checking_block()

crates/ruff_python_semantic/src/reference.rs

@@ -1,6 +1,7 @@
 use ruff_text_size::TextRange;
+use std::ops::Deref;
 
-use ruff_index::{newtype_index, IndexVec};
+use ruff_index::{newtype_index, IndexSlice, IndexVec};
 
 use crate::context::ExecutionContext;
 use crate::scope::ScopeId;
@@ -38,7 +39,7 @@ pub struct ReferenceId;
 pub struct References(IndexVec<ReferenceId, Reference>);
 
 impl References {
-    /// Pushes a new read reference and returns its unique id.
+    /// Pushes a new [`Reference`] and returns its [`ReferenceId`].
     pub fn push(
         &mut self,
         scope_id: ScopeId,
@@ -51,9 +52,12 @@ impl References {
             context,
         })
     }
+}
 
-    /// Returns the [`Reference`] with the given id.
+impl Deref for References {
-    pub fn resolve(&self, id: ReferenceId) -> &Reference {
+    type Target = IndexSlice<ReferenceId, Reference>;
-        &self.0[id]
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
     }
 }