From 2e00983762cccea91feca819aaee9ec71f4ad186 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 23 Aug 2023 00:38:08 +0530 Subject: [PATCH] Avoid `C417` for `lambda` with default and variadic parameters (#6752) ## Summary Avoid `C417` for `lambda` with default and variadic parameters. ## Test Plan `cargo test` and checking if it generates any autofix errors as test cases for `lambda` with default parameters already exists. fixes: #6715 --- .../fixtures/flake8_comprehensions/C417.py | 10 ++-- .../rules/unnecessary_map.rs | 47 ++++++++++++++----- ...8_comprehensions__tests__C417_C417.py.snap | 40 +++++++--------- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py index 7077acfad6bc5..a19c606a58986 100644 --- a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py +++ b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py @@ -15,11 +15,6 @@ _ = f"{set(map(lambda x: x % 2 == 0, nums))}" _ = f"{dict(map(lambda v: (v, v**2), nums))}" -# Error, but unfixable. -# For simple expressions, this could be: `(x if x else 1 for x in nums)`. -# For more complex expressions, this would differ: `(x + 2 if x else 3 for x in nums)`. -map(lambda x=1: x, nums) - # False negatives. map(lambda x=2, y=1: x + y, nums, nums) set(map(lambda x, y: x, nums, nums)) @@ -37,3 +32,8 @@ def func(arg1: int, arg2: int = 4): # Error: the `x` is overridden by the inner lambda. map(lambda x: lambda x: x, range(4)) + +# Ok because of the default parameters, and variadic arguments. +map(lambda x=1: x, nums) +map(lambda *args: len(args), range(4)) +map(lambda **kwargs: len(kwargs), range(4)) diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs index 8675682925fb7..11c0d1a6094b7 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs @@ -103,10 +103,17 @@ pub(crate) fn unnecessary_map( return; }; - if parameters - .as_ref() - .is_some_and(|parameters| late_binding(parameters, body)) - { + if parameters.as_ref().is_some_and(|parameters| { + late_binding(parameters, body) + || parameters + .posonlyargs + .iter() + .chain(¶meters.args) + .chain(¶meters.kwonlyargs) + .any(|param| param.default.is_some()) + || parameters.vararg.is_some() + || parameters.kwarg.is_some() + }) { return; } } @@ -137,10 +144,17 @@ pub(crate) fn unnecessary_map( return; }; - if parameters - .as_ref() - .is_some_and(|parameters| late_binding(parameters, body)) - { + if parameters.as_ref().is_some_and(|parameters| { + late_binding(parameters, body) + || parameters + .posonlyargs + .iter() + .chain(¶meters.args) + .chain(¶meters.kwonlyargs) + .any(|param| param.default.is_some()) + || parameters.vararg.is_some() + || parameters.kwarg.is_some() + }) { return; } } @@ -177,14 +191,21 @@ pub(crate) fn unnecessary_map( return; } - if parameters - .as_ref() - .is_some_and(|parameters| late_binding(parameters, body)) - { + if parameters.as_ref().is_some_and(|parameters| { + late_binding(parameters, body) + || parameters + .posonlyargs + .iter() + .chain(¶meters.args) + .chain(¶meters.kwonlyargs) + .any(|param| param.default.is_some()) + || parameters.vararg.is_some() + || parameters.kwarg.is_some() + }) { return; } } - } + }; let mut diagnostic = Diagnostic::new(UnnecessaryMap { object_type }, expr.range()); if checker.patch(diagnostic.kind.rule()) { diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap index acb6700d3fbed..c04bcb03b9fd8 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap +++ b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap @@ -226,7 +226,7 @@ C417.py:15:8: C417 [*] Unnecessary `map` usage (rewrite using a `set` comprehens 15 |+_ = f"{ {x % 2 == 0 for x in nums} }" 16 16 | _ = f"{dict(map(lambda v: (v, v**2), nums))}" 17 17 | -18 18 | # Error, but unfixable. +18 18 | # False negatives. C417.py:16:8: C417 [*] Unnecessary `map` usage (rewrite using a `dict` comprehension) | @@ -235,7 +235,7 @@ C417.py:16:8: C417 [*] Unnecessary `map` usage (rewrite using a `dict` comprehen 16 | _ = f"{dict(map(lambda v: (v, v**2), nums))}" | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417 17 | -18 | # Error, but unfixable. +18 | # False negatives. | = help: Replace `map` with a `dict` comprehension @@ -246,33 +246,27 @@ C417.py:16:8: C417 [*] Unnecessary `map` usage (rewrite using a `dict` comprehen 16 |-_ = f"{dict(map(lambda v: (v, v**2), nums))}" 16 |+_ = f"{ {v: v**2 for v in nums} }" 17 17 | -18 18 | # Error, but unfixable. -19 19 | # For simple expressions, this could be: `(x if x else 1 for x in nums)`. +18 18 | # False negatives. +19 19 | map(lambda x=2, y=1: x + y, nums, nums) -C417.py:21:1: C417 Unnecessary `map` usage (rewrite using a generator expression) +C417.py:34:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression) | -19 | # For simple expressions, this could be: `(x if x else 1 for x in nums)`. -20 | # For more complex expressions, this would differ: `(x + 2 if x else 3 for x in nums)`. -21 | map(lambda x=1: x, nums) - | ^^^^^^^^^^^^^^^^^^^^^^^^ C417 -22 | -23 | # False negatives. - | - = help: Replace `map` with a generator expression - -C417.py:39:1: C417 [*] Unnecessary `map` usage (rewrite using a generator expression) - | -38 | # Error: the `x` is overridden by the inner lambda. -39 | map(lambda x: lambda x: x, range(4)) +33 | # Error: the `x` is overridden by the inner lambda. +34 | map(lambda x: lambda x: x, range(4)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C417 +35 | +36 | # Ok because of the default parameters, and variadic arguments. | = help: Replace `map` with a generator expression ℹ Suggested fix -36 36 | map(lambda x: lambda: x, range(4)) -37 37 | -38 38 | # Error: the `x` is overridden by the inner lambda. -39 |-map(lambda x: lambda x: x, range(4)) - 39 |+(lambda x: x for x in range(4)) +31 31 | map(lambda x: lambda: x, range(4)) +32 32 | +33 33 | # Error: the `x` is overridden by the inner lambda. +34 |-map(lambda x: lambda x: x, range(4)) + 34 |+(lambda x: x for x in range(4)) +35 35 | +36 36 | # Ok because of the default parameters, and variadic arguments. +37 37 | map(lambda x=1: x, nums)