Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid autofixes for errors in f-strings (#561)

This commit is contained in:
Charlie Marsh 2022-11-02 22:31:57 -04:00 committed by GitHub
parent e473df1fe9
commit b42d77a4c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 102 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/check_ast.rs
View File

@ -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);
};

7
src/flake8_bugbear/plugins/assert_false.rs
View File

@ -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)) {

4
src/flake8_bugbear/plugins/assert_raises_exception.rs
View File

@ -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)),
));
}
}

4
src/flake8_bugbear/plugins/unary_prefix_increment.rs
View File

@ -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)),
))
}
}

4
src/flake8_bugbear/plugins/unused_loop_control_variable.rs
View File

@ -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.

96
src/flake8_comprehensions/checks.rs
View File

@ -54,13 +54,11 @@ pub fn unnecessary_generator_list(
keywords: &[Keyword],
locator: &SourceCodeLocator,
fix: bool,
location: Range,
) -> Option<Check> {
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<Check> {
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<Check> {
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<Check> {
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<Check> {
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<Check> {
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<Check> {
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<KeywordData>],
locator: &SourceCodeLocator,
fix: bool,
location: Range,
) -> Option<Check> {
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<Check> {
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<Check> {
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<Check> {
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<Check> {
pub fn unnecessary_call_around_sorted(
func: &Expr,
args: &[Expr],
location: Range,
) -> Option<Check> {
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<Check> {
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<Check> {
pub fn unnecessary_subscript_reversal(
func: &Expr,
args: &[Expr],
location: Range,
) -> Option<Check> {
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<Check> {
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<Check> {
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<Check> {
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<Check>
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));
}
}
}

11
src/flake8_print/checks.rs
View File

@ -9,25 +9,20 @@ pub fn print_call(
func: &Expr,
check_print: bool,
check_pprint: bool,
location: Range,
) -> Option<Check> {
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));
}
}
}

2
src/flake8_print/plugins/print_call.rs
View File

@ -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();