From e1c9d10863c8c0255776da5fc32b43f5b3c07ff1 Mon Sep 17 00:00:00 2001 From: InSync Date: Thu, 30 Jan 2025 01:45:55 +0700 Subject: [PATCH] [`flake8-comprehensions`] Do not emit `unnecessary-map` diagnostic when lambda has different arity (`C417`) (#15802) --- .../fixtures/flake8_comprehensions/C417.py | 19 ++++- .../rules/unnecessary_map.rs | 56 ++++++++------- ...8_comprehensions__tests__C417_C417.py.snap | 70 ++++--------------- 3 files changed, 60 insertions(+), 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 index 70dfcf841987f4..8380774d9fd53f 100644 --- 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,7 +49,7 @@ 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) @@ -57,3 +57,16 @@ def func(arg1: int, arg2: int = 4): # 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 index f8a50dca3b0423..8cb0a08f3e5564 100644 --- 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] = ¶meters.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/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap index bdd83974fd49dd..6f7c3e08814fac 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/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))}"