Skip to content

Commit

Permalink
Avoid C417 for lambda with default and variadic parameters (#6752)
Browse files Browse the repository at this point in the history 
## 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
  • Loading branch information
@dhruvmanila
dhruvmanila authored Aug 22, 2023
1 parent fb7caf4 commit 2e00983
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 41 deletions.
10 changes: 5 additions & 5 deletions crates/ruff/resources/test/fixtures/flake8_comprehensions/C417.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
47 changes: 34 additions & 13 deletions crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(&parameters.args)
.chain(&parameters.kwonlyargs)
.any(|param| param.default.is_some())
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
}) {
return;
}
}
Expand Down Expand Up @@ -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(&parameters.args)
.chain(&parameters.kwonlyargs)
.any(|param| param.default.is_some())
|| parameters.vararg.is_some()
|| parameters.kwarg.is_some()
}) {
return;
}
}
Expand Down Expand Up @@ -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(&parameters.args)
.chain(&parameters.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()) {
Expand Down
40 changes: 17 additions & 23 deletions ...ke8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C417_C417.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
|
Expand All @@ -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

Expand All @@ -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)


0 comments on commit 2e00983

Please sign in to comment.