Skip to content

Commit

Permalink
Insert parentheses for multi-argument generators (#12422)
Browse files Browse the repository at this point in the history
## Summary

Closes #12420.
  • Loading branch information
charliermarsh authored Jul 20, 2024
1 parent 4bcc96a commit 2c1926b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,18 @@
max([x.val for x in bar])
sum([x.val for x in bar], 0)

# Ok
# OK
sum(x.val for x in bar)
min(x.val for x in bar)
max(x.val for x in bar)
sum(x.val for x in bar, 0)

# Multi-line
sum(
[
delta
for delta in timedelta_list
if delta
],
dt.timedelta(),
)
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use ruff_python_ast::{self as ast, Expr, Keyword};

use ruff_diagnostics::Violation;
use ruff_diagnostics::{Diagnostic, FixAvailability};
use ruff_diagnostics::{Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::any_over_expr;
use ruff_text_size::Ranged;
use ruff_text_size::{Ranged, TextSize};

use crate::checkers::ast::Checker;

Expand Down Expand Up @@ -112,9 +112,30 @@ pub(crate) fn unnecessary_comprehension_in_call(
}

let mut diagnostic = Diagnostic::new(UnnecessaryComprehensionInCall, arg.range());
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_comprehension_in_call(expr, checker.locator(), checker.stylist())
});

if args.len() == 1 {
// If there's only one argument, remove the list or set brackets.
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_comprehension_in_call(expr, checker.locator(), checker.stylist())
});
} else {
// If there are multiple arguments, replace the list or set brackets with parentheses.
// If a function call has multiple arguments, one of which is a generator, then the
// generator must be parenthesized.

// Replace `[` with `(`.
let collection_start = Edit::replacement(
"(".to_string(),
arg.start(),
arg.start() + TextSize::from(1),
);

// Replace `]` with `)`.
let collection_end =
Edit::replacement(")".to_string(), arg.end() - TextSize::from(1), arg.end());

diagnostic.set_fix(Fix::unsafe_edits(collection_start, [collection_end]));
}
checker.diagnostics.push(diagnostic);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ C419_1.py:3:5: C419 [*] Unnecessary list comprehension
3 |+max(x.val for x in bar)
4 4 | sum([x.val for x in bar], 0)
5 5 |
6 6 | # Ok
6 6 | # OK

C419_1.py:4:5: C419 [*] Unnecessary list comprehension
|
Expand All @@ -61,7 +61,7 @@ C419_1.py:4:5: C419 [*] Unnecessary list comprehension
4 | sum([x.val for x in bar], 0)
| ^^^^^^^^^^^^^^^^^^^^ C419
5 |
6 | # Ok
6 | # OK
|
= help: Remove unnecessary list comprehension

Expand All @@ -70,7 +70,37 @@ C419_1.py:4:5: C419 [*] Unnecessary list comprehension
2 2 | min([x.val for x in bar])
3 3 | max([x.val for x in bar])
4 |-sum([x.val for x in bar], 0)
4 |+sum(x.val for x in bar, 0)
4 |+sum((x.val for x in bar), 0)
5 5 |
6 6 | # Ok
6 6 | # OK
7 7 | sum(x.val for x in bar)

C419_1.py:14:5: C419 [*] Unnecessary list comprehension
|
12 | # Multi-line
13 | sum(
14 | [
| _____^
15 | | delta
16 | | for delta in timedelta_list
17 | | if delta
18 | | ],
| |_____^ C419
19 | dt.timedelta(),
20 | )
|
= help: Remove unnecessary list comprehension

Unsafe fix
11 11 |
12 12 | # Multi-line
13 13 | sum(
14 |- [
14 |+ (
15 15 | delta
16 16 | for delta in timedelta_list
17 17 | if delta
18 |- ],
18 |+ ),
19 19 | dt.timedelta(),
20 20 | )

0 comments on commit 2c1926b

Please sign in to comment.