Fix false-positive unused-variable on unpacked loop variables

[mikeleppane]

Fixes #3911

What

A loop variable first bound by tuple unpacking and then reassigned inside the loop was reported as unused, even though it is read on every iteration:

def foo() -> Generator[int]:
    li, ri = 0, 100
    while li <= ri:
        mid = (li + ri) // 2
        if mid % 2 == 0:
            li = mid + 1
        else:
            ri = mid - 1   # before: "Variable `ri` is unused"; after: no warning
        yield li

The reporter saw it only with a Generator return type, but that was a coincidence. It reproduces with any return type, and with no read after the reassignment both li and ri get flagged.

Why

unused-variable is a binding-phase check: each assignment registers the name in a per-scope usage map, each read marks it used, and names still unused at scope exit are reported. mark_variable_used is a no-op if the name is not already in the map, so a name has to be registered before its reads to be tracked.

Only single-name assignments (x = ...) registered. Tuple/list unpacking, multi-target (a = b = ...), for, and with targets all flow through bind_target_name, which never called register_variable. So li, ri = 0, 100 left both names out of the map. The loop-header reads (while li <= ri, mid = (li + ri)) found nothing to mark, and the later single-name write ri = mid - 1 registered ri fresh as unused. With no read after that point in source order, it was reported. li survived only because yield li reads it after its own reassignment.

How

• bind_target_name now registers its target, but as tracking-only (allow_unused = true). The name is tracked so its reads land and the read-before-reassignment carry works across loop iterations, but it is never itself reported. This keeps the existing policy that only single-name assignments are flagged.
• The tracking-only registration is insert-if-absent, so it cannot overwrite a stricter existing entry. A prior dead store like x = 1; x, y = f() still reports x. A later single-name assignment is not tracking-only and still overwrites, staying reportable.
• VariableUsage gains an allow_unused field, matching ParameterUsage.allow_unused and ImportUsage.skip_unused_check, which already had equivalent flags.
• No change to for, with, _, or comprehension reporting. Comprehension targets reach bind_target_name but are inert, because the tracker only covers Module, Function, and Method scopes.

Files: binding/scope.rs, binding/target.rs, binding/bindings.rs, plus tests.

Test plan

All tests use the get_unused_variable_diagnostics harness in test/lsp/diagnostic.rs.

Test	What it checks
test_unused_variable_unpacked_reassigned_in_loop	the issue repro (with Generator) reports nothing
test_unused_variable_unpacked_reassigned_in_loop_no_trailing_read	the both-flagged variant (li and ri) reports nothing
test_unused_variable_single_then_unpack_still_reported	x = 1; x, y = (2,3) still reports x (no-clobber guard)
test_unused_variable_unpacking_target_not_reported	a, b = 1, 2; print(a) stays unflagged
test_unused_variable_for_loop_target_not_reported	for i in range(3) stays unflagged
test_unused_variable_with_target_not_reported	with cm as x stays unflagged
test_unused_variable_walrus_target_not_reported	(x := 5) stays unflagged
test_unused_variable_multi_target_not_reported	a = b = 5 stays unflagged
test_unused_variable_comprehension_target_not_reported	[1 for x in items] stays unflagged
test_unused_variable_comprehension_walrus_not_reported	a walrus hoisted out of a comprehension stays unflagged

Full lib suite: 5869 passed, 0 failed. Format and lint clean. A wide mypy_primer run over 73 projects showed zero diff, which is expected: unused-variable is an IDE-only hint and is never emitted by pyrefly check, so it does not appear in primer output.

Future improvements

This PR fixes the false positive without widening what gets reported. The check still only flags plain single-name local assignments. The cases below are deliberately left uncovered and could be follow-up PRs. Most of them need a dummy-name (_) exemption first, since pyrefly has none today and reporting these without one would flag _ everywhere.

• Unused tuple/list-unpacking targets: a, b = f(); print(a) does not flag b.
• Unused for targets: for i in range(3): pass does not flag i.
• Unused with ... as targets: with cm as x: pass does not flag x.
• Unused walrus targets: (x := 5) does not flag x.
• Unused multi-target names: a = b = 5; print(a) does not flag b.
• Comprehension targets: [1 for x in items] does not flag x. These live in a separate scope the tracker does not cover, so adding them is a larger change than the others.
• Module-level unused variables are never reported, by design, because another module can import them.
• Dead stores: x = 5; print(x); x = 7 does not flag the final x = 7, whose value is never read. This is tracked by the existing test_reassignment_false_negative, which carries a TODO.

A reasonable sequencing for the follow-up work: add the _ / dummy-name exemption, then decide per construct whether to report it (assignment unpacking is the least noisy candidate, for indices the most noisy), and run mypy_primer for each since these would change reported diagnostics. The allow_unused flag added here is the seam where that policy would be flipped per call site.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅