Keep JSDoc on expando hosts declared as arrows or function expressions - #4858
Keep JSDoc on expando hosts declared as arrows or function expressions#4858Yogesh Kumar (yogesh968) wants to merge 1 commit into
Conversation
Declaration emit turns an expando host into a `declare function` two different ways: function declarations go through UpdateFunctionDeclaration and keep the original node's position, while a variable whose initializer is an arrow function or function expression is synthesized with NewFunctionDeclaration and has no position at all. The printer resolves a node's comment range via EmitContext.CommentRange, which falls back to node.Loc. For the synthesized node that is a synthesized position, so emitLeadingCommentsOfNode skips leading comments and the JSDoc written above the variable statement never reaches the .d.ts. Set the comment range on the synthesized declaration to the variable statement it stands in for, which is where that JSDoc lives.
|
Yogesh Kumar (@yogesh968) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
|
Yogesh Kumar (@yogesh968) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Fixes #4295
Analysis
An expando host can be written either as a function declaration or as a variable
whose initializer is an arrow function / function expression. Declaration emit
turns both into a
declare function, but it builds them in two different ways intransformExpandoHost:The first branch goes through
Update..., so the result keeps the original node'sposition. The second branch synthesizes a node from scratch, so its
Locissynthesized.
Printer.emitCommentsBeforeNodeasksEmitContext.CommentRange(node)for the rangeto scan for leading comments, and that getter falls back to
node.Locwhen noexplicit range was set. For the synthesized node that is a synthesized position, so
emitLeadingCommentsOfNodetakes thePositionIsSynthesizedpath and skips leadingcomments entirely — the JSDoc written above the variable statement is silently
dropped from the
.d.ts.Given this input:
TypeScript 6.0.3 emits the comment, and tsgo does not:
Fix
Set an explicit comment range on the synthesized function declaration, pointing at
the variable statement — which is where JSDoc on the host is written, and whose
Pos()starts before the leading trivia. This mirrors what the siblingUpdate...branch gets for free, and matches the existing idiom a few hundredlines up in the same file (
SetOriginal+SetCommentRangeon a synthesizeddeclaration list).
Alternatives considered: also calling
EmitContext.SetOriginal(funcDecl, root).That copies the original's emit node and makes
MostOriginalresolve thesynthesized function back to the variable statement, which is more than is needed
here — the comment range is the only thing the printer is missing — so I kept the
change to the narrower of the two.
Effect on the table in #4295
Measured against
typescript@6.0.3,.jsinputs:Two rows converge onto 6.0 behavior. The second row does not: 6.0 drops the comment
when the expando arrow is exported inline, and after this change tsgo keeps it.
The
.tscolumns of that table are covered too: tsgo routes.tsexpando hoststhrough this same synthesis, so they gain the comment as well. (6.0 emits those as
declare const X: { (): null; args: ... }instead of a function/namespace pair —a separate shape difference that this change does not touch.)
I did not try to reproduce that omission. Both branches of
transformExpandoHostdescribe the same construct, and the only thing separating this row from the
ExpandoArrowNamedExportrow directly below it is where theexportkeyword sits,which does not seem like a distinction the emitter should be preserving. Suppressing
it again would mean special-casing exported variable statements purely to reproduce
what looks like an artifact of Strada's node reuse. The issue notes this row is
expected to change ("the comment emit fix for arrows will be a breaking change from
TS 6"). Happy to gate it if you would rather hold the line at exact 6.0 parity.
Testing
New test at
testdata/tests/cases/compiler/expandoArrowFunctionDeclarationEmitJSDoc.tscovering the named-export, inline-export and default-export forms, for both arrow
functions and function expressions, with function declarations alongside them as
controls. Baselines were accepted before the fix and re-accepted after, so the diff
on the baseline file shows exactly which comments the change adds.
No other baselines in the suite changed.
Disclosure
Per CONTRIBUTING.md: this patch was authored with AI assistance (Claude Code). I
picked the issue, reviewed the diff and the baseline changes, ran the build, tests,
lint and format locally, and I will be the one responding to review feedback here.
Copilot Checklist
I successfully ran these commands at the end of my session, and they completed without error:
Being straight about the two unticked boxes rather than claiming them — my machine ran
out of disk partway through and I could not get either to complete end to end.
What I did run green, individually:
The compiler baseline run is the one that matters most for this change, and it wrote
zero files into
testdata/baselines/local— every one of the ~49k reference baselinesmatched, so nothing outside the new test moved.
npx hereby lintnever got as far as linting: building the custom golangci-lint binaryfilled the disk (
compile: writing output: ... There is not enough space on the disk)../internal/fourslash/...is the other suite I did not run. Both should be covered byCI here — please shout if either turns up something and I will fix it.