[flake8-comprehensions] Do not emit unnecessary-map diagnostic wh…

…en lambda has different arity (`C417`) (#15802)
astral-sh · Jan 29, 2025 · e1c9d10 · e1c9d10
1 parent 23c9884
commit e1c9d10
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 85 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C417.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C417.py
@@ -6,12 +6,12 @@
 set(map(lambda x: x % 2 == 0, nums))
 dict(map(lambda v: (v, v**2), nums))
 dict(map(lambda v: [v, v**2], nums))
-map(lambda: "const", nums)
+
 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))}"
@@ -49,11 +49,24 @@ def func(arg1: int, arg2: int = 4):
 map(lambda x: x, (x, y, z))
 
 # See https://github.com/astral-sh/ruff/issues/14808
-# The following should be Ok since 
+# The following should be Ok since
 # named expressions are a syntax error inside comprehensions
 a = [1, 2, 3]
 b = map(lambda x: x, c := a)
 print(c)
 
 # Check nested as well
 map(lambda x:x, [c:=a])
+
+
+# https://github.com/astral-sh/ruff/issues/15796
+map(lambda: "const", nums)
+list(map(lambda x, y: x * y, nums))
+
+
+map(lambda: 1, "xyz")
+map(lambda x, y: x, [(1, 2), (3, 4)])
+list(map(lambda: 1, "xyz"))
+list(map(lambda x, y: x, [(1, 2), (3, 4)]))
+list(map(lambda: 1, "xyz"))
+list(map(lambda x, y: x, [(1, 2), (3, 4)]))
diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs
@@ -111,14 +111,7 @@ pub(crate) fn unnecessary_map(
                 return;
             }
 
-            if parameters.as_ref().is_some_and(|parameters| {
-                late_binding(parameters, body)
-                    || parameters
-                        .iter_non_variadic_params()
-                        .any(|param| param.default.is_some())
-                    || parameters.vararg.is_some()
-                    || parameters.kwarg.is_some()
-            }) {
+            if !lambda_has_expected_arity(parameters.as_deref(), body) {
                 return;
             }
         }
@@ -153,14 +146,7 @@ pub(crate) fn unnecessary_map(
                 return;
             };
 
-            if parameters.as_ref().is_some_and(|parameters| {
-                late_binding(parameters, body)
-                    || parameters
-                        .iter_non_variadic_params()
-                        .any(|param| param.default.is_some())
-                    || parameters.vararg.is_some()
-                    || parameters.kwarg.is_some()
-            }) {
+            if !lambda_has_expected_arity(parameters.as_deref(), body) {
                 return;
             }
         }
@@ -205,14 +191,7 @@ pub(crate) fn unnecessary_map(
                 return;
             }
 
-            if parameters.as_ref().is_some_and(|parameters| {
-                late_binding(parameters, body)
-                    || parameters
-                        .iter_non_variadic_params()
-                        .any(|param| param.default.is_some())
-                    || parameters.vararg.is_some()
-                    || parameters.kwarg.is_some()
-            }) {
+            if !lambda_has_expected_arity(parameters.as_deref(), body) {
                 return;
             }
         }
@@ -232,6 +211,35 @@ pub(crate) fn unnecessary_map(
     checker.diagnostics.push(diagnostic);
 }
 
+/// A lambda as the first argument to `map()` has the "expected" arity when:
+///
+/// * It has exactly one parameter
+/// * That parameter is not variadic
+/// * That parameter does not have a default value
+fn lambda_has_expected_arity(parameters: Option<&Parameters>, body: &Expr) -> bool {
+    let Some(parameters) = parameters else {
+        return false;
+    };
+
+    let [parameter] = &parameters.args[..] else {
+        return false;
+    };
+
+    if parameter.default.is_some() {
+        return false;
+    }
+
+    if parameters.vararg.is_some() || parameters.kwarg.is_some() {
+        return false;
+    }
+
+    if late_binding(parameters, body) {
+        return false;
+    }
+
+    true
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub(crate) enum ObjectType {
     Generator,

diff --git a/...prehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap b/...prehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap
@@ -82,7 +82,7 @@ C417.py:6:1: C417 [*] Unnecessary `map()` usage (rewrite using a set comprehensi
   6 |+{x % 2 == 0 for x in nums}
 7 7 | dict(map(lambda v: (v, v**2), nums))
 8 8 | dict(map(lambda v: [v, v**2], nums))
-9 9 | map(lambda: "const", nums)
+9 9 | 
 
 C417.py:7:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehension)
   |
@@ -91,7 +91,6 @@ C417.py:7:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehens
 7 | dict(map(lambda v: (v, v**2), nums))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
 8 | dict(map(lambda v: [v, v**2], nums))
-9 | map(lambda: "const", nums)
   |
   = help: Replace `map()` with a dict comprehension
 
@@ -102,7 +101,7 @@ C417.py:7:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehens
 7   |-dict(map(lambda v: (v, v**2), nums))
   7 |+{v: v**2 for v in nums}
 8 8 | dict(map(lambda v: [v, v**2], nums))
-9 9 | map(lambda: "const", nums)
+9 9 | 
 10 10 | map(lambda _: 3.0, nums)
 
 C417.py:8:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehension)
@@ -111,7 +110,7 @@ C417.py:8:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehens
  7 | dict(map(lambda v: (v, v**2), nums))
  8 | dict(map(lambda v: [v, v**2], nums))
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
- 9 | map(lambda: "const", nums)
+ 9 |
 10 | map(lambda _: 3.0, nums)
    |
    = help: Replace `map()` with a dict comprehension
@@ -122,35 +121,14 @@ C417.py:8:1: C417 [*] Unnecessary `map()` usage (rewrite using a dict comprehens
 7 7 | dict(map(lambda v: (v, v**2), nums))
 8   |-dict(map(lambda v: [v, v**2], nums))
   8 |+{v: v**2 for v in nums}
-9 9 | map(lambda: "const", nums)
+9 9 | 
 10 10 | map(lambda _: 3.0, nums)
 11 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
 
-C417.py:9:1: C417 [*] Unnecessary `map()` usage (rewrite using a generator expression)
-   |
- 7 | dict(map(lambda v: (v, v**2), nums))
- 8 | dict(map(lambda v: [v, v**2], nums))
- 9 | map(lambda: "const", nums)
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
-10 | map(lambda _: 3.0, nums)
-11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
-   |
-   = help: Replace `map()` with a generator expression
-
-ℹ Unsafe fix
-6  6  | set(map(lambda x: x % 2 == 0, nums))
-7  7  | dict(map(lambda v: (v, v**2), nums))
-8  8  | dict(map(lambda v: [v, v**2], nums))
-9     |-map(lambda: "const", nums)
-   9  |+("const" for _ in nums)
-10 10 | map(lambda _: 3.0, nums)
-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))
-
 C417.py:10:1: C417 [*] Unnecessary `map()` usage (rewrite using a generator expression)
    |
  8 | dict(map(lambda v: [v, v**2], nums))
- 9 | map(lambda: "const", nums)
+ 9 |
 10 | map(lambda _: 3.0, nums)
    | ^^^^^^^^^^^^^^^^^^^^^^^^ C417
 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
@@ -161,7 +139,7 @@ C417.py:10:1: C417 [*] Unnecessary `map()` usage (rewrite using a generator expr
 ℹ Unsafe fix
 7  7  | dict(map(lambda v: (v, v**2), nums))
 8  8  | dict(map(lambda v: [v, v**2], nums))
-9  9  | map(lambda: "const", nums)
+9  9  | 
 10    |-map(lambda _: 3.0, nums)
    10 |+(3.0 for _ in nums)
 11 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
@@ -170,7 +148,6 @@ C417.py:10:1: C417 [*] Unnecessary `map()` usage (rewrite using a generator expr
 
 C417.py:11:13: C417 [*] Unnecessary `map()` usage (rewrite using a generator expression)
    |
- 9 | map(lambda: "const", nums)
 10 | map(lambda _: 3.0, nums)
 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417
@@ -181,13 +158,13 @@ C417.py:11:13: C417 [*] Unnecessary `map()` usage (rewrite using a generator exp
 
 ℹ Unsafe fix
 8  8  | dict(map(lambda v: [v, v**2], nums))
-9  9  | map(lambda: "const", nums)
+9  9  | 
 10 10 | map(lambda _: 3.0, nums)
 11    |-_ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
    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 | list(map(lambda x, y: x * y, nums))
+14 14 | 
 
 C417.py:12:5: C417 [*] Unnecessary `map()` usage (rewrite using a generator expression)
    |
@@ -196,18 +173,17 @@ C417.py:12:5: C417 [*] Unnecessary `map()` usage (rewrite using a generator expr
 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
 
 ℹ Unsafe fix
-9  9  | map(lambda: "const", nums)
+9  9  | 
 10 10 | map(lambda _: 3.0, nums)
 11 11 | _ = "".join(map(lambda x: x in nums and "1" or "0", range(123)))
 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 | list(map(lambda x, y: x * y, nums))
+14 14 | 
 15 15 | 
 
 C417.py:13:14: C417 [*] Unnecessary `map()` usage (rewrite using a generator expression)
@@ -216,7 +192,6 @@ C417.py:13:14: C417 [*] Unnecessary `map()` usage (rewrite using a generator exp
 12 | all(map(lambda v: isinstance(v, dict), nums))
 13 | filter(func, map(lambda v: v, nums))
    |              ^^^^^^^^^^^^^^^^^^^^^^ C417
-14 | list(map(lambda x, y: x * y, nums))
    |
    = help: Replace `map()` with a generator expression
 
@@ -226,31 +201,10 @@ C417.py:13:14: C417 [*] Unnecessary `map()` usage (rewrite using a generator exp
 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 | list(map(lambda x, y: x * y, nums))
+14 14 | 
 15 15 | 
 16 16 | # When inside f-string, then the fix should be surrounded by whitespace
 
-C417.py:14:1: C417 [*] Unnecessary `map()` usage (rewrite using a list comprehension)
-   |
-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
-
-ℹ Unsafe 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
@@ -261,7 +215,7 @@ C417.py:17:8: C417 [*] Unnecessary `map()` usage (rewrite using a set comprehens
    = help: Replace `map()` with a set comprehension
 
 ℹ Unsafe fix
-14 14 | list(map(lambda x, y: x * y, nums))
+14 14 | 
 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))}"