Skip to content

[flake8-comprehensions] Skip C417 for lambdas with positional-only parameters#25272

Merged
ntBre merged 4 commits into
astral-sh:mainfrom
Dev-X25874:fix/c417-posonly-lambda-arity
May 21, 2026
Merged

[flake8-comprehensions] Skip C417 for lambdas with positional-only parameters#25272
ntBre merged 4 commits into
astral-sh:mainfrom
Dev-X25874:fix/c417-posonly-lambda-arity

Conversation

@Dev-X25874

Copy link
Copy Markdown
Contributor

Summary

lambda_has_expected_arity in the C417 (UnnecessaryMap) rule checked
parameters.args for exactly one element to confirm the lambda takes a
single argument, but never checked parameters.posonlyargs. A lambda
like lambda x, /, y: x + y has posonlyargs = [x] and args = [y],
so the slice pattern matched y as the sole element and the function
incorrectly returned true, causing ruff to raise a false C417
diagnostic and offer a broken autofix for a two-parameter lambda.

The fix adds an early return when parameters.posonlyargs is non-empty,
ensuring lambdas that combine positional-only and regular parameters are
correctly rejected before the arity check passes.

Test Plan

Added a new # Ok case to C417.py:

# Ok: lambda has a positional-only parameter alongside a regular parameter;
# total arity is 2, so rewriting to a comprehension is incorrect.
map(lambda x, /, y: x + y, nums)

This line produces no diagnostic after the fix, confirming the false
positive is gone.

@astral-sh-bot astral-sh-bot Bot requested a review from ntBre May 20, 2026 12:03
// Reject lambdas that also declare positional-only parameters, e.g. `lambda x, /, y: ...`.
// Such a lambda requires two arguments; `map()` over a single iterable is already broken
// at runtime and must not be rewritten as a comprehension.
if !parameters.posonlyargs.is_empty() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could just check parameters.len() == 1 and also remove the check on line 226.

@astral-sh-bot

astral-sh-bot Bot commented May 20, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre added the rule Implementing or modifying a lint rule label May 20, 2026
@Dev-X25874

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion! You're right, checking parameters.len() == 1 is much cleaner and handles all multi-parameter cases uniformly without needing the separate posonlyargs guard. Updated accordingly.

let Some(parameters) = lambda.parameters.as_deref() else {
return false;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to remove these blank lines, and we should be able to remove the check on line 220, as I said before. It's redundant with the total parameters.len() check, unless I'm missing something.

@Dev-X25874

Copy link
Copy Markdown
Contributor Author

Thanks for catching that! Restored the blank lines and removed the vararg/kwarg check since parameters.len() == 1 already makes it redundant.You said: commit message

@ntBre ntBre changed the title flake8-comprehensions: skip C417 for lambdas with positional-only parameters [flake8-comprehensions] Skip C417 for lambdas with positional-only parameters May 21, 2026
@ntBre ntBre merged commit 8a8e35e into astral-sh:main May 21, 2026
45 checks passed
@Dev-X25874 Dev-X25874 deleted the fix/c417-posonly-lambda-arity branch May 21, 2026 13:39
@Dev-X25874

Copy link
Copy Markdown
Contributor Author

Thanks for the merge and for the helpful review, ntBre — I really appreciate the suggestion to simplify the arity check. It made the fix cleaner and more robust.

thejchap pushed a commit to thejchap/ruff that referenced this pull request May 23, 2026
…y parameters (astral-sh#25272)

## Summary

`lambda_has_expected_arity` in the C417 (`UnnecessaryMap`) rule checked
`parameters.args` for exactly one element to confirm the lambda takes a
single argument, but never checked `parameters.posonlyargs`. A lambda
like `lambda x, /, y: x + y` has `posonlyargs = [x]` and `args = [y]`,
so the slice pattern matched `y` as the sole element and the function
incorrectly returned `true`, causing ruff to raise a false C417
diagnostic and offer a broken autofix for a two-parameter lambda.

The fix adds an early return when `parameters.posonlyargs` is non-empty,
ensuring lambdas that combine positional-only and regular parameters are
correctly rejected before the arity check passes.

## Test Plan

Added a new `# Ok` case to `C417.py`:

```python
# Ok: lambda has a positional-only parameter alongside a regular parameter;
# total arity is 2, so rewriting to a comprehension is incorrect.
map(lambda x, /, y: x + y, nums)
```

This line produces no diagnostic after the fix, confirming the false
positive is gone.
anishgirianish pushed a commit to anishgirianish/ruff that referenced this pull request May 28, 2026
…y parameters (astral-sh#25272)

## Summary

`lambda_has_expected_arity` in the C417 (`UnnecessaryMap`) rule checked
`parameters.args` for exactly one element to confirm the lambda takes a
single argument, but never checked `parameters.posonlyargs`. A lambda
like `lambda x, /, y: x + y` has `posonlyargs = [x]` and `args = [y]`,
so the slice pattern matched `y` as the sole element and the function
incorrectly returned `true`, causing ruff to raise a false C417
diagnostic and offer a broken autofix for a two-parameter lambda.

The fix adds an early return when `parameters.posonlyargs` is non-empty,
ensuring lambdas that combine positional-only and regular parameters are
correctly rejected before the arity check passes.

## Test Plan

Added a new `# Ok` case to `C417.py`:

```python
# Ok: lambda has a positional-only parameter alongside a regular parameter;
# total arity is 2, so rewriting to a comprehension is incorrect.
map(lambda x, /, y: x + y, nums)
```

This line produces no diagnostic after the fix, confirming the false
positive is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants