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 {
| signature.FunctionLikeData().Type = p.makeNewType(jsSignature.Type.AsJSDocReturnTag().TypeExpression, signature) | ||
| } | ||
| signature.FunctionLikeData().Parameters = p.newNodeList(jsSignature.Parameters.Loc, parameters) | ||
| signature.Loc = tag.AsJSDocOverloadTag().TagName.Loc |
There was a problem hiding this comment.
[nitpick] Consider using tag.Loc instead of TagName.Loc to preserve the full JSDoc tag span when assigning signature.Loc, ensuring consistent location tracking.
| signature.Loc = tag.AsJSDocOverloadTag().TagName.Loc | |
| signature.Loc = tag.Loc |
Copilot uses AI. Check for mistakes.
| @@ -2874,9 +2874,15 @@ func GetErrorRangeForNode(sourceFile *ast.SourceFile, node *ast.Node) core.TextR | |||
| } | |||
| return scanner.GetRangeOfTokenAtPosition(sourceFile, pos) | |||
| // This list is a work in progress. Add missing node kinds to improve their error spans | |||
| case ast.KindFunctionDeclaration, ast.KindMethodDeclaration: | |||
There was a problem hiding this comment.
[nitpick] Refactor the repeated NodeFlagsReparsed check in both the FunctionDeclaration/MethodDeclaration and Constructor cases into a shared helper or early switch to reduce duplication.
Copilot uses AI. Check for mistakes.
| @@ -3,19 +3,20 @@ | |||
| @@= skipped -0, +-1 lines =@@ | |||
| -<no content> | |||
| @@= skipped --1, +1 lines =@@ | |||
| +a.js(1,5): error TS1047: A rest parameter cannot be optional. | |||
There was a problem hiding this comment.
Is this an intentional change in behavior? Previously a postfix = meant optional-if-able-but-| undefined-otherwise.
| - /** | ||
| - * @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.
| func (f *NodeFactory) NewJSDocPropertyTag(tagName *IdentifierNode, name *EntityName, isBracketed bool, typeExpression *TypeNode, isNameFirst bool, comment *NodeList) *Node { | ||
| data := &JSDocPropertyTag{} | ||
| func (f *NodeFactory) newJSDocParameterOrPropertyTag(kind Kind, tagName *IdentifierNode, name *EntityName, isBracketed bool, typeExpression *TypeNode, isNameFirst bool, comment *NodeList) *Node { | ||
| data := &JSDocParameterTag{} |
There was a problem hiding this comment.
I feel like we should probably rename the base struct type here to JSDocParameterOrPropertyTag and alias type JSDocParameterTag = JSDocParameterOrPropertyTag - likewise, the As cast function should change to refer to the Or type, but it should be aliased to the subtypes for per-use clarity. As-is, something like
switch (node.Kind) {
case ast.KindJSDocParamterTag, ast.KindJSDocPropertyTag:
node.AsJSDocParameterTag() /// ???
}
just looks super sus, even though it (currently) works because of the underlying shapes - the aliased types and methods should help with that. Should also help if we ever get around to paying off the massive debt all this type erasure is incurring and start statically checking unions again...
@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.