Python/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff synced 2026-01-20 21:10:48 -05:00
Code Issues Packages Projects Releases Wiki Activity

Automatically remove unused variables (#1683)

Browse Source 
Closes #1460.
This commit is contained in:
Charlie Marsh
2023-01-06 22:06:04 -05:00
committed by GitHub
parent d5256f89b6
commit 0527fb9335
16 changed files with 666 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/ast/operations.rs
View File

@@ -212,7 +212,7 @@ pub fn is_unpacking_assignment(parent: &Stmt, child: &Expr) -> bool {
ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. }
);
// In `(a, b) = coords = (1, 2)`, `(a, b)` and `coords` are the targets, and
// `(a, b`) is a tuple. (We use "tuple" as a placeholder for any
// `(a, b)` is a tuple. (We use "tuple" as a placeholder for any
// unpackable expression.)
let targets_are_tuples = targets.iter().all(|item| {
matches!(

42
src/checkers/ast.rs
View File

@@ -76,10 +76,10 @@ pub struct Checker<'a> {
pub(crate) child_to_parent: FxHashMap<RefEquality<'a, Stmt>, RefEquality<'a, Stmt>>,
pub(crate) bindings: Vec<Binding<'a>>,
pub(crate) redefinitions: IntMap<usize, Vec<usize>>,
exprs: Vec<RefEquality<'a, Expr>>,
scopes: Vec<Scope<'a>>,
scope_stack: Vec<usize>,
dead_scopes: Vec<usize>,
pub(crate) exprs: Vec<RefEquality<'a, Expr>>,
pub(crate) scopes: Vec<Scope<'a>>,
pub(crate) scope_stack: Vec<usize>,
pub(crate) dead_scopes: Vec<usize>,
deferred_string_type_definitions: Vec<(Range, &'a str, bool, DeferralContext<'a>)>,
deferred_type_definitions: Vec<(&'a Expr, bool, DeferralContext<'a>)>,
deferred_functions: Vec<(&'a Stmt, DeferralContext<'a>, VisibleScope)>,
@@ -3062,10 +3062,28 @@ where
} {
if self.bindings[*index].used.is_none() {
if self.settings.enabled.contains(&CheckCode::F841) {
self.checks.push(Check::new(
let mut check = Check::new(
CheckKind::UnusedVariable(name.to_string()),
name_range,
));
);
if self.patch(&CheckCode::F841) {
match pyflakes::fixes::remove_exception_handler_assignment(
excepthandler,
self.locator,
) {
Ok(fix) => {
check.amend(fix);
}
Err(e) => {
error!(
"Failed to remove exception handler \
assignment: {}",
e
);
}
}
}
self.checks.push(check);
}
}
}
@@ -3801,18 +3819,10 @@ impl<'a> Checker<'a> {
let scope_index = scopes[scopes.len() - 1];
let parent_scope_index = scopes[scopes.len() - 2];
if self.settings.enabled.contains(&CheckCode::F841) {
self.checks.extend(pyflakes::checks::unused_variable(
&self.scopes[scope_index],
&self.bindings,
&self.settings.dummy_variable_rgx,
));
pyflakes::plugins::unused_variable(self, scope_index);
}
if self.settings.enabled.contains(&CheckCode::F842) {
self.checks.extend(pyflakes::checks::unused_annotation(
&self.scopes[scope_index],
&self.bindings,
&self.settings.dummy_variable_rgx,
));
pyflakes::plugins::unused_annotation(self, scope_index);
}
if self.settings.enabled.contains(&CheckCode::ARG001)
|| self.settings.enabled.contains(&CheckCode::ARG002)

2
src/linter.rs
View File

@@ -527,7 +527,7 @@ pub fn test_path(path: &Path, settings: &Settings) -> Result<Vec<Check>> {
// Detect autofixes that don't converge after multiple iterations.
if checks.iter().any(|check| check.fix.is_some()) {
let max_iterations = 3;
let max_iterations = 10;
let mut contents = contents.clone();
let mut iterations = 0;

62
src/pyflakes/checks.rs
View File

@@ -1,12 +1,11 @@
use std::string::ToString;
use regex::Regex;
use rustpython_parser::ast::{
Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind,
};
use crate::ast::helpers::except_range;
use crate::ast::types::{Binding, BindingKind, Range, Scope, ScopeKind};
use crate::ast::types::{Binding, Range, Scope, ScopeKind};
use crate::registry::{Check, CheckKind};
use crate::source_code_locator::SourceCodeLocator;
@@ -52,65 +51,6 @@ pub fn undefined_local(name: &str, scopes: &[&Scope], bindings: &[Binding]) -> O
None
}
/// F841
pub fn unused_variable(
scope: &Scope,
bindings: &[Binding],
dummy_variable_rgx: &Regex,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) {
return checks;
}
for (name, binding) in scope
.values
.iter()
.map(|(name, index)| (name, &bindings[*index]))
{
if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Assignment)
&& !dummy_variable_rgx.is_match(name)
&& name != &"__tracebackhide__"
&& name != &"__traceback_info__"
&& name != &"__traceback_supplement__"
{
checks.push(Check::new(
CheckKind::UnusedVariable((*name).to_string()),
binding.range,
));
}
}
checks
}
/// F842
pub fn unused_annotation(
scope: &Scope,
bindings: &[Binding],
dummy_variable_rgx: &Regex,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
for (name, binding) in scope
.values
.iter()
.map(|(name, index)| (name, &bindings[*index]))
{
if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Annotation)
&& !dummy_variable_rgx.is_match(name)
{
checks.push(Check::new(
CheckKind::UnusedAnnotation((*name).to_string()),
binding.range,
));
}
}
checks
}
/// F707
pub fn default_except_not_last(
handlers: &[Excepthandler],

35
src/pyflakes/fixes.rs
View File

@@ -1,6 +1,8 @@
use anyhow::{bail, Result};
use libcst_native::{Call, Codegen, CodegenState, Dict, DictElement, Expression};
use rustpython_ast::Expr;
use rustpython_ast::{Excepthandler, Expr};
use rustpython_parser::lexer;
use rustpython_parser::lexer::Tok;
use crate::ast::types::Range;
use crate::autofix::Fix;
@@ -97,3 +99,34 @@ pub fn remove_unused_keyword_arguments_from_format_call(
location.end_location,
))
}
/// Generate a `Fix` to remove the binding from an exception handler.
pub fn remove_exception_handler_assignment(
excepthandler: &Excepthandler,
locator: &SourceCodeLocator,
) -> Result<Fix> {
let contents = locator.slice_source_code_range(&Range::from_located(excepthandler));
let mut fix_start = None;
let mut fix_end = None;
// End of the token just before the `as` to the semicolon.
let mut prev = None;
for (start, tok, end) in
lexer::make_tokenizer_located(&contents, excepthandler.location).flatten()
{
if matches!(tok, Tok::As) {
fix_start = prev;
}
if matches!(tok, Tok::Colon) {
fix_end = Some(start);
break;
}
prev = Some(end);
}
if let (Some(start), Some(end)) = (fix_start, fix_end) {
Ok(Fix::deletion(start, end))
} else {
bail!("Could not find span of exception handler")
}
}

1
src/pyflakes/mod.rs
View File

@@ -103,6 +103,7 @@ mod tests {
#[test_case(CheckCode::F841, Path::new("F841_0.py"); "F841_0")]
#[test_case(CheckCode::F841, Path::new("F841_1.py"); "F841_1")]
#[test_case(CheckCode::F841, Path::new("F841_2.py"); "F841_2")]
#[test_case(CheckCode::F841, Path::new("F841_3.py"); "F841_3")]
#[test_case(CheckCode::F842, Path::new("F842.py"); "F842")]
#[test_case(CheckCode::F901, Path::new("F901.py"); "F901")]
fn checks(check_code: CheckCode, path: &Path) -> Result<()> {

4
src/pyflakes/plugins/mod.rs
View File

@@ -12,6 +12,8 @@ pub(crate) use strings::{
string_dot_format_extra_positional_arguments, string_dot_format_missing_argument,
string_dot_format_mixing_automatic,
};
pub use unused_annotation::unused_annotation;
pub use unused_variable::unused_variable;
mod assert_tuple;
mod f_string_missing_placeholders;
@@ -20,3 +22,5 @@ mod invalid_literal_comparisons;
mod invalid_print_syntax;
mod raise_not_implemented;
mod strings;
mod unused_annotation;
mod unused_variable;

23
src/pyflakes/plugins/unused_annotation.rs Normal file
View File

@@ -0,0 +1,23 @@
use crate::ast::types::BindingKind;
use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckKind};
/// F842
pub fn unused_annotation(checker: &mut Checker, scope: usize) {
let scope = &checker.scopes[scope];
for (name, binding) in scope
.values
.iter()
.map(|(name, index)| (name, &checker.bindings[*index]))
{
if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Annotation)
&& !checker.settings.dummy_variable_rgx.is_match(name)
{
checker.checks.push(Check::new(
CheckKind::UnusedAnnotation((*name).to_string()),
binding.range,
));
}
}
}

189
src/pyflakes/plugins/unused_variable.rs Normal file
View File

@@ -0,0 +1,189 @@
use log::error;
use rustpython_ast::{Expr, ExprKind, Stmt, StmtKind};
use crate::ast::types::{BindingKind, Range, RefEquality, ScopeKind};
use crate::autofix::helpers::delete_stmt;
use crate::autofix::Fix;
use crate::checkers::ast::Checker;
use crate::registry::{Check, CheckCode, CheckKind};
fn is_literal_or_name(expr: &Expr, checker: &Checker) -> bool {
// Accept any obvious literals or names.
if matches!(
expr.node,
ExprKind::Constant { .. }
| ExprKind::Name { .. }
| ExprKind::List { .. }
| ExprKind::Tuple { .. }
| ExprKind::Set { .. }
) {
return true;
}
// Accept empty initializers.
if let ExprKind::Call {
func,
args,
keywords,
} = &expr.node
{
if args.is_empty() && keywords.is_empty() {
if let ExprKind::Name { id, .. } = &func.node {
return (id == "set"
|| id == "list"
|| id == "tuple"
|| id == "dict"
|| id == "frozenset")
&& checker.is_builtin(id);
}
}
}
false
}
enum DeletionKind {
Whole,
Partial,
}
/// Generate a `Fix` to remove an unused variable assignment, given the
/// enclosing `Stmt` and the `Range` of the variable binding.
fn remove_unused_variable(
stmt: &Stmt,
range: &Range,
checker: &Checker,
) -> Option<(DeletionKind, Fix)> {
// First case: simple assignment (`x = 1`)
if let StmtKind::Assign { targets, value, .. } = &stmt.node {
if targets.len() == 1 && matches!(targets[0].node, ExprKind::Name { .. }) {
return if is_literal_or_name(value, checker) {
// If assigning to a constant (`x = 1`), delete the entire statement.
let parent = checker
.child_to_parent
.get(&RefEquality(stmt))
.map(std::convert::Into::into);
let deleted: Vec<&Stmt> = checker
.deletions
.iter()
.map(std::convert::Into::into)
.collect();
let locator = checker.locator;
match delete_stmt(stmt, parent, &deleted, locator) {
Ok(fix) => Some((DeletionKind::Whole, fix)),
Err(err) => {
error!("Failed to delete unused variable: {}", err);
None
}
}
} else {
// If the expression is more complex (`x = foo()`), remove the assignment,
// but preserve the right-hand side.
Some((
DeletionKind::Partial,
Fix::deletion(stmt.location, value.location),
))
};
}
}
// Second case: simple annotated assignment (`x: int = 1`)
if let StmtKind::AnnAssign {
target,
value: Some(value),
..
} = &stmt.node
{
if matches!(target.node, ExprKind::Name { .. }) {
return if is_literal_or_name(value, checker) {
// If assigning to a constant (`x = 1`), delete the entire statement.
let parent = checker
.child_to_parent
.get(&RefEquality(stmt))
.map(std::convert::Into::into);
let deleted: Vec<&Stmt> = checker
.deletions
.iter()
.map(std::convert::Into::into)
.collect();
let locator = checker.locator;
match delete_stmt(stmt, parent, &deleted, locator) {
Ok(fix) => Some((DeletionKind::Whole, fix)),
Err(err) => {
error!("Failed to delete unused variable: {}", err);
None
}
}
} else {
// If the expression is more complex (`x = foo()`), remove the assignment,
// but preserve the right-hand side.
Some((
DeletionKind::Partial,
Fix::deletion(stmt.location, value.location),
))
};
}
}
// Third case: withitem (`with foo() as x:`)
if let StmtKind::With { items, .. } = &stmt.node {
// Find the binding that matches the given `Range`.
// TODO(charlie): Store the `Withitem` in the `Binding`.
for item in items {
if let Some(optional_vars) = &item.optional_vars {
if optional_vars.location == range.location
&& optional_vars.end_location.unwrap() == range.end_location
{
return Some((
DeletionKind::Partial,
Fix::deletion(
item.context_expr.end_location.unwrap(),
optional_vars.end_location.unwrap(),
),
));
}
}
}
}
None
}
/// F841
pub fn unused_variable(checker: &mut Checker, scope: usize) {
let scope = &checker.scopes[scope];
if scope.uses_locals && matches!(scope.kind, ScopeKind::Function(..)) {
return;
}
for (name, binding) in scope
.values
.iter()
.map(|(name, index)| (name, &checker.bindings[*index]))
{
if binding.used.is_none()
&& matches!(binding.kind, BindingKind::Assignment)
&& !checker.settings.dummy_variable_rgx.is_match(name)
&& name != &"__tracebackhide__"
&& name != &"__traceback_info__"
&& name != &"__traceback_supplement__"
{
let mut check = Check::new(
CheckKind::UnusedVariable((*name).to_string()),
binding.range,
);
if checker.patch(&CheckCode::F841) {
if let Some(stmt) = binding.source.as_ref().map(std::convert::Into::into) {
if let Some((kind, fix)) = remove_unused_variable(stmt, &binding.range, checker)
{
if matches!(kind, DeletionKind::Whole) {
checker.deletions.insert(RefEquality(stmt));
}
check.amend(fix);
}
}
}
checker.checks.push(check);
}
}
}

54
src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841_0.py.snap
View File

@@ -10,7 +10,14 @@ expression: checks
end_location:
row: 3
column: 22
fix: ~
fix:
content: ""
location:
row: 3
column: 17
end_location:
row: 3
column: 22
parent: ~
- kind:
UnusedVariable: z
@@ -20,7 +27,14 @@ expression: checks
end_location:
row: 16
column: 5
fix: ~
fix:
content: ""
location:
row: 16
column: 4
end_location:
row: 16
column: 8
parent: ~
- kind:
UnusedVariable: foo
@@ -30,7 +44,14 @@ expression: checks
end_location:
row: 20
column: 7
fix: ~
fix:
content: ""
location:
row: 20
column: 0
end_location:
row: 21
column: 0
parent: ~
- kind:
UnusedVariable: a
@@ -70,7 +91,14 @@ expression: checks
end_location:
row: 51
column: 9
fix: ~
fix:
content: pass
location:
row: 51
column: 8
end_location:
row: 51
column: 13
parent: ~
- kind:
UnusedVariable: my_file
@@ -80,7 +108,14 @@ expression: checks
end_location:
row: 79
column: 32
fix: ~
fix:
content: ""
location:
row: 79
column: 21
end_location:
row: 79
column: 32
parent: ~
- kind:
UnusedVariable: my_file
@@ -90,6 +125,13 @@ expression: checks
end_location:
row: 85
column: 31
fix: ~
fix:
content: ""
location:
row: 85
column: 20
end_location:
row: 85
column: 31
parent: ~

215
src/pyflakes/snapshots/ruff__pyflakes__tests__F841_F841_3.py.snap Normal file
View File

@@ -0,0 +1,215 @@
---
source: src/pyflakes/mod.rs
expression: checks
---
- kind:
UnusedVariable: x
location:
row: 5
column: 4
end_location:
row: 5
column: 5
fix:
content: ""
location:
row: 5
column: 0
end_location:
row: 6
column: 0
parent: ~
- kind:
UnusedVariable: y
location:
row: 6
column: 4
end_location:
row: 6
column: 5
fix:
content: ""
location:
row: 6
column: 0
end_location:
row: 7
column: 0
parent: ~
- kind:
UnusedVariable: x
location:
row: 13
column: 4
end_location:
row: 13
column: 5
fix:
content: ""
location:
row: 13
column: 0
end_location:
row: 14
column: 0
parent: ~
- kind:
UnusedVariable: y
location:
row: 14
column: 4
end_location:
row: 14
column: 5
fix:
content: ""
location:
row: 14
column: 0
end_location:
row: 15
column: 0
parent: ~
- kind:
UnusedVariable: x1
location:
row: 21
column: 18
end_location:
row: 21
column: 20
fix:
content: ""
location:
row: 21
column: 14
end_location:
row: 21
column: 20
parent: ~
- kind:
UnusedVariable: x3
location:
row: 27
column: 19
end_location:
row: 27
column: 21
fix:
content: ""
location:
row: 27
column: 15
end_location:
row: 27
column: 21
parent: ~
- kind:
UnusedVariable: y3
location:
row: 27
column: 32
end_location:
row: 27
column: 34
fix:
content: ""
location:
row: 27
column: 28
end_location:
row: 27
column: 34
parent: ~
- kind:
UnusedVariable: z3
location:
row: 27
column: 45
end_location:
row: 27
column: 47
fix:
content: ""
location:
row: 27
column: 41
end_location:
row: 27
column: 47
parent: ~
- kind:
UnusedVariable: x1
location:
row: 32
column: 5
end_location:
row: 32
column: 7
fix: ~
parent: ~
- kind:
UnusedVariable: y1
location:
row: 32
column: 9
end_location:
row: 32
column: 11
fix: ~
parent: ~
- kind:
UnusedVariable: coords2
location:
row: 33
column: 15
end_location:
row: 33
column: 22
fix: ~
parent: ~
- kind:
UnusedVariable: coords3
location:
row: 34
column: 4
end_location:
row: 34
column: 11
fix: ~
parent: ~
- kind:
UnusedVariable: x1
location:
row: 40
column: 25
end_location:
row: 40
column: 27
fix:
content: ""
location:
row: 40
column: 21
end_location:
row: 40
column: 27
parent: ~
- kind:
UnusedVariable: x2
location:
row: 45
column: 46
end_location:
row: 45
column: 48
fix:
content: ""
location:
row: 45
column: 42
end_location:
row: 45
column: 48
parent: ~

72
src/pyflakes/snapshots/ruff__pyflakes__tests__f841_dummy_variable_rgx.snap
View File

@@ -10,7 +10,14 @@ expression: checks
end_location:
row: 3
column: 22
fix: ~
fix:
content: ""
location:
row: 3
column: 17
end_location:
row: 3
column: 22
parent: ~
- kind:
UnusedVariable: foo
@@ -20,7 +27,14 @@ expression: checks
end_location:
row: 20
column: 7
fix: ~
fix:
content: ""
location:
row: 20
column: 0
end_location:
row: 21
column: 0
parent: ~
- kind:
UnusedVariable: a
@@ -60,7 +74,14 @@ expression: checks
end_location:
row: 35
column: 5
fix: ~
fix:
content: ""
location:
row: 35
column: 0
end_location:
row: 36
column: 0
parent: ~
- kind:
UnusedVariable: __
@@ -70,7 +91,14 @@ expression: checks
end_location:
row: 36
column: 6
fix: ~
fix:
content: ""
location:
row: 36
column: 0
end_location:
row: 37
column: 0
parent: ~
- kind:
UnusedVariable: _discarded
@@ -80,7 +108,14 @@ expression: checks
end_location:
row: 37
column: 14
fix: ~
fix:
content: pass
location:
row: 37
column: 4
end_location:
row: 37
column: 18
parent: ~
- kind:
UnusedVariable: b
@@ -90,7 +125,14 @@ expression: checks
end_location:
row: 51
column: 9
fix: ~
fix:
content: pass
location:
row: 51
column: 8
end_location:
row: 51
column: 13
parent: ~
- kind:
UnusedVariable: my_file
@@ -100,7 +142,14 @@ expression: checks
end_location:
row: 79
column: 32
fix: ~
fix:
content: ""
location:
row: 79
column: 21
end_location:
row: 79
column: 32
parent: ~
- kind:
UnusedVariable: my_file
@@ -110,6 +159,13 @@ expression: checks
end_location:
row: 85
column: 31
fix: ~
fix:
content: ""
location:
row: 85
column: 20
end_location:
row: 85
column: 31
parent: ~

4
src/registry.rs
View File

@@ -3811,6 +3811,7 @@ impl CheckKind {
| CheckKind::UnsortedImports
| CheckKind::UnusedLoopControlVariable(..)
| CheckKind::UnusedNOQA(..)
| CheckKind::UnusedVariable(..)
| CheckKind::UseFixturesWithoutParameters
| CheckKind::UsePEP585Annotation(..)
| CheckKind::UsePEP604Annotation
@@ -4093,6 +4094,9 @@ impl CheckKind {
Some(format!("Rename unused `{name}` to `_{name}`"))
}
CheckKind::UnusedNOQA(..) => Some("Remove unused `noqa` directive".to_string()),
CheckKind::UnusedVariable(name) => {
Some(format!("Remove assignment to unused variable `{name}`"))
}
CheckKind::UsePEP585Annotation(name) => {
Some(format!("Replace `{name}` with `{}`", name.to_lowercase(),))
}

9
src/ruff/snapshots/ruff__ruff__tests__ruf100_0.snap
View File

@@ -136,7 +136,14 @@ expression: checks
end_location:
row: 29
column: 5
fix: ~
fix:
content: ""
location:
row: 29
column: 0
end_location:
row: 30
column: 0
parent: ~
- kind:
UnusedNOQA: