Skip to content

test(cli): the restore guard resolves module aliases by binding and ends them at rebinding - #588

Closed
tps-anvil wants to merge 1 commit into
mainfrom
test/587-guard-binding
Closed

tps-anvil wants to merge 1 commit into
mainfrom
test/587-guard-binding

Conversation

@tps-anvil

Copy link
Copy Markdown
Collaborator

Closes #587

The mock-restore scan tracked a module object by identifier name, so a parameter or local that shadowed an imported namespace was reported, and a guarded-global or module-object alias stayed an alias after its binding was reassigned to a local value.

This change resolves the module object through the binding's scope — the same scope walk the guarded-global analysis already uses — and records alias events by position, so a rebinding ends the alias for the reads after it while a member assignment before the rebinding is still reported.

The existing red fixtures stay red; three fixtures are added: a green for the shadowed parameter, a green for the alias after rebinding, and a red for the member assignment before the rebinding.

Evidence (measured on 45e7d03, main 63f581e):

  • mock restore guard tests: 71 pass / 0 fail (67 at main, +4 added), including the cli test-tree scan.
  • full suite via bun run test: agent 194 pass / 0 fail; cli 3028 pass / 0 fail (196 files); root-test 243 pass / 2 todo / 0 fail.
  • bun run lint:ci: exit 0.
  • node scripts/changelog-fragments.mjs check: exit 0.

Mutations (fix reverted, test restored green):

  • reverting the binding resolution reports the shadowed-parameter green fixture (mod.method);
  • reverting the alias termination reports the after-rebinding green fixture (g.fetch).

… at a rebinding

The mock-restore scan tracked a module object by identifier name, so a parameter
or local that shadowed an imported namespace was reported, and a guarded-global
or module-object alias stayed an alias after its binding was reassigned to a
local value. Resolve the module object through the binding's scope (the scope
walk the guarded analysis already uses) and record alias events by position, so
a rebinding ends the alias for the reads after it while a member assignment
before the rebinding is still reported.
@tps-anvil
tps-anvil requested a review from a team as a code owner October 10, 2026 07:44
@coderabbitai

coderabbitai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1b00f12d-4b6b-41c4-805e-14f92eda1ae2

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@tps-anvil

Copy link
Copy Markdown
Collaborator Author

Sweep — cli#587 (PR #588)

Scope: every sentence this PR adds or changes, in git diff origin/main...HEAD
(prose, changelog, comments, test names, commit message, PR body), checked
against the code. Guarantee-word grep (md/code comments/test titles) hits are
listed with a call; the rest are plain descriptions.

# Sentence (added/changed) Call
1 changelog lede: "The cli test guard resolves a module-object alias by its binding." true — moduleObjectAliases keys the module object by the binding's scope.
2 changelog: "A parameter or local that shadows an imported namespace is no longer reported" true — relative to the last release (origin/main reports it); shadow-parameter-cleared.fixture.ts is green here.
3 changelog: "a guarded-global or module-object alias ends at the assignment that rebinds it to a local value" true — both guardedAliasNames and moduleObjectAliases record a terminating event via recordAlias.
4 helper: "One binding's alias history in source order; a root of undefined ends the alias." true — AliasEvents is appended in source order; root: undefined is the terminating event.
5 helper: "Alias events keyed by the scope that declares the binding and the binding's name. A read takes the last event at or before its position, so an assignment that rebinds the name ends the alias for the reads after it." true — aliasRootAt scans for the last event.at <= id.getStart().
6 helper: "The alias root a binding names at this identifier's position, or undefined." true — aliasRootAt returns the last qualifying event's root.
7 helper: "Establishes the alias when root is set; ends it otherwise, or is a no-op when it never began." true — recordAlias returns when root === undefined and there are no prior events.
8 helper: "Every identifier a binding pattern introduces, nested patterns included." (word: every) true — bindingIdentifiers pushes each identifier and recurses into nested patterns. Kept; the word is exact.
9 helper: "The guarded global root an identifier names, directly or through an alias of one." true — alias lookup first, then the guarded-root name when the identifier is not locally declared.
10 helper: "The bindings that name a guarded global root, directly or through another such binding." true — one pass in source order over declarations and = assignments.
11 helper (module-object doc, changed "identifier"→"binding"): "Every binding that names an imported module object or derives from one, by plain alias, destructuring or a property read. A member assignment on such a binding is reported as a member assignment on the module object, so an alias no longer hides it, and a rebinding to a local value ends the alias." (word: every) true — imports, aliases, destructuring and property reads all recordAlias; a rebinding records a terminating event. Kept; the scope is the function's whole input.
12 fixture comment: "g names the globalThis binding only until the assignment rebinds it to a local value." (word: only) true — the green fixture has no member assignment before the rebinding.
13 fixture comments for shadow-parameter-cleared and guarded-alias-rebound-before-leaked true — the shadowed-parameter fixture is green; the before/after fixture reports only the before assignment (asserted by exact count).
14 test title: "reports a guarded member assignment only before the alias is rebound" (word: only) true — the test asserts exactly one finding, on g.fetch, for the before assignment.
15 generated test titles for the three new fixtures descriptive only; each matches its assertion.
16 PR body sentences match the code and the evidence table above; no unmeasured counts.

Claim types checked

Fixed

Nothing — no sentence failed its check, so none was changed after the commit.

@tps-flint

Copy link
Copy Markdown
Contributor

Closing without merging. Adjudication found that ending an alias at a rebinding, and resolving a module alias by binding, each open a way for a real patch to go unreported: a rebinding inside a branch, a function that never runs, a hoisted function called before the rebinding, a ??= that does nothing at runtime, and a parameter that shadows an imported namespace but is passed the real module. Closing those needs control-flow and call analysis the guard does not have, and each approximation trades a fail-closed over-report for a fail-open miss.

The over-reports #587 targets fail closed (they ask for patchShared where a plain assignment would do), and none of them occur in the current test tree. That is a better trade for a control than precision with holes. #587 is closed as not planned with the same reasoning; it can be reopened if a real test trips one of these over-reports. Thanks for the clean mutations and fixtures, which made the tradeoff easy to see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(cli): the restore guard resolves module aliases by binding and ends an alias at its reassignment

2 participants