fix(semantic): typed-return concise arrow opens a scope and declares params (#60) - #63
Merged
Merged
Conversation
…params (#60) The `parseParenthesized` typed-return arrow branch (`(params): T => body`) built the arrow_fn node without ever calling emitScopeOpen/emitParamDeclaresFromRange, so for untyped params with an explicit return type the arrow opened no scope and declared no parameters — they were absent from the scope tree, breaking every scope-aware analysis (no-unused-vars, no-shadow, no-undef, redeclaration, …). The typed-param path (`(a: T): U =>`) and the empty-param path already worked. Open the arrow scope and declare the params before parsing the body, mirroring the other arrow paths, and reuse the function-level `param_ev_mark` to re-home parameter default references into the arrow scope (#56 parity). The body now parses inside the scope so its references resolve to the params. The branch has a speculative backtrack for the `cond ? (a): T => body : alt` conditional-consequent ambiguity. Scope events are emitted only when no such backtrack is possible (`!saved_cc`) — otherwise they would orphan on the backtrack — so that rare ambiguous case keeps its prior (no-scope) behavior; no events to undo on the backtrack path. Validated: full suite green; TS conformance identical to baseline (17910/17913, 1210/1223); semantic sweep over 19,233 files, 0 crashes, with the expected deltas — +45 arrow scopes, +47 parameter symbols, and -47 references (the spurious param-NAME reads from the cover parse are now correctly cancelled as declarations rather than counted as reads).
Add the highest-value guard the suite lacked: a typed-return arrow whose params are never referenced (`(a, b): number => 0`) still declares them in the arrow scope — proving declaration is independent of use. Add rest- and destructured- param shapes, a type-predicate return (`a is string`, no crash + declared), a ref pin to the js_ts test (body parses inside the new scope), and a regression pin that the conditional-consequent ambiguity (`c ? (a): T => a : alt`, the case the fix gates out via !saved_cc) still analyzes without diagnostics.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #60.
Problem
The
parseParenthesizedtyped-return arrow branch ((params): T => body) built thearrow_fnnode without callingemitScopeOpen/emitParamDeclaresFromRange. So for untyped params with an explicit return type, the arrow opened no scope and declared no parameters — they were absent from the scope tree entirely, breaking every scope-aware analysis (no-unused-vars,no-shadow,no-undef, redeclaration, …).The typed-param path (
(a: T): U =>, vialooksLikeTsArrowParams) and the empty-param path already worked.Fix
Open the arrow scope and declare the params before parsing the body, mirroring the other arrow paths, and reuse the function-level
param_ev_markto re-home parameter-default references into the arrow scope (#56 parity). The body then parses inside the scope so its references resolve to the params.This branch has a speculative backtrack for the
cond ? (a): T => body : altconditional-consequent ambiguity. Scope events are emitted only when no backtrack is possible (!saved_cc) — otherwise they would orphan on the backtrack — so that rare ambiguous case keeps its prior (no-scope) behavior, and there are no events to undo on the backtrack path.Validation
arrow_functionscope and the body resolves to them; default re-homes to the param (semantic: arrow-function default value does not resolve sibling/self parameters (deferred param-declare ordering) #56 parity); js_ts-mode parity.