From aced2bf24fd07152bceafbef1f1adf24a4f347a4 Mon Sep 17 00:00:00 2001 From: Elliott Johnson Date: Fri, 14 Aug 2026 15:37:30 -0600 Subject: [PATCH 1/3] Revert "fix: revert jsdoc type cast support (for now) (#165)" This reverts commit e51f38fb99dc4c4f6257a2d88e407390be18cfc4. --- src/languages/ts/index.js | 36 +++++++++++++++++-- .../comment-jsdoc-type-cast/expected.js | 2 ++ .../comment-jsdoc-type-cast/expected.js.map | 11 ++++++ test/samples/comment-jsdoc-type-cast/input.js | 3 ++ 4 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 test/samples/comment-jsdoc-type-cast/expected.js create mode 100644 test/samples/comment-jsdoc-type-cast/expected.js.map create mode 100644 test/samples/comment-jsdoc-type-cast/input.js diff --git a/src/languages/ts/index.js b/src/languages/ts/index.js index f3b4cdd..0f9145b 100644 --- a/src/languages/ts/index.js +++ b/src/languages/ts/index.js @@ -315,9 +315,11 @@ export default (options = {}) => { * @param {{ line: number, column: number } | null} from * @param {{ line: number, column: number }} to * @param {boolean} pad + * @param {boolean} [is_next_to_expression] */ - function flush_comments_until(context, from, to, pad) { + function flush_comments_until(context, from, to, pad, is_next_to_expression = false) { let first = true; + let jsdoc_type_casts = 0; while (comment_index < comments.length) { const comment = comments[comment_index]; @@ -331,10 +333,24 @@ export default (options = {}) => { first = false; write_comment(comment, context); + // Acorn removes the parentheses that give a JSDoc `@type` comment cast semantics. + // We have to do a best guess (because we don't have access to the original source) + // to detect it based on the comment starting with `* @type {`, and only when + // it's an expression (e.g. `const foo = /** @type {number} */ (1);`), not something + // else like a statement (e.g. `/** @type {number} */ let foo;`). + const is_jsdoc_type_cast = + is_next_to_expression && + comment.type === 'Block' && + /(?:^|\n)\s*\*\s*@type\s*{/.test(comment.value); + + if (is_jsdoc_type_cast) { + context.write(' ('); + jsdoc_type_casts += 1; + } if (comment.loc.end.line < to.line) { context.newline(); - } else if (pad) { + } else if (pad && !is_jsdoc_type_cast) { context.write(' '); } @@ -343,6 +359,8 @@ export default (options = {}) => { break; } } + + return jsdoc_type_casts; } /** @@ -1040,12 +1058,24 @@ export default (options = {}) => { _(node, context, visit) { write_additional_comments(context, options.getLeadingComments?.(node), 'leading'); + let jsdoc_type_casts = 0; + if (node.loc) { - flush_comments_until(context, null, node.loc.start, true); + jsdoc_type_casts = flush_comments_until( + context, + null, + node.loc.start, + true, + node.type in EXPRESSIONS_PRECEDENCE + ); } visit(node); + if (jsdoc_type_casts > 0) { + context.write(')'.repeat(jsdoc_type_casts)); + } + // a JSX empty expression prints nothing and exists only to hold the // comments inside `{...}`. Flush them here, otherwise they are written // by whichever node comes next — after the closing brace, where they diff --git a/test/samples/comment-jsdoc-type-cast/expected.js b/test/samples/comment-jsdoc-type-cast/expected.js new file mode 100644 index 0000000..2138f53 --- /dev/null +++ b/test/samples/comment-jsdoc-type-cast/expected.js @@ -0,0 +1,2 @@ +const foo = /** @type {number} */ (1); +const bar = /** @type {number} */ (/** @type {number} */ (1)); diff --git a/test/samples/comment-jsdoc-type-cast/expected.js.map b/test/samples/comment-jsdoc-type-cast/expected.js.map new file mode 100644 index 0000000..5a8e256 --- /dev/null +++ b/test/samples/comment-jsdoc-type-cast/expected.js.map @@ -0,0 +1,11 @@ +{ + "version": 3, + "names": [], + "sources": [ + "input.js" + ], + "sourcesContent": [ + "const foo = /** @type {number} */ (1);\n\nconst bar = /** @type {number} */ (/** @type {number} */ (1));\n" + ], + "mappings": "AAAA,MAAM,AAAA,GAAG,0BAA0B,CAAC;AAEpC,MAAM,AAAA,GAAG,iDAAiD,CAAC" +} diff --git a/test/samples/comment-jsdoc-type-cast/input.js b/test/samples/comment-jsdoc-type-cast/input.js new file mode 100644 index 0000000..4edc268 --- /dev/null +++ b/test/samples/comment-jsdoc-type-cast/input.js @@ -0,0 +1,3 @@ +const foo = /** @type {number} */ (1); + +const bar = /** @type {number} */ (/** @type {number} */ (1)); From a17dab2a6cb4a92ce2453b9009127962ef934334 Mon Sep 17 00:00:00 2001 From: Elliott Johnson Date: Fri, 14 Aug 2026 15:37:58 -0600 Subject: [PATCH 2/3] fix: don't treat JSDoc @type comments on bindings as type casts Fixes #164 by tracking nodes in binding positions (variable declarator ids, function/method/arrow params, catch clause params) and excluding them from JSDoc type cast detection, so `let /** @type {T} */ x;` prints valid output while `/** @type {T} */ (expr)` casts keep their parentheses. --- .changeset/hot-doors-shake.md | 5 ++ src/languages/ts/index.js | 70 ++++++++++++++++--- .../comment-jsdoc-type-annotation/expected.js | 14 ++++ .../expected.js.map | 11 +++ .../comment-jsdoc-type-annotation/input.js | 13 ++++ 5 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 .changeset/hot-doors-shake.md create mode 100644 test/samples/comment-jsdoc-type-annotation/expected.js create mode 100644 test/samples/comment-jsdoc-type-annotation/expected.js.map create mode 100644 test/samples/comment-jsdoc-type-annotation/input.js diff --git a/.changeset/hot-doors-shake.md b/.changeset/hot-doors-shake.md new file mode 100644 index 0000000..4ee838e --- /dev/null +++ b/.changeset/hot-doors-shake.md @@ -0,0 +1,5 @@ +--- +'esrap': patch +--- + +fix: re-introduce JSDoc type cast support, but don't treat JSDoc `@type` comments on binding identifiers (variable declarator ids, function parameters, catch params) as type casts diff --git a/src/languages/ts/index.js b/src/languages/ts/index.js index 0f9145b..cd95dec 100644 --- a/src/languages/ts/index.js +++ b/src/languages/ts/index.js @@ -74,6 +74,37 @@ const OPERATOR_PRECEDENCE = { '**': 13 }; +/** + * Nodes in binding positions (variable declarator ids, function parameters, + * catch clause params, etc). A JSDoc `@type` comment attached to one of these + * is an annotation on the binding, not a type cast, so it must not be wrapped + * in parentheses (see https://github.com/sveltejs/esrap/issues/164). + * @type {WeakSet} + */ +const BINDINGS = new WeakSet(); + +/** + * Marks a node as occupying a binding position. + * @template T + * @param {T} node + * @returns {T} + */ +function binding(node) { + if (node && typeof node === 'object') BINDINGS.add(node); + return node; +} + +/** + * Marks an array of nodes (e.g. function params) as binding positions. + * @template {any[]} T + * @param {T} nodes + * @returns {T} + */ +function bindings(nodes) { + if (nodes) for (const node of nodes) binding(node); + return nodes; +} + /** * Writes `keyword` bounded by source map locations for the exact character span, * so breakpoints line up on keywords (not only identifiers and braces). @@ -823,7 +854,12 @@ export default (options = {}) => { } context.write('('); - sequence(context, node.params, (node.returnType ?? node.body).loc?.start ?? null, false); + sequence( + context, + bindings(node.params), + (node.returnType ?? node.body).loc?.start ?? null, + false + ); context.write(')'); if (node.returnType) context.visit(node.returnType); @@ -886,7 +922,7 @@ export default (options = {}) => { context.write('('); sequence( context, - node.value.params, + bindings(node.value.params), (node.value.returnType ?? node.value.body)?.loc?.start ?? node.loc?.end ?? null, false ); @@ -1009,7 +1045,7 @@ export default (options = {}) => { sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - node.parameters ?? node.params, + bindings(node.parameters ?? node.params), // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions (node.typeAnnotation ?? node.returnType)?.loc?.start ?? null, false @@ -1038,7 +1074,7 @@ export default (options = {}) => { sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - node.parameters ?? node.params, + bindings(node.parameters ?? node.params), // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions node.typeAnnotation?.typeAnnotation?.loc?.start ?? node.returnType?.typeAnnotation?.loc?.start ?? @@ -1066,7 +1102,7 @@ export default (options = {}) => { null, node.loc.start, true, - node.type in EXPRESSIONS_PRECEDENCE + node.type in EXPRESSIONS_PRECEDENCE && !BINDINGS.has(node) ); } @@ -1106,7 +1142,12 @@ export default (options = {}) => { } context.write('('); - sequence(context, node.params, (node.returnType ?? node.body).loc?.start ?? null, false); + sequence( + context, + bindings(node.params), + (node.returnType ?? node.body).loc?.start ?? null, + false + ); context.write(')'); if (node.returnType) context.visit(node.returnType); @@ -1624,7 +1665,7 @@ export default (options = {}) => { context.write('('); sequence( context, - node.value.params, + bindings(node.value.params), (node.value.returnType ?? node.value.body).loc?.start ?? null, false ); @@ -1787,7 +1828,7 @@ export default (options = {}) => { if (node.handler.param) { write_keyword(context, node.handler, 'catch', '('); - context.visit(node.handler.param); + context.visit(binding(node.handler.param)); context.write(') '); } else { write_keyword(context, node.handler, 'catch', ' '); @@ -1914,7 +1955,12 @@ export default (options = {}) => { } context.write('('); - sequence(context, node.params, node.returnType?.loc?.start ?? node.loc?.end ?? null, false); + sequence( + context, + bindings(node.params), + node.returnType?.loc?.start ?? node.loc?.end ?? null, + false + ); context.write(')'); if (node.returnType) { @@ -2234,7 +2280,7 @@ export default (options = {}) => { sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - node.parameters ?? node.params, + bindings(node.parameters ?? node.params), // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions (node.typeAnnotation ?? node.returnType)?.loc?.start ?? null, false @@ -2747,7 +2793,9 @@ function same_module_name(a, b) { function handle_var_declarator(node, context, no_in) { // `definite` sits on the declarator, but `!` belongs between the name and the // type annotation — both of which are written by the identifier's own visitor - context.visit(node.definite ? /** @type {any} */ ({ ...node.id, definite: true }) : node.id); + context.visit( + binding(node.definite ? /** @type {any} */ ({ ...node.id, definite: true }) : node.id) + ); if (node.init) { context.write(' = '); diff --git a/test/samples/comment-jsdoc-type-annotation/expected.js b/test/samples/comment-jsdoc-type-annotation/expected.js new file mode 100644 index 0000000..d674fb2 --- /dev/null +++ b/test/samples/comment-jsdoc-type-annotation/expected.js @@ -0,0 +1,14 @@ +let /** @type {number | undefined} */ computed; + +let /** @type {number | undefined} */ a, + /** @type {string} */ b; + +function foo(/** @type {number} */ x, /** @type {string} */ y) {} + +const fn = (/** @type {number} */ z) => z; + +try { + foo(1, ''); +} catch(/** @type {any} */ e) {} + +const value = /** @type {number} */ (computed); \ No newline at end of file diff --git a/test/samples/comment-jsdoc-type-annotation/expected.js.map b/test/samples/comment-jsdoc-type-annotation/expected.js.map new file mode 100644 index 0000000..1dadee7 --- /dev/null +++ b/test/samples/comment-jsdoc-type-annotation/expected.js.map @@ -0,0 +1,11 @@ +{ + "version": 3, + "names": [], + "sources": [ + "input.js" + ], + "sourcesContent": [ + "let /** @type {number | undefined} */ computed;\n\nlet /** @type {number | undefined} */ a, /** @type {string} */ b;\n\nfunction foo(/** @type {number} */ x, /** @type {string} */ y) {}\n\nconst fn = (/** @type {number} */ z) => z;\n\ntry {\n\tfoo(1, '');\n} catch (/** @type {any} */ e) {}\n\nconst value = /** @type {number} */ (computed);\n" + ], + "mappings": "AAAA,IAAI,kCAAkC,QAAQ;;AAE9C,IAAI,kCAAkC,CAAC;uBAAwB,CAAC;;AAEhE,QAAQ,CAAC,GAAG,uBAAuB,CAAC,wBAAwB,CAAC,EAAE,CAAC,AAAA,CAAC;;AAEjE,MAAM,AAAA,EAAE,0BAA0B,CAAC,KAAK,CAAC;;AAEzC,GAAG,CAAC,CAAC;CACJ,GAAG,CAAC,CAAC,EAAE,EAAE;AACV,CAAC,CAAC,KAAK,oBAAqB,CAAC,EAAE,CAAC,AAAA,CAAC;;AAEjC,MAAM,AAAA,KAAK,0BAA0B,QAAQ" +} \ No newline at end of file diff --git a/test/samples/comment-jsdoc-type-annotation/input.js b/test/samples/comment-jsdoc-type-annotation/input.js new file mode 100644 index 0000000..7578f6d --- /dev/null +++ b/test/samples/comment-jsdoc-type-annotation/input.js @@ -0,0 +1,13 @@ +let /** @type {number | undefined} */ computed; + +let /** @type {number | undefined} */ a, /** @type {string} */ b; + +function foo(/** @type {number} */ x, /** @type {string} */ y) {} + +const fn = (/** @type {number} */ z) => z; + +try { + foo(1, ''); +} catch (/** @type {any} */ e) {} + +const value = /** @type {number} */ (computed); From 93d797ed676a4d9fa4a83089b2f0f18bb29396b9 Mon Sep 17 00:00:00 2001 From: Elliott Johnson Date: Fri, 14 Aug 2026 15:50:14 -0600 Subject: [PATCH 3/3] refactor: rename binding trackers and make them void for CQS --- src/languages/ts/index.js | 69 ++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/languages/ts/index.js b/src/languages/ts/index.js index cd95dec..46f993b 100644 --- a/src/languages/ts/index.js +++ b/src/languages/ts/index.js @@ -85,24 +85,20 @@ const BINDINGS = new WeakSet(); /** * Marks a node as occupying a binding position. - * @template T - * @param {T} node - * @returns {T} + * @param {object | null | undefined} node + * @returns {void} */ -function binding(node) { +function track_binding(node) { if (node && typeof node === 'object') BINDINGS.add(node); - return node; } /** * Marks an array of nodes (e.g. function params) as binding positions. - * @template {any[]} T - * @param {T} nodes - * @returns {T} + * @param {(object | null | undefined)[] | null | undefined} nodes + * @returns {void} */ -function bindings(nodes) { - if (nodes) for (const node of nodes) binding(node); - return nodes; +function track_bindings(nodes) { + if (nodes) for (const node of nodes) track_binding(node); } /** @@ -853,13 +849,9 @@ export default (options = {}) => { context.visit(node.typeParameters); } + track_bindings(node.params); context.write('('); - sequence( - context, - bindings(node.params), - (node.returnType ?? node.body).loc?.start ?? null, - false - ); + sequence(context, node.params, (node.returnType ?? node.body).loc?.start ?? null, false); context.write(')'); if (node.returnType) context.visit(node.returnType); @@ -919,10 +911,11 @@ export default (options = {}) => { // @ts-expect-error `typeParameters` lives on the method node, not its value if (node.typeParameters) context.visit(node.typeParameters); + track_bindings(node.value.params); context.write('('); sequence( context, - bindings(node.value.params), + node.value.params, (node.value.returnType ?? node.value.body)?.loc?.start ?? node.loc?.end ?? null, false ); @@ -1041,11 +1034,13 @@ export default (options = {}) => { context.visit(node.typeParameters); } + // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions + track_bindings(node.parameters ?? node.params); context.write('('); sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - bindings(node.parameters ?? node.params), + node.parameters ?? node.params, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions (node.typeAnnotation ?? node.returnType)?.loc?.start ?? null, false @@ -1070,11 +1065,13 @@ export default (options = {}) => { } if (node.typeParameters) context.visit(node.typeParameters); + // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions + track_bindings(node.parameters ?? node.params); context.write('('); sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - bindings(node.parameters ?? node.params), + node.parameters ?? node.params, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions node.typeAnnotation?.typeAnnotation?.loc?.start ?? node.returnType?.typeAnnotation?.loc?.start ?? @@ -1141,13 +1138,9 @@ export default (options = {}) => { context.visit(node.typeParameters); } + track_bindings(node.params); context.write('('); - sequence( - context, - bindings(node.params), - (node.returnType ?? node.body).loc?.start ?? null, - false - ); + sequence(context, node.params, (node.returnType ?? node.body).loc?.start ?? null, false); context.write(')'); if (node.returnType) context.visit(node.returnType); @@ -1662,10 +1655,11 @@ export default (options = {}) => { if (node.computed) context.write('[', token_before(node.key.loc?.start)); context.visit(node.key); if (node.computed) context.write(']', token_at(node.key.loc?.end)); + track_bindings(node.value.params); context.write('('); sequence( context, - bindings(node.value.params), + node.value.params, (node.value.returnType ?? node.value.body).loc?.start ?? null, false ); @@ -1828,7 +1822,8 @@ export default (options = {}) => { if (node.handler.param) { write_keyword(context, node.handler, 'catch', '('); - context.visit(binding(node.handler.param)); + track_binding(node.handler.param); + context.visit(node.handler.param); context.write(') '); } else { write_keyword(context, node.handler, 'catch', ' '); @@ -1954,13 +1949,9 @@ export default (options = {}) => { context.visit(node.typeParameters); } + track_bindings(node.params); context.write('('); - sequence( - context, - bindings(node.params), - node.returnType?.loc?.start ?? node.loc?.end ?? null, - false - ); + sequence(context, node.params, node.returnType?.loc?.start ?? node.loc?.end ?? null, false); context.write(')'); if (node.returnType) { @@ -2276,11 +2267,13 @@ export default (options = {}) => { context.visit(node.typeParameters); } + // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions + track_bindings(node.parameters ?? node.params); context.write('('); sequence( context, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions - bindings(node.parameters ?? node.params), + node.parameters ?? node.params, // @ts-expect-error `acorn-typescript` and `@typescript-eslint/types` have slightly different type definitions (node.typeAnnotation ?? node.returnType)?.loc?.start ?? null, false @@ -2793,9 +2786,9 @@ function same_module_name(a, b) { function handle_var_declarator(node, context, no_in) { // `definite` sits on the declarator, but `!` belongs between the name and the // type annotation — both of which are written by the identifier's own visitor - context.visit( - binding(node.definite ? /** @type {any} */ ({ ...node.id, definite: true }) : node.id) - ); + const id = node.definite ? /** @type {any} */ ({ ...node.id, definite: true }) : node.id; + track_binding(id); + context.visit(id); if (node.init) { context.write(' = ');