From e690a51c206df6afdb4f15f5928f5ca1e981957c Mon Sep 17 00:00:00 2001 From: Eric San Date: Thu, 25 Jun 2026 13:50:02 +0800 Subject: [PATCH 1/2] fix(parser): parenthesized type as an arrow return type (#62) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `looksLikeFunctionTypeParams` ended with a "scan to the matching `)` then check for `=>`" heuristic that classified ANY `( … ) =>` as a function type, ignoring the content. So an arrow return type that is a parenthesized type — `const f = (): (void) => {}` — was read as the function type `(void) => {}`, swallowing the arrow's own `=>`; the whole arrow then became an ErrorNode. `(void)`, `(T | U)`, `(readonly T[])` etc. all hit this. Replace the scan-forward with TS's isUnambiguouslyStartOfFunctionType logic: skip an optional parameter-property modifier and exactly ONE parameter binding (identifier / `this` / destructuring pattern — NOT a reserved bare-type keyword like `void`/`null`/`true`/`false`/`this`, and NOT a leading `(`), then require a parameter separator (`:` `?` `=` `,`) or `)` immediately followed by `=>`. A type operator after the first identifier (`T | U`, `T[]`) therefore correctly falls through to a parenthesized type, leaving the `=>` for the enclosing arrow. Validated: full suite green; TS conformance back at baseline 17910/17913 · 1210/1223 (an initial over-strict version regressed 5 parameter-list/function- type cases — `(public B) =>` modifiers and `({}?: T) =>` optional patterns — now covered by the modifier-skip and the `?` separator); babel 1928/1928 · 1548/1548; test262 3966/3966 · 1389/1389; semantic sweep 0 crashes with only positive deltas (+8 scopes / +37 symbols / +28 refs — arrows recovered from ErrorNodes), diagnostics unchanged. --- src/typescript.zig | 75 ++++++++++++++++++++++++++++++++----------- tests/parser_test.zig | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 19 deletions(-) diff --git a/src/typescript.zig b/src/typescript.zig index 48d62d4..72a4ec0 100644 --- a/src/typescript.zig +++ b/src/typescript.zig @@ -848,30 +848,67 @@ fn looksLikeFunctionTypeParams(p: *Parser) bool { } } - // Scan forward to find closing `)` and check for `=>` + // No annotation/optional/comma matched above, so the only remaining function- + // type forms are a single bare parameter: `(id) =>`, `(id, …) =>`, `({pat}) =>`, + // `([pat]) =>`, or `(id = default) =>`. Recognise them by skipping exactly ONE + // parameter binding and requiring a parameter-separator next (`:` `,` `=`) or a + // `)` immediately followed by `=>`. + // + // Crucially, a reserved keyword that forms a bare type on its own + // (`void`/`null`/`true`/`false`/`this`), a leading `(`, or a type operator after + // the first identifier (`T | U`, `T & U`, `T[]`) means the parens are a + // parenthesized TYPE — so a trailing `=>` belongs to an *enclosing* arrow, not a + // function type here. (Matches TS's isUnambiguouslyStartOfFunctionType: the old + // "scan to `)` then check `=>`" heuristic mis-read `(void) =>` etc. as fn types, + // discarding the enclosing arrow — see #62.) p.restore(saved); _ = p.advance(); // skip `(` - var depth: u32 = 1; - var limit: u32 = 0; - while (depth > 0 and !p.isAtEnd() and limit < 200) : (limit += 1) { - switch (p.peek()) { - .l_paren => { - depth += 1; - _ = p.advance(); - }, - .r_paren => { - depth -= 1; - if (depth == 0) { - _ = p.advance(); // consume `)` - return p.peek() == .arrow; + // Skip a leading parameter-property modifier (public/private/protected/readonly) + // when another binding token follows — `(public B) =>` is a function type + // (mirrors parseFunctionTypeParam). A modifier not followed by a binding stays + // the parameter name (`(public) =>`). + if (p.peekAt(1) == .identifier or p.peekAt(1) == .kw_this or + p.peekAt(1) == .l_brace or p.peekAt(1) == .l_bracket) + { + const is_modifier = p.peek() == .kw_readonly or (p.peek() == .identifier and blk: { + const txt = p.tokenText(p.tokIdx()); + break :blk std.mem.eql(u8, txt, "public") or std.mem.eql(u8, txt, "private") or + std.mem.eql(u8, txt, "protected") or std.mem.eql(u8, txt, "readonly"); + }); + if (is_modifier) _ = p.advance(); + } + switch (p.peek()) { + // Destructuring-pattern parameter — skip the balanced `{…}`/`[…]`. + .l_brace, .l_bracket => { + _ = p.advance(); + var depth: u32 = 1; + var limit: u32 = 0; + while (depth > 0 and !p.isAtEnd() and limit < 200) : (limit += 1) { + switch (p.peek()) { + .l_brace, .l_bracket => depth += 1, + .r_brace, .r_bracket => depth -= 1, + else => {}, } _ = p.advance(); - }, - else => _ = p.advance(), - } + } + if (depth != 0) return false; + }, + .identifier => _ = p.advance(), + // A keyword may name a parameter, except the reserved keywords that are a + // complete bare type by themselves. + else => |first| { + if (!first.isKeyword() or first == .kw_void or first == .kw_null or + first == .kw_true or first == .kw_false or first == .kw_this) + return false; + _ = p.advance(); + }, } - - return false; + // After the first parameter binding: a separator (`:` `?` `=` `,`) or `) =>`. + return switch (p.peek()) { + .colon, .question, .comma, .equal => true, + .r_paren => p.peekAt(1) == .arrow, + else => false, + }; } /// Parse a simple parenthesized type: `(Type)`. diff --git a/tests/parser_test.zig b/tests/parser_test.zig index 949000b..1e3a15f 100644 --- a/tests/parser_test.zig +++ b/tests/parser_test.zig @@ -926,3 +926,71 @@ test "lang:js_ts angle-bracket type assertion parsed as TS (not JSX) (#32)" { defer tree.deinit(testing.allocator); try expectNoErrors(&tree); } + +// ── #62: parenthesized type as an arrow return type ─────────────────────── + +fn hasNodeTag(tree: *const ast.Ast, tag: Node.Tag) bool { + const tags = tree.nodes.items(.tag); + var i: u32 = 0; + while (i < tree.nodes.len) : (i += 1) { + if (tags[i] == tag) return true; + } + return false; +} + +test "arrow with a parenthesized return type parses, not an ErrorNode (#62)" { + // `(): (void) => {}` — the `: (` must read a parenthesized return TYPE, not a + // function type that swallows the arrow's own `=>`. Previously the whole arrow + // became an ErrorNode. + const cases = [_][]const u8{ + "const f = (): (void) => {};", + "const f = (): (T | U) => x;", + "const f = (): (readonly string[]) => x;", + "const f = (x): (void) => {};", + "const o = { key: (): (void) => { x(); } };", + "const f = (): (() => void) => g;", // parenthesized function-type return + }; + for (cases) |src| { + var tree = try parseTs(src); + defer tree.deinit(testing.allocator); + try expectNoErrors(&tree); + try testing.expect(hasNodeTag(&tree, .arrow_fn)); + try testing.expect(!hasNodeTag(&tree, .error_node)); + } + // js_ts mode parses it the same way. + var jt = try parseJsTs("const f = (): (void) => {};"); + defer jt.deinit(testing.allocator); + try expectNoErrors(&jt); + try testing.expect(hasNodeTag(&jt, .arrow_fn)); +} + +test "parenthesized return type is a TSParenthesizedType; function types still parse (#62)" { + { + var tree = try parseTs("const f = (): (void) => {};"); + defer tree.deinit(testing.allocator); + try testing.expect(hasNodeTag(&tree, .ts_parenthesized_type)); + } + // Genuine function types must still be recognized (the heuristic that decides + // `( … ) =>` is a function type vs a parenthesized type must not regress). + const fn_types = [_][]const u8{ + "type A = (a) => void;", + "type B = (a, b) => void;", + "type C = ({x}) => void;", + "type D = (...r: any[]) => void;", + "type E = (a?) => void;", + "type F = (a = 1) => void;", + "function A(): (public B) => C {}", // parameter-property modifier + "type G = ({}?: { x: string }) => void;", // optional destructuring param + }; + for (fn_types) |src| { + var tree = try parseTs(src); + defer tree.deinit(testing.allocator); + try expectNoErrors(&tree); + try testing.expect(hasNodeTag(&tree, .ts_function_type)); + } + // A bare parenthesized type stays a parenthesized type. + var pt = try parseTs("type T = (void);"); + defer pt.deinit(testing.allocator); + try expectNoErrors(&pt); + try testing.expect(hasNodeTag(&pt, .ts_parenthesized_type)); +} From 26789bdadd520eaafb6d63d767944c1f46cf0e40 Mon Sep 17 00:00:00 2001 From: Eric San Date: Thu, 25 Jun 2026 14:18:50 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(parser):=20address=20#62=20review=20?= =?UTF-8?q?=E2=80=94=20scope-event/diagnostic=20leaks,=20`(this)`,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings on the looksLikeFunctionTypeParams rewrite: 1. Scope-event leak (regression review). Classifying expression-shaped parens like `(b = 1)` as function-type params makes the conditional-consequent typed-arrow speculation in parseParenthesized run parseFunctionType, which emits scope_open/declare events. The backtrack restored tok/nodes/extra but NOT the event stream, leaking a phantom function scope + parameter (wrong reference data for `c ? x : (b = 1) && b`). Add Parser.resetEventsTo and truncate events on both backtrack paths. 2. Memory leak: that same speculation also allocPrints a diagnostic (no `=>` after the fake params); the backtrack's shrinkRetainingCapacity dropped it without freeing the message. Add Parser.truncateDiagnostics (frees discarded messages, matching the tree's deinit) and use it on the backtrack paths. 3. `(this) =>` fidelity (correctness review): a bare `this` parameter is a valid function type in TS, so accept `.kw_this` as a parameter binding instead of treating `(this)` as a parenthesized type. Tests: pin the arrow's actual ArrowData.return_type (not a loose whole-tree hasNodeTag scan); add union/intersection/conditional return-type cases; add a semantic regression test that the conditional-with-paren-alternate leaks no phantom scope or duplicate parameter. Validated: full suite green (no leaks under the testing allocator); TS conformance 17910/17913 · 1210/1223; babel 1928/1928 · 1548/1548; test262 3966/3966 · 1389/1389. --- src/expressions.zig | 12 +++++++++--- src/parser.zig | 29 ++++++++++++++++++++++++++++- src/typescript.zig | 7 ++++--- tests/parser_test.zig | 29 ++++++++++++++++++++++++++++- tests/semantic_test.zig | 21 +++++++++++++++++++++ 5 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/expressions.zig b/src/expressions.zig index 268d0c2..8ea7449 100644 --- a/src/expressions.zig +++ b/src/expressions.zig @@ -3539,6 +3539,10 @@ fn parseParenthesized(p: *Parser) Error!NodeIndex { const saved_diag_len = p.diagnostics.items.len; const saved_nodes_len = p.nodes.len; const saved_extra_len = p.extra_data.items.len; + // Speculatively parsing the return type can emit scope events (e.g. a type + // that looks like a function type) — discard them if we backtrack so they + // don't leak into the enclosing scope (#62). + const saved_ev = p.eventMark(); // Snapshot scratch contents so we can restore even after scratchPop. const params_count = p.scratch.items.len - scratch_top; var params_snapshot: [16]u32 = undefined; @@ -3612,16 +3616,17 @@ fn parseParenthesized(p: *Parser) Error!NodeIndex { // Backtrack to bare-paren and let the caller see `:` again. if (saved_cc and p.peek() != .colon and can_snapshot) { p.tok_i = saved_tok; - p.diagnostics.shrinkRetainingCapacity(saved_diag_len); + p.truncateDiagnostics(saved_diag_len); p.nodes.len = @intCast(saved_nodes_len); p.extra_data.shrinkRetainingCapacity(saved_extra_len); + p.resetEventsTo(saved_ev); // discard the speculative return-type's events // Restore scratch contents (scratchPop above truncated to scratch_top). p.scratch.shrinkRetainingCapacity(scratch_top); for (params_snapshot[0..params_count]) |raw| { try p.scratch.append(p.gpa, raw); } // Fall through to the bare-paren interpretation below. - // (emit_arrow_scope is false here, so no scope events were emitted.) + // (emit_arrow_scope is false here, so no arrow-scope events were emitted.) } else { if (emit_arrow_scope) try p.emitScopeClose(.none); const extra = try p.addExtra(ast.ArrowData, .{ @@ -3641,9 +3646,10 @@ fn parseParenthesized(p: *Parser) Error!NodeIndex { } else { // Type didn't parse or no `=>` — backtrack the type annotation only. p.tok_i = saved_tok; - p.diagnostics.shrinkRetainingCapacity(saved_diag_len); + p.truncateDiagnostics(saved_diag_len); p.nodes.len = @intCast(saved_nodes_len); p.extra_data.shrinkRetainingCapacity(saved_extra_len); + p.resetEventsTo(saved_ev); // discard the speculative return-type's events } } diff --git a/src/parser.zig b/src/parser.zig index 58479f8..2c7c2f5 100644 --- a/src/parser.zig +++ b/src/parser.zig @@ -1142,11 +1142,24 @@ pub const Parser = struct { } /// The current scope-event high-water mark. Paired with `rehomeParamRefs` - /// to bracket the events emitted while parsing an arrow's parameter list. + /// to bracket the events emitted while parsing an arrow's parameter list, + /// or with `resetEventsTo` to discard events emitted during a speculative + /// parse that is later backtracked. pub inline fn eventMark(self: *const Parser) u32 { return @intCast(self.ev_len); } + /// Discard every scope event emitted at or after `mark` — the event-stream + /// analogue of restoring `tok_i`/`nodes`/`extra_data` after a speculative + /// parse fails. Stale `ref_event_idx` entries left pointing past the new + /// length are harmless: their consumers re-check `ev_idx < ev_len` and the + /// event's node before acting, and `emitReference` overwrites the slot when + /// the node index is reused after the backtrack. + pub inline fn resetEventsTo(self: *Parser, mark: u32) void { + if (!self.emit_scope_events) return; + if (mark < self.ev_len) self.ev_len = mark; + } + /// Re-home parameter default-initializer (and computed-key) references from /// the enclosing scope into the just-opened arrow/parameter scope. /// @@ -8792,6 +8805,20 @@ pub const Parser = struct { self.extra_data.shrinkRetainingCapacity(s.extra_len); } + /// Truncate the diagnostics list to `len`, freeing the message strings of the + /// discarded entries. emitDiagnostic allocPrints messages with `gpa` and the + /// tree's deinit frees them — but only those still in the list, so a plain + /// `shrinkRetainingCapacity` after a speculative parse would leak the dropped + /// messages. Use this when backtracking a speculation that may have diagnosed. + pub fn truncateDiagnostics(self: *Parser, len: usize) void { + var i = self.diagnostics.items.len; + while (i > len) { + i -= 1; + self.gpa.free(self.diagnostics.items[i].message); + } + self.diagnostics.shrinkRetainingCapacity(len); + } + /// Restore parser position from a checkpoint. pub fn restore(self: *Parser, saved: u32) void { self.tok_i = saved; diff --git a/src/typescript.zig b/src/typescript.zig index 72a4ec0..af6cdde 100644 --- a/src/typescript.zig +++ b/src/typescript.zig @@ -893,12 +893,13 @@ fn looksLikeFunctionTypeParams(p: *Parser) bool { } if (depth != 0) return false; }, - .identifier => _ = p.advance(), + .identifier, .kw_this => _ = p.advance(), // A keyword may name a parameter, except the reserved keywords that are a - // complete bare type by themselves. + // complete bare type by themselves. (`this` is accepted above — `(this) =>` + // is a function type in TS.) else => |first| { if (!first.isKeyword() or first == .kw_void or first == .kw_null or - first == .kw_true or first == .kw_false or first == .kw_this) + first == .kw_true or first == .kw_false) return false; _ = p.advance(); }, diff --git a/tests/parser_test.zig b/tests/parser_test.zig index 1e3a15f..516128b 100644 --- a/tests/parser_test.zig +++ b/tests/parser_test.zig @@ -944,7 +944,9 @@ test "arrow with a parenthesized return type parses, not an ErrorNode (#62)" { // became an ErrorNode. const cases = [_][]const u8{ "const f = (): (void) => {};", - "const f = (): (T | U) => x;", + "const f = (): (T | U) => x;", // union + "const f = (): (T & U) => x;", // intersection + "const f = (): (A extends B ? C : D) => x;", // conditional type "const f = (): (readonly string[]) => x;", "const f = (x): (void) => {};", "const o = { key: (): (void) => { x(); } };", @@ -994,3 +996,28 @@ test "parenthesized return type is a TSParenthesizedType; function types still p try expectNoErrors(&pt); try testing.expect(hasNodeTag(&pt, .ts_parenthesized_type)); } + +fn firstNodeOfTag(tree: *const ast.Ast, tag: Node.Tag) ?NodeIndex { + const tags = tree.nodes.items(.tag); + var i: u32 = 0; + while (i < tree.nodes.len) : (i += 1) { + if (tags[i] == tag) return NodeIndex.fromInt(i); + } + return null; +} + +test "arrow's actual return_type node is the parenthesized type (#62)" { + // Pin the precise shape (not just `hasNodeTag` anywhere in the tree): the + // arrow's ArrowData.return_type is a TSTypeAnnotation wrapping the + // parenthesized type. + var tree = try parseTs("const f = (): (void) => {};"); + defer tree.deinit(testing.allocator); + try expectNoErrors(&tree); + const datas = tree.nodes.items(.data); + const arrow = firstNodeOfTag(&tree, .arrow_fn) orelse return error.NoArrow; + const ad = tree.extraData(ast.ArrowData, @intFromEnum(datas[arrow.toInt()].lhs)); + try testing.expect(ad.return_type != .none); + try expectNodeTag(&tree, ad.return_type, .ts_type_annotation); + // The annotation's inner node (data.lhs) is the parenthesized type. + try expectNodeTag(&tree, datas[ad.return_type.toInt()].lhs, .ts_parenthesized_type); +} diff --git a/tests/semantic_test.zig b/tests/semantic_test.zig index 02e2a4a..9690d52 100644 --- a/tests/semantic_test.zig +++ b/tests/semantic_test.zig @@ -1518,3 +1518,24 @@ test "overloaded interface method creates independent scopes per overload (#30)" } try testing.expectEqual(@as(u32, 2), count); } + +test "conditional with a paren-expression alternate leaks no phantom scope (#62)" { + // Speculatively parsing the consequent's typed-arrow return type tries the + // alternate `(b = 1)` as a function type, emitting scope/declare events; the + // backtrack must discard them so no phantom function scope or duplicate `b` + // parameter leaks into the graph. + var r = try analyzeTsSource("function f(b){ return c ? (b) : (b = 1) && b; }"); + defer r.deinit(testing.allocator); + // Exactly one function scope (f); the discarded speculation adds none. + try testing.expectEqual(@as(u32, 1), countScopesOfKind(&r, .function)); + try testing.expectEqual(@as(u32, 0), countScopesOfKind(&r, .arrow_function)); + // Only the real `b` parameter — no phantom duplicate. + var b_params: u32 = 0; + var i: u32 = 0; + while (i < r.symbols.count()) : (i += 1) { + const id = SymbolId.fromInt(i); + if (r.symbols.getBindingKind(id) == .parameter and + std.mem.eql(u8, r.symbols.getName(id), "b")) b_params += 1; + } + try testing.expectEqual(@as(u32, 1), b_params); +}