From bbc38fea73bd30c343c1f76029d0b2fa7d4d8cc0 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 12 Nov 2022 11:39:09 -0500 Subject: [PATCH] Avoid generating empty statement bodies (#700) --- resources/test/fixtures/F401_0.py | 1 - resources/test/fixtures/U010.py | 1 + src/check_ast.rs | 18 +++-- src/flake8_print/plugins/print_call.rs | 4 +- .../plugins/deprecated_unittest_alias.rs | 1 + .../plugins/super_call_with_parameters.rs | 1 + src/pyupgrade/plugins/type_of_primitive.rs | 1 + src/pyupgrade/plugins/unnecessary_abspath.rs | 1 + .../plugins/unnecessary_future_import.rs | 11 +++- .../plugins/unnecessary_lru_cache_params.rs | 1 + .../plugins/use_pep585_annotation.rs | 1 + .../plugins/use_pep604_annotation.rs | 1 + .../plugins/useless_metaclass_type.rs | 3 +- .../plugins/useless_object_inheritance.rs | 1 + .../ruff__linter__tests__F401_F401_0.py.snap | 38 +++++------ .../ruff__linter__tests__U010_U010.py.snap | 65 ++++++++++++------- 16 files changed, 94 insertions(+), 55 deletions(-) diff --git a/resources/test/fixtures/F401_0.py b/resources/test/fixtures/F401_0.py index 24afc4c6fe..8694df15b9 100644 --- a/resources/test/fixtures/F401_0.py +++ b/resources/test/fixtures/F401_0.py @@ -28,7 +28,6 @@ from blah import ClassA, ClassB, ClassC if TYPE_CHECKING: from models import Fruit, Nut, Vegetable - if TYPE_CHECKING: import shelve import importlib diff --git a/resources/test/fixtures/U010.py b/resources/test/fixtures/U010.py index 452a50ea5c..eb8ece827e 100644 --- a/resources/test/fixtures/U010.py +++ b/resources/test/fixtures/U010.py @@ -7,6 +7,7 @@ from __future__ import invalid_module, generators if True: from __future__ import generator_stop + from __future__ import generators if True: from __future__ import generator_stop diff --git a/src/check_ast.rs b/src/check_ast.rs index 1be705a966..39dd16a5d5 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -2411,16 +2411,20 @@ impl<'a> Checker<'a> { .iter() .map(|index| self.parents[*index]) .collect(); - - let removal_fn = match kind { + match match kind { ImportKind::Import => pyflakes::fixes::remove_unused_imports, ImportKind::ImportFrom => pyflakes::fixes::remove_unused_import_froms, - }; - - match removal_fn(self.locator, &full_names, child, parent, &deleted) { - Ok(fix) => Some(fix), + }( + self.locator, &full_names, child, parent, &deleted + ) { + Ok(fix) => { + if fix.patch.content.is_empty() || fix.patch.content == "pass" { + self.deletions.insert(defined_by); + } + Some(fix) + } Err(e) => { - error!("Failed to fix unused imports: {}", e); + error!("Failed to remove unused imports: {}", e); None } } diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 435621c942..c6cabceb18 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -7,6 +7,7 @@ use crate::check_ast::Checker; use crate::checks::CheckCode; use crate::flake8_print::checks; +/// T201, T203 pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { if let Some(mut check) = checks::print_call( expr, @@ -26,7 +27,6 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { .iter() .map(|index| checker.parents[*index]) .collect(); - match helpers::remove_stmt( checker.parents[context.defined_by], context.defined_in.map(|index| checker.parents[index]), @@ -38,7 +38,7 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { } check.amend(fix) } - Err(e) => error!("Failed to fix unused imports: {}", e), + Err(e) => error!("Failed to remove print call: {}", e), } } } diff --git a/src/pyupgrade/plugins/deprecated_unittest_alias.rs b/src/pyupgrade/plugins/deprecated_unittest_alias.rs index 04ca6d6267..b66a85b6ad 100644 --- a/src/pyupgrade/plugins/deprecated_unittest_alias.rs +++ b/src/pyupgrade/plugins/deprecated_unittest_alias.rs @@ -28,6 +28,7 @@ static DEPRECATED_ALIASES: Lazy> = Lazy::ne ]) }); +/// U005 pub fn deprecated_unittest_alias(checker: &mut Checker, expr: &Expr) { if let ExprKind::Attribute { value, attr, .. } = &expr.node { if let Some(target) = DEPRECATED_ALIASES.get(attr.as_str()) { diff --git a/src/pyupgrade/plugins/super_call_with_parameters.rs b/src/pyupgrade/plugins/super_call_with_parameters.rs index 31fe9c6c6f..b743477566 100644 --- a/src/pyupgrade/plugins/super_call_with_parameters.rs +++ b/src/pyupgrade/plugins/super_call_with_parameters.rs @@ -5,6 +5,7 @@ use crate::check_ast::Checker; use crate::pyupgrade; use crate::pyupgrade::checks; +/// U008 pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { // Only bother going through the super check at all if we're in a `super` call. // (We check this in `check_super_args` too, so this is just an optimization.) diff --git a/src/pyupgrade/plugins/type_of_primitive.rs b/src/pyupgrade/plugins/type_of_primitive.rs index 723fcd7183..6a7575428b 100644 --- a/src/pyupgrade/plugins/type_of_primitive.rs +++ b/src/pyupgrade/plugins/type_of_primitive.rs @@ -6,6 +6,7 @@ use crate::check_ast::Checker; use crate::checks::CheckKind; use crate::pyupgrade::checks; +/// U003 pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let Some(mut check) = checks::type_of_primitive(func, args, Range::from_located(expr)) { if checker.patch() { diff --git a/src/pyupgrade/plugins/unnecessary_abspath.rs b/src/pyupgrade/plugins/unnecessary_abspath.rs index 9ccc90f5ac..d2851bcc30 100644 --- a/src/pyupgrade/plugins/unnecessary_abspath.rs +++ b/src/pyupgrade/plugins/unnecessary_abspath.rs @@ -5,6 +5,7 @@ use crate::autofix::Fix; use crate::check_ast::Checker; use crate::pyupgrade::checks; +/// U002 pub fn unnecessary_abspath(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { if let Some(mut check) = checks::unnecessary_abspath(func, args, Range::from_located(expr)) { if checker.patch() { diff --git a/src/pyupgrade/plugins/unnecessary_future_import.rs b/src/pyupgrade/plugins/unnecessary_future_import.rs index b994b05095..0e807be99d 100644 --- a/src/pyupgrade/plugins/unnecessary_future_import.rs +++ b/src/pyupgrade/plugins/unnecessary_future_import.rs @@ -1,5 +1,6 @@ use std::collections::BTreeSet; +use log::error; use rustpython_ast::{AliasData, Located}; use rustpython_parser::ast::Stmt; @@ -62,14 +63,20 @@ pub fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, names: &[Lo .iter() .map(|index| checker.parents[*index]) .collect(); - if let Ok(fix) = fixes::remove_unnecessary_future_import( + match fixes::remove_unnecessary_future_import( checker.locator, &removable_index, checker.parents[context.defined_by], context.defined_in.map(|index| checker.parents[index]), &deleted, ) { - check.amend(fix); + Ok(fix) => { + if fix.patch.content.is_empty() || fix.patch.content == "pass" { + checker.deletions.insert(context.defined_by); + } + check.amend(fix); + } + Err(e) => error!("Failed to remove __future__ import: {}", e), } } checker.add_check(check); diff --git a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs index 58e4565063..1279013c8e 100644 --- a/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs +++ b/src/pyupgrade/plugins/unnecessary_lru_cache_params.rs @@ -3,6 +3,7 @@ use rustpython_parser::ast::Expr; use crate::check_ast::Checker; use crate::pyupgrade::{checks, fixes}; +/// U011 pub fn unnecessary_lru_cache_params(checker: &mut Checker, decorator_list: &[Expr]) { if let Some(mut check) = checks::unnecessary_lru_cache_params( decorator_list, diff --git a/src/pyupgrade/plugins/use_pep585_annotation.rs b/src/pyupgrade/plugins/use_pep585_annotation.rs index 3498a437ac..fb5a018ce4 100644 --- a/src/pyupgrade/plugins/use_pep585_annotation.rs +++ b/src/pyupgrade/plugins/use_pep585_annotation.rs @@ -5,6 +5,7 @@ use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; +/// U006 pub fn use_pep585_annotation(checker: &mut Checker, expr: &Expr, id: &str) { let mut check = Check::new( CheckKind::UsePEP585Annotation(id.to_string()), diff --git a/src/pyupgrade/plugins/use_pep604_annotation.rs b/src/pyupgrade/plugins/use_pep604_annotation.rs index e71c7c5eca..9be264325f 100644 --- a/src/pyupgrade/plugins/use_pep604_annotation.rs +++ b/src/pyupgrade/plugins/use_pep604_annotation.rs @@ -41,6 +41,7 @@ fn union(elts: &[Expr]) -> Expr { } } +/// U007 pub fn use_pep604_annotation(checker: &mut Checker, expr: &Expr, value: &Expr, slice: &Expr) { if checker.match_typing_module(value, "Optional") { let mut check = Check::new(CheckKind::UsePEP604Annotation, Range::from_located(expr)); diff --git a/src/pyupgrade/plugins/useless_metaclass_type.rs b/src/pyupgrade/plugins/useless_metaclass_type.rs index bcff798545..70b2bb2785 100644 --- a/src/pyupgrade/plugins/useless_metaclass_type.rs +++ b/src/pyupgrade/plugins/useless_metaclass_type.rs @@ -6,6 +6,7 @@ use crate::autofix::helpers; use crate::check_ast::Checker; use crate::pyupgrade::checks; +/// U001 pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, targets: &[Expr]) { if let Some(mut check) = checks::useless_metaclass_type(targets, value, Range::from_located(stmt)) @@ -29,7 +30,7 @@ pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, } check.amend(fix) } - Err(e) => error!("Failed to fix unused imports: {}", e), + Err(e) => error!("Failed to fix remove metaclass type: {}", e), } } checker.add_check(check); diff --git a/src/pyupgrade/plugins/useless_object_inheritance.rs b/src/pyupgrade/plugins/useless_object_inheritance.rs index c3cf4d23f8..4cb6789daf 100644 --- a/src/pyupgrade/plugins/useless_object_inheritance.rs +++ b/src/pyupgrade/plugins/useless_object_inheritance.rs @@ -4,6 +4,7 @@ use crate::check_ast::Checker; use crate::pyupgrade; use crate::pyupgrade::checks; +/// U004 pub fn useless_object_inheritance( checker: &mut Checker, stmt: &Stmt, diff --git a/src/snapshots/ruff__linter__tests__F401_F401_0.py.snap b/src/snapshots/ruff__linter__tests__F401_F401_0.py.snap index c2f1a1f3c1..a67353f8cb 100644 --- a/src/snapshots/ruff__linter__tests__F401_F401_0.py.snap +++ b/src/snapshots/ruff__linter__tests__F401_F401_0.py.snap @@ -67,19 +67,19 @@ expression: checks - - shelve - false location: - row: 33 + row: 32 column: 4 end_location: - row: 33 + row: 32 column: 17 fix: patch: content: "" location: - row: 33 + row: 32 column: 0 end_location: - row: 34 + row: 33 column: 0 applied: false - kind: @@ -87,39 +87,39 @@ expression: checks - - importlib - false location: - row: 34 + row: 33 column: 4 end_location: - row: 34 + row: 33 column: 20 fix: patch: - content: "" + content: pass location: - row: 34 - column: 0 + row: 33 + column: 4 end_location: - row: 35 - column: 0 + row: 33 + column: 20 applied: false - kind: UnusedImport: - - pathlib - false location: - row: 38 + row: 37 column: 4 end_location: - row: 38 + row: 37 column: 18 fix: patch: content: "" location: - row: 38 + row: 37 column: 0 end_location: - row: 39 + row: 38 column: 0 applied: false - kind: @@ -127,19 +127,19 @@ expression: checks - - pickle - false location: - row: 53 + row: 52 column: 8 end_location: - row: 53 + row: 52 column: 21 fix: patch: content: pass location: - row: 53 + row: 52 column: 8 end_location: - row: 53 + row: 52 column: 21 applied: false diff --git a/src/snapshots/ruff__linter__tests__U010_U010.py.snap b/src/snapshots/ruff__linter__tests__U010_U010.py.snap index dfc2aca003..2838d98b22 100644 --- a/src/snapshots/ruff__linter__tests__U010_U010.py.snap +++ b/src/snapshots/ruff__linter__tests__U010_U010.py.snap @@ -129,52 +129,71 @@ expression: checks end_location: row: 9 column: 41 - fix: - patch: - content: pass - location: - row: 9 - column: 4 - end_location: - row: 9 - column: 41 - applied: false -- kind: - UnnecessaryFutureImport: - - generator_stop - location: - row: 12 - column: 4 - end_location: - row: 12 - column: 41 fix: patch: content: "" location: - row: 12 + row: 9 column: 0 end_location: - row: 13 + row: 10 column: 0 applied: false - kind: UnnecessaryFutureImport: - generators + location: + row: 10 + column: 4 + end_location: + row: 10 + column: 37 + fix: + patch: + content: pass + location: + row: 10 + column: 4 + end_location: + row: 10 + column: 37 + applied: false +- kind: + UnnecessaryFutureImport: + - generator_stop location: row: 13 column: 4 end_location: row: 13 + column: 41 + fix: + patch: + content: "" + location: + row: 13 + column: 0 + end_location: + row: 14 + column: 0 + applied: false +- kind: + UnnecessaryFutureImport: + - generators + location: + row: 14 + column: 4 + end_location: + row: 14 column: 53 fix: patch: content: from __future__ import invalid_module location: - row: 13 + row: 14 column: 4 end_location: - row: 13 + row: 14 column: 53 applied: false