Skip to content

Commit 76d9009

Browse files
authored
[pycodestyle] Fix E731 autofix creating a syntax error for expressions spanned across multiple lines (astral-sh#18479)
1 parent 0152229 commit 76d9009

File tree

3 files changed

+95
-8
lines changed

3 files changed

+95
-8
lines changed

crates/ruff_linter/resources/test/fixtures/pycodestyle/E731.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,17 @@ class FilterDataclass:
176176
x = lambda: (
177177
# comment
178178
y := 10
179-
)
179+
)
180+
181+
# https://github.com/astral-sh/ruff/issues/18475
182+
foo_tooltip = (
183+
lambda x, data: f"\nfoo: {data['foo'][int(x)]}"
184+
if data["foo"] is not None
185+
else ""
186+
)
187+
188+
foo_tooltip = (
189+
lambda x, data: f"\nfoo: {data['foo'][int(x)]}" +
190+
more
191+
192+
)

crates/ruff_linter/src/rules/pycodestyle/rules/lambda_assignment.rs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ pub(crate) fn lambda_assignment(
9292
let first_line = checker.locator().line_str(stmt.start());
9393
let indentation = leading_indentation(first_line);
9494
let mut indented = String::new();
95-
for (idx, line) in function(id, lambda, annotation, checker)
95+
for (idx, line) in function(id, lambda, annotation, stmt, checker)
9696
.universal_newlines()
9797
.enumerate()
9898
{
@@ -177,6 +177,7 @@ fn function(
177177
name: &str,
178178
lambda: &ExprLambda,
179179
annotation: Option<&Expr>,
180+
stmt: &Stmt,
180181
checker: &Checker,
181182
) -> String {
182183
// Use a dummy body. It gets replaced at the end with the actual body.
@@ -236,7 +237,7 @@ fn function(
236237
});
237238
let generated = checker.generator().stmt(&func);
238239

239-
return replace_trailing_ellipsis_with_original_expr(generated, lambda, checker);
240+
return replace_trailing_ellipsis_with_original_expr(generated, lambda, stmt, checker);
240241
}
241242
}
242243
let function = Stmt::FunctionDef(ast::StmtFunctionDef {
@@ -251,12 +252,13 @@ fn function(
251252
});
252253
let generated = checker.generator().stmt(&function);
253254

254-
replace_trailing_ellipsis_with_original_expr(generated, lambda, checker)
255+
replace_trailing_ellipsis_with_original_expr(generated, lambda, stmt, checker)
255256
}
256257

257258
fn replace_trailing_ellipsis_with_original_expr(
258259
mut generated: String,
259260
lambda: &ExprLambda,
261+
stmt: &Stmt,
260262
checker: &Checker,
261263
) -> String {
262264
let original_expr_range = parenthesized_range(
@@ -267,14 +269,28 @@ fn replace_trailing_ellipsis_with_original_expr(
267269
)
268270
.unwrap_or(lambda.body.range());
269271

270-
let original_expr_in_source = checker.locator().slice(original_expr_range);
272+
// This prevents the autofix of introducing a syntax error if the lambda's body is an
273+
// expression spanned across multiple lines. To avoid the syntax error we preserve
274+
// the parenthesis around the body.
275+
let original_expr_in_source = if parenthesized_range(
276+
lambda.into(),
277+
stmt.into(),
278+
checker.comment_ranges(),
279+
checker.source(),
280+
)
281+
.is_some()
282+
{
283+
format!("({})", checker.locator().slice(original_expr_range))
284+
} else {
285+
checker.locator().slice(original_expr_range).to_string()
286+
};
271287

272288
let placeholder_ellipsis_start = generated.rfind("...").unwrap();
273289
let placeholder_ellipsis_end = placeholder_ellipsis_start + "...".len();
274290

275291
generated.replace_range(
276292
placeholder_ellipsis_start..placeholder_ellipsis_end,
277-
original_expr_in_source,
293+
&original_expr_in_source,
278294
);
279295
generated
280296
}

crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__E731_E731.py.snap

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ E731.py:139:5: E731 Do not assign a `lambda` expression, use a `def`
318318
138 138 | class TemperatureScales(Enum):
319319
139 |- CELSIUS = (lambda deg_c: deg_c)
320320
139 |+ def CELSIUS(deg_c):
321-
140 |+ return deg_c
321+
140 |+ return (deg_c)
322322
140 141 | FAHRENHEIT = (lambda deg_c: deg_c * 9 / 5 + 32)
323323
141 142 |
324324
142 143 |
@@ -338,7 +338,7 @@ E731.py:140:5: E731 Do not assign a `lambda` expression, use a `def`
338338
139 139 | CELSIUS = (lambda deg_c: deg_c)
339339
140 |- FAHRENHEIT = (lambda deg_c: deg_c * 9 / 5 + 32)
340340
140 |+ def FAHRENHEIT(deg_c):
341-
141 |+ return deg_c * 9 / 5 + 32
341+
141 |+ return (deg_c * 9 / 5 + 32)
342342
141 142 |
343343
142 143 |
344344
143 144 | # Regression test for: https://github.com/astral-sh/ruff/issues/7141
@@ -449,6 +449,8 @@ E731.py:176:1: E731 [*] Do not assign a `lambda` expression, use a `def`
449449
178 | | y := 10
450450
179 | | )
451451
| |_^ E731
452+
180 |
453+
181 | # https://github.com/astral-sh/ruff/issues/18475
452454
|
453455
= help: Rewrite `x` as a `def`
454456

@@ -462,3 +464,59 @@ E731.py:176:1: E731 [*] Do not assign a `lambda` expression, use a `def`
462464
177 178 | # comment
463465
178 179 | y := 10
464466
179 180 | )
467+
468+
E731.py:182:1: E731 [*] Do not assign a `lambda` expression, use a `def`
469+
|
470+
181 | # https://github.com/astral-sh/ruff/issues/18475
471+
182 | / foo_tooltip = (
472+
183 | | lambda x, data: f"\nfoo: {data['foo'][int(x)]}"
473+
184 | | if data["foo"] is not None
474+
185 | | else ""
475+
186 | | )
476+
| |_^ E731
477+
187 |
478+
188 | foo_tooltip = (
479+
|
480+
= help: Rewrite `foo_tooltip` as a `def`
481+
482+
Unsafe fix
483+
179 179 | )
484+
180 180 |
485+
181 181 | # https://github.com/astral-sh/ruff/issues/18475
486+
182 |-foo_tooltip = (
487+
183 |- lambda x, data: f"\nfoo: {data['foo'][int(x)]}"
488+
182 |+def foo_tooltip(x, data):
489+
183 |+ return (f"\nfoo: {data['foo'][int(x)]}"
490+
184 184 | if data["foo"] is not None
491+
185 |- else ""
492+
186 |-)
493+
185 |+ else "")
494+
187 186 |
495+
188 187 | foo_tooltip = (
496+
189 188 | lambda x, data: f"\nfoo: {data['foo'][int(x)]}" +
497+
498+
E731.py:188:1: E731 [*] Do not assign a `lambda` expression, use a `def`
499+
|
500+
186 | )
501+
187 |
502+
188 | / foo_tooltip = (
503+
189 | | lambda x, data: f"\nfoo: {data['foo'][int(x)]}" +
504+
190 | | more
505+
191 | |
506+
192 | | )
507+
| |_^ E731
508+
|
509+
= help: Rewrite `foo_tooltip` as a `def`
510+
511+
Unsafe fix
512+
185 185 | else ""
513+
186 186 | )
514+
187 187 |
515+
188 |-foo_tooltip = (
516+
189 |- lambda x, data: f"\nfoo: {data['foo'][int(x)]}" +
517+
190 |- more
518+
191 |-
519+
192 |-)
520+
188 |+def foo_tooltip(x, data):
521+
189 |+ return (f"\nfoo: {data['foo'][int(x)]}" +
522+
190 |+ more)

0 commit comments

Comments
 (0)