Skip to content

Commit a1579d8

Browse files
authored
[pylint] Fix PLW0108 autofix introducing a syntax error when the lambda's body contains an assignment expression (#18678)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary This PR also supresses the fix if the assignment expression target shadows one of the lambda's parameters. Fixes #18675 <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan Add regression tests. <!-- How was it tested? -->
1 parent 32c5418 commit a1579d8

3 files changed

Lines changed: 64 additions & 12 deletions

File tree

crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_lambda.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,7 @@
5757
# lambda uses an additional keyword
5858
_ = lambda *args: f(*args, y=1)
5959
_ = lambda *args: f(*args, y=x)
60+
61+
# https://github.com/astral-sh/ruff/issues/18675
62+
_ = lambda x: (string := str)(x)
63+
_ = lambda x: ((x := 1) and str)(x)

crates/ruff_linter/src/rules/pylint/rules/unnecessary_lambda.rs

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ruff_python_ast::{self as ast, Expr, ExprLambda, Parameter, ParameterWithDef
44
use ruff_text_size::Ranged;
55

66
use crate::checkers::ast::Checker;
7-
use crate::{AlwaysFixableViolation, Applicability, Edit, Fix};
7+
use crate::{Edit, Fix, FixAvailability, Violation};
88

99
/// ## What it does
1010
/// Checks for `lambda` definitions that consist of a single function call
@@ -46,14 +46,16 @@ use crate::{AlwaysFixableViolation, Applicability, Edit, Fix};
4646
#[derive(ViolationMetadata)]
4747
pub(crate) struct UnnecessaryLambda;
4848

49-
impl AlwaysFixableViolation for UnnecessaryLambda {
49+
impl Violation for UnnecessaryLambda {
50+
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
51+
5052
#[derive_message_formats]
5153
fn message(&self) -> String {
5254
"Lambda may be unnecessary; consider inlining inner function".to_string()
5355
}
5456

55-
fn fix_title(&self) -> String {
56-
"Inline function call".to_string()
57+
fn fix_title(&self) -> Option<String> {
58+
Some("Inline function call".to_string())
5759
}
5860
}
5961

@@ -199,7 +201,7 @@ pub(crate) fn unnecessary_lambda(checker: &Checker, lambda: &ExprLambda) {
199201
finder.names
200202
};
201203

202-
for name in names {
204+
for name in &names {
203205
if let Some(binding_id) = checker.semantic().resolve_name(name) {
204206
let binding = checker.semantic().binding(binding_id);
205207
if checker.semantic().is_current_scope(binding.scope) {
@@ -209,13 +211,33 @@ pub(crate) fn unnecessary_lambda(checker: &Checker, lambda: &ExprLambda) {
209211
}
210212

211213
let mut diagnostic = checker.report_diagnostic(UnnecessaryLambda, lambda.range());
212-
diagnostic.set_fix(Fix::applicable_edit(
213-
Edit::range_replacement(
214-
checker.locator().slice(func.as_ref()).to_string(),
215-
lambda.range(),
216-
),
217-
Applicability::Unsafe,
218-
));
214+
// Suppress the fix if the assignment expression target shadows one of the lambda's parameters.
215+
// This is necessary to avoid introducing a change in the behavior of the program.
216+
for name in names {
217+
if let Some(binding_id) = checker.semantic().lookup_symbol(name.id()) {
218+
let binding = checker.semantic().binding(binding_id);
219+
if checker
220+
.semantic()
221+
.current_scope()
222+
.shadowed_binding(binding_id)
223+
.is_some()
224+
&& binding
225+
.expression(checker.semantic())
226+
.is_some_and(Expr::is_named_expr)
227+
{
228+
return;
229+
}
230+
}
231+
}
232+
233+
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
234+
if func.is_named_expr() {
235+
format!("({})", checker.locator().slice(func.as_ref()))
236+
} else {
237+
checker.locator().slice(func.as_ref()).to_string()
238+
},
239+
lambda.range(),
240+
)));
219241
}
220242

221243
/// Identify all `Expr::Name` nodes in an AST.

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0108_unnecessary_lambda.py.snap

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,29 @@ unnecessary_lambda.py:10:5: PLW0108 [*] Lambda may be unnecessary; consider inli
155155
11 11 |
156156
12 12 | # default value in lambda parameters
157157
13 13 | _ = lambda x=42: print(x)
158+
159+
unnecessary_lambda.py:62:5: PLW0108 [*] Lambda may be unnecessary; consider inlining inner function
160+
|
161+
61 | # https://github.com/astral-sh/ruff/issues/18675
162+
62 | _ = lambda x: (string := str)(x)
163+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0108
164+
63 | _ = lambda x: ((x := 1) and str)(x)
165+
|
166+
= help: Inline function call
167+
168+
ℹ Unsafe fix
169+
59 59 | _ = lambda *args: f(*args, y=x)
170+
60 60 |
171+
61 61 | # https://github.com/astral-sh/ruff/issues/18675
172+
62 |-_ = lambda x: (string := str)(x)
173+
62 |+_ = (string := str)
174+
63 63 | _ = lambda x: ((x := 1) and str)(x)
175+
176+
unnecessary_lambda.py:63:5: PLW0108 Lambda may be unnecessary; consider inlining inner function
177+
|
178+
61 | # https://github.com/astral-sh/ruff/issues/18675
179+
62 | _ = lambda x: (string := str)(x)
180+
63 | _ = lambda x: ((x := 1) and str)(x)
181+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0108
182+
|
183+
= help: Inline function call

0 commit comments

Comments
 (0)