v0.2.1: The Scope-Blind Bug That Quietly Killed var_to_const_let on Real Codebases

Three days after v0.2.0 shipped, the first bug report came in. The var_to_const_let transform — supposedly one of the safest, most mechanical TypeScript transforms in the catalog — was dropping entire files from the diff on real codebases. The transform looked at the file, decided every var was reassigned, and refused itself for every binding. Net effect: you ran the transform, you got no changes, no error, no clue why.

A bug that produces no error is the worst kind of bug. There is nothing to grep the logs for. The user runs the command, the command exits zero, the diff is empty, and the only signal that something went wrong is that the work you expected to happen did not happen. We want to be honest about that. v0.2.0 shipped with a transform that silently no-opped on the exact codebases it was supposed to help.

The bug

The transform needed to answer one question per var binding: is this binding ever reassigned? If yes, rewrite to let. If no, rewrite to const. Simple decision, two outcomes, one input.

Our v0.2.0 implementation answered that question with a textual scan of the file. Find every assignment expression in the source. Check whether the left-hand side identifier text matches the var name we are currently considering. If any match exists anywhere in the file, mark the binding as reassigned.

This works perfectly on fixtures where every variable has a unique name. It falls over the instant a real codebase reuses a common identifier in two unrelated scopes.

function a() {
  var i = 0;     // never reassigned; should become const
}

function b() {
  var i = 0;
  i++;           // reassigned; should become let
}

The old check looked at the whole file, saw the i++ in function b, and concluded that every var named i in the file was reassigned. Both functions came back unchanged. The transform reported success. The diff was empty.

Why no test caught it

Every unit-test fixture for var_to_const_let used unique identifiers per scope. The fixtures were named things like x, y, result, temp, alpha, beta. Never two scopes with the same identifier. The bug was structurally invisible to the test suite because the test suite did not contain a single name collision.

Real TypeScript codebases reuse i, len, result, data, key, value constantly. Every for-loop counter is i. Every map callback parameter is value. Every reduce accumulator is result. The first time the transform ran on anything that was not a fixture, it hit a name collision in the first file it touched and refused itself out of the diff.

The pattern is worth naming: a test suite that uses unique identifiers per fixture cannot catch scope-blind logic. Add at least one fixture that reuses the same identifier in two unrelated scopes and the bug surfaces immediately. We did not have that fixture. We do now.

The fix

Scope-correct reference resolution via ts-morph's findReferencesAsNodes. For each var binding, ask the TypeScript compiler for the references to that binding — the ones the compiler resolves to the same symbol. Then check whether any of those references is on the LHS of an assignment. Two unrelated i bindings in two functions have two unrelated symbols. The references for one binding do not include the other.

This is the entire fix. It is fewer lines than the textual scan it replaced. It is also slower per binding because it calls into the type checker, but the slowdown is invisible at the file level because the transform is gated by a cheap precondition before any reference resolution runs.

While we were rewriting the analysis, we found a separate latent bug. The original implementation skipped over var i initializers inside for (var i = 0; ...) headers — the visitor was descending into the for-statement body but not its initializer clause. So even on fixtures where the textual scan would have produced the right answer, the for-loop counters were never considered. Fixed in the same patch.

While we were in there

Two unrelated bugs landed in the same patch release because they were small and the release was already going out.

• The 32 KB tree-sitter crash. The analyze command crashed on files larger than roughly 32 KB because tree-sitter's native binding rejects oversized string input by default. Parsing now uses the streaming callback-input form, and an unparseable file is skipped with a logged warning rather than aborting the whole run. A large generated file in someone's repo should not take down the entire analysis.
• format_to_fstring grew up. v0.2.0 handled %s and %d. v0.2.1 adds %.2f, %x, %o, %e, %g, full width and precision specifiers, and the literal %% escape. Mapping %(name)s-style named conversions, non-literal format targets, and dynamic asterisk widths is still conservatively skipped — the transform refuses rather than guesses. Refusing is always a valid outcome for a Refactron transform. Guessing is not.

The lesson

Text matching is not scope analysis. If you write an AST transform without using your AST library's reference resolver, you have written your own reference resolver — and yours is going to be buggy because the compiler authors have spent years on theirs. The fix is not careful regex. The fix is using the symbol table the compiler has already built and is offering you for free.

We added a regression test that reuses identifiers across scopes and would catch any future scope-blind refactor. Every new transform we ship now has at least one fixture with deliberate name collisions in unrelated scopes. The cost of writing that fixture is roughly five minutes. The cost of not writing it was a patch release.