Add overload tag #900
Add overload tag #900
Conversation
@overload emits a synthetic bodyless function declaration, which then behaves just like a real overload. To make errors behave nicely, I had to omit some position checks; those errors can't apply to @overload tags anyway. I also removed one @overload-specific error and went back to the standard one. I ran into the same problem with @template as I saw with @typedef, which means that name lookup fails on all @template tags as far as I can tell. The fix requires setting Host to a parent synthetic node on nested synthetic nodes, because name resolution always prefers Host for those nodes. I applied my fix but it still didn't instantiate the type parameters correctly, so I'm going to fix it in a separate PR.
- Extract question token creation to a function. - Create a signature with a bunch of nil fields, and set them at the end, allowing makeNewType to set the host in the standard way. - Also, move question token creation for parameters so that it's created even if there's no type on the parameter tag.
There was a problem hiding this comment.
Pull Request Overview
Adds support for JSDoc @overload tags by emitting synthetic overload declarations and updates related JSDoc tag handling, AST nodes, binding, checking, and encoder logic.
- Implemented
@overloadparsing inreparser.goand synthetic signature creation - Unified
JSDocPropertyTagandJSDocParameterTaginto a single tag type with shared factory methods - Updated checker, binder, AST, and encoder to respect
NodeFlagsReparsedand new tag kinds - Regenerated baseline snapshots for various overload and type cases
Reviewed Changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/.../jsFileMethodOverloads3.* | Updated symbol and error baselines for overloads |
| testdata/.../jsFileMethodOverloads2.* | Updated types and symbols baselines for overloads |
| testdata/.../jsFileMethodOverloads.* | Updated types and symbols baselines for overloads |
| testdata/.../jsFileFunctionOverloads2.* | Updated types and symbols baselines for overloads |
| testdata/.../jsFileFunctionOverloads.* | Updated types and symbols baselines for overloads |
| internal/parser/reparser.go | Added @overload support and makeQuestionIfOptional helper |
| internal/checker/checker.go | Skips synthetic overload declarations by flag |
| internal/binder/binder.go | Extends error-range logic for reparsed nodes |
| internal/ast/ast.go | Merged property/parameter tag types and accessors |
| internal/api/encoder/encoder.go | Adjusted encoder masks for unified JSDoc tags |
Comments suppressed due to low confidence (1)
internal/ast/ast.go:9136
- [nitpick] The method name
UpdateJSDocParameterTagnow handles both parameter and property tags; consider renaming it toUpdateJSDocParameterOrPropertyTagfor clarity.
func (f *NodeFactory) UpdateJSDocParameterTag(kind Kind, node *JSDocParameterTag, tagName *IdentifierNode, name *EntityName, isBracketed bool, typeExpression *TypeNode, isNameFirst bool, comment *NodeList) *Node {
testdata/baselines/reference/submoduleAccepted/conformance/jsdocParseStarEquals.errors.txt.diff
Show resolved
Hide resolved
| - /** | ||
| - * @overload | ||
| - ~~~~~~~~ | ||
| +!!! error TS7010: 'id', which lacks return-type annotation, implicitly has an 'any' return type. |
There was a problem hiding this comment.
This is just the wrong error message, no? JSDoc overloads are supposed to use the version without a declaration name because they're nameless in the source, right? Even if the synthetic one we make now copies the host function's name. As-is, this error is kinda ambiguous, since nowhere does id appear in the overload descriptor.
There was a problem hiding this comment.
I disagree and collapsed the two intentionally, but if you feel strongly I can change it back.
The reason I like having the name in the error message is that the function name appears a few lines after the tag in the source text. When I read the diff, I feel like the Strada error message withholds useful information on a technicality.
There was a problem hiding this comment.
As-is, to me, the error feels like it's talking about the host declaration rather than the underlined one, and the span's in the wrong place.
There was a problem hiding this comment.
If you wanted to compromise, the message This overload of 'id' lacks a return-type annotation and implicitly has an 'any' return type. would be more apt, I think.
Also improve type of `findMatchingParameter`
@overloademits a synthetic bodyless function declaration, which then behaves just like a real overload.To make errors behave nicely, I had to omit some position checks; those errors can't apply to
@overloadtags anyway. I also removed one@overload-specific error and went back to the standard one.I ran into the same problem with
@template(type parameters) as I saw with@typedef. The fix requires setting Host to a parent synthetic node on nested synthetic nodes, because name resolution always prefers Host for those nodes. I created the signature first, with Parameters and Type nil, so that I could usemakeNewTypeto set the Host in the usual way when creating parameters and return type. Then I set the Parameters and Type afterward.I applied that same fix to
@typedefbut it still didn't instantiate the type parameters correctly, so I'm going to fix it in a separate PR.There is a good deal of churn because I also removed JSDocPropertyTag as a separate type -- it's now a JSDocParameterTag with a different kind, just like JSExportAssignment and other reparsed nodes.