Skip to content

Commit cb00129

Browse files
charliermarshcharliermarsh
committed
Add more
1 parent c5fa0cc commit cb00129

6 files changed

Lines changed: 170 additions & 195 deletions

File tree

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,11 @@
88
t5 = tuple(
99
(1, 2)
1010
)
11+
12+
tuple( # comment
13+
[1, 2]
14+
)
15+
16+
tuple([ # comment
17+
1, 2
18+
])

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -657,9 +657,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
657657
);
658658
}
659659
if checker.enabled(Rule::UnnecessaryLiteralSet) {
660-
flake8_comprehensions::rules::unnecessary_literal_set(
661-
checker, expr, func, args, keywords,
662-
);
660+
flake8_comprehensions::rules::unnecessary_literal_set(checker, call);
663661
}
664662
if checker.enabled(Rule::UnnecessaryLiteralDict) {
665663
flake8_comprehensions::rules::unnecessary_literal_dict(
@@ -677,9 +675,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
677675
);
678676
}
679677
if checker.enabled(Rule::UnnecessaryLiteralWithinTupleCall) {
680-
flake8_comprehensions::rules::unnecessary_literal_within_tuple_call(
681-
checker, expr, func, args, keywords,
682-
);
678+
flake8_comprehensions::rules::unnecessary_literal_within_tuple_call(checker, call);
683679
}
684680
if checker.enabled(Rule::UnnecessaryLiteralWithinListCall) {
685681
flake8_comprehensions::rules::unnecessary_literal_within_list_call(checker, call);

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

Lines changed: 1 addition & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use libcst_native::{
66
Arg, AssignEqual, AssignTargetExpression, Call, Comment, CompFor, Dict, DictComp, DictElement,
77
Element, EmptyLine, Expression, GeneratorExp, LeftCurlyBrace, LeftParen, LeftSquareBracket,
88
List, ListComp, Name, ParenthesizableWhitespace, ParenthesizedNode, ParenthesizedWhitespace,
9-
RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace,
9+
RightCurlyBrace, RightParen, RightSquareBracket, SetComp, SimpleString, SimpleWhitespace,
1010
TrailingWhitespace, Tuple,
1111
};
1212

@@ -145,95 +145,6 @@ pub(crate) fn fix_unnecessary_list_comprehension_dict(
145145
))
146146
}
147147

148-
/// Drop a trailing comma from a list of tuple elements.
149-
fn drop_trailing_comma<'a>(
150-
tuple: &Tuple<'a>,
151-
) -> Result<(
152-
Vec<Element<'a>>,
153-
ParenthesizableWhitespace<'a>,
154-
ParenthesizableWhitespace<'a>,
155-
)> {
156-
let whitespace_after = tuple
157-
.lpar
158-
.first()
159-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
160-
.whitespace_after
161-
.clone();
162-
let whitespace_before = tuple
163-
.rpar
164-
.first()
165-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
166-
.whitespace_before
167-
.clone();
168-
169-
let mut elements = tuple.elements.clone();
170-
if elements.len() == 1 {
171-
if let Some(Element::Simple {
172-
value,
173-
comma: Some(..),
174-
..
175-
}) = elements.last()
176-
{
177-
if whitespace_before == ParenthesizableWhitespace::default()
178-
&& whitespace_after == ParenthesizableWhitespace::default()
179-
{
180-
elements[0] = Element::Simple {
181-
value: value.clone(),
182-
comma: None,
183-
};
184-
}
185-
}
186-
}
187-
188-
Ok((elements, whitespace_after, whitespace_before))
189-
}
190-
191-
/// (C405) Convert `set((1, 2))` to `{1, 2}`.
192-
pub(crate) fn fix_unnecessary_literal_set(expr: &Expr, checker: &Checker) -> Result<Edit> {
193-
let locator = checker.locator();
194-
let stylist = checker.stylist();
195-
196-
// Expr(Call(List|Tuple)))) -> Expr(Set)))
197-
let module_text = locator.slice(expr);
198-
let mut tree = match_expression(module_text)?;
199-
let call = match_call_mut(&mut tree)?;
200-
let arg = match_arg(call)?;
201-
202-
let (elements, whitespace_after, whitespace_before) = match &arg.value {
203-
Expression::Tuple(inner) => drop_trailing_comma(inner)?,
204-
Expression::List(inner) => (
205-
inner.elements.clone(),
206-
inner.lbracket.whitespace_after.clone(),
207-
inner.rbracket.whitespace_before.clone(),
208-
),
209-
_ => {
210-
bail!("Expected Expression::Tuple | Expression::List");
211-
}
212-
};
213-
214-
if elements.is_empty() {
215-
call.args = vec![];
216-
} else {
217-
tree = Expression::Set(Box::new(Set {
218-
elements,
219-
lbrace: LeftCurlyBrace { whitespace_after },
220-
rbrace: RightCurlyBrace { whitespace_before },
221-
lpar: vec![],
222-
rpar: vec![],
223-
}));
224-
}
225-
226-
Ok(Edit::range_replacement(
227-
pad_expression(
228-
tree.codegen_stylist(stylist),
229-
expr.range(),
230-
checker.locator(),
231-
checker.semantic(),
232-
),
233-
expr.range(),
234-
))
235-
}
236-
237148
/// (C406) Convert `dict([(1, 2)])` to `{1: 2}`.
238149
pub(crate) fn fix_unnecessary_literal_dict(expr: &Expr, checker: &Checker) -> Result<Edit> {
239150
let locator = checker.locator();
@@ -511,56 +422,6 @@ pub(crate) fn pad_end(
511422
}
512423
}
513424

514-
/// (C409) Convert `tuple([1, 2])` to `tuple(1, 2)`
515-
pub(crate) fn fix_unnecessary_literal_within_tuple_call(
516-
expr: &Expr,
517-
locator: &Locator,
518-
stylist: &Stylist,
519-
) -> Result<Edit> {
520-
let module_text = locator.slice(expr);
521-
let mut tree = match_expression(module_text)?;
522-
let call = match_call_mut(&mut tree)?;
523-
let arg = match_arg(call)?;
524-
let (elements, whitespace_after, whitespace_before) = match &arg.value {
525-
Expression::Tuple(inner) => (
526-
&inner.elements,
527-
&inner
528-
.lpar
529-
.first()
530-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
531-
.whitespace_after,
532-
&inner
533-
.rpar
534-
.first()
535-
.ok_or_else(|| anyhow::anyhow!("Expected at least one set of parentheses"))?
536-
.whitespace_before,
537-
),
538-
Expression::List(inner) => (
539-
&inner.elements,
540-
&inner.lbracket.whitespace_after,
541-
&inner.rbracket.whitespace_before,
542-
),
543-
_ => {
544-
bail!("Expected Expression::Tuple | Expression::List");
545-
}
546-
};
547-
548-
tree = Expression::Tuple(Box::new(Tuple {
549-
elements: elements.clone(),
550-
lpar: vec![LeftParen {
551-
whitespace_after: whitespace_after.clone(),
552-
}],
553-
rpar: vec![RightParen {
554-
whitespace_before: whitespace_before.clone(),
555-
}],
556-
}));
557-
558-
Ok(Edit::range_replacement(
559-
tree.codegen_stylist(stylist),
560-
expr.range(),
561-
))
562-
}
563-
564425
/// (C411) Convert `list([i * i for i in x])` to `[i * i for i in x]`.
565426
pub(crate) fn fix_unnecessary_list_call(
566427
expr: &Expr,

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

Lines changed: 70 additions & 19 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::{self as ast, Expr};
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

@@ -53,16 +51,13 @@ impl AlwaysFixableViolation for UnnecessaryLiteralSet {
5351
}
5452

5553
/// C405 (`set([1, 2])`)
56-
pub(crate) fn unnecessary_literal_set(
57-
checker: &mut Checker,
58-
expr: &Expr,
59-
func: &Expr,
60-
args: &[Expr],
61-
keywords: &[Keyword],
62-
) {
63-
let Some(argument) =
64-
helpers::exactly_one_argument_with_matching_function("set", func, args, keywords)
65-
else {
54+
pub(crate) fn unnecessary_literal_set(checker: &mut Checker, call: &ast::ExprCall) {
55+
let Some(argument) = helpers::exactly_one_argument_with_matching_function(
56+
"set",
57+
&call.func,
58+
&call.arguments.args,
59+
&call.arguments.keywords,
60+
) else {
6661
return;
6762
};
6863
if !checker.semantic().is_builtin("set") {
@@ -73,13 +68,69 @@ pub(crate) fn unnecessary_literal_set(
7368
Expr::Tuple(_) => "tuple",
7469
_ => return,
7570
};
71+
7672
let mut diagnostic = Diagnostic::new(
7773
UnnecessaryLiteralSet {
7874
obj_type: kind.to_string(),
7975
},
80-
expr.range(),
76+
call.range(),
8177
);
82-
diagnostic
83-
.try_set_fix(|| fixes::fix_unnecessary_literal_set(expr, checker).map(Fix::unsafe_edit));
78+
79+
// Convert `set((1, 2))` to `{1, 2}`.
80+
diagnostic.set_fix({
81+
let elts = match &argument {
82+
Expr::List(ast::ExprList { elts, .. }) => elts,
83+
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts,
84+
_ => unreachable!(),
85+
};
86+
87+
match elts.as_slice() {
88+
// If the list or tuple is empty, replace the entire call with `set()`.
89+
[] => Fix::unsafe_edit(Edit::range_replacement("set()".to_string(), call.range())),
90+
// If it's a single-element tuple (with no whitespace around it), remove the trailing
91+
// comma.
92+
[elt]
93+
if argument.is_tuple_expr()
94+
// The element must start right after the `(`.
95+
&& elt.start() == argument.start() + TextSize::new(1)
96+
// The element must be followed by exactly one comma and a closing `)`.
97+
&& elt.end() + TextSize::new(2) == argument.end() =>
98+
{
99+
// Replace from the start of the call to the start of the inner element.
100+
let call_start = Edit::replacement(
101+
pad_start("{", call.range(), checker.locator(), checker.semantic()),
102+
call.start(),
103+
elt.start(),
104+
);
105+
106+
// Replace from the end of the inner element to the end of the call with `}`.
107+
let call_end = Edit::replacement(
108+
pad_end("}", call.range(), checker.locator(), checker.semantic()),
109+
elt.end(),
110+
call.end(),
111+
);
112+
113+
Fix::unsafe_edits(call_start, [call_end])
114+
}
115+
_ => {
116+
// Replace from the start of the call to the start of the inner list or tuple with `{`.
117+
let call_start = Edit::replacement(
118+
pad_start("{", call.range(), checker.locator(), checker.semantic()),
119+
call.start(),
120+
argument.start() + TextSize::from(1),
121+
);
122+
123+
// Replace from the end of the inner list or tuple to the end of the call with `}`.
124+
let call_end = Edit::replacement(
125+
pad_end("}", call.range(), checker.locator(), checker.semantic()),
126+
argument.end() - TextSize::from(1),
127+
call.end(),
128+
);
129+
130+
Fix::unsafe_edits(call_start, [call_end])
131+
}
132+
}
133+
});
134+
84135
checker.diagnostics.push(diagnostic);
85136
}

0 commit comments

Comments
 (0)