Skip to content

Commit c5fa0cc

Browse files
charliermarshcharliermarsh
authored
Remove CST-based fixers for C400, C401, C410, and C418 (#9819)
1 parent dd77d29 commit c5fa0cc

9 files changed

Lines changed: 188 additions & 231 deletions

File tree

‎crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C410.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,12 @@
22
l2 = list((1, 2))
33
l3 = list([])
44
l4 = list(())
5+
6+
7+
list( # comment
8+
[1, 2]
9+
)
10+
11+
list([ # comment
12+
1, 2
13+
])

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -638,14 +638,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
638638
flake8_bandit::rules::tarfile_unsafe_members(checker, call);
639639
}
640640
if checker.enabled(Rule::UnnecessaryGeneratorList) {
641-
flake8_comprehensions::rules::unnecessary_generator_list(
642-
checker, expr, func, args, keywords,
643-
);
641+
flake8_comprehensions::rules::unnecessary_generator_list(checker, call);
644642
}
645643
if checker.enabled(Rule::UnnecessaryGeneratorSet) {
646-
flake8_comprehensions::rules::unnecessary_generator_set(
647-
checker, expr, func, args, keywords,
648-
);
644+
flake8_comprehensions::rules::unnecessary_generator_set(checker, call);
649645
}
650646
if checker.enabled(Rule::UnnecessaryGeneratorDict) {
651647
flake8_comprehensions::rules::unnecessary_generator_dict(
@@ -686,14 +682,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
686682
);
687683
}
688684
if checker.enabled(Rule::UnnecessaryLiteralWithinListCall) {
689-
flake8_comprehensions::rules::unnecessary_literal_within_list_call(
690-
checker, expr, func, args, keywords,
691-
);
685+
flake8_comprehensions::rules::unnecessary_literal_within_list_call(checker, call);
692686
}
693687
if checker.enabled(Rule::UnnecessaryLiteralWithinDictCall) {
694-
flake8_comprehensions::rules::unnecessary_literal_within_dict_call(
695-
checker, expr, func, args, keywords,
696-
);
688+
flake8_comprehensions::rules::unnecessary_literal_within_dict_call(checker, call);
697689
}
698690
if checker.enabled(Rule::UnnecessaryListCall) {
699691
flake8_comprehensions::rules::unnecessary_list_call(checker, expr, func, args);

‎crates/ruff_linter/src/rules/flake8_comprehensions/fixes.rs‎

Lines changed: 0 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -29,73 +29,6 @@ use crate::{
2929
},
3030
};
3131

32-
/// (C400) Convert `list(x for x in y)` to `[x for x in y]`.
33-
pub(crate) fn fix_unnecessary_generator_list(
34-
expr: &Expr,
35-
locator: &Locator,
36-
stylist: &Stylist,
37-
) -> Result<Edit> {
38-
// Expr(Call(GeneratorExp)))) -> Expr(ListComp)))
39-
let module_text = locator.slice(expr);
40-
let mut tree = match_expression(module_text)?;
41-
let call = match_call_mut(&mut tree)?;
42-
let arg = match_arg(call)?;
43-
44-
let generator_exp = match_generator_exp(&arg.value)?;
45-
46-
tree = Expression::ListComp(Box::new(ListComp {
47-
elt: generator_exp.elt.clone(),
48-
for_in: generator_exp.for_in.clone(),
49-
lbracket: LeftSquareBracket {
50-
whitespace_after: call.whitespace_before_args.clone(),
51-
},
52-
rbracket: RightSquareBracket {
53-
whitespace_before: arg.whitespace_after_arg.clone(),
54-
},
55-
lpar: generator_exp.lpar.clone(),
56-
rpar: generator_exp.rpar.clone(),
57-
}));
58-
59-
Ok(Edit::range_replacement(
60-
tree.codegen_stylist(stylist),
61-
expr.range(),
62-
))
63-
}
64-
65-
/// (C401) Convert `set(x for x in y)` to `{x for x in y}`.
66-
pub(crate) fn fix_unnecessary_generator_set(expr: &Expr, checker: &Checker) -> Result<Edit> {
67-
let locator = checker.locator();
68-
let stylist = checker.stylist();
69-
70-
// Expr(Call(GeneratorExp)))) -> Expr(SetComp)))
71-
let module_text = locator.slice(expr);
72-
let mut tree = match_expression(module_text)?;
73-
let call = match_call_mut(&mut tree)?;
74-
let arg = match_arg(call)?;
75-
76-
let generator_exp = match_generator_exp(&arg.value)?;
77-
78-
tree = Expression::SetComp(Box::new(SetComp {
79-
elt: generator_exp.elt.clone(),
80-
for_in: generator_exp.for_in.clone(),
81-
lbrace: LeftCurlyBrace {
82-
whitespace_after: call.whitespace_before_args.clone(),
83-
},
84-
rbrace: RightCurlyBrace {
85-
whitespace_before: arg.whitespace_after_arg.clone(),
86-
},
87-
lpar: generator_exp.lpar.clone(),
88-
rpar: generator_exp.rpar.clone(),
89-
}));
90-
91-
let content = tree.codegen_stylist(stylist);
92-
93-
Ok(Edit::range_replacement(
94-
pad_expression(content, expr.range(), checker.locator(), checker.semantic()),
95-
expr.range(),
96-
))
97-
}
98-
9932
/// (C402) Convert `dict((x, x) for x in range(3))` to `{x: x for x in
10033
/// range(3)}`.
10134
pub(crate) fn fix_unnecessary_generator_dict(expr: &Expr, checker: &Checker) -> Result<Edit> {
@@ -628,58 +561,6 @@ pub(crate) fn fix_unnecessary_literal_within_tuple_call(
628561
))
629562
}
630563

631-
/// (C410) Convert `list([1, 2])` to `[1, 2]`
632-
pub(crate) fn fix_unnecessary_literal_within_list_call(
633-
expr: &Expr,
634-
locator: &Locator,
635-
stylist: &Stylist,
636-
) -> Result<Edit> {
637-
let module_text = locator.slice(expr);
638-
let mut tree = match_expression(module_text)?;
639-
let call = match_call_mut(&mut tree)?;
640-
let arg = match_arg(call)?;
641-
let (elements, whitespace_after, whitespace_before) = match &arg.value {
642-
Expression::Tuple(inner) => (
643-
&inner.elements,
644-
&inner
645-
.lpar
646-
.first()
647-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
648-
.whitespace_after,
649-
&inner
650-
.rpar
651-
.first()
652-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
653-
.whitespace_before,
654-
),
655-
Expression::List(inner) => (
656-
&inner.elements,
657-
&inner.lbracket.whitespace_after,
658-
&inner.rbracket.whitespace_before,
659-
),
660-
_ => {
661-
bail!("Expected Expression::Tuple | Expression::List");
662-
}
663-
};
664-
665-
tree = Expression::List(Box::new(List {
666-
elements: elements.clone(),
667-
lbracket: LeftSquareBracket {
668-
whitespace_after: whitespace_after.clone(),
669-
},
670-
rbracket: RightSquareBracket {
671-
whitespace_before: whitespace_before.clone(),
672-
},
673-
lpar: vec![],
674-
rpar: vec![],
675-
}));
676-
677-
Ok(Edit::range_replacement(
678-
tree.codegen_stylist(stylist),
679-
expr.range(),
680-
))
681-
}
682-
683564
/// (C411) Convert `list([i * i for i in x])` to `[i * i for i in x]`.
684565
pub(crate) fn fix_unnecessary_list_call(
685566
expr: &Expr,
@@ -1094,25 +975,6 @@ pub(crate) fn fix_unnecessary_map(
1094975
Ok(Edit::range_replacement(content, expr.range()))
1095976
}
1096977

1097-
/// (C418) Convert `dict({"a": 1})` to `{"a": 1}`
1098-
pub(crate) fn fix_unnecessary_literal_within_dict_call(
1099-
expr: &Expr,
1100-
locator: &Locator,
1101-
stylist: &Stylist,
1102-
) -> Result<Edit> {
1103-
let module_text = locator.slice(expr);
1104-
let mut tree = match_expression(module_text)?;
1105-
let call = match_call_mut(&mut tree)?;
1106-
let arg = match_arg(call)?;
1107-
1108-
tree = arg.value.clone();
1109-
1110-
Ok(Edit::range_replacement(
1111-
tree.codegen_stylist(stylist),
1112-
expr.range(),
1113-
))
1114-
}
1115-
1116978
/// (C419) Convert `[i for i in a]` into `i for i in a`
1117979
pub(crate) fn fix_unnecessary_comprehension_any_all(
1118980
expr: &Expr,

‎crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_list.rs‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
1-
use ruff_python_ast::{Expr, Keyword};
2-
3-
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
1+
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
42
use ruff_macros::{derive_message_formats, violation};
5-
use ruff_text_size::Ranged;
3+
use ruff_python_ast as ast;
4+
use ruff_text_size::{Ranged, TextSize};
65

76
use crate::checkers::ast::Checker;
87

9-
use crate::rules::flake8_comprehensions::fixes;
10-
118
use super::helpers;
129

1310
/// ## What it does
@@ -47,27 +44,40 @@ impl AlwaysFixableViolation for UnnecessaryGeneratorList {
4744
}
4845

4946
/// C400 (`list(generator)`)
50-
pub(crate) fn unnecessary_generator_list(
51-
checker: &mut Checker,
52-
expr: &Expr,
53-
func: &Expr,
54-
args: &[Expr],
55-
keywords: &[Keyword],
56-
) {
57-
let Some(argument) =
58-
helpers::exactly_one_argument_with_matching_function("list", func, args, keywords)
59-
else {
47+
pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::ExprCall) {
48+
let Some(argument) = helpers::exactly_one_argument_with_matching_function(
49+
"list",
50+
&call.func,
51+
&call.arguments.args,
52+
&call.arguments.keywords,
53+
) else {
6054
return;
6155
};
6256
if !checker.semantic().is_builtin("list") {
6357
return;
6458
}
65-
if let Expr::GeneratorExp(_) = argument {
66-
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, expr.range());
67-
diagnostic.try_set_fix(|| {
68-
fixes::fix_unnecessary_generator_list(expr, checker.locator(), checker.stylist())
69-
.map(Fix::unsafe_edit)
59+
if argument.is_generator_exp_expr() {
60+
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, call.range());
61+
62+
// Convert `list(x for x in y)` to `[x for x in y]`.
63+
diagnostic.set_fix({
64+
// Replace `list(` with `[`.
65+
let call_start = Edit::replacement(
66+
"[".to_string(),
67+
call.start(),
68+
call.arguments.start() + TextSize::from(1),
69+
);
70+
71+
// Replace `)` with `]`.
72+
let call_end = Edit::replacement(
73+
"]".to_string(),
74+
call.arguments.end() - TextSize::from(1),
75+
call.end(),
76+
);
77+
78+
Fix::unsafe_edits(call_start, [call_end])
7079
});
80+
7181
checker.diagnostics.push(diagnostic);
7282
}
7383
}

‎crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_generator_set.rs‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
use ruff_python_ast::{Expr, Keyword};
2-
3-
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
1+
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
42
use ruff_macros::{derive_message_formats, violation};
5-
use ruff_text_size::Ranged;
3+
use ruff_python_ast as ast;
4+
use ruff_text_size::{Ranged, TextSize};
65

76
use crate::checkers::ast::Checker;
8-
9-
use crate::rules::flake8_comprehensions::fixes;
7+
use crate::rules::flake8_comprehensions::fixes::{pad_end, pad_start};
108

119
use super::helpers;
1210

@@ -47,26 +45,40 @@ impl AlwaysFixableViolation for UnnecessaryGeneratorSet {
4745
}
4846

4947
/// C401 (`set(generator)`)
50-
pub(crate) fn unnecessary_generator_set(
51-
checker: &mut Checker,
52-
expr: &Expr,
53-
func: &Expr,
54-
args: &[Expr],
55-
keywords: &[Keyword],
56-
) {
57-
let Some(argument) =
58-
helpers::exactly_one_argument_with_matching_function("set", func, args, keywords)
59-
else {
48+
pub(crate) fn unnecessary_generator_set(checker: &mut Checker, call: &ast::ExprCall) {
49+
let Some(argument) = helpers::exactly_one_argument_with_matching_function(
50+
"set",
51+
&call.func,
52+
&call.arguments.args,
53+
&call.arguments.keywords,
54+
) else {
6055
return;
6156
};
6257
if !checker.semantic().is_builtin("set") {
6358
return;
6459
}
65-
if let Expr::GeneratorExp(_) = argument {
66-
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, expr.range());
67-
diagnostic.try_set_fix(|| {
68-
fixes::fix_unnecessary_generator_set(expr, checker).map(Fix::unsafe_edit)
60+
if argument.is_generator_exp_expr() {
61+
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, call.range());
62+
63+
// Convert `set(x for x in y)` to `{x for x in y}`.
64+
diagnostic.set_fix({
65+
// Replace `set(` with `}`.
66+
let call_start = Edit::replacement(
67+
pad_start("{", call.range(), checker.locator(), checker.semantic()),
68+
call.start(),
69+
call.arguments.start() + TextSize::from(1),
70+
);
71+
72+
// Replace `)` with `}`.
73+
let call_end = Edit::replacement(
74+
pad_end("}", call.range(), checker.locator(), checker.semantic()),
75+
call.arguments.end() - TextSize::from(1),
76+
call.end(),
77+
);
78+
79+
Fix::unsafe_edits(call_start, [call_end])
6980
});
81+
7082
checker.diagnostics.push(diagnostic);
7183
}
7284
}

‎crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_list_comprehension_set.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &a
5757
}
5858
if argument.is_list_comp_expr() {
5959
let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, call.range());
60-
diagnostic.try_set_fix(|| {
60+
diagnostic.set_fix({
6161
// Replace `set(` with `{`.
6262
let call_start = Edit::replacement(
6363
pad_start("{", call.range(), checker.locator(), checker.semantic()),
@@ -79,10 +79,7 @@ pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &a
7979
// Delete the close bracket (`]`).
8080
let argument_end = Edit::deletion(argument.end() - TextSize::from(1), argument.end());
8181

82-
Ok(Fix::unsafe_edits(
83-
call_start,
84-
[argument_start, argument_end, call_end],
85-
))
82+
Fix::unsafe_edits(call_start, [argument_start, argument_end, call_end])
8683
});
8784
checker.diagnostics.push(diagnostic);
8885
}

0 commit comments

Comments
 (0)