From d380b37a092242c459c57deec8c1286b86e06435 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 7 Aug 2024 11:17:56 +0100 Subject: [PATCH] Add a new `Binding::is_unused` method (#12729) --- .../ruff_linter/src/checkers/ast/analyze/bindings.rs | 2 +- .../rules/unused_loop_control_variable.rs | 3 ++- .../flake8_unused_arguments/rules/unused_arguments.rs | 2 +- .../src/rules/pyflakes/rules/unused_annotation.rs | 2 +- .../src/rules/pyflakes/rules/unused_variable.rs | 2 +- .../ruff_linter/src/rules/pylint/rules/no_self_use.rs | 2 +- .../src/rules/ruff/rules/sort_dunder_slots.rs | 2 +- crates/ruff_python_semantic/src/binding.rs | 11 ++++++++++- crates/ruff_python_semantic/src/model.rs | 2 +- 9 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs index 6400f5a52081f..fe6faeafa6244 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/bindings.rs @@ -22,7 +22,7 @@ pub(crate) fn bindings(checker: &mut Checker) { for binding in &*checker.semantic.bindings { if checker.enabled(Rule::UnusedVariable) { if binding.kind.is_bound_exception() - && !binding.is_used() + && binding.is_unused() && !checker .settings .dummy_variable_rgx diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index d7cc7191219d8..bf8f7318a3b45 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -4,6 +4,7 @@ use ruff_python_ast as ast; use ruff_python_ast::helpers; use ruff_python_ast::helpers::{NameFinder, StoredNameFinder}; use ruff_python_ast::visitor::Visitor; +use ruff_python_semantic::Binding; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -137,7 +138,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast .get_all(name) .map(|binding_id| checker.semantic().binding(binding_id)) .filter(|binding| binding.start() >= expr.start()) - .all(|binding| !binding.is_used()) + .all(Binding::is_unused) { diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( rename, diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index c37888f1a7587..558b408831302 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -298,7 +298,7 @@ fn call<'a>( .get(arg.name.as_str()) .map(|binding_id| semantic.binding(binding_id))?; if binding.kind.is_argument() - && !binding.is_used() + && binding.is_unused() && !dummy_variable_rgx.is_match(arg.name.as_str()) { Some(Diagnostic::new( diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_annotation.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_annotation.rs index b6e0bd9e9ad21..ad5e383db692f 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_annotation.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_annotation.rs @@ -42,7 +42,7 @@ pub(crate) fn unused_annotation( for (name, range) in scope.bindings().filter_map(|(name, binding_id)| { let binding = checker.semantic().binding(binding_id); if binding.kind.is_annotation() - && !binding.is_used() + && binding.is_unused() && !checker.settings.dummy_variable_rgx.is_match(name) { Some((name.to_string(), binding.range())) diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_variable.rs index d0f2714824296..dec723706bf3f 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_variable.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_variable.rs @@ -262,9 +262,9 @@ pub(crate) fn unused_variable(checker: &Checker, scope: &Scope, diagnostics: &mu || binding.kind.is_named_expr_assignment() || binding.kind.is_with_item_var()) && (!binding.is_unpacked_assignment() || checker.settings.preview.is_enabled()) + && binding.is_unused() && !binding.is_nonlocal() && !binding.is_global() - && !binding.is_used() && !checker.settings.dummy_variable_rgx.is_match(name) && !matches!( name, diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index e2f14d09bd329..19420e7b4fc74 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -123,7 +123,7 @@ pub(crate) fn no_self_use( if scope .get("self") .map(|binding_id| semantic.binding(binding_id)) - .is_some_and(|binding| binding.kind.is_argument() && !binding.is_used()) + .is_some_and(|binding| binding.kind.is_argument() && binding.is_unused()) { diagnostics.push(Diagnostic::new( NoSelfUse { diff --git a/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs b/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs index dbc40493b8520..210e69145fdcf 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/sort_dunder_slots.rs @@ -124,7 +124,7 @@ pub(crate) fn sort_dunder_slots(checker: &Checker, binding: &Binding) -> Option< if let Some(sorted_source_code) = display.generate_sorted_source_code(&items, checker) { let edit = Edit::range_replacement(sorted_source_code, display.range()); - let applicability = if display.kind.is_set_literal() || !binding.is_used() { + let applicability = if display.kind.is_set_literal() || binding.is_unused() { Applicability::Safe } else { Applicability::Unsafe diff --git a/crates/ruff_python_semantic/src/binding.rs b/crates/ruff_python_semantic/src/binding.rs index d4cc059088ca8..9b9c74aa81fe2 100644 --- a/crates/ruff_python_semantic/src/binding.rs +++ b/crates/ruff_python_semantic/src/binding.rs @@ -36,9 +36,18 @@ pub struct Binding<'a> { } impl<'a> Binding<'a> { + /// Return `true` if this [`Binding`] is unused. + /// + /// This method is the opposite of [`Binding::is_used`]. + pub fn is_unused(&self) -> bool { + self.references.is_empty() + } + /// Return `true` if this [`Binding`] is used. + /// + /// This method is the opposite of [`Binding::is_unused`]. pub fn is_used(&self) -> bool { - !self.references.is_empty() + !self.is_unused() } /// Returns an iterator over all references for the current [`Binding`]. diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 4a632279ae12f..184fb9496b0b0 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1455,7 +1455,7 @@ impl<'a> SemanticModel<'a> { .get_all(id) .map(|binding_id| self.binding(binding_id)) .filter(|binding| binding.start() >= expr.start()) - .all(|binding| !binding.is_used()) + .all(Binding::is_unused) } _ => false, }