Fix nested calls to `sorted` with differing arguments (#5761)

## Summary

Nested calls to `sorted` can only be collapsed if the calls are
identical (i.e., they have the exact same keyword arguments).
Update C414 to only flag such cases.

Fixes #5712

## Test Plan

Updated snapshots.
Tested against flake8-comprehensions. It incorrectly flags these cases.
This commit is contained in:
Justin Prieto 2023-07-14 09:43:47 -04:00 committed by GitHub
parent fb46579d30
commit 816f7644a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 124 additions and 69 deletions

View File

@ -12,7 +12,8 @@ set(reversed(x))
sorted(list(x)) sorted(list(x))
sorted(tuple(x)) sorted(tuple(x))
sorted(sorted(x)) sorted(sorted(x))
sorted(sorted(x, key=lambda y: y)) sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
sorted(sorted(x, reverse=True), reverse=True)
sorted(reversed(x)) sorted(reversed(x))
sorted(list(x), key=lambda y: y) sorted(list(x), key=lambda y: y)
tuple( tuple(
@ -21,3 +22,9 @@ tuple(
"o"] "o"]
) )
) )
# Nested sorts with differing keyword arguments. Not flagged.
sorted(sorted(x, key=lambda y: y))
sorted(sorted(x, key=lambda y: y), key=lambda x: x)
sorted(sorted(x), reverse=True)
sorted(sorted(x, reverse=False), reverse=True)

View File

@ -2772,7 +2772,7 @@ where
} }
if self.enabled(Rule::UnnecessaryDoubleCastOrProcess) { if self.enabled(Rule::UnnecessaryDoubleCastOrProcess) {
flake8_comprehensions::rules::unnecessary_double_cast_or_process( flake8_comprehensions::rules::unnecessary_double_cast_or_process(
self, expr, func, args, self, expr, func, args, keywords,
); );
} }
if self.enabled(Rule::UnnecessarySubscriptReversal) { if self.enabled(Rule::UnnecessarySubscriptReversal) {

View File

@ -1,7 +1,8 @@
use rustpython_parser::ast::{self, Expr, Ranged}; use rustpython_parser::ast::{self, Expr, Keyword, Ranged};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableKeyword;
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::registry::AsRule; use crate::registry::AsRule;
@ -69,6 +70,7 @@ pub(crate) fn unnecessary_double_cast_or_process(
expr: &Expr, expr: &Expr,
func: &Expr, func: &Expr,
args: &[Expr], args: &[Expr],
outer_kw: &[Keyword],
) { ) {
let Some(outer) = helpers::expr_name(func) else { let Some(outer) = helpers::expr_name(func) else {
return; return;
@ -84,7 +86,12 @@ pub(crate) fn unnecessary_double_cast_or_process(
let Some(arg) = args.first() else { let Some(arg) = args.first() else {
return; return;
}; };
let Expr::Call(ast::ExprCall { func, .. }) = arg else { let Expr::Call(ast::ExprCall {
func,
keywords: inner_kw,
..
}) = arg
else {
return; return;
}; };
let Some(inner) = helpers::expr_name(func) else { let Some(inner) = helpers::expr_name(func) else {
@ -94,6 +101,21 @@ pub(crate) fn unnecessary_double_cast_or_process(
return; return;
} }
// Avoid collapsing nested `sorted` calls with non-identical keyword arguments
// (i.e., `key`, `reverse`).
if inner == "sorted" && outer == "sorted" {
if inner_kw.len() != outer_kw.len() {
return;
}
if !inner_kw.iter().all(|inner| {
outer_kw
.iter()
.any(|outer| ComparableKeyword::from(inner) == ComparableKeyword::from(outer))
}) {
return;
}
}
// Ex) set(tuple(...)) // Ex) set(tuple(...))
// Ex) list(tuple(...)) // Ex) list(tuple(...))
// Ex) set(set(...)) // Ex) set(set(...))

View File

@ -226,7 +226,7 @@ C414.py:12:1: C414 [*] Unnecessary `list` call within `sorted()`
12 |+sorted(x) 12 |+sorted(x)
13 13 | sorted(tuple(x)) 13 13 | sorted(tuple(x))
14 14 | sorted(sorted(x)) 14 14 | sorted(sorted(x))
15 15 | sorted(sorted(x, key=lambda y: y)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()` C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()`
| |
@ -235,7 +235,7 @@ C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()`
13 | sorted(tuple(x)) 13 | sorted(tuple(x))
| ^^^^^^^^^^^^^^^^ C414 | ^^^^^^^^^^^^^^^^ C414
14 | sorted(sorted(x)) 14 | sorted(sorted(x))
15 | sorted(sorted(x, key=lambda y: y)) 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
| |
= help: Remove the inner `tuple` call = help: Remove the inner `tuple` call
@ -246,8 +246,8 @@ C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()`
13 |-sorted(tuple(x)) 13 |-sorted(tuple(x))
13 |+sorted(x) 13 |+sorted(x)
14 14 | sorted(sorted(x)) 14 14 | sorted(sorted(x))
15 15 | sorted(sorted(x, key=lambda y: y)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 16 | sorted(reversed(x)) 16 16 | sorted(sorted(x, reverse=True), reverse=True)
C414.py:14:1: C414 [*] Unnecessary `sorted` call within `sorted()` C414.py:14:1: C414 [*] Unnecessary `sorted` call within `sorted()`
| |
@ -255,8 +255,8 @@ C414.py:14:1: C414 [*] Unnecessary `sorted` call within `sorted()`
13 | sorted(tuple(x)) 13 | sorted(tuple(x))
14 | sorted(sorted(x)) 14 | sorted(sorted(x))
| ^^^^^^^^^^^^^^^^^ C414 | ^^^^^^^^^^^^^^^^^ C414
15 | sorted(sorted(x, key=lambda y: y)) 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 | sorted(reversed(x)) 16 | sorted(sorted(x, reverse=True), reverse=True)
| |
= help: Remove the inner `sorted` call = help: Remove the inner `sorted` call
@ -266,18 +266,18 @@ C414.py:14:1: C414 [*] Unnecessary `sorted` call within `sorted()`
13 13 | sorted(tuple(x)) 13 13 | sorted(tuple(x))
14 |-sorted(sorted(x)) 14 |-sorted(sorted(x))
14 |+sorted(x) 14 |+sorted(x)
15 15 | sorted(sorted(x, key=lambda y: y)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 16 | sorted(reversed(x)) 16 16 | sorted(sorted(x, reverse=True), reverse=True)
17 17 | sorted(list(x), key=lambda y: y) 17 17 | sorted(reversed(x))
C414.py:15:1: C414 [*] Unnecessary `sorted` call within `sorted()` C414.py:15:1: C414 [*] Unnecessary `sorted` call within `sorted()`
| |
13 | sorted(tuple(x)) 13 | sorted(tuple(x))
14 | sorted(sorted(x)) 14 | sorted(sorted(x))
15 | sorted(sorted(x, key=lambda y: y)) 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414
16 | sorted(reversed(x)) 16 | sorted(sorted(x, reverse=True), reverse=True)
17 | sorted(list(x), key=lambda y: y) 17 | sorted(reversed(x))
| |
= help: Remove the inner `sorted` call = help: Remove the inner `sorted` call
@ -285,77 +285,103 @@ C414.py:15:1: C414 [*] Unnecessary `sorted` call within `sorted()`
12 12 | sorted(list(x)) 12 12 | sorted(list(x))
13 13 | sorted(tuple(x)) 13 13 | sorted(tuple(x))
14 14 | sorted(sorted(x)) 14 14 | sorted(sorted(x))
15 |-sorted(sorted(x, key=lambda y: y)) 15 |-sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
15 |+sorted(x, ) 15 |+sorted(x, reverse=False, key=foo)
16 16 | sorted(reversed(x)) 16 16 | sorted(sorted(x, reverse=True), reverse=True)
17 17 | sorted(list(x), key=lambda y: y) 17 17 | sorted(reversed(x))
18 18 | tuple( 18 18 | sorted(list(x), key=lambda y: y)
C414.py:16:1: C414 [*] Unnecessary `reversed` call within `sorted()` C414.py:16:1: C414 [*] Unnecessary `sorted` call within `sorted()`
| |
14 | sorted(sorted(x)) 14 | sorted(sorted(x))
15 | sorted(sorted(x, key=lambda y: y)) 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 | sorted(reversed(x)) 16 | sorted(sorted(x, reverse=True), reverse=True)
| ^^^^^^^^^^^^^^^^^^^ C414 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414
17 | sorted(list(x), key=lambda y: y) 17 | sorted(reversed(x))
18 | tuple( 18 | sorted(list(x), key=lambda y: y)
| |
= help: Remove the inner `reversed` call = help: Remove the inner `sorted` call
Suggested fix Suggested fix
13 13 | sorted(tuple(x)) 13 13 | sorted(tuple(x))
14 14 | sorted(sorted(x)) 14 14 | sorted(sorted(x))
15 15 | sorted(sorted(x, key=lambda y: y)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 |-sorted(reversed(x)) 16 |-sorted(sorted(x, reverse=True), reverse=True)
16 |+sorted(x) 16 |+sorted(x, reverse=True)
17 17 | sorted(list(x), key=lambda y: y) 17 17 | sorted(reversed(x))
18 18 | tuple( 18 18 | sorted(list(x), key=lambda y: y)
19 19 | list( 19 19 | tuple(
C414.py:17:1: C414 [*] Unnecessary `list` call within `sorted()` C414.py:17:1: C414 [*] Unnecessary `reversed` call within `sorted()`
| |
15 | sorted(sorted(x, key=lambda y: y)) 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 | sorted(reversed(x)) 16 | sorted(sorted(x, reverse=True), reverse=True)
17 | sorted(list(x), key=lambda y: y) 17 | sorted(reversed(x))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 | ^^^^^^^^^^^^^^^^^^^ C414
18 | tuple( 18 | sorted(list(x), key=lambda y: y)
19 | list( 19 | tuple(
| |
= help: Remove the inner `list` call = help: Remove the inner `reversed` call
Suggested fix Suggested fix
14 14 | sorted(sorted(x)) 14 14 | sorted(sorted(x))
15 15 | sorted(sorted(x, key=lambda y: y)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
16 16 | sorted(reversed(x)) 16 16 | sorted(sorted(x, reverse=True), reverse=True)
17 |-sorted(list(x), key=lambda y: y) 17 |-sorted(reversed(x))
17 |+sorted(x, key=lambda y: y) 17 |+sorted(x)
18 18 | tuple( 18 18 | sorted(list(x), key=lambda y: y)
19 19 | list( 19 19 | tuple(
20 20 | [x, 3, "hell"\ 20 20 | list(
C414.py:18:1: C414 [*] Unnecessary `list` call within `tuple()` C414.py:18:1: C414 [*] Unnecessary `list` call within `sorted()`
| |
16 | sorted(reversed(x)) 16 | sorted(sorted(x, reverse=True), reverse=True)
17 | sorted(list(x), key=lambda y: y) 17 | sorted(reversed(x))
18 | / tuple( 18 | sorted(list(x), key=lambda y: y)
19 | | list( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414
20 | | [x, 3, "hell"\ 19 | tuple(
21 | | "o"] 20 | list(
22 | | )
23 | | )
| |_^ C414
| |
= help: Remove the inner `list` call = help: Remove the inner `list` call
Suggested fix Suggested fix
16 16 | sorted(reversed(x)) 15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo)
17 17 | sorted(list(x), key=lambda y: y) 16 16 | sorted(sorted(x, reverse=True), reverse=True)
18 18 | tuple( 17 17 | sorted(reversed(x))
19 |- list( 18 |-sorted(list(x), key=lambda y: y)
20 |- [x, 3, "hell"\ 18 |+sorted(x, key=lambda y: y)
19 |+ [x, 3, "hell"\ 19 19 | tuple(
21 20 | "o"] 20 20 | list(
22 21 | ) 21 21 | [x, 3, "hell"\
23 |-)
C414.py:19:1: C414 [*] Unnecessary `list` call within `tuple()`
|
17 | sorted(reversed(x))
18 | sorted(list(x), key=lambda y: y)
19 | / tuple(
20 | | list(
21 | | [x, 3, "hell"\
22 | | "o"]
23 | | )
24 | | )
| |_^ C414
25 |
26 | # Nested sorts with differing keyword arguments. Not flagged.
|
= help: Remove the inner `list` call
Suggested fix
17 17 | sorted(reversed(x))
18 18 | sorted(list(x), key=lambda y: y)
19 19 | tuple(
20 |- list(
21 |- [x, 3, "hell"\
20 |+ [x, 3, "hell"\
22 21 | "o"]
23 22 | )
24 |-)
25 23 |
26 24 | # Nested sorts with differing keyword arguments. Not flagged.
27 25 | sorted(sorted(x, key=lambda y: y))