Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-comprehensions] Do not emit unnecessary-map diagnostic when lambda has different arity (C417) #15802

Merged
merged 2 commits into from
Jan 29, 2025
Merged

[flake8-comprehensions] Do not emit unnecessary-map diagnostic when lambda has different arity (C417) #15802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10b34d6
[`flake8-comprehensions`] Do not emit diagnostic when lambda has diff…
InSyncWithFoo Jan 29, 2025
06bd19e
Per review
InSyncWithFoo Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C417.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))}"
Expand Down Expand Up @@ -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)]))
56 changes: 32 additions & 24 deletions crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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,
Expand Down
70 changes: 12 additions & 58 deletions ...prehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C417_C417.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
|
Expand All @@ -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

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

Expand All @@ -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
Expand All @@ -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))}"
Expand Down
Loading