[flake8-comprehensions] Skip C416 if comprehension contains unpacking

crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C416.py

@@ -8,6 +8,7 @@
 {k: v for k, v in y}
 {k: v for k, v in d.items()}
 [(k, v) for k, v in d.items()]
+[(k, v) for [k, v] in d.items()]
 {k: (a, b) for k, (a, b) in d.items()}
 
 [i for i, in z]
@@ -22,3 +23,7 @@
 
 # Regression test for: https://github.com/astral-sh/ruff/issues/7196
 any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
+
+zz = [[1], [2], [3]]
+[(i,) for (i,) in zz]  # != list(zz)
+{(i,) for (i,) in zz}  # != set(zz)

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs

@@ -1,7 +1,7 @@
 use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
 use ruff_macros::{derive_message_formats, ViolationMetadata};
-use ruff_python_ast::comparable::ComparableExpr;
 use ruff_python_ast::{self as ast, Comprehension, Expr};
+use ruff_python_semantic::analyze::typing;
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -115,16 +115,22 @@ pub(crate) fn unnecessary_dict_comprehension(
     let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else {
         return;
     };
-    let [target_key, target_value] = elts.as_slice() else {
+    let [Expr::Name(ast::ExprName { id: target_key, .. }), Expr::Name(ast::ExprName {
+        id: target_value, ..
+    })] = elts.as_slice()
+    else {
         return;
     };
-    if ComparableExpr::from(key) != ComparableExpr::from(target_key) {
+    let Expr::Name(ast::ExprName { id: key, .. }) = &key else {
         return;
-    }
-    if ComparableExpr::from(value) != ComparableExpr::from(target_value) {
+    };
+    let Expr::Name(ast::ExprName { id: value, .. }) = &value else {
         return;
+    };
+
+    if target_key == key && target_value == value {
+        add_diagnostic(checker, expr);
     }
-    add_diagnostic(checker, expr);
 }
 
 /// C416
@@ -140,10 +146,83 @@ pub(crate) fn unnecessary_list_set_comprehension(
     if !generator.ifs.is_empty() || generator.is_async {
         return;
     }
-    if ComparableExpr::from(elt) != ComparableExpr::from(&generator.target) {
-        return;
+    if is_dict_items(checker, &generator.iter) {
+        match (&generator.target, elt) {
+            // [(k, v) for k, v in dict.items()] or [(k, v) for [k, v] in dict.items()]
+            (
+                Expr::Tuple(ast::ExprTuple {
+                    elts: target_elts, ..
+                })
+                | Expr::List(ast::ExprList {
+                    elts: target_elts, ..
+                }),
+                Expr::Tuple(ast::ExprTuple { elts, .. }),
+            ) => {
+                let [Expr::Name(ast::ExprName { id: target_key, .. }), Expr::Name(ast::ExprName {
+                    id: target_value, ..
+                })] = target_elts.as_slice()
+                else {
+                    return;
+                };
+                let [Expr::Name(ast::ExprName { id: key, .. }), Expr::Name(ast::ExprName { id: value, .. })] =
+                    elts.as_slice()
+                else {
+                    return;
+                };
+                if target_key == key && target_value == value {
+                    add_diagnostic(checker, expr);
+                }
+            }
+            // [x for x in dict.items()]
+            (
+                Expr::Name(ast::ExprName {
+                    id: target_name, ..
+                }),
+                Expr::Name(ast::ExprName { id: elt_name, .. }),
+            ) if target_name == elt_name => {
+                add_diagnostic(checker, expr);
+            }
+            _ => {}
+        }
+    } else {
+        // [x for x in iterable]
+        let Expr::Name(ast::ExprName {
+            id: target_name, ..
+        }) = &generator.target
+        else {
+            return;
+        };
+        let Expr::Name(ast::ExprName { id: elt_name, .. }) = &elt else {
+            return;
+        };
+        if elt_name == target_name {
+            add_diagnostic(checker, expr);
+        }
     }
-    add_diagnostic(checker, expr);
+}
+
+fn is_dict_items(checker: &Checker, expr: &Expr) -> bool {
+    let Expr::Call(ast::ExprCall { func, .. }) = expr else {
+        return false;
+    };
+
+    let Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = func.as_ref() else {
+        return false;
+    };
+
+    if attr.as_str() != "items" {
+        return false;
+    }
+
+    let Expr::Name(name) = value.as_ref() else {
+        return false;
+    };
+
+    let Some(id) = checker.semantic().resolve_name(name) else {
+        return false;
+    };
+
+    typing::is_dict(checker.semantic().binding(id), checker.semantic())
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C416_C416.py.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/ruff_linter/src/rules/flake8_comprehensions/mod.rs
+snapshot_kind: text
 ---
 C416.py:6:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
   |
@@ -61,7 +62,7 @@ C416.py:8:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
   8 |+dict(y)
 9 9 | {k: v for k, v in d.items()}
 10 10 | [(k, v) for k, v in d.items()]
-11 11 | {k: (a, b) for k, (a, b) in d.items()}
+11 11 | [(k, v) for [k, v] in d.items()]
 
 C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
    |
@@ -70,7 +71,7 @@ C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
  9 | {k: v for k, v in d.items()}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
 10 | [(k, v) for k, v in d.items()]
-11 | {k: (a, b) for k, (a, b) in d.items()}
+11 | [(k, v) for [k, v] in d.items()]
    |
    = help: Rewrite using `dict()`
 
@@ -81,16 +82,17 @@ C416.py:9:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
 9     |-{k: v for k, v in d.items()}
    9  |+dict(d.items())
 10 10 | [(k, v) for k, v in d.items()]
-11 11 | {k: (a, b) for k, (a, b) in d.items()}
-12 12 | 
+11 11 | [(k, v) for [k, v] in d.items()]
+12 12 | {k: (a, b) for k, (a, b) in d.items()}
 
 C416.py:10:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
    |
  8 | {k: v for k, v in y}
  9 | {k: v for k, v in d.items()}
 10 | [(k, v) for k, v in d.items()]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
-11 | {k: (a, b) for k, (a, b) in d.items()}
+11 | [(k, v) for [k, v] in d.items()]
+12 | {k: (a, b) for k, (a, b) in d.items()}
    |
    = help: Rewrite using `list()`
 
@@ -100,42 +102,46 @@ C416.py:10:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
 9  9  | {k: v for k, v in d.items()}
 10    |-[(k, v) for k, v in d.items()]
    10 |+list(d.items())
-11 11 | {k: (a, b) for k, (a, b) in d.items()}
-12 12 | 
-13 13 | [i for i, in z]
+11 11 | [(k, v) for [k, v] in d.items()]
+12 12 | {k: (a, b) for k, (a, b) in d.items()}
+13 13 | 
 
-C416.py:11:1: C416 [*] Unnecessary dict comprehension (rewrite using `dict()`)
+C416.py:11:1: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
    |
  9 | {k: v for k, v in d.items()}
 10 | [(k, v) for k, v in d.items()]
-11 | {k: (a, b) for k, (a, b) in d.items()}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
-12 | 
-13 | [i for i, in z]
+11 | [(k, v) for [k, v] in d.items()]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C416
+12 | {k: (a, b) for k, (a, b) in d.items()}
    |
-   = help: Rewrite using `dict()`
+   = help: Rewrite using `list()`
 
 ℹ Unsafe fix
 8  8  | {k: v for k, v in y}
 9  9  | {k: v for k, v in d.items()}
 10 10 | [(k, v) for k, v in d.items()]
-11    |-{k: (a, b) for k, (a, b) in d.items()}
-   11 |+dict(d.items())
-12 12 | 
-13 13 | [i for i, in z]
-14 14 | [i for i, j in y]
+11    |-[(k, v) for [k, v] in d.items()]
+   11 |+list(d.items())
+12 12 | {k: (a, b) for k, (a, b) in d.items()}
+13 13 | 
+14 14 | [i for i, in z]
 
-C416.py:24:70: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
+C416.py:25:70: C416 [*] Unnecessary list comprehension (rewrite using `list()`)
    |
-23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
-24 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
+24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
+25 | any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
    |                                                                      ^^^^^^^^^^^^^^^^^^^^^^^ C416
+26 | 
+27 | zz = [[1], [2], [3]]
    |
    = help: Rewrite using `list()`
 
 ℹ Unsafe fix
-21 21 | {k: v if v else None for k, v in y}
-22 22 | 
-23 23 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
-24    |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
-   24 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))
+22 22 | {k: v if v else None for k, v in y}
+23 23 | 
+24 24 | # Regression test for: https://github.com/astral-sh/ruff/issues/7196
+25    |-any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in[t for t in SymbolType])
+   25 |+any(len(symbol_table.get_by_type(symbol_type)) > 0 for symbol_type in list(SymbolType))
+26 26 | 
+27 27 | zz = [[1], [2], [3]]
+28 28 | [(i,) for (i,) in zz]  # != list(zz)