B023: don't flag a function that is only called inside the loop

[Eljees]

Fixes #468. Fixes #380.

check_for_b023 warns whenever a function defined in a loop closes over the loop variable. But a function whose every reference is a direct call in the loop body cannot outlive the iteration it was defined in, so the value it closes over is the one the author meant:

def f():
    results = []
    for x in range(3):
        def g():
            return x
        results.append(g())      # called here, never stored
    return results

The existing safe_functions notion covered only a fixed set of shapes — filter/map/reduce, a key= argument, and return lambda: x. This replaces it with the rule you both described on the issues: warn when the name escapes the loop or is referenced as anything other than a direct call, and stay silent otherwise. @jakkdl spelled it out on #380 and @cooperlees agreed with it on #468; I've implemented both halves plus one guard of my own — a decorated definition keeps warning, because a decorator can store the function.

Measured

python -m pytest: 79 passed, 2 skipped — before and after, on 2155484.

Still flagged (the name escapes):

shape
[lambda: i for i in range(3)]	flagged
out.append(lambda: i)	flagged
return g from inside the loop	flagged
d[i] = g	flagged
@staticmethod above the definition	flagged

No longer flagged (every reference is a direct call):

shape
results.append(g())	silent
print(g())	silent

tests/eval_files/b023.py gains the new cases in the project's declarative format; the existing expectations are unchanged — the one line that moves in the diff is the same line at a new offset, not a changed expectation.

black and isort are clean. rstcheck reports two unreferenced hyperlink targets in README.rst at lines 456 and 463 — that is identical on a clean checkout and unrelated to the change here.

AI-assisted (LLM used for drafting); the runs above are mine.

[Copilot]

Pull request overview

Updates B023 to exempt loop-defined functions referenced only through direct calls within the loop.

Changes:

• Adds direct-call reference analysis for loop-defined functions.
• Adds regression and escape-case coverage.
• Documents the behavior change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
bugbear.py	Implements immediate-call detection.
tests/eval_files/b023.py	Adds B023 evaluation cases.
README.rst	Updates the changelog.

Suppressed comments (2)

bugbear.py:1160

• These checks still accept direct calls from nested functions. If f() is called once at loop level and also from a stored wrapper, both Name nodes are call targets inside ast.walk(loop_node), so f is exempted even though wrapper() can invoke it after the loop. Track each reference's lexical owner and reject calls reached through another nested function (while handling self-recursion separately if desired).

            if (
                not isinstance(node.ctx, ast.Load)
                or id(node) not in call_targets
                or id(node) not in in_loop
            ):

bugbear.py:1150

• ast.walk(loop_node) is not sufficient proof that a call occurs in the defining iteration: it includes the loop's else suite, and it also accepts a call textually before the def (which invokes the previous iteration's binding). Both cases let the function outlive its defining iteration but are marked safe. Exclude orelse and require each accepted call to be reached after the matching definition on every relevant control-flow path.

        root = self.node_stack[0] if self.node_stack else loop_node
        in_loop = {id(node) for node in ast.walk(loop_node)}
        call_targets = {
            id(node.func)
            for node in ast.walk(root)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cooperlees]

LGTM - Thanks for this.

I think copilot has found some nice things to polish up - Feel free to state why it's wrong tho if it is.

[Eljees]

Thanks — the Copilot points hold up, and four of them are the same root cause: I proved "called in this iteration" with ast.walk, which is neither scope-aware nor order-aware.

Concretely:

• Deferred bodies (1) is a real hole. async def and generator functions return an object instead of running, so the late-binding read survives the call. I will restrict the exemption to synchronous, non-generator definitions.
• Identifier-only keying (2) and calls reached through a nested function (5) are the same defect seen from two sides: a reference has to be tied to the definition that binds it, and matching identifier text alone is the wrong key. I will resolve references against the defining scope.
• ast.walk(loop_node) covering the else suite and accepting a call that precedes the def (4) is correct as well; the search has to be restricted to the statements that can run in the same iteration after the definition.
• The quadratic walk (3) goes away with the same rewrite: one reference/call index built per scope instead of two walks per loop.

I will push a revision with regression cases for each of the five and report back. If you would rather see this as a narrower rule — for example exempting only a direct call in the loop body at statement level — say so and I will cut it to that instead.

[cooperlees]

I'm happy to start simpler, but if you're happy to do all five go for it. Both ways are an imrpovement so will take what ever you have time for :) ... thanks!

[Eljees]

Went with all five. Also rebased onto main, so the conflict is gone.

The rewrite ties the exemption to the definition rather than to the identifier. A name is exempted only when it has at least one reference and every reference in the scope holding the loop is a call that runs in the same iteration as the def.

review point	what changed
deferred bodies	only a plain def qualifies now — an async def builds a coroutine and a generator function builds a generator, so the read of the loop variable is deferred
identifier-only keying	a name bound anywhere else in the scope is skipped entirely, so an unrelated binding can no longer decide the outcome
ast.walk(loop_node) too wide	the search covers the loop body only: the else suite runs after the loop, and a call above the def invokes the previous iteration's binding
calls through a nested function	a reference reached through a nested function, lambda or generator expression disqualifies
two walks per loop	the parent links, name uses and call targets of a scope are built once and cached

One case the rewrite surfaced that none of us had listed: a definition nothing refers to was being exempted, because "every reference is a call" is vacuously true on an empty set. f_bad_1 in the existing fixture is exactly that, and it is right to keep reporting it — the name stays bound after the loop, so it can be called later with the last iteration's values. Fixed, and the existing expectation is what caught it.

Measured

tests/eval_files/b023.py now holds 31 expectations, all produced exactly — nothing missing, nothing extra. Four are new, one per unsound case:

• awaited_later — an async def that is called
• iterated_later — a generator function that is called
• called_above_the_def — the call sits above the def
• called_in_the_else_suite — the call sits in the else suite

The nested-function case (wrapper calling target) was already in the fixture from the first round and still reports.

python -m pytest: 79 passed, 2 skipped — identical to the baseline on 838c365. black and isort clean on both files. rstcheck reports one unreferenced hyperlink target in README.rst, and does so on an untouched checkout as well.

If any of the five reads as over-strict to you, say which and I will loosen that one rather than the set.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

bugbear.py:1164

• This only considers def statements that are direct children of the loop. A function defined and called within an if, try, or with in the loop still cannot outlive that iteration, but it remains flagged—for example, for i in xs: if cond: def f(): return i; f(). Please collect candidates through immediate control-flow suites while still stopping at nested lexical/deferred scopes.

        for statement in body:
            if (
                isinstance(statement, ast.FunctionDef)
                and not statement.decorator_list
                and not _defines_a_generator(statement)
            ):

bugbear.py:1183

• global and nonlocal declarations are not represented as ast.Name stores, so _is_rebound misses them. Inside a function, global f; for i in xs: def f(): return i; f() is therefore exempted even though each definition is stored globally and can be called after the loop with the final i. Please exclude candidates whose binding is declared global or nonlocal in the enclosing scope.

            # matching a reference by its identifier alone is only sound while
            # the name has exactly one binding in this scope
            if len(definitions) != 1 or _is_rebound(name, names_used, parents):
                continue