Use BindingId copies in lieu of &BindingId in semantic model meth…

…ods (#4633)
astral-sh · May 24, 2023 · f0e173d · f0e173d
1 parent f4f1b1d
commit f0e173d
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 51 deletions.
diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs
@@ -298,7 +298,7 @@ where
 
                         if let Some(binding_id) = binding_id {
                             self.semantic_model.add_local_reference(
-                                *binding_id,
+                                binding_id,
                                 stmt.range(),
                                 ExecutionContext::Runtime,
                             );
@@ -1894,7 +1894,7 @@ where
                         .global_scope()
                         .get(name)
                         .map_or(true, |binding_id| {
-                            self.semantic_model.bindings[*binding_id]
+                            self.semantic_model.bindings[binding_id]
                                 .kind
                                 .is_annotation()
                         })
@@ -1958,7 +1958,7 @@ where
                         .global_scope()
                         .get(name)
                         .map_or(true, |binding_id| {
-                            self.semantic_model.bindings[*binding_id]
+                            self.semantic_model.bindings[binding_id]
                                 .kind
                                 .is_annotation()
                         })
@@ -4003,7 +4003,7 @@ where
                             );
                         }
 
-                        let definition = self.semantic_model.scope().get(name).copied();
+                        let definition = self.semantic_model.scope().get(name);
                         self.handle_node_store(
                             name,
                             &Expr::Name(ast::ExprName {
@@ -4017,9 +4017,9 @@ where
 
                         if let Some(binding_id) = {
                             let scope = self.semantic_model.scope_mut();
-                            &scope.remove(name)
+                            scope.remove(name)
                         } {
-                            if !self.semantic_model.is_used(*binding_id) {
+                            if !self.semantic_model.is_used(binding_id) {
                                 if self.enabled(Rule::UnusedVariable) {
                                     let mut diagnostic = Diagnostic::new(
                                         pyflakes::rules::UnusedVariable { name: name.into() },
@@ -4292,7 +4292,7 @@ impl<'a> Checker<'a> {
             .ancestors(self.semantic_model.scope_id)
             .enumerate()
             .find_map(|(stack_index, scope)| {
-                scope.get(name).map(|binding_id| (stack_index, *binding_id))
+                scope.get(name).map(|binding_id| (stack_index, binding_id))
             })
         {
             let existing = &self.semantic_model.bindings[existing_binding_id];
@@ -4387,7 +4387,7 @@ impl<'a> Checker<'a> {
         let scope = &mut self.semantic_model.scopes[scope_id];
 
         let binding = if let Some(binding_id) = scope.get(name) {
-            let existing = &self.semantic_model.bindings[*binding_id];
+            let existing = &self.semantic_model.bindings[binding_id];
             match &existing.kind {
                 BindingKind::Builtin => {
                     // Avoid overriding builtins.
@@ -4521,7 +4521,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.bindings[binding_id].kind.is_global()
                     })
                 {
                     pep8_naming::rules::non_lowercase_variable_in_function(self, expr, parent, id);
@@ -4639,7 +4639,7 @@ impl<'a> Checker<'a> {
                     if let Stmt::AugAssign(_) = parent {
                         if let Some(binding_id) = scope.get("__all__") {
                             if let BindingKind::Export(Export { names: existing }) =
-                                &self.semantic_model.bindings[*binding_id].kind
+                                &self.semantic_model.bindings[binding_id].kind
                             {
                                 names.extend_from_slice(existing);
                             }
@@ -4926,7 +4926,6 @@ impl<'a> Checker<'a> {
             let global_scope = self.semantic_model.global_scope();
             let all_names: Option<(&[&str], TextRange)> = global_scope
                 .get("__all__")
-                .copied()
                 .map(|binding_id| &self.semantic_model.bindings[binding_id])
                 .and_then(|binding| match &binding.kind {
                     BindingKind::Export(Export { names }) => {
@@ -4939,7 +4938,7 @@ impl<'a> Checker<'a> {
                 (
                     names
                         .iter()
-                        .filter_map(|name| global_scope.get(name).copied())
+                        .filter_map(|name| global_scope.get(name))
                         .collect(),
                     range,
                 )
@@ -4961,7 +4960,6 @@ impl<'a> Checker<'a> {
             .semantic_model
             .global_scope()
             .get("__all__")
-            .copied()
             .map(|binding_id| &self.semantic_model.bindings[binding_id])
             .and_then(|binding| match &binding.kind {
                 BindingKind::Export(Export { names }) => Some((names.as_slice(), binding.range)),
@@ -4981,7 +4979,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.bindings[binding_id])
                             .filter(|binding| {
                                 flake8_type_checking::helpers::is_valid_runtime_import(
                                     &self.semantic_model,
@@ -5042,7 +5040,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.bindings[binding_id];
                     if binding.kind.is_global() {
                         if let Some(source) = binding.source {
                             let stmt = &self.semantic_model.stmts[source];
@@ -5069,7 +5067,7 @@ impl<'a> Checker<'a> {
             // the bindings are in different scopes.
             if self.enabled(Rule::RedefinedWhileUnused) {
                 for (name, binding_id) in scope.bindings() {
-                    let binding = &self.semantic_model.bindings[*binding_id];
+                    let binding = &self.semantic_model.bindings[binding_id];
 
                     if matches!(
                         binding.kind,
@@ -5082,10 +5080,10 @@ impl<'a> Checker<'a> {
                         }
 
                         if let Some(shadowed_ids) =
-                            self.semantic_model.shadowed_bindings.get(binding_id)
+                            self.semantic_model.shadowed_bindings.get(&binding_id)
                         {
-                            for binding_id in shadowed_ids {
-                                let rebound = &self.semantic_model.bindings[*binding_id];
+                            for binding_id in shadowed_ids.iter().copied() {
+                                let rebound = &self.semantic_model.bindings[binding_id];
                                 #[allow(deprecated)]
                                 let line = self.locator.compute_line_index(
                                     binding
@@ -5127,7 +5125,7 @@ impl<'a> Checker<'a> {
                         .collect()
                 };
                 for binding_id in scope.binding_ids() {
-                    let binding = &self.semantic_model.bindings[*binding_id];
+                    let binding = &self.semantic_model.bindings[binding_id];
 
                     if let Some(diagnostic) =
                         flake8_type_checking::rules::runtime_import_in_type_checking_block(
@@ -5166,7 +5164,7 @@ impl<'a> Checker<'a> {
                     FxHashMap::default();
 
                 for binding_id in scope.binding_ids() {
-                    let binding = &self.semantic_model.bindings[*binding_id];
+                    let binding = &self.semantic_model.bindings[binding_id];
 
                     if binding.is_used() || binding.is_explicit_export() {
                         continue;
@@ -5388,7 +5386,7 @@ impl<'a> Checker<'a> {
         let global_scope = self.semantic_model.global_scope();
         let exports: Option<&[&str]> = global_scope
             .get("__all__")
-            .map(|binding_id| &self.semantic_model.bindings[*binding_id])
+            .map(|binding_id| &self.semantic_model.bindings[binding_id])
             .and_then(|binding| match &binding.kind {
                 BindingKind::Export(Export { names }) => Some(names.as_slice()),
                 _ => None,

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
@@ -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().bindings[binding_id])
                     .all(|binding| !binding.is_used())
                 {
                     #[allow(deprecated)]

diff --git a/crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs
@@ -293,7 +293,7 @@ fn call<'a>(
     for arg in args {
         if let Some(binding) = values
             .get(arg.arg.as_str())
-            .map(|binding_id| &bindings[*binding_id])
+            .map(|binding_id| &bindings[binding_id])
         {
             if binding.kind.is_argument()
                 && !binding.is_used()

diff --git a/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs b/crates/ruff/src/rules/pyflakes/rules/undefined_local.rs
@@ -43,7 +43,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().bindings[binding_id])
             {
                 // And has already been accessed in the current scope...
                 if let Some(range) = binding.references().find_map(|reference_id| {

diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs b/crates/ruff/src/rules/pyflakes/rules/unused_annotation.rs
@@ -24,9 +24,7 @@ pub(crate) fn unused_annotation(checker: &mut Checker, scope: ScopeId) {
     let bindings: Vec<_> = scope
         .bindings()
         .filter_map(|(name, binding_id)| {
-            let name = *name;
-            let binding = &checker.semantic_model().bindings[*binding_id];
-
+            let binding = &checker.semantic_model().bindings[binding_id];
             if binding.kind.is_annotation()
                 && !binding.is_used()
                 && !checker.settings.dummy_variable_rgx.is_match(name)
@@ -39,11 +37,8 @@ pub(crate) fn unused_annotation(checker: &mut Checker, scope: ScopeId) {
         .collect();
 
     for (name, range) in bindings {
-        checker.diagnostics.push(Diagnostic::new(
-            UnusedAnnotation {
-                name: (*name).to_string(),
-            },
-            range,
-        ));
+        checker
+            .diagnostics
+            .push(Diagnostic::new(UnusedAnnotation { name }, range));
     }
 }
diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs
@@ -320,7 +320,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().bindings[binding_id]))
         .filter_map(|(name, binding)| {
             if (binding.kind.is_assignment() || binding.kind.is_named_expr_assignment())
                 && !binding.is_used()

diff --git a/crates/ruff/src/rules/pylint/rules/global_statement.rs b/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().bindings[binding_id];
         if binding.kind.is_global() {
             let source = checker.semantic_model().stmts[binding
                 .source

diff --git a/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs b/crates/ruff/src/rules/pyupgrade/rules/useless_object_inheritance.rs
@@ -37,7 +37,7 @@ fn rule(name: &str, bases: &[Expr], scope: &Scope, bindings: &Bindings) -> Optio
         if !matches!(
             scope
                 .get(id.as_str())
-                .map(|binding_id| &bindings[*binding_id]),
+                .map(|binding_id| &bindings[binding_id]),
             None | Some(Binding {
                 kind: BindingKind::Builtin,
                 ..

diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs
@@ -110,7 +110,7 @@ impl<'a> SemanticModel<'a> {
     pub fn find_binding(&self, member: &str) -> Option<&Binding> {
         self.scopes()
             .find_map(|scope| scope.get(member))
-            .map(|binding_id| &self.bindings[*binding_id])
+            .map(|binding_id| &self.bindings[binding_id])
     }
 
     /// Return `true` if `member` is bound as a builtin.
@@ -124,7 +124,7 @@ 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).copied() {
+            if let Some(binding_id) = self.scopes.global().get(symbol) {
                 // Mark the binding as used.
                 let context = self.execution_context();
                 let reference_id = self.references.push(ScopeId::global(), range, context);
@@ -160,7 +160,7 @@ impl<'a> SemanticModel<'a> {
                 }
             }
 
-            if let Some(binding_id) = scope.get(symbol).copied() {
+            if let Some(binding_id) = scope.get(symbol) {
                 // Mark the binding as used.
                 let context = self.execution_context();
                 let reference_id = self.references.push(self.scope_id, range, context);
@@ -255,7 +255,7 @@ impl<'a> SemanticModel<'a> {
             return None;
         }
 
-        self.scopes[scope_id].get(full_name).copied()
+        self.scopes[scope_id].get(full_name)
     }
 
     /// Resolves the [`Expr`] to a fully-qualified symbol-name, if `value` resolves to an imported
@@ -352,7 +352,7 @@ impl<'a> SemanticModel<'a> {
         member: &str,
     ) -> Option<(&Stmt, String)> {
         self.scopes().enumerate().find_map(|(scope_index, scope)| {
-            scope.binding_ids().copied().find_map(|binding_id| {
+            scope.binding_ids().find_map(|binding_id| {
                 let binding = &self.bindings[binding_id];
                 match &binding.kind {
                     // Ex) Given `module="sys"` and `object="exit"`:

diff --git a/crates/ruff_python_semantic/src/reference.rs b/crates/ruff_python_semantic/src/reference.rs
@@ -24,8 +24,8 @@ impl Reference {
         self.range
     }
 
-    pub const fn context(&self) -> &ExecutionContext {
-        &self.context
+    pub const fn context(&self) -> ExecutionContext {
+        self.context
     }
 }
 

diff --git a/crates/ruff_python_semantic/src/scope.rs b/crates/ruff_python_semantic/src/scope.rs
@@ -45,8 +45,8 @@ impl<'a> Scope<'a> {
     }
 
     /// Returns the [id](BindingId) of the binding bound to the given name.
-    pub fn get(&self, name: &str) -> Option<&BindingId> {
-        self.bindings.get(name)
+    pub fn get(&self, name: &str) -> Option<BindingId> {
+        self.bindings.get(name).copied()
     }
 
     /// Adds a new binding with the given name to this scope.
@@ -70,22 +70,23 @@ impl<'a> Scope<'a> {
     }
 
     /// Returns the ids of all bindings defined in this scope.
-    pub fn binding_ids(&self) -> std::collections::hash_map::Values<&str, BindingId> {
-        self.bindings.values()
+    pub fn binding_ids(&self) -> impl Iterator<Item = BindingId> + '_ {
+        self.bindings.values().copied()
     }
 
     /// Returns a tuple of the name and id of all bindings defined in this scope.
-    pub fn bindings(&self) -> std::collections::hash_map::Iter<&'a str, BindingId> {
-        self.bindings.iter()
+    pub fn bindings(&self) -> impl Iterator<Item = (&str, BindingId)> + '_ {
+        self.bindings.iter().map(|(&name, &id)| (name, id))
     }
 
     /// Returns an iterator over all [bindings](BindingId) bound to the given name, including
     /// those that were shadowed by later bindings.
-    pub fn bindings_for_name(&self, name: &str) -> impl Iterator<Item = &BindingId> {
+    pub fn bindings_for_name(&self, name: &str) -> impl Iterator<Item = BindingId> + '_ {
         self.bindings
             .get(name)
             .into_iter()
             .chain(self.shadowed_bindings.get(name).into_iter().flatten().rev())
+            .copied()
     }
 
     /// Adds a reference to a star import (e.g., `from sys import *`) to this scope.