diff --git a/src/check_ast.rs b/src/check_ast.rs index 016c3467e9..9ed8f3d8b2 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -129,7 +129,9 @@ impl<'a> Checker<'a> { /// Return `true` if a patch should be generated under the given autofix /// `Mode`. pub fn patch(&self) -> bool { - self.autofix.patch() + // TODO(charlie): We can't fix errors in f-strings until RustPython adds + // location data. + self.autofix.patch() && self.in_f_string.is_none() } /// Return `true` if the `Expr` is a reference to `typing.${target}`. @@ -995,6 +997,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1008,6 +1011,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1021,6 +1025,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1035,6 +1040,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1044,7 +1050,10 @@ where if self.settings.enabled.contains(&CheckCode::C404) { if let Some(check) = flake8_comprehensions::checks::unnecessary_list_comprehension_dict( - expr, func, args, keywords, + func, + args, + keywords, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1059,6 +1068,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1066,7 +1076,10 @@ where if self.settings.enabled.contains(&CheckCode::C406) { if let Some(check) = flake8_comprehensions::checks::unnecessary_literal_dict( - expr, func, args, keywords, + func, + args, + keywords, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1080,6 +1093,7 @@ where keywords, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1093,6 +1107,7 @@ where args, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1107,6 +1122,7 @@ where args, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1120,6 +1136,7 @@ where args, self.locator, self.patch(), + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; @@ -1128,7 +1145,9 @@ where if self.settings.enabled.contains(&CheckCode::C413) { if let Some(check) = flake8_comprehensions::checks::unnecessary_call_around_sorted( - expr, func, args, + func, + args, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1138,7 +1157,9 @@ where if self.settings.enabled.contains(&CheckCode::C414) { if let Some(check) = flake8_comprehensions::checks::unnecessary_double_cast_or_process( - expr, func, args, + func, + args, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1148,7 +1169,9 @@ where if self.settings.enabled.contains(&CheckCode::C415) { if let Some(check) = flake8_comprehensions::checks::unnecessary_subscript_reversal( - expr, func, args, + func, + args, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); @@ -1156,9 +1179,11 @@ where } if self.settings.enabled.contains(&CheckCode::C417) { - if let Some(check) = - flake8_comprehensions::checks::unnecessary_map(expr, func, args) - { + if let Some(check) = flake8_comprehensions::checks::unnecessary_map( + func, + args, + self.locate_check(Range::from_located(expr)), + ) { self.checks.push(check); }; } @@ -1335,7 +1360,10 @@ where ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { if self.settings.enabled.contains(&CheckCode::C416) { if let Some(check) = flake8_comprehensions::checks::unnecessary_comprehension( - expr, elt, generators, + expr, + elt, + generators, + self.locate_check(Range::from_located(expr)), ) { self.checks.push(check); }; diff --git a/src/flake8_bugbear/plugins/assert_false.rs b/src/flake8_bugbear/plugins/assert_false.rs index ebb25d9f08..95976f192b 100644 --- a/src/flake8_bugbear/plugins/assert_false.rs +++ b/src/flake8_bugbear/plugins/assert_false.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind}; -use crate::ast::types::Range; +use crate::ast::types::{CheckLocator, Range}; use crate::autofix::Fix; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -42,7 +42,10 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: &Optio .. } = &test.node { - let mut check = Check::new(CheckKind::DoNotAssertFalse, Range::from_located(test)); + let mut check = Check::new( + CheckKind::DoNotAssertFalse, + checker.locate_check(Range::from_located(test)), + ); if checker.patch() { let mut generator = SourceGenerator::new(); if let Ok(()) = generator.unparse_stmt(&assertion_error(msg)) { diff --git a/src/flake8_bugbear/plugins/assert_raises_exception.rs b/src/flake8_bugbear/plugins/assert_raises_exception.rs index c408f4b7b9..a3278df157 100644 --- a/src/flake8_bugbear/plugins/assert_raises_exception.rs +++ b/src/flake8_bugbear/plugins/assert_raises_exception.rs @@ -1,7 +1,7 @@ use rustpython_ast::{ExprKind, Stmt, Withitem}; use crate::ast::helpers::match_name_or_attr; -use crate::ast::types::Range; +use crate::ast::types::{CheckLocator, Range}; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -17,7 +17,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With { checker.add_check(Check::new( CheckKind::NoAssertRaisesException, - Range::from_located(stmt), + checker.locate_check(Range::from_located(stmt)), )); } } diff --git a/src/flake8_bugbear/plugins/unary_prefix_increment.rs b/src/flake8_bugbear/plugins/unary_prefix_increment.rs index 16e22a1cfe..487412fb81 100644 --- a/src/flake8_bugbear/plugins/unary_prefix_increment.rs +++ b/src/flake8_bugbear/plugins/unary_prefix_increment.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Expr, ExprKind, Unaryop}; -use crate::ast::types::Range; +use crate::ast::types::{CheckLocator, Range}; use crate::check_ast::Checker; use crate::checks::{Check, CheckKind}; @@ -11,7 +11,7 @@ pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop, if matches!(op, Unaryop::UAdd) { checker.add_check(Check::new( CheckKind::UnaryPrefixIncrement, - Range::from_located(expr), + checker.locate_check(Range::from_located(expr)), )) } } diff --git a/src/flake8_bugbear/plugins/unused_loop_control_variable.rs b/src/flake8_bugbear/plugins/unused_loop_control_variable.rs index 9eb0cd84dd..4937a3c97b 100644 --- a/src/flake8_bugbear/plugins/unused_loop_control_variable.rs +++ b/src/flake8_bugbear/plugins/unused_loop_control_variable.rs @@ -2,7 +2,7 @@ use std::collections::BTreeMap; use rustpython_ast::{Expr, ExprKind, Stmt}; -use crate::ast::types::Range; +use crate::ast::types::{CheckLocator, Range}; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::autofix::Fix; @@ -64,7 +64,7 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body: let mut check = Check::new( CheckKind::UnusedLoopControlVariable(name.to_string()), - Range::from_located(expr), + checker.locate_check(Range::from_located(expr)), ); if checker.patch() { // Prefix the variable name with an underscore. diff --git a/src/flake8_comprehensions/checks.rs b/src/flake8_comprehensions/checks.rs index 11d502f62d..ee98115c0a 100644 --- a/src/flake8_comprehensions/checks.rs +++ b/src/flake8_comprehensions/checks.rs @@ -54,13 +54,11 @@ pub fn unnecessary_generator_list( keywords: &[Keyword], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("list", func, args, keywords)?; if let ExprKind::GeneratorExp { .. } = argument { - let mut check = Check::new( - CheckKind::UnnecessaryGeneratorList, - Range::from_located(expr), - ); + let mut check = Check::new(CheckKind::UnnecessaryGeneratorList, location); if fix { match fixes::fix_unnecessary_generator_list(locator, expr) { Ok(fix) => check.amend(fix), @@ -80,13 +78,11 @@ pub fn unnecessary_generator_set( keywords: &[Keyword], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; if let ExprKind::GeneratorExp { .. } = argument { - let mut check = Check::new( - CheckKind::UnnecessaryGeneratorSet, - Range::from_located(expr), - ); + let mut check = Check::new(CheckKind::UnnecessaryGeneratorSet, location); if fix { match fixes::fix_unnecessary_generator_set(locator, expr) { Ok(fix) => check.amend(fix), @@ -106,15 +102,13 @@ pub fn unnecessary_generator_dict( keywords: &[Keyword], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("dict", func, args, keywords)?; if let ExprKind::GeneratorExp { elt, .. } = argument { match &elt.node { ExprKind::Tuple { elts, .. } if elts.len() == 2 => { - let mut check = Check::new( - CheckKind::UnnecessaryGeneratorDict, - Range::from_located(expr), - ); + let mut check = Check::new(CheckKind::UnnecessaryGeneratorDict, location); if fix { match fixes::fix_unnecessary_generator_dict(locator, expr) { Ok(fix) => check.amend(fix), @@ -137,13 +131,11 @@ pub fn unnecessary_list_comprehension_set( keywords: &[Keyword], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; if let ExprKind::ListComp { .. } = &argument { - let mut check = Check::new( - CheckKind::UnnecessaryListComprehensionSet, - Range::from_located(expr), - ); + let mut check = Check::new(CheckKind::UnnecessaryListComprehensionSet, location); if fix { match fixes::fix_unnecessary_list_comprehension_set(locator, expr) { Ok(fix) => check.amend(fix), @@ -157,10 +149,10 @@ pub fn unnecessary_list_comprehension_set( /// C404 (`dict([...])`) pub fn unnecessary_list_comprehension_dict( - expr: &Expr, func: &Expr, args: &[Expr], keywords: &[Keyword], + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("dict", func, args, keywords)?; if let ExprKind::ListComp { elt, .. } = &argument { @@ -168,7 +160,7 @@ pub fn unnecessary_list_comprehension_dict( ExprKind::Tuple { elts, .. } if elts.len() == 2 => { return Some(Check::new( CheckKind::UnnecessaryListComprehensionDict, - Range::from_located(expr), + location, )); } _ => {} @@ -185,6 +177,7 @@ pub fn unnecessary_literal_set( keywords: &[Keyword], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("set", func, args, keywords)?; let kind = match argument { @@ -192,10 +185,7 @@ pub fn unnecessary_literal_set( ExprKind::Tuple { .. } => "tuple", _ => return None, }; - let mut check = Check::new( - CheckKind::UnnecessaryLiteralSet(kind.to_string()), - Range::from_located(expr), - ); + let mut check = Check::new(CheckKind::UnnecessaryLiteralSet(kind.to_string()), location); if fix { match fixes::fix_unnecessary_literal_set(locator, expr) { Ok(fix) => check.amend(fix), @@ -207,10 +197,10 @@ pub fn unnecessary_literal_set( /// C406 (`dict([(1, 2)])`) pub fn unnecessary_literal_dict( - expr: &Expr, func: &Expr, args: &[Expr], keywords: &[Keyword], + location: Range, ) -> Option { let argument = exactly_one_argument_with_matching_function("dict", func, args, keywords)?; let (kind, elts) = match argument { @@ -228,7 +218,7 @@ pub fn unnecessary_literal_dict( Some(Check::new( CheckKind::UnnecessaryLiteralDict(kind.to_string()), - Range::from_located(expr), + location, )) } @@ -240,6 +230,7 @@ pub fn unnecessary_collection_call( keywords: &[Located], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { if !args.is_empty() { return None; @@ -256,7 +247,7 @@ pub fn unnecessary_collection_call( }; let mut check = Check::new( CheckKind::UnnecessaryCollectionCall(id.to_string()), - Range::from_located(expr), + location, ); if fix { // TODO(charlie): Support fixing `dict(a=1)`. @@ -277,6 +268,7 @@ pub fn unnecessary_literal_within_tuple_call( args: &[Expr], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = first_argument_with_matching_function("tuple", func, args)?; let argument_kind = match argument { @@ -286,7 +278,7 @@ pub fn unnecessary_literal_within_tuple_call( }; let mut check = Check::new( CheckKind::UnnecessaryLiteralWithinTupleCall(argument_kind.to_string()), - Range::from_located(expr), + location, ); if fix { match fixes::fix_unnecessary_literal_within_tuple_call(locator, expr) { @@ -304,6 +296,7 @@ pub fn unnecessary_literal_within_list_call( args: &[Expr], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = first_argument_with_matching_function("list", func, args)?; let argument_kind = match argument { @@ -313,7 +306,7 @@ pub fn unnecessary_literal_within_list_call( }; let mut check = Check::new( CheckKind::UnnecessaryLiteralWithinListCall(argument_kind.to_string()), - Range::from_located(expr), + location, ); if fix { match fixes::fix_unnecessary_literal_within_list_call(locator, expr) { @@ -331,10 +324,11 @@ pub fn unnecessary_list_call( args: &[Expr], locator: &SourceCodeLocator, fix: bool, + location: Range, ) -> Option { let argument = first_argument_with_matching_function("list", func, args)?; if let ExprKind::ListComp { .. } = argument { - let mut check = Check::new(CheckKind::UnnecessaryListCall, Range::from_located(expr)); + let mut check = Check::new(CheckKind::UnnecessaryListCall, location); if fix { match fixes::fix_unnecessary_list_call(locator, expr) { Ok(fix) => check.amend(fix), @@ -347,7 +341,11 @@ pub fn unnecessary_list_call( } /// C413 -pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { +pub fn unnecessary_call_around_sorted( + func: &Expr, + args: &[Expr], + location: Range, +) -> Option { let outer = function_name(func)?; if !(outer == "list" || outer == "reversed") { return None; @@ -356,7 +354,7 @@ pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) - if function_name(func)? == "sorted" { return Some(Check::new( CheckKind::UnnecessaryCallAroundSorted(outer.to_string()), - Range::from_located(expr), + location, )); } } @@ -365,19 +363,19 @@ pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) - /// C414 pub fn unnecessary_double_cast_or_process( - expr: &Expr, func: &Expr, args: &[Expr], + location: Range, ) -> Option { let outer = function_name(func)?; if !["list", "tuple", "set", "reversed", "sorted"].contains(&outer) { return None; } - fn new_check(inner: &str, outer: &str, expr: &Expr) -> Check { + fn new_check(inner: &str, outer: &str, location: Range) -> Check { Check::new( CheckKind::UnnecessaryDoubleCastOrProcess(inner.to_string(), outer.to_string()), - Range::from_located(expr), + location, ) } @@ -387,24 +385,28 @@ pub fn unnecessary_double_cast_or_process( if (outer == "set" || outer == "sorted") && (inner == "list" || inner == "tuple" || inner == "reversed" || inner == "sorted") { - return Some(new_check(inner, outer, expr)); + return Some(new_check(inner, outer, location)); } // Ex) list(tuple(...)) if (outer == "list" || outer == "tuple") && (inner == "list" || inner == "tuple") { - return Some(new_check(inner, outer, expr)); + return Some(new_check(inner, outer, location)); } // Ex) set(set(...)) if outer == "set" && inner == "set" { - return Some(new_check(inner, outer, expr)); + return Some(new_check(inner, outer, location)); } } None } /// C415 -pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { +pub fn unnecessary_subscript_reversal( + func: &Expr, + args: &[Expr], + location: Range, +) -> Option { let first_arg = args.first()?; let id = function_name(func)?; if !["set", "sorted", "reversed"].contains(&id) { @@ -427,7 +429,7 @@ pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) - if *val == BigInt::from(1) { return Some(Check::new( CheckKind::UnnecessarySubscriptReversal(id.to_string()), - Range::from_located(expr), + location, )); } } @@ -444,6 +446,7 @@ pub fn unnecessary_comprehension( expr: &Expr, elt: &Expr, generators: &[Comprehension], + location: Range, ) -> Option { if generators.len() != 1 { return None; @@ -464,30 +467,27 @@ pub fn unnecessary_comprehension( }; Some(Check::new( CheckKind::UnnecessaryComprehension(expr_kind.to_string()), - Range::from_located(expr), + location, )) } /// C417 -pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { - fn new_check(kind: &str, expr: &Expr) -> Check { - Check::new( - CheckKind::UnnecessaryMap(kind.to_string()), - Range::from_located(expr), - ) +pub fn unnecessary_map(func: &Expr, args: &[Expr], location: Range) -> Option { + fn new_check(kind: &str, location: Range) -> Check { + Check::new(CheckKind::UnnecessaryMap(kind.to_string()), location) } let id = function_name(func)?; match id { "map" => { if args.len() == 2 && matches!(&args[0].node, ExprKind::Lambda { .. }) { - return Some(new_check("generator", expr)); + return Some(new_check("generator", location)); } } "list" | "set" => { if let ExprKind::Call { func, args, .. } = &args.first()?.node { let argument = first_argument_with_matching_function("map", func, args)?; if let ExprKind::Lambda { .. } = argument { - return Some(new_check(id, expr)); + return Some(new_check(id, location)); } } } @@ -498,7 +498,7 @@ pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option if let ExprKind::Lambda { body, .. } = &argument { if matches!(&body.node, ExprKind::Tuple { elts, .. } | ExprKind::List { elts, .. } if elts.len() == 2) { - return Some(new_check(id, expr)); + return Some(new_check(id, location)); } } } diff --git a/src/flake8_print/checks.rs b/src/flake8_print/checks.rs index 241e8ca677..1fd6b98b1a 100644 --- a/src/flake8_print/checks.rs +++ b/src/flake8_print/checks.rs @@ -9,25 +9,20 @@ pub fn print_call( func: &Expr, check_print: bool, check_pprint: bool, + location: Range, ) -> Option { if let ExprKind::Name { id, .. } = &func.node { if check_print && id == "print" { return Some(Check::new(CheckKind::PrintFound, Range::from_located(expr))); } else if check_pprint && id == "pprint" { - return Some(Check::new( - CheckKind::PPrintFound, - Range::from_located(expr), - )); + return Some(Check::new(CheckKind::PPrintFound, location)); } } if let ExprKind::Attribute { value, attr, .. } = &func.node { if let ExprKind::Name { id, .. } = &value.node { if check_pprint && id == "pprint" && attr == "pprint" { - return Some(Check::new( - CheckKind::PPrintFound, - Range::from_located(expr), - )); + return Some(Check::new(CheckKind::PPrintFound, location)); } } } diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 98221c0e47..35ea9bd5b4 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -1,6 +1,7 @@ use log::error; use rustpython_ast::{Expr, Stmt, StmtKind}; +use crate::ast::types::{CheckLocator, Range}; use crate::autofix::helpers; use crate::check_ast::Checker; use crate::checks::CheckCode; @@ -12,6 +13,7 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { func, checker.settings.enabled.contains(&CheckCode::T201), checker.settings.enabled.contains(&CheckCode::T203), + checker.locate_check(Range::from_located(expr)), ) { if checker.patch() { let context = checker.binding_context();