Skip to content

[pyflakes] Skip undefined-name (F821) for conditionally deleted variables#24088

Merged
dylwil3 merged 6 commits intoastral-sh:mainfrom
dylwil3:try-else-undefined
Mar 21, 2026
Merged

[pyflakes] Skip undefined-name (F821) for conditionally deleted variables#24088
dylwil3 merged 6 commits intoastral-sh:mainfrom
dylwil3:try-else-undefined

Conversation

@dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Mar 20, 2026 •
edited
Loading

We already skip F821 for things like this:

x = 1
if cond:
    del x
print(x) # no diagnostic here

But when we checked for "conditional branches" we were only looking for match, if, and while statements:

ruff/crates/ruff_python_ast/src/helpers.rs

Lines 1110 to 1128 in f32b9bb

/// Check if a node is part of a conditional branch.
pub fn on_conditional_branch<'a>(parents: &mut impl Iterator<Item = &'a Stmt>) -> bool {
parents.any(|parent| {
if matches!(parent, Stmt::If(_) | Stmt::While(_) | Stmt::Match(_)) {
return true;
}
if let Stmt::Expr(ast::StmtExpr {
value,
range: _,
node_index: _,
}) = parent
{
if value.is_if_expr() {
return true;
}
}
false
})
}

Here we also check for except handlers and else clauses. We continue to regard try statements as "unconditionally executing" even though that's not quite true.

Closes #6242

As an aside: I originally wanted to use the BindingKind variant ConditionalDeletion introduced way back in #10415, but it turns out we never actually constructed an instance of it! We could, if we ever needed to track the "thing that was conditionally deleted". But since that doesn't seem to have ever arisen in practice, I just got rid of the variant and its references altogether.

@dylwil3 dylwil3 added bug Something isn't working rule Implementing or modifying a lint rule labels Mar 20, 2026
@dylwil3 dylwil3 marked this pull request as ready for review March 20, 2026 19:02
@astral-sh-bot astral-sh-bot bot requested a review from ntBre March 20, 2026 19:02
@astral-sh-bot
Copy link

astral-sh-bot bot commented Mar 20, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+4 -0 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)

prefecthq/prefect (+4 -0 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ src/prefect/_internal/concurrency/calls.py:432:45: RUF100 [*] Unused `noqa` directive (unused: `F821`)
+ src/prefect/_internal/concurrency/calls.py:433:53: RUF100 [*] Unused `noqa` directive (unused: `F821`)
+ src/prefect/_internal/concurrency/calls.py:467:45: RUF100 [*] Unused `noqa` directive (unused: `F821`)
+ src/prefect/_internal/concurrency/calls.py:468:59: RUF100 [*] Unused `noqa` directive (unused: `F821`)

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF100 4 4 0 0 0

@dylwil3 dylwil3 removed the request for review from ntBre March 20, 2026 22:41
@dylwil3 dylwil3 merged commit 1a2e402 into astral-sh:main Mar 21, 2026
42 checks passed
@dylwil3 dylwil3 deleted the try-else-undefined branch March 21, 2026 13:17
@dylwil3 dylwil3 mentioned this pull request Mar 24, 2026
Closed
@BrewTestBot BrewTestBot mentioned this pull request Mar 26, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

bug Something isn't working rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

F821 "Undefined name" when variable is deleted in another branch

3 participants

@dylwil3 @charliermarsh @ntBre