Use structured types for C417 comprehension target (#7190)

Rather than manually joining the arguments as a comma-separated string,
and treating that comma-separated string as a name.

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

@@ -11,6 +11,7 @@ map(lambda _: 3.0, nums)
 _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
 all(map(lambda v: isinstance(v, dict), nums))
 filter(func, map(lambda v: v, nums))
+list(map(lambda x, y: x * y, nums))
 
 # When inside f-string, then the fix should be surrounded by whitespace
 _ = f"{set(map(lambda x: x % 2 == 0, nums))}"

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

@@ -957,11 +957,29 @@ pub(crate) fn fix_unnecessary_map(
         _ => bail!("Expected a call or lambda"),
     };
 
-    // Format the lambda arguments as a comma-separated list.
-    let mut args_str = lambda.params.params.iter().map(|f| f.name.value).join(", ");
-    if args_str.is_empty() {
-        args_str = "_".to_string();
-    }
+    // Format the lambda target.
+    let target = match lambda.params.params.as_slice() {
+        // Ex) `lambda: x`
+        [] => AssignTargetExpression::Name(Box::new(Name {
+            value: "_",
+            lpar: vec![],
+            rpar: vec![],
+        })),
+        // Ex) `lambda x: y`
+        [param] => AssignTargetExpression::Name(Box::new(param.name.clone())),
+        // Ex) `lambda x, y: z`
+        params => AssignTargetExpression::Tuple(Box::new(Tuple {
+            elements: params
+                .iter()
+                .map(|param| Element::Simple {
+                    value: Expression::Name(Box::new(param.name.clone())),
+                    comma: None,
+                })
+                .collect(),
+            lpar: vec![],
+            rpar: vec![],
+        })),
+    };
 
     // Parenthesize the iterator, if necessary, as in:
     // ```python
@@ -978,11 +996,7 @@ pub(crate) fn fix_unnecessary_map(
     };
 
     let compfor = Box::new(CompFor {
-        target: AssignTargetExpression::Name(Box::new(Name {
-            value: args_str.as_str(),
-            lpar: vec![],
-            rpar: vec![],
-        })),
+        target,
         iter,
         ifs: vec![],
         inner_for_in: None,

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

@@ -187,7 +187,7 @@ C417.py:11:13: C417 [*] Unnecessary `map` usage (rewrite using a generator expre
    11 |+_ = "".join((x in nums and "1" or "0" for x in range(123)))
 12 12 | all(map(lambda v: isinstance(v, dict), nums))
 13 13 | filter(func, map(lambda v: v, nums))
-14 14 | 
+14 14 | list(map(lambda x, y: x * y, nums))
 
 C417.py:12:5: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
    |
@@ -196,6 +196,7 @@ C417.py:12:5: C417 [*] Unnecessary `map` usage (rewrite using a generator expres
 12 | all(map(lambda v: isinstance(v, dict), nums))
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
 13 | filter(func, map(lambda v: v, nums))
+14 | list(map(lambda x, y: x * y, nums))
    |
    = help: Replace `map` with a generator expression
 
@@ -206,8 +207,8 @@ C417.py:12:5: C417 [*] Unnecessary `map` usage (rewrite using a generator expres
 12    |-all(map(lambda v: isinstance(v, dict), nums))
    12 |+all((isinstance(v, dict) for v in nums))
 13 13 | filter(func, map(lambda v: v, nums))
-14 14 | 
-15 15 | # When inside f-string, then the fix should be surrounded by whitespace
+14 14 | list(map(lambda x, y: x * y, nums))
+15 15 | 
 
 C417.py:13:14: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
    |
@@ -215,8 +216,7 @@ C417.py:13:14: C417 [*] Unnecessary `map` usage (rewrite using a generator expre
 12 | all(map(lambda v: isinstance(v, dict), nums))
 13 | filter(func, map(lambda v: v, nums))
    |              ^^^^^^^^^^^^^^^^^^^^^^ C417
-14 | 
-15 | # When inside f-string, then the fix should be surrounded by whitespace
+14 | list(map(lambda x, y: x * y, nums))
    |
    = help: Replace `map` with a generator expression
 
@@ -226,121 +226,142 @@ C417.py:13:14: C417 [*] Unnecessary `map` usage (rewrite using a generator expre
 12 12 | all(map(lambda v: isinstance(v, dict), nums))
 13    |-filter(func, map(lambda v: v, nums))
    13 |+filter(func, (v for v in nums))
-14 14 | 
-15 15 | # When inside f-string, then the fix should be surrounded by whitespace
-16 16 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+14 14 | list(map(lambda x, y: x * y, nums))
+15 15 | 
+16 16 | # When inside f-string, then the fix should be surrounded by whitespace
 
-C417.py:16:8: C417 [*] Unnecessary `map` usage (rewrite using a `set` comprehension)
+C417.py:14:1: C417 [*] Unnecessary `map` usage (rewrite using a `list` comprehension)
    |
-15 | # When inside f-string, then the fix should be surrounded by whitespace
-16 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+12 | all(map(lambda v: isinstance(v, dict), nums))
+13 | filter(func, map(lambda v: v, nums))
+14 | list(map(lambda x, y: x * y, nums))
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
+15 | 
+16 | # When inside f-string, then the fix should be surrounded by whitespace
+   |
+   = help: Replace `map` with a `list` comprehension
+
+ℹ Suggested fix
+11 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
+12 12 | all(map(lambda v: isinstance(v, dict), nums))
+13 13 | filter(func, map(lambda v: v, nums))
+14    |-list(map(lambda x, y: x * y, nums))
+   14 |+[x * y for x, y in nums]
+15 15 | 
+16 16 | # When inside f-string, then the fix should be surrounded by whitespace
+17 17 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+
+C417.py:17:8: C417 [*] Unnecessary `map` usage (rewrite using a `set` comprehension)
+   |
+16 | # When inside f-string, then the fix should be surrounded by whitespace
+17 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-17 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
+18 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
    |
    = help: Replace `map` with a `set` comprehension
 
 ℹ Suggested fix
-13 13 | filter(func, map(lambda v: v, nums))
-14 14 | 
-15 15 | # When inside f-string, then the fix should be surrounded by whitespace
-16    |-_ = f"{set(map(lambda x: x % 2 == 0, nums))}"
-   16 |+_ = f"{ {x % 2 == 0 for x in nums} }"
-17 17 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
-18 18 | 
-19 19 | # False negatives.
+14 14 | list(map(lambda x, y: x * y, nums))
+15 15 | 
+16 16 | # When inside f-string, then the fix should be surrounded by whitespace
+17    |-_ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+   17 |+_ = f"{ {x % 2 == 0 for x in nums} }"
+18 18 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
+19 19 | 
+20 20 | # False negatives.
 
-C417.py:17:8: C417 [*] Unnecessary `map` usage (rewrite using a `dict` comprehension)
+C417.py:18:8: C417 [*] Unnecessary `map` usage (rewrite using a `dict` comprehension)
    |
-15 | # When inside f-string, then the fix should be surrounded by whitespace
-16 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
-17 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
+16 | # When inside f-string, then the fix should be surrounded by whitespace
+17 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+18 | _ = f"{dict(map(lambda v: (v, v**2), nums))}"
    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-18 | 
-19 | # False negatives.
+19 | 
+20 | # False negatives.
    |
    = help: Replace `map` with a `dict` comprehension
 
 ℹ Suggested fix
-14 14 | 
-15 15 | # When inside f-string, then the fix should be surrounded by whitespace
-16 16 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
-17    |-_ = f"{dict(map(lambda v: (v, v**2), nums))}"
-   17 |+_ = f"{ {v: v**2 for v in nums} }"
-18 18 | 
-19 19 | # False negatives.
-20 20 | map(lambda x=2, y=1: x + y, nums, nums)
+15 15 | 
+16 16 | # When inside f-string, then the fix should be surrounded by whitespace
+17 17 | _ = f"{set(map(lambda x: x % 2 == 0, nums))}"
+18    |-_ = f"{dict(map(lambda v: (v, v**2), nums))}"
+   18 |+_ = f"{ {v: v**2 for v in nums} }"
+19 19 | 
+20 20 | # False negatives.
+21 21 | map(lambda x=2, y=1: x + y, nums, nums)
 
-C417.py:35:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
+C417.py:36:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
    |
-34 | # Error: the `x` is overridden by the inner lambda.
-35 | map(lambda x: lambda x: x, range(4))
+35 | # Error: the `x` is overridden by the inner lambda.
+36 | map(lambda x: lambda x: x, range(4))
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-36 | 
-37 | # Ok because of the default parameters, and variadic arguments.
+37 | 
+38 | # Ok because of the default parameters, and variadic arguments.
    |
    = help: Replace `map` with a generator expression
 
 ℹ Suggested fix
-32 32 | map(lambda x: lambda: x, range(4))
-33 33 | 
-34 34 | # Error: the `x` is overridden by the inner lambda.
-35    |-map(lambda x: lambda x: x, range(4))
-   35 |+(lambda x: x for x in range(4))
-36 36 | 
-37 37 | # Ok because of the default parameters, and variadic arguments.
-38 38 | map(lambda x=1: x, nums)
-
-C417.py:46:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
-   |
-45 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
-46 | map(lambda x: x, y if y else z)
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-47 | map(lambda x: x, (y if y else z))
-48 | map(lambda x: x, (x, y, z))
-   |
-   = help: Replace `map` with a generator expression
-
-ℹ Suggested fix
-43 43 | dict(map(lambda k, v: (k, v), keys, values))
-44 44 | 
-45 45 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
-46    |-map(lambda x: x, y if y else z)
-   46 |+(x for x in (y if y else z))
-47 47 | map(lambda x: x, (y if y else z))
-48 48 | map(lambda x: x, (x, y, z))
+33 33 | map(lambda x: lambda: x, range(4))
+34 34 | 
+35 35 | # Error: the `x` is overridden by the inner lambda.
+36    |-map(lambda x: lambda x: x, range(4))
+   36 |+(lambda x: x for x in range(4))
+37 37 | 
+38 38 | # Ok because of the default parameters, and variadic arguments.
+39 39 | map(lambda x=1: x, nums)
 
 C417.py:47:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
    |
-45 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
-46 | map(lambda x: x, y if y else z)
-47 | map(lambda x: x, (y if y else z))
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-48 | map(lambda x: x, (x, y, z))
+46 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
+47 | map(lambda x: x, y if y else z)
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
+48 | map(lambda x: x, (y if y else z))
+49 | map(lambda x: x, (x, y, z))
    |
    = help: Replace `map` with a generator expression
 
 ℹ Suggested fix
-44 44 | 
-45 45 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
-46 46 | map(lambda x: x, y if y else z)
-47    |-map(lambda x: x, (y if y else z))
+44 44 | dict(map(lambda k, v: (k, v), keys, values))
+45 45 | 
+46 46 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
+47    |-map(lambda x: x, y if y else z)
    47 |+(x for x in (y if y else z))
-48 48 | map(lambda x: x, (x, y, z))
+48 48 | map(lambda x: x, (y if y else z))
+49 49 | map(lambda x: x, (x, y, z))
 
 C417.py:48:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
    |
-46 | map(lambda x: x, y if y else z)
-47 | map(lambda x: x, (y if y else z))
-48 | map(lambda x: x, (x, y, z))
+46 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
+47 | map(lambda x: x, y if y else z)
+48 | map(lambda x: x, (y if y else z))
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
+49 | map(lambda x: x, (x, y, z))
+   |
+   = help: Replace `map` with a generator expression
+
+ℹ Suggested fix
+45 45 | 
+46 46 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
+47 47 | map(lambda x: x, y if y else z)
+48    |-map(lambda x: x, (y if y else z))
+   48 |+(x for x in (y if y else z))
+49 49 | map(lambda x: x, (x, y, z))
+
+C417.py:49:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression)
+   |
+47 | map(lambda x: x, y if y else z)
+48 | map(lambda x: x, (y if y else z))
+49 | map(lambda x: x, (x, y, z))
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
    |
    = help: Replace `map` with a generator expression
 
 ℹ Suggested fix
-45 45 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
-46 46 | map(lambda x: x, y if y else z)
-47 47 | map(lambda x: x, (y if y else z))
-48    |-map(lambda x: x, (x, y, z))
-   48 |+(x for x in (x, y, z))
+46 46 | # Regression test for: https://github.com/astral-sh/ruff/issues/7121
+47 47 | map(lambda x: x, y if y else z)
+48 48 | map(lambda x: x, (y if y else z))
+49    |-map(lambda x: x, (x, y, z))
+   49 |+(x for x in (x, y, z))