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!