Repository navigation
Fix JSDoc @param rest parsing (#320) - #321
Open
kungfooman wants to merge 3 commits into
Open
kungfooman wants to merge 3 commits into
kungfooman wants to merge 3 commits into
Conversation
Drop the written-text array sniff in tagRestElementType: the expander already gathers ...T into one array level, so variadicity is just marker plus isArrayNode. Covers paren and generic spellings for free.
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 #320
Primer
JSDoc rest annotations come in two spellings. The documented spelling puts the dots inside the braces (
@param {...number[]} values): the...marks the parameter variadic and thenumber[]inside gives the element type. The bare spelling puts only the dots there (@param {...number} values): in the TypeScript grammar a rest parameter must still be an array type, so...numbergathers the call arguments intonumber[]and the parameter behaves as one fixed element of that array type, while...number[]gathers intonumber[][]so each argument is checked againstnumber[]. Both spellings reach the transpiler as the same string, and the type expander erases the marker:expandTypeDepFree('...number')returns{array, number}the same way it returns it for a plainnumber[], so after expansion no trace of...remains.The overload layer converts each
@overloadblock into a positional tuple insrc-transpiler/overloadToTuple.js, where adotDotflag on atupleMembermakes the validator treat it as variadic (src-runtime/expandTupleElements.js,src-runtime/inspectOverloads.js). Because the expander erased the marker, the old code recovered it with a second regex over the raw section string (/@param\s+\{\s*\.\.\.[^}]*\}\s+(\w+)/g), then applied an array-shape gate: rest applied only when the expanded type was anarraywhose element was itself anarray. Two parses of one string, two regex dialects, and neither knew about the other. Anything the regexes disagreed on silently became a different type.What broke, gap by gap, each verified against
tsc --noEmit --allowJs --checkJs --stricton the same file:{T[]} ...name(dots before the name) fragmented into dotted-path parts (['', '', '', 'values']), tripped theconsole.assertinparseJSDoc.js, emitted Skipping warnings, and built a phantom empty-named member plus a non-rest member. tsc rejects the spelling at parse (TS1003 Identifier expected,TS8032 qualified name). RTI accepted it with phantom members.{...T}lost variadicity in overloads:{array, number}fails the nested-array gate, so@param {...number} valuesbecame a fixednumber[]member. tsc treats the same tag as variadic-over-number-vs-array-shape (acceptsg(1), rejects the gatheredg([1,2])with[number]vsnumber), and RTI's own plain path agreed with tsc; only the overload tuple diverged, and no probe pinned it because thegenerateTypeChecks.spec.jsrest tests and theoverloads-input.mjssumfixture exercise only...T[].{...number[][]}the unwrap-then-check-array logic yields variadic-over-number[]by counting one level from dots and one from brackets, i.e. string counting, not structure.[... ]) silently droppeddotDotand stayed fixed-arity. tsc rejects it (TS2370 a rest parameter must be of an array type, equivalently no call matches). RTI accepted it as fixed-arity optional, and no fixture covered it.What changed
src-transpiler/parseJSDocTags.js(new): a tag-level scanner that walks each@paramline, extracts the brace content and the head name token with the existingextractCurlyContent/extractNameAndOptionalityhelpers, and records whether the rest marker sits before the name ({T[]} ...name). Dots inside braces, optional brackets, dotted paths, same-line tags, and trailing prose each have a spec pinning the scan.src-transpiler/tagRestElementType.js(new): resolves one tag's rest shape. A brace marker with an optional name is theTS2370error and resolves tonever, so every call warns. A...T[]marker (written array) turns variadic over the written element type. A...Tmarker turns variadic over the gathered element itself, matching the tsc accept/reject matrix call by call. Anything else passes through untouched.src-transpiler/overloadToTuple.js: consumes the tags instead of the regex. Members whose marker sits before the name are dropped (matching tsc's parse rejection); every other member resolves throughtagRestElementType. Branch bodies use braces per review convention, and the poison loop is a plainforwithcontinueinstead of amap/filterchain.src-transpiler/expandTypeDepFree.js: a leading...element union now splits the gathered element at the first top-level|,,,;, or?before wrapping, so...number|stringgathers onlynumber((number[])|string, as tsc parses...number: number) instead of swallowing the whole tail. Depth tracking skips bracketed groups (Array<>, parens, tuples, objects).test/typechecking/overload-rest-input.mjsplusoverload-rest-output.mjs(transpiler parity) andoverload-rest-errors.json(runtime: exactlydocumented('x',1,'bad'),bare('x',[1,2]),optionalRest([1,2]),misplaced(1),misplaced([1])throw, in order);overloadToTuple.spec.jsgains bare-rest, optional-rest, and misplaced-rest tests; newparseJSDocTags.spec.jsandtagRestElementType.spec.jswired intotest_runtime.js. The old expectations fail on the pre-fix tree (bare rest fixed-arity, optional rest non-never, poison phantom kept), and the full gate passes on the new tree.Worked demonstration (
node run-jsdoc.js --emiton the minimal overload input, emission verbatim): docs in are the two-blocksumdoc above; emission out is a singleinspectOverloads([...arguments], [{tuple: {type: tuple, elements: [{tupleMember, label: string}, {tupleMember, values: {array, number}, dotDot: true}]}, templates: {}}], 'sum', 'arguments')guard; at runtimesum('x', 1, 2)passes,sum('x', 1, 'bad')warnsArgument of type ["x", 1, "bad"] is not assignable to parameter of type [label: string, ...values: number[]], and no other call shape changes.Latent holes surfaced
{number[]} ...valuesin the plain (non-overload) path still writes phantom{'': ...}entries throughparseJSDocwith Skipping warnings: the tuple path now drops the member, butparseJSDocitself still fragments leading dots into dotted-path parts and theAsserterdefault path warnsMissing param. Tolerated deliberately: tsc rejects the spelling before checking, the errors file pins the overload verdict, and restructuringparseJSDoc's name grammar is out of scope.parseJSDoc's name regex admits[/]/=prose but not{/}: a@paramline without braces never scans, and a description containing a literal@paramat a line start with a non-tag gap is skipped by the gap rule. The tag scanner mirrors the old line rule (gap must be empty,*, or/**, and must not contain@), so both layers agree; a prose line that defeats one defeats both, which the same-line spec pins.tagRestElementTypedecides array-ness from the written text (endsWith('[]'),startsWith('Array<')) rather than the expanded node, because the expander erases which array level the dots contributed. A parenthesized written array (...(number[])) orreadonlyprefix would misclassify the written shape; tsc probes forreadonlyshow it still gathers variadically, so the text check errs toward the documented path and the nested-union probes (...number|string,...(number|string),...Array<number>) pin the covered spellings.