[flake8-return] Fix RET504 false positive when variable is read in except/finally#24484
[flake8-return] Fix RET504 false positive when variable is read in except/finally#24484anishgirianish wants to merge 3 commits intoastral-sh:mainfrom
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RUF100 | 1 | 1 | 0 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)
mlflow/mlflow (+1 -0 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview
+ mlflow/telemetry/track.py:31:32: RUF100 [*] Unused `noqa` directive (unused: `RET504`)
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RUF100 | 1 | 1 | 0 | 0 | 0 |
…lly-false-positive
|
Can you take another look at the ecosystem report. It's unclear to me why https://github.com/apache/airflow/blob/4a499ad4241876850e4ad93d1db42e3d7b71a634/airflow-core/src/airflow/api/common/mark_tasks.py#L108-L124 disappears (there's no try...finally involved) |
|
@MichaReiser, thanks for catching the airflow regression earlier, which pointed me in the right direction. Would like to request you for your re-review whenever you get a chance. Thank you |
| parents | ||
| .iter() | ||
| .filter_map(|stmt| stmt.as_try_stmt()) | ||
| .any(|stmt_try| any_over_body(&stmt_try.finalbody, &name_read)) |
There was a problem hiding this comment.
I think this is incorrect if the current branch is the finalbody:
def f():
try:
pass
finally:
x = foo()
return x
Summary
Fixes #17292
RET504 flagged assignments as unnecessary even when the variable was read in
finally/except, breaking runtime behavior on fix.Checks that the binding has only one reference (the return itself) before flagging
Test Plan