fix(parser,types): clear the ParseError long tail + Cluster A logical-assignment narrowing#1260
Merged
Conversation
Seven unrelated, independently-scoped parser gaps found while triaging the last of the #99 Track-1 ParseError bucket: - Object-type/interface members separated only by a newline (no ';'/',') now use the same ASI fallback as top-level interface members (`bar(): { baz: T }` followed by `baz: T` on the next line). - A parenthesized member/index expression is a valid assignment target for '=', compound, and logical-assignment operators alike (`(a.b) &&= v`) — parens don't change whether an expression is a reference; unwrap them (recursively) before dispatch. - `async` is a contextual keyword, not exclusively a function/arrow marker: only commit to parsing an async function/arrow when it's immediately followed by `function`/`<`/`(`; otherwise treat it as a plain identifier reference (`if (async)`, a destructured `async` binding). - `import.meta` used as a statement (`import.meta.foo();`) was routed to the *static* import-declaration parser because the statement dispatcher only excluded dynamic-import's `import(`, not `import.` — both are expressions, not declarations. - `export { x as y }` (a bare specifier list, no declaration) inside `declare global`/`declare module` bodies was unhandled — only `export <declaration>` forms were. - `export * as ns from './mod'` (ES2020 namespace re-export) wasn't recognized; only bare `export * from` was. Threads a new Stmt.Export.NamespaceExportName through parsing (the re-export binding step is a no-op for single-file checking either way). - An untagged template with an invalid escape sequence (`` `\xtraordinary` ``) is a real syntax error (TS1125) but a recoverable one — tsc keeps checking the rest of the file. The parser now substitutes an empty string for that part and records the offending line(s) on the AST node instead of aborting the parse; the checker reports TS1125 there. Tagged templates are unaffected (their raw form already tolerated this). Baseline: 7 ParseError -> 0 (296-test subset fully parses now); 3 flip straight to Pass, the rest land in Fail (real, now-measurable type-checking gaps instead of a fatal abort). 0 regressions.
203 -> 207 Pass, 7 -> 0 ParseError. 0 regressions.
…if-guards Three gaps in CheckLogicalAssign / the if-condition type-guard analyzer, all needed together to clear logicalAssignment4's remaining diagnostics: - Post-assignment narrowing: `results ||= []; results.push(100)` (the statement form, not chained directly onto the assignment) wasn't narrowed for the second statement at all. CheckLogicalAssign now installs the assignment's result type into the environment the same way CheckAssign already does for a plain `x = v`. - `&&=` only evaluates/assigns its RHS when the LHS is truthy — narrow the LHS to its truthy constituents while checking the RHS (mirrors plain `&&`'s CheckLogical). Without this, `thing &&= thing.original` spuriously flagged `thing` as possibly undefined on a read `&&=`'s own short-circuit already guarantees is safe. - `if (x OP= y)` is now recognized as a type-guard condition (new AnalyzeLogicalAssignGuard): the LHS narrows the same way a bare `if (x)` would, using its post-assignment type. For `&&=` specifically, a truthy result additionally proves the RHS ran and was itself truthy — if the RHS is a bare identifier, narrow that too (`||=`/`??=` don't get this: their truthy branch is also reachable via an already-truthy/non-nullish LHS that never touched the RHS). Also fixes a real pre-existing bug surfaced while wiring the above: the "pick the wider of two candidate types" pattern in CheckLogicalAssign's resultType computation had its two IsCompatible branches' return values swapped, collapsing a union to its narrower constituent instead of the wider one. Scoped to CheckLogicalAssign only — CheckLogical (plain &&/||) has the identical pattern and is unaudited. Flips logicalAssignment4 and (unplanned bonus) logicalAssignment11 from Fail to Pass, 0 regressions. logicalAssignment5 needs a distinct new checker feature entirely (TS2722 "cannot invoke possibly undefined" does not exist anywhere in the codebase); logicalAssignment6/7 need argument compatibility checking to continue after a nullable-receiver diagnostic fires, instead of stopping there — both left as separate follow-ups.
209 -> 211 Pass. 0 regressions.
3 tasks
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.
Summary
Follow-up to #1259 (merged) — continuation of epic #99 / #80 Track-1 (TS conformance). #1259 was merged before this branch's later commits were pushed, so this PR carries the remainder, across two pieces of work:
1. Clearing the ParseError long tail (7 tests)
Seven unrelated, independently-scoped parser gaps found while clearing what was left of the ParseError bucket:
;/,) now use the same ASI fallback as top-level interface members (bar(): { baz: T }followed bybaz: Ton the next line).=, compound, and logical-assignment operators alike ((a.b) &&= v) — parens don't change whether an expression is a reference; unwrap them (recursively) before dispatch.asyncis a contextual keyword, not exclusively a function/arrow marker: only commit to parsing an async function/arrow when it's immediately followed byfunction/</(; otherwise treat it as a plain identifier reference (if (async), a destructuredasyncbinding).import.metaused as a statement (import.meta.foo();) was routed to the static import-declaration parser because the statement dispatcher only excluded dynamic-import'simport(, notimport.— both are expressions, not declarations.export { x as y }(a bare specifier list, no declaration) insidedeclare global/declare modulebodies was unhandled — onlyexport <declaration>forms were.export * as ns from './mod'(ES2020 namespace re-export) wasn't recognized; only bareexport * fromwas.`\xtraordinary`) is a real syntax error (TS1125) but a recoverable one — tsc keeps checking the rest of the file. The parser now substitutes an empty string for that part and records the offending line(s) instead of aborting the parse; the checker reports TS1125 there. Tagged templates are unaffected.2. Cluster A: compound logical-assignment narrowing (logicalAssignment4/11)
Three gaps in
CheckLogicalAssign/ the if-condition type-guard analyzer, all needed together:results ||= []; results.push(100)(statement form) wasn't narrowed for the second statement —CheckLogicalAssignnow installs the assignment's result type into the environment the same wayCheckAssignalready does for plainx = v.&&=short-circuit narrowing within its own RHS:thing &&= thing.originalspuriously flaggedthingas possibly undefined on a read&&='s own short-circuit already guarantees is safe — narrow the LHS to truthy while checking the RHS, mirroring plain&&.if (x OP= y)as a type-guard condition (newAnalyzeLogicalAssignGuard): the LHS narrows like a bareif (x); for&&=specifically, a truthy result additionally proves the RHS ran and was itself truthy, so a bare-identifier RHS narrows too (verified against tsc's own baseline —||=/??=do NOT get this).CheckLogicalAssign's result-type computation had its twoIsCompatiblebranches' return values swapped, collapsing a union to its narrower constituent instead of the wider one.Not fixed, left as separate follow-ups (distinct root causes from narrowing):
logicalAssignment5needs a new checker feature — TS2722 "cannot invoke an object which is possibly undefined" doesn't exist anywhere in the codebase.logicalAssignment6/7need argument-compatibility checking to continue after a nullable-receiver diagnostic fires, instead of stopping there.Result: TS conformance subset baseline 203 → 211 Pass, 7 → 0 ParseError, 0 regressions across both pieces of work.
Test plan
dotnet test(SharpTS.Tests) — 15452/15452 passingSharpTS.Test262interpreted + compiled baselines — 0 failuresSharpTS.TypeScriptConformancebaseline — 211 Pass / 69 Fail / 0 ParseError / 17 Skipped / 1 TypeCheckError, 0 regressions vs. committed baseline