diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py index c590ed3fcc..29fb74661a 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C414.py @@ -12,7 +12,8 @@ set(reversed(x)) sorted(list(x)) sorted(tuple(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(list(x), key=lambda y: y) tuple( @@ -21,3 +22,9 @@ tuple( "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) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 3d3921f025..f999b586d9 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2772,7 +2772,7 @@ where } if self.enabled(Rule::UnnecessaryDoubleCastOrProcess) { flake8_comprehensions::rules::unnecessary_double_cast_or_process( - self, expr, func, args, + self, expr, func, args, keywords, ); } if self.enabled(Rule::UnnecessarySubscriptReversal) { diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs index 2c71d2d11a..857920c9f3 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs @@ -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_macros::{derive_message_formats, violation}; +use ruff_python_ast::comparable::ComparableKeyword; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -69,6 +70,7 @@ pub(crate) fn unnecessary_double_cast_or_process( expr: &Expr, func: &Expr, args: &[Expr], + outer_kw: &[Keyword], ) { let Some(outer) = helpers::expr_name(func) else { return; @@ -84,7 +86,12 @@ pub(crate) fn unnecessary_double_cast_or_process( let Some(arg) = args.first() else { return; }; - let Expr::Call(ast::ExprCall { func, .. }) = arg else { + let Expr::Call(ast::ExprCall { + func, + keywords: inner_kw, + .. + }) = arg + else { return; }; let Some(inner) = helpers::expr_name(func) else { @@ -94,6 +101,21 @@ pub(crate) fn unnecessary_double_cast_or_process( 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) list(tuple(...)) // Ex) set(set(...)) diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap index 9c6af7a48a..fb2d7cefc9 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C414_C414.py.snap @@ -226,7 +226,7 @@ C414.py:12:1: C414 [*] Unnecessary `list` call within `sorted()` 12 |+sorted(x) 13 13 | sorted(tuple(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()` | @@ -235,7 +235,7 @@ C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()` 13 | sorted(tuple(x)) | ^^^^^^^^^^^^^^^^ C414 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 @@ -246,8 +246,8 @@ C414.py:13:1: C414 [*] Unnecessary `tuple` call within `sorted()` 13 |-sorted(tuple(x)) 13 |+sorted(x) 14 14 | sorted(sorted(x)) -15 15 | sorted(sorted(x, key=lambda y: y)) -16 16 | sorted(reversed(x)) +15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 16 | sorted(sorted(x, reverse=True), reverse=True) 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)) 14 | sorted(sorted(x)) | ^^^^^^^^^^^^^^^^^ C414 -15 | sorted(sorted(x, key=lambda y: y)) -16 | sorted(reversed(x)) +15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 | sorted(sorted(x, reverse=True), reverse=True) | = 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)) 14 |-sorted(sorted(x)) 14 |+sorted(x) -15 15 | sorted(sorted(x, key=lambda y: y)) -16 16 | sorted(reversed(x)) -17 17 | sorted(list(x), key=lambda y: y) +15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 16 | sorted(sorted(x, reverse=True), reverse=True) +17 17 | sorted(reversed(x)) C414.py:15:1: C414 [*] Unnecessary `sorted` call within `sorted()` | 13 | sorted(tuple(x)) 14 | sorted(sorted(x)) -15 | sorted(sorted(x, key=lambda y: y)) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 -16 | sorted(reversed(x)) -17 | sorted(list(x), key=lambda y: y) +15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 +16 | sorted(sorted(x, reverse=True), reverse=True) +17 | sorted(reversed(x)) | = 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)) 13 13 | sorted(tuple(x)) 14 14 | sorted(sorted(x)) -15 |-sorted(sorted(x, key=lambda y: y)) - 15 |+sorted(x, ) -16 16 | sorted(reversed(x)) -17 17 | sorted(list(x), key=lambda y: y) -18 18 | tuple( +15 |-sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) + 15 |+sorted(x, reverse=False, key=foo) +16 16 | sorted(sorted(x, reverse=True), reverse=True) +17 17 | sorted(reversed(x)) +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)) -15 | sorted(sorted(x, key=lambda y: y)) -16 | sorted(reversed(x)) - | ^^^^^^^^^^^^^^^^^^^ C414 -17 | sorted(list(x), key=lambda y: y) -18 | tuple( +15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 | sorted(sorted(x, reverse=True), reverse=True) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 +17 | sorted(reversed(x)) +18 | sorted(list(x), key=lambda y: y) | - = help: Remove the inner `reversed` call + = help: Remove the inner `sorted` call ℹ Suggested fix 13 13 | sorted(tuple(x)) 14 14 | sorted(sorted(x)) -15 15 | sorted(sorted(x, key=lambda y: y)) -16 |-sorted(reversed(x)) - 16 |+sorted(x) -17 17 | sorted(list(x), key=lambda y: y) -18 18 | tuple( -19 19 | list( +15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 |-sorted(sorted(x, reverse=True), reverse=True) + 16 |+sorted(x, reverse=True) +17 17 | sorted(reversed(x)) +18 18 | sorted(list(x), key=lambda y: y) +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)) -16 | sorted(reversed(x)) -17 | sorted(list(x), key=lambda y: y) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 -18 | tuple( -19 | list( +15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 | sorted(sorted(x, reverse=True), reverse=True) +17 | sorted(reversed(x)) + | ^^^^^^^^^^^^^^^^^^^ C414 +18 | sorted(list(x), key=lambda y: y) +19 | tuple( | - = help: Remove the inner `list` call + = help: Remove the inner `reversed` call ℹ Suggested fix 14 14 | sorted(sorted(x)) -15 15 | sorted(sorted(x, key=lambda y: y)) -16 16 | sorted(reversed(x)) -17 |-sorted(list(x), key=lambda y: y) - 17 |+sorted(x, key=lambda y: y) -18 18 | tuple( -19 19 | list( -20 20 | [x, 3, "hell"\ +15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 16 | sorted(sorted(x, reverse=True), reverse=True) +17 |-sorted(reversed(x)) + 17 |+sorted(x) +18 18 | sorted(list(x), key=lambda y: y) +19 19 | tuple( +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)) -17 | sorted(list(x), key=lambda y: y) -18 | / tuple( -19 | | list( -20 | | [x, 3, "hell"\ -21 | | "o"] -22 | | ) -23 | | ) - | |_^ C414 +16 | sorted(sorted(x, reverse=True), reverse=True) +17 | sorted(reversed(x)) +18 | sorted(list(x), key=lambda y: y) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C414 +19 | tuple( +20 | list( | = help: Remove the inner `list` call ℹ Suggested fix -16 16 | sorted(reversed(x)) -17 17 | sorted(list(x), key=lambda y: y) -18 18 | tuple( -19 |- list( -20 |- [x, 3, "hell"\ - 19 |+ [x, 3, "hell"\ -21 20 | "o"] -22 21 | ) -23 |-) +15 15 | sorted(sorted(x, key=foo, reverse=False), reverse=False, key=foo) +16 16 | sorted(sorted(x, reverse=True), reverse=True) +17 17 | sorted(reversed(x)) +18 |-sorted(list(x), key=lambda y: y) + 18 |+sorted(x, key=lambda y: y) +19 19 | tuple( +20 20 | list( +21 21 | [x, 3, "hell"\ + +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))