Skip to content

Commit

Permalink
[flake8-comprehensions] Parenthesize sorted when needed for `unne…
Browse files Browse the repository at this point in the history
…cessary-call-around-sorted` (`C413`) (#15825)

If there is any `ParenthesizedWhitespace` (in the sense of LibCST) after
the function name `sorted` and before the arguments, then we must wrap
`sorted` with parentheses after removing the surrounding function.

Closes #15789
  • Loading branch information
dylwil3 authored Jan 30, 2025
1 parent 56f956a commit 13cf3e6
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,13 @@
# Regression test for: https://github.com/astral-sh/ruff/issues/10335
reversed(sorted([1, 2, 3], reverse=False or True))
reversed(sorted([1, 2, 3], reverse=(False or True)))

# These fixes need to be parenthesized to avoid syntax errors and behavior
# changes.
# See https://github.com/astral-sh/ruff/issues/15789
reversed(sorted
(""))
list(sorted
(""))
list(sorted
("xy"))
11 changes: 11 additions & 0 deletions crates/ruff_linter/src/rules/flake8_comprehensions/fixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,17 @@ pub(crate) fn fix_unnecessary_call_around_sorted(
}
};

let inner_needs_parens = matches!(
inner_call.whitespace_after_func,
ParenthesizableWhitespace::ParenthesizedWhitespace(_)
);

if let Expression::Name(outer_name) = &*outer_call.func {
if outer_name.value == "list" {
tree = Expression::Call(Box::new((*inner_call).clone()));
if inner_needs_parens {
tree = tree.with_parens(LeftParen::default(), RightParen::default());
};
} else {
// If the `reverse` argument is used...
let args = if inner_call.args.iter().any(|arg| {
Expand Down Expand Up @@ -503,6 +511,9 @@ pub(crate) fn fix_unnecessary_call_around_sorted(
whitespace_after_func: inner_call.whitespace_after_func.clone(),
whitespace_before_args: inner_call.whitespace_before_args.clone(),
}));
if inner_needs_parens {
tree = tree.with_parens(LeftParen::default(), RightParen::default());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,13 +244,17 @@ C413.py:18:1: C413 [*] Unnecessary `reversed()` call around `sorted()`
18 |-reversed(sorted([1, 2, 3], reverse=False or True))
18 |+sorted([1, 2, 3], reverse=not (False or True))
19 19 | reversed(sorted([1, 2, 3], reverse=(False or True)))
20 20 |
21 21 | # These fixes need to be parenthesized to avoid syntax errors and behavior

C413.py:19:1: C413 [*] Unnecessary `reversed()` call around `sorted()`
|
17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335
18 | reversed(sorted([1, 2, 3], reverse=False or True))
19 | reversed(sorted([1, 2, 3], reverse=(False or True)))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C413
20 |
21 | # These fixes need to be parenthesized to avoid syntax errors and behavior
|
= help: Remove unnecessary `reversed()` call

Expand All @@ -260,3 +264,70 @@ C413.py:19:1: C413 [*] Unnecessary `reversed()` call around `sorted()`
18 18 | reversed(sorted([1, 2, 3], reverse=False or True))
19 |-reversed(sorted([1, 2, 3], reverse=(False or True)))
19 |+sorted([1, 2, 3], reverse=not (False or True))
20 20 |
21 21 | # These fixes need to be parenthesized to avoid syntax errors and behavior
22 22 | # changes.

C413.py:24:1: C413 [*] Unnecessary `reversed()` call around `sorted()`
|
22 | # changes.
23 | # See https://github.com/astral-sh/ruff/issues/15789
24 | / reversed(sorted
25 | | (""))
| |_____^ C413
26 | list(sorted
27 | (""))
|
= help: Remove unnecessary `reversed()` call

Unsafe fix
21 21 | # These fixes need to be parenthesized to avoid syntax errors and behavior
22 22 | # changes.
23 23 | # See https://github.com/astral-sh/ruff/issues/15789
24 |-reversed(sorted
25 |-(""))
24 |+(sorted
25 |+("", reverse=True))
26 26 | list(sorted
27 27 | (""))
28 28 | list(sorted

C413.py:26:1: C413 [*] Unnecessary `list()` call around `sorted()`
|
24 | reversed(sorted
25 | (""))
26 | / list(sorted
27 | | (""))
| |_________^ C413
28 | list(sorted
29 | ("xy"))
|
= help: Remove unnecessary `list()` call

Safe fix
23 23 | # See https://github.com/astral-sh/ruff/issues/15789
24 24 | reversed(sorted
25 25 | (""))
26 |-list(sorted
26 |+(sorted
27 27 | (""))
28 28 | list(sorted
29 29 | ("xy"))

C413.py:28:1: C413 [*] Unnecessary `list()` call around `sorted()`
|
26 | list(sorted
27 | (""))
28 | / list(sorted
29 | | ("xy"))
| |_______^ C413
|
= help: Remove unnecessary `list()` call

Safe fix
25 25 | (""))
26 26 | list(sorted
27 27 | (""))
28 |-list(sorted
28 |+(sorted
29 29 | ("xy"))

0 comments on commit 13cf3e6

Please sign in to comment.