From aaeab0ecf1ba4aaffeefb8ca5c53e77336878f80 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 8 Dec 2022 21:31:08 -0500 Subject: [PATCH] Implement F811 (`RedefinedWhileUnused`) (#1137) --- README.md | 2 +- flake8_to_ruff/src/parser.rs | 1 - resources/test/fixtures/pyflakes/F811_0.py | 11 + resources/test/fixtures/pyflakes/F811_1.py | 1 + resources/test/fixtures/pyflakes/F811_10.py | 8 + resources/test/fixtures/pyflakes/F811_11.py | 9 + resources/test/fixtures/pyflakes/F811_12.py | 7 + resources/test/fixtures/pyflakes/F811_13.py | 8 + resources/test/fixtures/pyflakes/F811_14.py | 10 + resources/test/fixtures/pyflakes/F811_15.py | 5 + resources/test/fixtures/pyflakes/F811_16.py | 9 + resources/test/fixtures/pyflakes/F811_17.py | 10 + resources/test/fixtures/pyflakes/F811_18.py | 14 + resources/test/fixtures/pyflakes/F811_19.py | 7 + resources/test/fixtures/pyflakes/F811_2.py | 1 + resources/test/fixtures/pyflakes/F811_20.py | 14 + resources/test/fixtures/pyflakes/F811_3.py | 1 + resources/test/fixtures/pyflakes/F811_4.py | 1 + resources/test/fixtures/pyflakes/F811_5.py | 1 + resources/test/fixtures/pyflakes/F811_6.py | 7 + resources/test/fixtures/pyflakes/F811_7.py | 8 + resources/test/fixtures/pyflakes/F811_8.py | 8 + resources/test/fixtures/pyflakes/F811_9.py | 7 + src/ast/branch_detection.rs | 105 ++++++ src/ast/mod.rs | 1 + src/ast/types.rs | 103 +++++- src/check_ast.rs | 319 ++++++++++++------ src/checks.rs | 8 + src/checks_gen.rs | 8 + src/flake8_print/plugins/print_call.rs | 22 +- src/pyflakes/checks.rs | 28 +- src/pyflakes/mod.rs | 21 ++ ...ruff__pyflakes__tests__F811_F811_0.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_1.py.snap | 16 + ...uff__pyflakes__tests__F811_F811_10.py.snap | 6 + ...uff__pyflakes__tests__F811_F811_11.py.snap | 6 + ...uff__pyflakes__tests__F811_F811_12.py.snap | 16 + ...uff__pyflakes__tests__F811_F811_13.py.snap | 6 + ...uff__pyflakes__tests__F811_F811_14.py.snap | 6 + ...uff__pyflakes__tests__F811_F811_15.py.snap | 16 + ...uff__pyflakes__tests__F811_F811_16.py.snap | 16 + ...uff__pyflakes__tests__F811_F811_17.py.snap | 27 ++ ...uff__pyflakes__tests__F811_F811_18.py.snap | 6 + ...uff__pyflakes__tests__F811_F811_19.py.snap | 6 + ...ruff__pyflakes__tests__F811_F811_2.py.snap | 16 + ...uff__pyflakes__tests__F811_F811_20.py.snap | 6 + ...ruff__pyflakes__tests__F811_F811_3.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_4.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_5.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_6.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_7.py.snap | 6 + ...ruff__pyflakes__tests__F811_F811_8.py.snap | 16 + ...ruff__pyflakes__tests__F811_F811_9.py.snap | 6 + src/pylint/plugins/use_sys_exit.rs | 6 +- .../plugins/super_call_with_parameters.rs | 6 +- .../plugins/unnecessary_future_import.rs | 16 +- .../plugins/useless_metaclass_type.rs | 18 +- 57 files changed, 882 insertions(+), 186 deletions(-) create mode 100644 resources/test/fixtures/pyflakes/F811_0.py create mode 100644 resources/test/fixtures/pyflakes/F811_1.py create mode 100644 resources/test/fixtures/pyflakes/F811_10.py create mode 100644 resources/test/fixtures/pyflakes/F811_11.py create mode 100644 resources/test/fixtures/pyflakes/F811_12.py create mode 100644 resources/test/fixtures/pyflakes/F811_13.py create mode 100644 resources/test/fixtures/pyflakes/F811_14.py create mode 100644 resources/test/fixtures/pyflakes/F811_15.py create mode 100644 resources/test/fixtures/pyflakes/F811_16.py create mode 100644 resources/test/fixtures/pyflakes/F811_17.py create mode 100644 resources/test/fixtures/pyflakes/F811_18.py create mode 100644 resources/test/fixtures/pyflakes/F811_19.py create mode 100644 resources/test/fixtures/pyflakes/F811_2.py create mode 100644 resources/test/fixtures/pyflakes/F811_20.py create mode 100644 resources/test/fixtures/pyflakes/F811_3.py create mode 100644 resources/test/fixtures/pyflakes/F811_4.py create mode 100644 resources/test/fixtures/pyflakes/F811_5.py create mode 100644 resources/test/fixtures/pyflakes/F811_6.py create mode 100644 resources/test/fixtures/pyflakes/F811_7.py create mode 100644 resources/test/fixtures/pyflakes/F811_8.py create mode 100644 resources/test/fixtures/pyflakes/F811_9.py create mode 100644 src/ast/branch_detection.rs create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_0.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_1.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_10.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_11.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_12.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_13.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_14.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_15.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_16.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_17.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_18.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_19.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_2.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_20.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_3.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_4.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_5.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_6.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_7.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_8.py.snap create mode 100644 src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_9.py.snap diff --git a/README.md b/README.md index f86094c2b0..41beba5a9c 100644 --- a/README.md +++ b/README.md @@ -948,7 +948,7 @@ automatically convert your existing configuration.) Ruff can be used as a drop-in replacement for Flake8 when used (1) without or with a small number of plugins, (2) alongside Black, and (3) on Python 3 code. -Under those conditions, Ruff implements every rule in Flake8, with the exception of `F811`. +Under those conditions, Ruff implements every rule in Flake8. Ruff also re-implements some of the most popular Flake8 plugins and related code quality tools natively, including: diff --git a/flake8_to_ruff/src/parser.rs b/flake8_to_ruff/src/parser.rs index 4608bd68b6..d9742ced1e 100644 --- a/flake8_to_ruff/src/parser.rs +++ b/flake8_to_ruff/src/parser.rs @@ -135,7 +135,6 @@ fn tokenize_files_to_codes_mapping(value: &str) -> Vec { } /// Parse a 'files-to-codes' mapping, mimicking Flake8's internal logic. -/// /// See: pub fn parse_files_to_codes_mapping(value: &str) -> Result> { if value.trim().is_empty() { diff --git a/resources/test/fixtures/pyflakes/F811_0.py b/resources/test/fixtures/pyflakes/F811_0.py new file mode 100644 index 0000000000..09a41b9d1c --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_0.py @@ -0,0 +1,11 @@ +def foo(x): + return x + + +@foo +def bar(): + pass + + +def bar(): + pass diff --git a/resources/test/fixtures/pyflakes/F811_1.py b/resources/test/fixtures/pyflakes/F811_1.py new file mode 100644 index 0000000000..2f186ff128 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_1.py @@ -0,0 +1 @@ +import fu as FU, bar as FU diff --git a/resources/test/fixtures/pyflakes/F811_10.py b/resources/test/fixtures/pyflakes/F811_10.py new file mode 100644 index 0000000000..5bb8083dd1 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_10.py @@ -0,0 +1,8 @@ +"""Test that importing a module twice using a nested does not issue a warning.""" +try: + if True: + if True: + import os +except: + import os +os.path diff --git a/resources/test/fixtures/pyflakes/F811_11.py b/resources/test/fixtures/pyflakes/F811_11.py new file mode 100644 index 0000000000..bf84fc7428 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_11.py @@ -0,0 +1,9 @@ +try: + from aa import mixer +except AttributeError: + from bb import mixer +except RuntimeError: + from cc import mixer +except: + from dd import mixer +mixer(123) diff --git a/resources/test/fixtures/pyflakes/F811_12.py b/resources/test/fixtures/pyflakes/F811_12.py new file mode 100644 index 0000000000..8bc9edbd4e --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_12.py @@ -0,0 +1,7 @@ +try: + from aa import mixer +except ImportError: + pass +else: + from bb import mixer +mixer(123) diff --git a/resources/test/fixtures/pyflakes/F811_13.py b/resources/test/fixtures/pyflakes/F811_13.py new file mode 100644 index 0000000000..8d534b52b4 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_13.py @@ -0,0 +1,8 @@ +try: + import funca +except ImportError: + from bb import funca + from bb import funcb +else: + from bbb import funcb +print(funca, funcb) diff --git a/resources/test/fixtures/pyflakes/F811_14.py b/resources/test/fixtures/pyflakes/F811_14.py new file mode 100644 index 0000000000..2fe2532da2 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_14.py @@ -0,0 +1,10 @@ +try: + import b +except ImportError: + b = Ellipsis + from bb import a +else: + from aa import a +finally: + a = 42 +print(a, b) diff --git a/resources/test/fixtures/pyflakes/F811_15.py b/resources/test/fixtures/pyflakes/F811_15.py new file mode 100644 index 0000000000..6c176318e6 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_15.py @@ -0,0 +1,5 @@ +import fu + + +def fu(): + pass diff --git a/resources/test/fixtures/pyflakes/F811_16.py b/resources/test/fixtures/pyflakes/F811_16.py new file mode 100644 index 0000000000..4ee48502f7 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_16.py @@ -0,0 +1,9 @@ +"""Test that shadowing a global with a nested function generates a warning.""" + +import fu + + +def bar(): + def baz(): + def fu(): + pass diff --git a/resources/test/fixtures/pyflakes/F811_17.py b/resources/test/fixtures/pyflakes/F811_17.py new file mode 100644 index 0000000000..fa3ec02814 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_17.py @@ -0,0 +1,10 @@ +"""Test that shadowing a global name with a nested function generates a warning.""" +import fu + + +def bar(): + import fu + + def baz(): + def fu(): + pass diff --git a/resources/test/fixtures/pyflakes/F811_18.py b/resources/test/fixtures/pyflakes/F811_18.py new file mode 100644 index 0000000000..05d8b758c7 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_18.py @@ -0,0 +1,14 @@ +"""Test that a global import which is redefined locally, but used later in another scope does not generate a warning.""" + +import unittest, transport + + +class GetTransportTestCase(unittest.TestCase): + def test_get_transport(self): + transport = 'transport' + self.assertIsNotNone(transport) + + +class TestTransportMethodArgs(unittest.TestCase): + def test_send_defaults(self): + transport.Transport() diff --git a/resources/test/fixtures/pyflakes/F811_19.py b/resources/test/fixtures/pyflakes/F811_19.py new file mode 100644 index 0000000000..6082315720 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_19.py @@ -0,0 +1,7 @@ +"""If an imported name is redefined by a class statement which also uses that name in the bases list, no warning is emitted.""" + +from fu import bar + + +class bar(bar): + pass diff --git a/resources/test/fixtures/pyflakes/F811_2.py b/resources/test/fixtures/pyflakes/F811_2.py new file mode 100644 index 0000000000..0311b23383 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_2.py @@ -0,0 +1 @@ +from moo import fu as FU, bar as FU diff --git a/resources/test/fixtures/pyflakes/F811_20.py b/resources/test/fixtures/pyflakes/F811_20.py new file mode 100644 index 0000000000..9c5d3a96f5 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_20.py @@ -0,0 +1,14 @@ +""" + Test that shadowing a global with a class attribute does not produce a + warning. + """ + +import fu + + +class bar: + # STOPSHIP: This errors. + fu = 1 + + +print(fu) diff --git a/resources/test/fixtures/pyflakes/F811_3.py b/resources/test/fixtures/pyflakes/F811_3.py new file mode 100644 index 0000000000..bbe1104532 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_3.py @@ -0,0 +1 @@ +import fu; fu = 3 diff --git a/resources/test/fixtures/pyflakes/F811_4.py b/resources/test/fixtures/pyflakes/F811_4.py new file mode 100644 index 0000000000..d0bcfef2b3 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_4.py @@ -0,0 +1 @@ +import fu; fu, bar = 3 diff --git a/resources/test/fixtures/pyflakes/F811_5.py b/resources/test/fixtures/pyflakes/F811_5.py new file mode 100644 index 0000000000..f6ff4f09a9 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_5.py @@ -0,0 +1 @@ +import fu; [fu, bar] = 3 diff --git a/resources/test/fixtures/pyflakes/F811_6.py b/resources/test/fixtures/pyflakes/F811_6.py new file mode 100644 index 0000000000..ff3686fb13 --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_6.py @@ -0,0 +1,7 @@ +"""Test that importing a module twice within an if block does raise a warning.""" + +i = 2 +if i == 1: + import os + import os +os.path diff --git a/resources/test/fixtures/pyflakes/F811_7.py b/resources/test/fixtures/pyflakes/F811_7.py new file mode 100644 index 0000000000..8a1231144c --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_7.py @@ -0,0 +1,8 @@ +"""Test that importing a module twice in if-else blocks does not raise a warning.""" + +i = 2 +if i == 1: + import os +else: + import os +os.path diff --git a/resources/test/fixtures/pyflakes/F811_8.py b/resources/test/fixtures/pyflakes/F811_8.py new file mode 100644 index 0000000000..d4340c2f8b --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_8.py @@ -0,0 +1,8 @@ +"""Test that importing a module twice in a try block does raise a warning.""" + +try: + import os + import os +except: + pass +os.path diff --git a/resources/test/fixtures/pyflakes/F811_9.py b/resources/test/fixtures/pyflakes/F811_9.py new file mode 100644 index 0000000000..29aa4e7bab --- /dev/null +++ b/resources/test/fixtures/pyflakes/F811_9.py @@ -0,0 +1,7 @@ +"""Test that importing a module twice in a try block does not raise a warning.""" + +try: + import os +except: + import os +os.path diff --git a/src/ast/branch_detection.rs b/src/ast/branch_detection.rs new file mode 100644 index 0000000000..8ef3c8e8ff --- /dev/null +++ b/src/ast/branch_detection.rs @@ -0,0 +1,105 @@ +use std::cmp::Ordering; + +use rustc_hash::FxHashMap; +use rustpython_ast::ExcepthandlerKind::ExceptHandler; +use rustpython_ast::Stmt; +use rustpython_parser::ast::StmtKind; + +use crate::ast::types::RefEquality; + +/// Return the common ancestor of `left` and `right` below `stop`, or `None`. +fn common_ancestor<'a>( + left: &'a RefEquality<'a, Stmt>, + right: &'a RefEquality<'a, Stmt>, + stop: Option<&'a RefEquality<'a, Stmt>>, + depths: &'a FxHashMap, usize>, + child_to_parent: &'a FxHashMap, RefEquality<'a, Stmt>>, +) -> Option<&'a RefEquality<'a, Stmt>> { + if let Some(stop) = stop { + if left == stop || right == stop { + return None; + } + } + if left == right { + return Some(left); + } + + let left_depth = depths.get(left)?; + let right_depth = depths.get(right)?; + match left_depth.cmp(right_depth) { + Ordering::Less => common_ancestor( + left, + child_to_parent.get(right)?, + stop, + depths, + child_to_parent, + ), + Ordering::Equal => common_ancestor( + child_to_parent.get(left)?, + child_to_parent.get(right)?, + stop, + depths, + child_to_parent, + ), + Ordering::Greater => common_ancestor( + child_to_parent.get(left)?, + right, + stop, + depths, + child_to_parent, + ), + } +} + +/// Return the alternative branches for a given node. +fn alternatives<'a>(node: &'a RefEquality<'a, Stmt>) -> Vec>> { + match &node.0.node { + StmtKind::If { body, .. } => vec![body.iter().map(RefEquality).collect()], + StmtKind::Try { + body, + handlers, + orelse, + .. + } => vec![body.iter().chain(orelse.iter()).map(RefEquality).collect()] + .into_iter() + .chain(handlers.iter().map(|handler| { + let ExceptHandler { body, .. } = &handler.node; + body.iter().map(RefEquality).collect() + })) + .collect(), + _ => vec![], + } +} + +/// Return `true` if `node` is a descendent of any of the nodes in `ancestors`. +fn descendant_of<'a>( + node: &RefEquality<'a, Stmt>, + ancestors: &[RefEquality<'a, Stmt>], + stop: &RefEquality<'a, Stmt>, + depths: &FxHashMap, usize>, + child_to_parent: &FxHashMap, RefEquality<'a, Stmt>>, +) -> bool { + ancestors.iter().any(|ancestor| { + common_ancestor(node, ancestor, Some(stop), depths, child_to_parent).is_some() + }) +} + +/// Return `true` if `left` and `right` are on different branches of an `if` or +/// `try` statement. +pub fn different_forks<'a>( + left: &RefEquality<'a, Stmt>, + right: &RefEquality<'a, Stmt>, + depths: &FxHashMap, usize>, + child_to_parent: &FxHashMap, RefEquality<'a, Stmt>>, +) -> bool { + if let Some(ancestor) = common_ancestor(left, right, None, depths, child_to_parent) { + for items in alternatives(ancestor) { + let l = descendant_of(left, &items, ancestor, depths, child_to_parent); + let r = descendant_of(right, &items, ancestor, depths, child_to_parent); + if l ^ r { + return true; + } + } + } + false +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 9e807a28df..855f89af71 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -1,3 +1,4 @@ +pub mod branch_detection; pub mod cast; pub mod function_type; pub mod helpers; diff --git a/src/ast/types.rs b/src/ast/types.rs index 2bcbace0af..f0dbf1db8f 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -88,38 +88,117 @@ impl<'a> Scope<'a> { } } -#[derive(Clone, Debug)] -pub struct BindingContext { - pub defined_by: usize, - pub defined_in: Option, -} - #[derive(Clone, Debug)] pub enum BindingKind { Annotation, Argument, Assignment, - // TODO(charlie): This seems to be a catch-all. Binding, LoopVar, Global, Nonlocal, Builtin, ClassDefinition, - Definition, + FunctionDefinition, Export(Vec), FutureImportation, StarImportation(Option, Option), - Importation(String, String, BindingContext), - FromImportation(String, String, BindingContext), - SubmoduleImportation(String, String, BindingContext), + Importation(String, String), + FromImportation(String, String), + SubmoduleImportation(String, String), } #[derive(Clone, Debug)] -pub struct Binding { +pub struct Binding<'a> { pub kind: BindingKind, pub range: Range, + /// The statement in which the `Binding` was defined. + pub source: Option>, /// Tuple of (scope index, range) indicating the scope and range at which /// the binding was last used. pub used: Option<(usize, Range)>, + /// A list of pointers to `Binding` instances that redefined this binding. + pub redefined: Vec, } + +// Pyflakes defines the following binding hierarchy (via inheritance): +// Binding +// ExportBinding +// Annotation +// Argument +// Assignment +// NamedExprAssignment +// Definition +// FunctionDefinition +// ClassDefinition +// Builtin +// Importation +// SubmoduleImportation +// ImportationFrom +// StarImportation +// FutureImportation + +impl<'a> Binding<'a> { + pub fn is_definition(&self) -> bool { + matches!( + self.kind, + BindingKind::ClassDefinition + | BindingKind::FunctionDefinition + | BindingKind::Builtin + | BindingKind::FutureImportation + | BindingKind::StarImportation(..) + | BindingKind::Importation(..) + | BindingKind::FromImportation(..) + | BindingKind::SubmoduleImportation(..) + ) + } + + pub fn redefines(&self, existing: &'a Binding) -> bool { + match &self.kind { + BindingKind::Importation(_, full_name) | BindingKind::FromImportation(_, full_name) => { + if let BindingKind::SubmoduleImportation(_, existing_full_name) = &existing.kind { + return full_name == existing_full_name; + } + } + BindingKind::SubmoduleImportation(_, full_name) => { + if let BindingKind::Importation(_, existing_full_name) + | BindingKind::FromImportation(_, existing_full_name) + | BindingKind::SubmoduleImportation(_, existing_full_name) = &existing.kind + { + return full_name == existing_full_name; + } + } + BindingKind::Annotation => { + return false; + } + BindingKind::FutureImportation => { + return false; + } + BindingKind::StarImportation(..) => { + return false; + } + _ => {} + } + existing.is_definition() + } +} + +#[derive(Debug, Copy, Clone)] +pub struct RefEquality<'a, T>(pub &'a T); + +impl<'a, T> std::hash::Hash for RefEquality<'a, T> { + fn hash(&self, state: &mut H) + where + H: std::hash::Hasher, + { + (self.0 as *const T).hash(state); + } +} + +impl<'a, 'b, T> PartialEq> for RefEquality<'a, T> { + fn eq(&self, other: &RefEquality<'b, T>) -> bool { + std::ptr::eq(self.0, other.0) + } +} + +impl<'a, T> Eq for RefEquality<'a, T> {} diff --git a/src/check_ast.rs b/src/check_ast.rs index 22db1ec5c2..f80e3eba73 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1,6 +1,5 @@ //! Lint rules based on AST traversal. -use std::collections::BTreeMap; use std::path::Path; use log::error; @@ -17,11 +16,10 @@ use crate::ast::helpers::{ use crate::ast::operations::extract_all_names; use crate::ast::relocate::relocate_expr; use crate::ast::types::{ - Binding, BindingContext, BindingKind, ClassDef, FunctionDef, Lambda, Node, Range, Scope, - ScopeKind, + Binding, BindingKind, ClassDef, FunctionDef, Lambda, Node, Range, RefEquality, Scope, ScopeKind, }; use crate::ast::visitor::{walk_excepthandler, Visitor}; -use crate::ast::{helpers, operations, visitor}; +use crate::ast::{branch_detection, cast, helpers, operations, visitor}; use crate::checks::{Check, CheckCode, CheckKind, DeferralKeyword}; use crate::docstrings::definition::{Definition, DefinitionKind, Documentable}; use crate::python::builtins::{BUILTINS, MAGIC_GLOBALS}; @@ -38,12 +36,12 @@ use crate::{ flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_debugger, flake8_import_conventions, flake8_print, flake8_return, flake8_tidy_imports, flake8_unused_arguments, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, - pylint, pyupgrade, + pylint, pyupgrade, visibility, }; const GLOBAL_SCOPE_INDEX: usize = 0; -type DeferralContext = (Vec, Vec); +type DeferralContext<'a> = (Vec, Vec>); #[allow(clippy::struct_excessive_bools)] pub struct Checker<'a> { @@ -58,23 +56,24 @@ pub struct Checker<'a> { definitions: Vec<(Definition<'a>, Visibility)>, // Edit tracking. // TODO(charlie): Instead of exposing deletions, wrap in a public API. - pub(crate) deletions: FxHashSet, + pub(crate) deletions: FxHashSet>, // Import tracking. pub(crate) from_imports: FxHashMap<&'a str, FxHashSet<&'a str>>, pub(crate) import_aliases: FxHashMap<&'a str, &'a str>, // Retain all scopes and parent nodes, along with a stack of indexes to track which are active // at various points in time. - pub(crate) parents: Vec<&'a Stmt>, - pub(crate) parent_stack: Vec, - pub(crate) bindings: Vec, + pub(crate) parents: Vec>, + pub(crate) depths: FxHashMap, usize>, + pub(crate) child_to_parent: FxHashMap, RefEquality<'a, Stmt>>, + pub(crate) bindings: Vec>, scopes: Vec>, scope_stack: Vec, dead_scopes: Vec, - deferred_string_type_definitions: Vec<(Range, &'a str, bool, DeferralContext)>, - deferred_type_definitions: Vec<(&'a Expr, bool, DeferralContext)>, - deferred_functions: Vec<(&'a Stmt, DeferralContext, VisibleScope)>, - deferred_lambdas: Vec<(&'a Expr, DeferralContext)>, - deferred_assignments: Vec<(usize, DeferralContext)>, + 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)>, + deferred_lambdas: Vec<(&'a Expr, DeferralContext<'a>)>, + deferred_assignments: Vec<(usize, DeferralContext<'a>)>, // Internal, derivative state. visible_scope: VisibleScope, in_f_string: Option, @@ -110,7 +109,8 @@ impl<'a> Checker<'a> { from_imports: FxHashMap::default(), import_aliases: FxHashMap::default(), parents: vec![], - parent_stack: vec![], + depths: FxHashMap::default(), + child_to_parent: FxHashMap::default(), bindings: vec![], scopes: vec![], scope_stack: vec![], @@ -223,11 +223,7 @@ where if !self.seen_import_boundary && !helpers::is_assignment_to_a_dunder(node) && !operations::in_nested_block( - &mut self - .parent_stack - .iter() - .rev() - .map(|index| self.parents[*index]), + &mut self.parents.iter().rev().map(|node| node.0), ) { self.seen_import_boundary = true; @@ -249,6 +245,8 @@ where kind: BindingKind::Global, used: usage, range: Range::from_located(stmt), + source: None, + redefined: vec![], }); scope.values.insert(name, index); } @@ -287,6 +285,8 @@ where kind: BindingKind::Nonlocal, used: usage, range: Range::from_located(stmt), + source: None, + redefined: vec![], }); scope.values.insert(name, index); } @@ -318,8 +318,7 @@ where if self.settings.enabled.contains(&CheckCode::F701) { if let Some(check) = pyflakes::checks::break_outside_loop( stmt, - &self.parents, - &self.parent_stack, + &mut self.parents.iter().rev().map(|node| node.0).skip(1), ) { self.add_check(check); } @@ -329,8 +328,7 @@ where if self.settings.enabled.contains(&CheckCode::F702) { if let Some(check) = pyflakes::checks::continue_outside_loop( stmt, - &self.parents, - &self.parent_stack, + &mut self.parents.iter().rev().map(|node| node.0).skip(1), ) { self.add_check(check); } @@ -500,9 +498,11 @@ where self.add_binding( name, Binding { - kind: BindingKind::Definition, + kind: BindingKind::FunctionDefinition, used: None, range: Range::from_located(stmt), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); } @@ -608,10 +608,11 @@ where kind: BindingKind::SubmoduleImportation( name.to_string(), full_name.to_string(), - self.binding_context(), ), used: None, range: Range::from_located(alias), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); } else { @@ -630,7 +631,6 @@ where kind: BindingKind::Importation( name.to_string(), full_name.to_string(), - self.binding_context(), ), // Treat explicit re-export as usage (e.g., `import applications // as applications`). @@ -652,6 +652,8 @@ where None }, range: Range::from_located(alias), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); } @@ -795,6 +797,8 @@ where Range::from_located(alias), )), range: Range::from_located(alias), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); @@ -825,6 +829,8 @@ where kind: BindingKind::StarImportation(*level, module.clone()), used: None, range: Range::from_located(stmt), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); @@ -872,11 +878,7 @@ where self.add_binding( name, Binding { - kind: BindingKind::FromImportation( - name.to_string(), - full_name, - self.binding_context(), - ), + kind: BindingKind::FromImportation(name.to_string(), full_name), // Treat explicit re-export as usage (e.g., `from .applications // import FastAPI as FastAPI`). used: if alias @@ -897,6 +899,8 @@ where None }, range, + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); } @@ -1143,7 +1147,7 @@ where self.deferred_functions.push(( stmt, - (self.scope_stack.clone(), self.parent_stack.clone()), + (self.scope_stack.clone(), self.parents.clone()), self.visible_scope.clone(), )); } @@ -1221,6 +1225,8 @@ where kind: BindingKind::ClassDefinition, used: None, range: Range::from_located(stmt), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); }; @@ -1256,13 +1262,13 @@ where Range::from_located(expr), value, self.in_annotation, - (self.scope_stack.clone(), self.parent_stack.clone()), + (self.scope_stack.clone(), self.parents.clone()), )); } else { self.deferred_type_definitions.push(( expr, self.in_annotation, - (self.scope_stack.clone(), self.parent_stack.clone()), + (self.scope_stack.clone(), self.parents.clone()), )); } return; @@ -1345,7 +1351,7 @@ where self.check_builtin_shadowing(id, Range::from_located(expr), true); - self.handle_node_store(id, expr, self.current_parent()); + self.handle_node_store(id, expr); } ExprContext::Del => self.handle_node_delete(expr), } @@ -2053,7 +2059,7 @@ where Range::from_located(expr), value, self.in_annotation, - (self.scope_stack.clone(), self.parent_stack.clone()), + (self.scope_stack.clone(), self.parents.clone()), )); } if self.settings.enabled.contains(&CheckCode::S104) { @@ -2136,7 +2142,7 @@ where match &expr.node { ExprKind::Lambda { .. } => { self.deferred_lambdas - .push((expr, (self.scope_stack.clone(), self.parent_stack.clone()))); + .push((expr, (self.scope_stack.clone(), self.parents.clone()))); } ExprKind::Call { func, @@ -2387,7 +2393,6 @@ where ctx: ExprContext::Store, }, ), - self.current_parent(), ); } @@ -2402,7 +2407,6 @@ where ctx: ExprContext::Store, }, ), - self.current_parent(), ); walk_excepthandler(self, excepthandler); @@ -2484,6 +2488,8 @@ where kind: BindingKind::Argument, used: None, range: Range::from_located(arg), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); @@ -2510,14 +2516,20 @@ where impl<'a> Checker<'a> { fn push_parent(&mut self, parent: &'a Stmt) { - self.parent_stack.push(self.parents.len()); - self.parents.push(parent); + let num_existing = self.parents.len(); + self.parents.push(RefEquality(parent)); + self.depths + .insert(self.parents[num_existing].clone(), num_existing); + if num_existing > 0 { + self.child_to_parent.insert( + self.parents[num_existing].clone(), + self.parents[num_existing - 1].clone(), + ); + } } fn pop_parent(&mut self) { - self.parent_stack - .pop() - .expect("Attempted to pop without scope"); + self.parents.pop().expect("Attempted to pop without scope"); } fn push_scope(&mut self, scope: Scope<'a>) { @@ -2535,12 +2547,15 @@ impl<'a> Checker<'a> { fn bind_builtins(&mut self) { let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; + for builtin in BUILTINS.iter().chain(MAGIC_GLOBALS.iter()) { let index = self.bindings.len(); self.bindings.push(Binding { kind: BindingKind::Builtin, range: Range::default(), used: None, + source: None, + redefined: vec![], }); scope.values.insert(builtin, index); } @@ -2551,48 +2566,88 @@ impl<'a> Checker<'a> { } pub fn current_scopes(&self) -> impl Iterator { - self.scope_stack.iter().rev().map(|s| &self.scopes[*s]) + self.scope_stack + .iter() + .rev() + .map(|index| &self.scopes[*index]) } - pub fn current_parent(&self) -> &'a Stmt { - self.parents[*(self.parent_stack.last().expect("No parent found"))] + pub fn current_parent(&self) -> &RefEquality<'a, Stmt> { + self.parents.iter().rev().next().expect("No parent found") } - pub fn binding_context(&self) -> BindingContext { - let mut rev = self.parent_stack.iter().rev().fuse(); - let defined_by = *rev.next().expect("Expected to bind within a statement"); - let defined_in = rev.next().copied(); - BindingContext { - defined_by, - defined_in, - } + pub fn current_grandparent(&self) -> Option<&RefEquality<'a, Stmt>> { + self.parents.iter().rev().nth(1) } - fn add_binding<'b>(&mut self, name: &'b str, binding: Binding) + fn add_binding<'b>(&mut self, name: &'b str, binding: Binding<'a>) where 'b: 'a, { - if self.settings.enabled.contains(&CheckCode::F402) { - let scope = self.current_scope(); - if let Some(index) = scope.values.get(&name) { - let existing = &self.bindings[*index]; - if matches!(binding.kind, BindingKind::LoopVar) - && matches!( - existing.kind, - BindingKind::Importation(..) - | BindingKind::FromImportation(..) - | BindingKind::SubmoduleImportation(..) - | BindingKind::StarImportation(..) - | BindingKind::FutureImportation - ) - { - self.add_check(Check::new( - CheckKind::ImportShadowedByLoopVar( - name.to_string(), - existing.range.location.row(), - ), - binding.range, - )); + let index = self.bindings.len(); + + if let Some((stack_index, scope_index)) = self + .scope_stack + .iter() + .rev() + .enumerate() + .find(|(_, scope_index)| self.scopes[**scope_index].values.contains_key(&name)) + { + let existing_index = self.scopes[*scope_index].values.get(&name).unwrap(); + let existing = &self.bindings[*existing_index]; + let in_current_scope = stack_index == 0; + if !matches!(existing.kind, BindingKind::Builtin) + && existing.source.as_ref().map_or(true, |left| { + binding.source.as_ref().map_or(true, |right| { + !branch_detection::different_forks( + left, + right, + &self.depths, + &self.child_to_parent, + ) + }) + }) + { + let existing_is_import = matches!( + existing.kind, + BindingKind::Importation(..) + | BindingKind::FromImportation(..) + | BindingKind::SubmoduleImportation(..) + | BindingKind::StarImportation(..) + | BindingKind::FutureImportation + ); + if matches!(binding.kind, BindingKind::LoopVar) && existing_is_import { + if self.settings.enabled.contains(&CheckCode::F402) { + self.add_check(Check::new( + CheckKind::ImportShadowedByLoopVar( + name.to_string(), + existing.range.location.row(), + ), + binding.range, + )); + } + } else if in_current_scope { + if existing.used.is_none() + && binding.redefines(existing) + && (!self.settings.dummy_variable_rgx.is_match(name) || existing_is_import) + && !(matches!(existing.kind, BindingKind::FunctionDefinition) + && visibility::is_overload( + self, + cast::decorator_list(existing.source.as_ref().unwrap().0), + )) + { + if self.settings.enabled.contains(&CheckCode::F811) { + self.add_check(Check::new( + CheckKind::RedefinedWhileUnused( + name.to_string(), + existing.range.location.row(), + ), + binding.range, + )); + } + } + } else if existing_is_import && binding.redefines(existing) { + self.bindings[*existing_index].redefined.push(index); } } } @@ -2603,13 +2658,11 @@ impl<'a> Checker<'a> { let binding = match scope.values.get(&name) { None => binding, Some(index) => Binding { - kind: binding.kind, - range: binding.range, used: self.bindings[*index].used, + ..binding }, }; - let index = self.bindings.len(); self.bindings.push(binding); let scope = &mut self.scopes[*(self.scope_stack.last().expect("No current scope found"))]; @@ -2646,9 +2699,9 @@ impl<'a> Checker<'a> { // import pyarrow as pa // import pyarrow.csv // print(pa.csv.read_csv("test.csv")) - if let BindingKind::Importation(name, full_name, _) - | BindingKind::FromImportation(name, full_name, _) - | BindingKind::SubmoduleImportation(name, full_name, _) = + if let BindingKind::Importation(name, full_name) + | BindingKind::FromImportation(name, full_name) + | BindingKind::SubmoduleImportation(name, full_name) = &self.bindings[*index].kind { let has_alias = full_name @@ -2728,10 +2781,12 @@ impl<'a> Checker<'a> { } } - fn handle_node_store<'b>(&mut self, id: &'b str, expr: &Expr, parent: &Stmt) + fn handle_node_store<'b>(&mut self, id: &'b str, expr: &Expr) where 'b: 'a, { + let parent = self.current_parent().0; + if self.settings.enabled.contains(&CheckCode::F823) { let scopes: Vec<&Scope> = self .scope_stack @@ -2775,6 +2830,8 @@ impl<'a> Checker<'a> { kind: BindingKind::Annotation, used: None, range: Range::from_located(expr), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); return; @@ -2791,6 +2848,8 @@ impl<'a> Checker<'a> { kind: BindingKind::LoopVar, used: None, range: Range::from_located(expr), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); return; @@ -2803,6 +2862,8 @@ impl<'a> Checker<'a> { kind: BindingKind::Binding, used: None, range: Range::from_located(expr), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); return; @@ -2822,6 +2883,8 @@ impl<'a> Checker<'a> { kind: BindingKind::Export(extract_all_names(parent, current, &self.bindings)), used: None, range: Range::from_located(expr), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); return; @@ -2833,6 +2896,8 @@ impl<'a> Checker<'a> { kind: BindingKind::Assignment, used: None, range: Range::from_located(expr), + source: Some(self.current_parent().clone()), + redefined: vec![], }, ); } @@ -2842,13 +2907,8 @@ impl<'a> Checker<'a> { 'b: 'a, { if let ExprKind::Name { id, .. } = &expr.node { - if operations::on_conditional_branch( - &mut self - .parent_stack - .iter() - .rev() - .map(|index| self.parents[*index]), - ) { + if operations::on_conditional_branch(&mut self.parents.iter().rev().map(|node| node.0)) + { return; } @@ -2892,7 +2952,7 @@ impl<'a> Checker<'a> { self.deferred_type_definitions.pop() { self.scope_stack = scopes; - self.parent_stack = parents; + self.parents = parents; self.in_annotation = in_annotation; self.in_type_definition = true; self.in_deferred_type_definition = true; @@ -2925,7 +2985,7 @@ impl<'a> Checker<'a> { } for (expr, (in_annotation, (scopes, parents))) in allocator.iter().zip(stacks) { self.scope_stack = scopes; - self.parent_stack = parents; + self.parents = parents; self.in_annotation = in_annotation; self.in_type_definition = true; self.in_deferred_string_type_definition = true; @@ -2938,7 +2998,7 @@ impl<'a> Checker<'a> { fn check_deferred_functions(&mut self) { while let Some((stmt, (scopes, parents), visibility)) = self.deferred_functions.pop() { self.scope_stack = scopes.clone(); - self.parent_stack = parents.clone(); + self.parents = parents.clone(); self.visible_scope = visibility; match &stmt.node { @@ -2983,7 +3043,7 @@ impl<'a> Checker<'a> { fn check_deferred_lambdas(&mut self) { while let Some((expr, (scopes, parents))) = self.deferred_lambdas.pop() { self.scope_stack = scopes.clone(); - self.parent_stack = parents.clone(); + self.parents = parents.clone(); if let ExprKind::Lambda { args, body } = &expr.node { self.push_scope(Scope::new(ScopeKind::Lambda(Lambda { args, body }))); @@ -3038,6 +3098,7 @@ impl<'a> Checker<'a> { fn check_dead_scopes(&mut self) { if !self.settings.enabled.contains(&CheckCode::F401) && !self.settings.enabled.contains(&CheckCode::F405) + && !self.settings.enabled.contains(&CheckCode::F811) && !self.settings.enabled.contains(&CheckCode::F822) { return; @@ -3072,6 +3133,41 @@ impl<'a> Checker<'a> { } } + if self.settings.enabled.contains(&CheckCode::F811) { + for (name, index) in &scope.values { + let binding = &self.bindings[*index]; + + if matches!( + binding.kind, + BindingKind::Importation(..) + | BindingKind::FromImportation(..) + | BindingKind::SubmoduleImportation(..) + | BindingKind::StarImportation(..) + | BindingKind::FutureImportation + ) { + // Skip used exports from `__all__` + if binding.used.is_some() + || all_names + .as_ref() + .map(|names| names.contains(name)) + .unwrap_or_default() + { + continue; + } + + for index in &binding.redefined { + checks.push(Check::new( + CheckKind::RedefinedWhileUnused( + (*name).to_string(), + binding.range.location.row(), + ), + self.bindings[*index].range, + )); + } + } + } + } + if self.settings.enabled.contains(&CheckCode::F405) { if scope.import_starred { if let Some(all_binding) = all_binding { @@ -3108,15 +3204,17 @@ impl<'a> Checker<'a> { // Collect all unused imports by location. (Multiple unused imports at the same // location indicates an `import from`.) type UnusedImport<'a> = (&'a String, &'a Range); + type BindingContext<'a, 'b> = + (&'a RefEquality<'b, Stmt>, Option<&'a RefEquality<'b, Stmt>>); - let mut unused: BTreeMap<(usize, Option), Vec> = - BTreeMap::new(); + let mut unused: FxHashMap> = FxHashMap::default(); for (name, index) in &scope.values { let binding = &self.bindings[*index]; - let (BindingKind::Importation(_, full_name, context) - | BindingKind::SubmoduleImportation(_, full_name, context) - | BindingKind::FromImportation(_, full_name, context)) = &binding.kind else { continue; }; + + let (BindingKind::Importation(_, full_name) + | BindingKind::SubmoduleImportation(_, full_name) + | BindingKind::FromImportation(_, full_name)) = &binding.kind else { continue; }; // Skip used exports from `__all__` if binding.used.is_some() @@ -3128,24 +3226,23 @@ impl<'a> Checker<'a> { continue; } + let defined_by = binding.source.as_ref().unwrap(); + let defined_in = self.child_to_parent.get(defined_by); unused - .entry((context.defined_by, context.defined_in)) + .entry((defined_by, defined_in)) .or_default() .push((full_name, &binding.range)); } for ((defined_by, defined_in), unused_imports) in unused { - let child = self.parents[defined_by]; - let parent = defined_in.map(|defined_in| self.parents[defined_in]); + let child = defined_by.0; + let parent = defined_in.map(|defined_in| defined_in.0); let ignore_init = self.settings.ignore_init_module_imports && self.path.ends_with("__init__.py"); let fix = if !ignore_init && self.patch(&CheckCode::F401) { - let deleted: Vec<&Stmt> = self - .deletions - .iter() - .map(|index| self.parents[*index]) - .collect(); + let deleted: Vec<&Stmt> = + self.deletions.iter().map(|node| node.0).collect(); match pyflakes::fixes::remove_unused_imports( self.locator, &unused_imports, @@ -3155,7 +3252,7 @@ impl<'a> Checker<'a> { ) { Ok(fix) => { if fix.content.is_empty() || fix.content == "pass" { - self.deletions.insert(defined_by); + self.deletions.insert(defined_by.clone()); } Some(fix) } diff --git a/src/checks.rs b/src/checks.rs index 31667c5efd..bca406669a 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -87,6 +87,7 @@ pub enum CheckCode { F706, F707, F722, + F811, F821, F822, F823, @@ -616,6 +617,7 @@ pub enum CheckKind { PercentFormatStarRequiresSequence, PercentFormatUnsupportedFormatCharacter(char), RaiseNotImplemented, + RedefinedWhileUnused(String, usize), ReturnOutsideFunction, StringDotFormatExtraNamedArguments(Vec), StringDotFormatExtraPositionalArguments(Vec), @@ -932,6 +934,7 @@ impl CheckCode { CheckCode::F706 => CheckKind::ReturnOutsideFunction, CheckCode::F707 => CheckKind::DefaultExceptNotLast, CheckCode::F722 => CheckKind::ForwardAnnotationSyntaxError("...".to_string()), + CheckCode::F811 => CheckKind::RedefinedWhileUnused("...".to_string(), 1), CheckCode::F821 => CheckKind::UndefinedName("...".to_string()), CheckCode::F822 => CheckKind::UndefinedExport("...".to_string()), CheckCode::F823 => CheckKind::UndefinedLocal("...".to_string()), @@ -1359,6 +1362,7 @@ impl CheckCode { CheckCode::F706 => CheckCategory::Pyflakes, CheckCode::F707 => CheckCategory::Pyflakes, CheckCode::F722 => CheckCategory::Pyflakes, + CheckCode::F811 => CheckCategory::Pyflakes, CheckCode::F821 => CheckCategory::Pyflakes, CheckCode::F822 => CheckCategory::Pyflakes, CheckCode::F823 => CheckCategory::Pyflakes, @@ -1508,6 +1512,7 @@ impl CheckKind { CheckKind::TypeComparison => &CheckCode::E721, CheckKind::UndefinedExport(_) => &CheckCode::F822, CheckKind::UndefinedLocal(_) => &CheckCode::F823, + CheckKind::RedefinedWhileUnused(..) => &CheckCode::F811, CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UnusedImport(..) => &CheckCode::F401, CheckKind::UnusedVariable(_) => &CheckCode::F841, @@ -1846,6 +1851,9 @@ impl CheckKind { CheckKind::RaiseNotImplemented => { "`raise NotImplemented` should be `raise NotImplementedError`".to_string() } + CheckKind::RedefinedWhileUnused(name, line) => { + format!("Redefinition of unused `{name}` from line {line}") + } CheckKind::ReturnOutsideFunction => { "`return` statement outside of a function/method".to_string() } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index bd27331e2e..c210f8637b 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -243,6 +243,8 @@ pub enum CheckCodePrefix { F72, F722, F8, + F81, + F811, F82, F821, F822, @@ -1026,6 +1028,7 @@ impl CheckCodePrefix { CheckCode::F706, CheckCode::F707, CheckCode::F722, + CheckCode::F811, CheckCode::F821, CheckCode::F822, CheckCode::F823, @@ -1158,12 +1161,15 @@ impl CheckCodePrefix { CheckCodePrefix::F72 => vec![CheckCode::F722], CheckCodePrefix::F722 => vec![CheckCode::F722], CheckCodePrefix::F8 => vec![ + CheckCode::F811, CheckCode::F821, CheckCode::F822, CheckCode::F823, CheckCode::F831, CheckCode::F841, ], + CheckCodePrefix::F81 => vec![CheckCode::F811], + CheckCodePrefix::F811 => vec![CheckCode::F811], CheckCodePrefix::F82 => vec![CheckCode::F821, CheckCode::F822, CheckCode::F823], CheckCodePrefix::F821 => vec![CheckCode::F821], CheckCodePrefix::F822 => vec![CheckCode::F822], @@ -2033,6 +2039,8 @@ impl CheckCodePrefix { CheckCodePrefix::F72 => SuffixLength::Two, CheckCodePrefix::F722 => SuffixLength::Three, CheckCodePrefix::F8 => SuffixLength::One, + CheckCodePrefix::F81 => SuffixLength::Two, + CheckCodePrefix::F811 => SuffixLength::Three, CheckCodePrefix::F82 => SuffixLength::Two, CheckCodePrefix::F821 => SuffixLength::Three, CheckCodePrefix::F822 => SuffixLength::Three, diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 66bd0f611a..88ba301874 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -19,24 +19,14 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { }; if checker.patch(check.kind.code()) { - let context = checker.binding_context(); - if matches!( - checker.parents[context.defined_by].node, - StmtKind::Expr { .. } - ) { - let deleted: Vec<&Stmt> = checker - .deletions - .iter() - .map(|index| checker.parents[*index]) - .collect(); - match helpers::remove_stmt( - checker.parents[context.defined_by], - context.defined_in.map(|index| checker.parents[index]), - &deleted, - ) { + let defined_by = checker.current_parent(); + let defined_in = checker.current_grandparent(); + if matches!(defined_by.0.node, StmtKind::Expr { .. }) { + let deleted: Vec<&Stmt> = checker.deletions.iter().map(|node| node.0).collect(); + match helpers::remove_stmt(defined_by.0, defined_in.map(|node| node.0), &deleted) { Ok(fix) => { if fix.content.is_empty() || fix.content == "pass" { - checker.deletions.insert(context.defined_by); + checker.deletions.insert(defined_by.clone()); } check.amend(fix); } diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index a3aac7ad13..c223b75e7f 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -554,12 +554,13 @@ pub fn starred_expressions( } /// F701 -pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize]) -> Option { +pub fn break_outside_loop<'a>( + stmt: &'a Stmt, + parents: &mut impl Iterator, +) -> Option { let mut allowed: bool = false; - let mut parent = stmt; - for index in parent_stack.iter().rev() { - let child = parent; - parent = parents[*index]; + let mut child = stmt; + for parent in parents { match &parent.node { StmtKind::For { orelse, .. } | StmtKind::AsyncFor { orelse, .. } @@ -569,7 +570,6 @@ pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize] break; } } - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } | StmtKind::ClassDef { .. } => { @@ -577,6 +577,7 @@ pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize] } _ => {} } + child = parent; } if allowed { @@ -590,16 +591,13 @@ pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize] } /// F702 -pub fn continue_outside_loop( - stmt: &Stmt, - parents: &[&Stmt], - parent_stack: &[usize], +pub fn continue_outside_loop<'a>( + stmt: &'a Stmt, + parents: &mut impl Iterator, ) -> Option { let mut allowed: bool = false; - let mut parent = stmt; - for index in parent_stack.iter().rev() { - let child = parent; - parent = parents[*index]; + let mut child = stmt; + for parent in parents { match &parent.node { StmtKind::For { orelse, .. } | StmtKind::AsyncFor { orelse, .. } @@ -609,7 +607,6 @@ pub fn continue_outside_loop( break; } } - StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } | StmtKind::ClassDef { .. } => { @@ -617,6 +614,7 @@ pub fn continue_outside_loop( } _ => {} } + child = parent; } if allowed { diff --git a/src/pyflakes/mod.rs b/src/pyflakes/mod.rs index c625702375..9a14babbe7 100644 --- a/src/pyflakes/mod.rs +++ b/src/pyflakes/mod.rs @@ -66,6 +66,27 @@ mod tests { #[test_case(CheckCode::F706, Path::new("F706.py"); "F706")] #[test_case(CheckCode::F707, Path::new("F707.py"); "F707")] #[test_case(CheckCode::F722, Path::new("F722.py"); "F722")] + #[test_case(CheckCode::F811, Path::new("F811_0.py"); "F811_0")] + #[test_case(CheckCode::F811, Path::new("F811_1.py"); "F811_1")] + #[test_case(CheckCode::F811, Path::new("F811_2.py"); "F811_2")] + #[test_case(CheckCode::F811, Path::new("F811_3.py"); "F811_3")] + #[test_case(CheckCode::F811, Path::new("F811_4.py"); "F811_4")] + #[test_case(CheckCode::F811, Path::new("F811_5.py"); "F811_5")] + #[test_case(CheckCode::F811, Path::new("F811_6.py"); "F811_6")] + #[test_case(CheckCode::F811, Path::new("F811_7.py"); "F811_7")] + #[test_case(CheckCode::F811, Path::new("F811_8.py"); "F811_8")] + #[test_case(CheckCode::F811, Path::new("F811_9.py"); "F811_9")] + #[test_case(CheckCode::F811, Path::new("F811_10.py"); "F811_10")] + #[test_case(CheckCode::F811, Path::new("F811_11.py"); "F811_11")] + #[test_case(CheckCode::F811, Path::new("F811_12.py"); "F811_12")] + #[test_case(CheckCode::F811, Path::new("F811_13.py"); "F811_13")] + #[test_case(CheckCode::F811, Path::new("F811_14.py"); "F811_14")] + #[test_case(CheckCode::F811, Path::new("F811_15.py"); "F811_15")] + #[test_case(CheckCode::F811, Path::new("F811_16.py"); "F811_16")] + #[test_case(CheckCode::F811, Path::new("F811_17.py"); "F811_17")] + #[test_case(CheckCode::F811, Path::new("F811_18.py"); "F811_18")] + #[test_case(CheckCode::F811, Path::new("F811_19.py"); "F811_19")] + #[test_case(CheckCode::F811, Path::new("F811_20.py"); "F811_20")] #[test_case(CheckCode::F821, Path::new("F821_0.py"); "F821_0")] #[test_case(CheckCode::F821, Path::new("F821_1.py"); "F821_1")] #[test_case(CheckCode::F821, Path::new("F821_2.py"); "F821_2")] diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_0.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_0.py.snap new file mode 100644 index 0000000000..7ef6c3c446 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_0.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - bar + - 6 + location: + row: 10 + column: 0 + end_location: + row: 12 + column: 0 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_1.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_1.py.snap new file mode 100644 index 0000000000..7e8d19a991 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_1.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - FU + - 1 + location: + row: 1 + column: 17 + end_location: + row: 1 + column: 26 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_10.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_10.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_10.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_11.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_11.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_11.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_12.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_12.py.snap new file mode 100644 index 0000000000..4cbcb125b7 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_12.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - mixer + - 2 + location: + row: 6 + column: 19 + end_location: + row: 6 + column: 24 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_13.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_13.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_13.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_14.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_14.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_14.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_15.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_15.py.snap new file mode 100644 index 0000000000..bd59e9be76 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_15.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 1 + location: + row: 4 + column: 0 + end_location: + row: 6 + column: 0 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_16.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_16.py.snap new file mode 100644 index 0000000000..cb0592ba62 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_16.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 3 + location: + row: 8 + column: 8 + end_location: + row: 10 + column: 0 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_17.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_17.py.snap new file mode 100644 index 0000000000..dd1453afed --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_17.py.snap @@ -0,0 +1,27 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 2 + location: + row: 6 + column: 11 + end_location: + row: 6 + column: 13 + fix: ~ +- kind: + RedefinedWhileUnused: + - fu + - 6 + location: + row: 9 + column: 8 + end_location: + row: 11 + column: 0 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_18.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_18.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_18.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_19.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_19.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_19.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_2.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_2.py.snap new file mode 100644 index 0000000000..d9c73dfcd7 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_2.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - FU + - 1 + location: + row: 1 + column: 26 + end_location: + row: 1 + column: 35 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_20.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_20.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_20.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_3.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_3.py.snap new file mode 100644 index 0000000000..44c564e9ce --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_3.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 1 + location: + row: 1 + column: 11 + end_location: + row: 1 + column: 13 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_4.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_4.py.snap new file mode 100644 index 0000000000..44c564e9ce --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_4.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 1 + location: + row: 1 + column: 11 + end_location: + row: 1 + column: 13 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_5.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_5.py.snap new file mode 100644 index 0000000000..8e2c7754af --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_5.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - fu + - 1 + location: + row: 1 + column: 12 + end_location: + row: 1 + column: 14 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_6.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_6.py.snap new file mode 100644 index 0000000000..7b7582522a --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_6.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - os + - 5 + location: + row: 6 + column: 11 + end_location: + row: 6 + column: 13 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_7.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_7.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_7.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_8.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_8.py.snap new file mode 100644 index 0000000000..3efca65ec5 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_8.py.snap @@ -0,0 +1,16 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +- kind: + RedefinedWhileUnused: + - os + - 4 + location: + row: 5 + column: 11 + end_location: + row: 5 + column: 13 + fix: ~ + diff --git a/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_9.py.snap b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_9.py.snap new file mode 100644 index 0000000000..43ccad66f9 --- /dev/null +++ b/src/pyflakes/snapshots/ruff__pyflakes__tests__F811_F811_9.py.snap @@ -0,0 +1,6 @@ +--- +source: src/pyflakes/mod.rs +expression: checks +--- +[] + diff --git a/src/pylint/plugins/use_sys_exit.rs b/src/pylint/plugins/use_sys_exit.rs index 688661e14e..320fdf1333 100644 --- a/src/pylint/plugins/use_sys_exit.rs +++ b/src/pylint/plugins/use_sys_exit.rs @@ -30,13 +30,13 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) - // e.g. module=sys object=exit // `import sys` -> `sys.exit` // `import sys as sys2` -> `sys2.exit` - BindingKind::Importation(name, full_name, _) if full_name == module => { + BindingKind::Importation(name, full_name) if full_name == module => { Some(format!("{name}.{member}")) } // e.g. module=os.path object=join // `from os.path import join` -> `join` // `from os.path import join as join2` -> `join2` - BindingKind::FromImportation(name, full_name, _) + BindingKind::FromImportation(name, full_name) if full_name == &format!("{module}.{member}") => { Some(name.to_string()) @@ -50,7 +50,7 @@ fn get_member_import_name_alias(checker: &Checker, module: &str, member: &str) - } // e.g. module=os.path object=join // `import os.path ` -> `os.path.join` - BindingKind::SubmoduleImportation(_, full_name, _) if full_name == module => { + BindingKind::SubmoduleImportation(_, full_name) if full_name == module => { Some(format!("{full_name}.{member}")) } // Non-imports. diff --git a/src/pyupgrade/plugins/super_call_with_parameters.rs b/src/pyupgrade/plugins/super_call_with_parameters.rs index 5fee7af156..e0df17097c 100644 --- a/src/pyupgrade/plugins/super_call_with_parameters.rs +++ b/src/pyupgrade/plugins/super_call_with_parameters.rs @@ -13,11 +13,7 @@ pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Exp return; } let scope = checker.current_scope(); - let parents: Vec<&Stmt> = checker - .parent_stack - .iter() - .map(|index| checker.parents[*index]) - .collect(); + let parents: Vec<&Stmt> = checker.parents.iter().map(|node| node.0).collect(); let Some(mut check) = checks::super_args(scope, &parents, expr, func, args) else { return; }; diff --git a/src/pyupgrade/plugins/unnecessary_future_import.rs b/src/pyupgrade/plugins/unnecessary_future_import.rs index 510be0bd2b..372de40cee 100644 --- a/src/pyupgrade/plugins/unnecessary_future_import.rs +++ b/src/pyupgrade/plugins/unnecessary_future_import.rs @@ -62,23 +62,21 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo ), Range::from_located(stmt), ); + if checker.patch(check.kind.code()) { - let context = checker.binding_context(); - let deleted: Vec<&Stmt> = checker - .deletions - .iter() - .map(|index| checker.parents[*index]) - .collect(); + let deleted: Vec<&Stmt> = checker.deletions.iter().map(|node| node.0).collect(); + let defined_by = checker.current_parent(); + let defined_in = checker.current_grandparent(); match fixes::remove_unnecessary_future_import( checker.locator, &removable_index, - checker.parents[context.defined_by], - context.defined_in.map(|index| checker.parents[index]), + defined_by.0, + defined_in.map(|node| node.0), &deleted, ) { Ok(fix) => { if fix.content.is_empty() || fix.content == "pass" { - checker.deletions.insert(context.defined_by); + checker.deletions.insert(defined_by.clone()); } check.amend(fix); } diff --git a/src/pyupgrade/plugins/useless_metaclass_type.rs b/src/pyupgrade/plugins/useless_metaclass_type.rs index 381a539a46..fe50bed9ef 100644 --- a/src/pyupgrade/plugins/useless_metaclass_type.rs +++ b/src/pyupgrade/plugins/useless_metaclass_type.rs @@ -13,21 +13,13 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, return; }; if checker.patch(check.kind.code()) { - let context = checker.binding_context(); - let deleted: Vec<&Stmt> = checker - .deletions - .iter() - .map(|index| checker.parents[*index]) - .collect(); - - match helpers::remove_stmt( - checker.parents[context.defined_by], - context.defined_in.map(|index| checker.parents[index]), - &deleted, - ) { + let deleted: Vec<&Stmt> = checker.deletions.iter().map(|node| node.0).collect(); + let defined_by = checker.current_parent(); + let defined_in = checker.current_grandparent(); + match helpers::remove_stmt(defined_by.0, defined_in.map(|node| node.0), &deleted) { Ok(fix) => { if fix.content.is_empty() || fix.content == "pass" { - checker.deletions.insert(context.defined_by); + checker.deletions.insert(defined_by.clone()); } check.amend(fix); }