[flake8-comprehensions] bugfix for C413 autofix (#2804)

README.md

@@ -1047,7 +1047,7 @@ For more, see [flake8-comprehensions](https://pypi.org/project/flake8-comprehens
 | C409 | unnecessary-literal-within-tuple-call | Unnecessary `{literal}` literal passed to `tuple()` (rewrite as a `tuple` literal) | 🛠 |
 | C410 | unnecessary-literal-within-list-call | Unnecessary `{literal}` literal passed to `list()` (remove the outer call to `list()`) | 🛠 |
 | C411 | unnecessary-list-call | Unnecessary `list` call (remove the outer call to `list()`) | 🛠 |
-| C413 | unnecessary-call-around-sorted | Unnecessary `{func}` call around `sorted()` | 🛠 |
+| C413 | [unnecessary-call-around-sorted](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-call-around-sorted.md) | Unnecessary `{func}` call around `sorted()` | 🛠 |
 | C414 | [unnecessary-double-cast-or-process](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unnecessary-double-cast-or-process.md) | Unnecessary `{inner}` call within `{outer}()` | 🛠 |
 | C415 | unnecessary-subscript-reversal | Unnecessary subscript reversal of iterable within `{func}()` |  |
 | C416 | unnecessary-comprehension | Unnecessary `{obj_type}` comprehension (rewrite using `{obj_type}()`) | 🛠 |

crates/ruff/resources/test/fixtures/flake8_comprehensions/C413.py

@@ -4,7 +4,9 @@ list(sorted(x))
 reversed(sorted(x))
 reversed(sorted(x, key=lambda e: e))
 reversed(sorted(x, reverse=True))
-
+reversed(sorted(x, key=lambda e: e, reverse=True))
+reversed(sorted(x, reverse=True, key=lambda e: e))
+reversed(sorted(x, reverse=False))
 
 def reversed(*args, **kwargs):
     return None

crates/ruff/src/rules/flake8_comprehensions/fixes.rs

@@ -743,6 +743,7 @@ pub fn fix_unnecessary_call_around_sorted(
         if outer_name.value == "list" {
             body.value = Expression::Call(inner_call.clone());
         } else {
+            // If the `reverse` argument is used
             let args = if inner_call.args.iter().any(|arg| {
                 matches!(
                     arg.keyword,
@@ -752,7 +753,46 @@ pub fn fix_unnecessary_call_around_sorted(
                     })
                 )
             }) {
-                inner_call.args.clone()
+                // Negate the `reverse` argument
+                inner_call
+                    .args
+                    .clone()
+                    .into_iter()
+                    .map(|mut arg| {
+                        if matches!(
+                            arg.keyword,
+                            Some(Name {
+                                value: "reverse",
+                                ..
+                            })
+                        ) {
+                            if let Expression::Name(ref val) = arg.value {
+                                if val.value == "True" {
+                                    // TODO: even better would be to drop the argument, as False is the default
+                                    arg.value = Expression::Name(Box::new(Name {
+                                        value: "False",
+                                        lpar: vec![],
+                                        rpar: vec![],
+                                    }));
+                                    arg
+                                } else if val.value == "False" {
+                                    arg.value = Expression::Name(Box::new(Name {
+                                        value: "True",
+                                        lpar: vec![],
+                                        rpar: vec![],
+                                    }));
+                                    arg
+                                } else {
+                                    arg
+                                }
+                            } else {
+                                arg
+                            }
+                        } else {
+                            arg
+                        }
+                    })
+                    .collect_vec()
             } else {
                 let mut args = inner_call.args.clone();
                 args.push(Arg {

crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs

@@ -10,6 +10,29 @@ use crate::rules::flake8_comprehensions::fixes;
 use crate::violation::AlwaysAutofixableViolation;
 
 define_violation!(
+    /// ## What it does
+    /// Checks for unnecessary `list` or `reversed` calls around `sorted`
+    /// calls.
+    ///
+    /// ## Why is this bad?
+    /// It is unnecessary to use `list` around `sorted`, as the latter already
+    /// returns a list.
+    ///
+    /// It is also unnecessary to use `reversed` around `sorted`, as the latter
+    /// has a `reverse` argument that can be used in lieu of an additional
+    /// `reversed` call.
+    ///
+    /// In both cases, it's clearer to avoid the redundant call.
+    ///
+    /// ## Examples
+    /// ```python
+    /// reversed(sorted(iterable))
+    /// ```
+    ///
+    /// Use instead:
+    /// ```python
+    /// sorted(iterable, reverse=True)
+    /// ```
     pub struct UnnecessaryCallAroundSorted {
         pub func: String,
     }

crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C413_C413.py.snap

@@ -1,5 +1,5 @@
 ---
-source: src/rules/flake8_comprehensions/mod.rs
+source: crates/ruff/src/rules/flake8_comprehensions/mod.rs
 expression: diagnostics
 ---
 - kind:
@@ -70,7 +70,7 @@ expression: diagnostics
     column: 33
   fix:
     content:
-      - "sorted(x, reverse=True)"
+      - "sorted(x, reverse=False)"
     location:
       row: 6
       column: 0
@@ -78,4 +78,61 @@ expression: diagnostics
       row: 6
       column: 33
   parent: ~
+- kind:
+    UnnecessaryCallAroundSorted:
+      func: reversed
+  location:
+    row: 7
+    column: 0
+  end_location:
+    row: 7
+    column: 50
+  fix:
+    content:
+      - "sorted(x, key=lambda e: e, reverse=False)"
+    location:
+      row: 7
+      column: 0
+    end_location:
+      row: 7
+      column: 50
+  parent: ~
+- kind:
+    UnnecessaryCallAroundSorted:
+      func: reversed
+  location:
+    row: 8
+    column: 0
+  end_location:
+    row: 8
+    column: 50
+  fix:
+    content:
+      - "sorted(x, reverse=False, key=lambda e: e)"
+    location:
+      row: 8
+      column: 0
+    end_location:
+      row: 8
+      column: 50
+  parent: ~
+- kind:
+    UnnecessaryCallAroundSorted:
+      func: reversed
+  location:
+    row: 9
+    column: 0
+  end_location:
+    row: 9
+    column: 34
+  fix:
+    content:
+      - "sorted(x, reverse=True)"
+    location:
+      row: 9
+      column: 0
+    end_location:
+      row: 9
+      column: 34
+  parent: ~
 

docs/rules/unnecessary-call-around-sorted.md

@@ -0,0 +1,29 @@
+# unnecessary-call-around-sorted (C413)
+
+Derived from the **flake8-comprehensions** linter.
+
+Autofix is always available.
+
+## What it does
+Checks for unnecessary `list` or `reversed` calls around `sorted`
+calls.
+
+## Why is this bad?
+It is unnecessary to use `list` around `sorted`, as the latter already
+returns a list.
+
+It is also unnecessary to use `reversed` around `sorted`, as the latter
+has a `reverse` argument that can be used in lieu of an additional
+`reversed` call.
+
+In both cases, it's clearer to avoid the redundant call.
+
+## Examples
+```python
+reversed(sorted(iterable))
+```
+
+Use instead:
+```python
+sorted(iterable, reverse=True)
+```