Skip to content

Deep clone JSDoc #1412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions internal/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -9063,7 +9063,6 @@ func (node *JSDocLinkCode) Name() *DeclarationName {

type JSDocTypeExpression struct {
TypeNodeBase
Host *Node
Type *TypeNode
}

Expand Down Expand Up @@ -9321,7 +9320,6 @@ type JSDocTemplateTag struct {
JSDocTagBase
Constraint *Node
TypeParameters *TypeParameterList
Host *Node
}

func (f *NodeFactory) NewJSDocTemplateTag(tagName *IdentifierNode, constraint *Node, typeParameters *TypeParameterList, comment *NodeList) *Node {
Expand Down Expand Up @@ -9816,10 +9814,9 @@ func (node *JSDocThisTag) Clone(f NodeFactoryCoercible) *Node {
// JSDocImportTag
type JSDocImportTag struct {
JSDocTagBase
JSImportDeclaration *ImportDeclaration
ImportClause *Declaration
ModuleSpecifier *Expression
Attributes *Node
ImportClause *Declaration
ModuleSpecifier *Expression
Attributes *Node
}

func (f *NodeFactory) NewJSDocImportTag(tagName *IdentifierNode, importClause *Declaration, moduleSpecifier *Node, attributes *Node, comment *NodeList) *Node {
Expand Down
21 changes: 18 additions & 3 deletions internal/ast/deepclone.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ast
import "github.com/microsoft/typescript-go/internal/core"

// Ideally, this would get cached on the node factory so there's only ever one set of closures made per factory
func getDeepCloneVisitor(f *NodeFactory) *NodeVisitor {
func getDeepCloneVisitor(f *NodeFactory, syntheticLocation bool) *NodeVisitor {
var visitor *NodeVisitor
visitor = NewNodeVisitor(
func(node *Node) *Node {
Expand All @@ -15,7 +15,9 @@ func getDeepCloneVisitor(f *NodeFactory) *NodeVisitor {
// In strada, `factory.cloneNode` was dynamic and did _not_ clone positions for any "special cases", meanwhile
// Node.Clone in corsa reliably uses `Update` calls for all nodes and so copies locations by default.
// Deep clones are done to copy a node across files, so here, we explicitly make the location range synthetic on all cloned nodes
c.Loc = core.NewTextRange(-1, -1)
if syntheticLocation {
c.Loc = core.NewTextRange(-1, -1)
}
return c
},
f,
Expand Down Expand Up @@ -46,5 +48,18 @@ func getDeepCloneVisitor(f *NodeFactory) *NodeVisitor {
}

func (f *NodeFactory) DeepCloneNode(node *Node) *Node {
return getDeepCloneVisitor(f).VisitNode(node)
return getDeepCloneVisitor(f, true /*syntheticLocation*/).VisitNode(node)
}

func (f *NodeFactory) DeepCloneReparse(node *Node) *Node {
if node != nil {
node = getDeepCloneVisitor(f, false /*syntheticLocation*/).VisitNode(node)
SetParentInChildren(node)
node.Flags |= NodeFlagsReparsed
}
return node
}

func (f *NodeFactory) DeepCloneReparseModifiers(modifiers *ModifierList) *ModifierList {
return getDeepCloneVisitor(f, false /*syntheticLocation*/).VisitModifiers(modifiers)
}
13 changes: 2 additions & 11 deletions internal/ast/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,17 +867,6 @@ func WalkUpParenthesizedTypes(node *TypeNode) *Node {
return node
}

func GetEffectiveTypeParent(parent *Node) *Node {
if parent != nil && IsInJSFile(parent) {
if parent.Kind == KindJSDocTypeExpression && parent.AsJSDocTypeExpression().Host != nil {
parent = parent.AsJSDocTypeExpression().Host
} else if parent.Kind == KindJSDocTemplateTag && parent.AsJSDocTemplateTag().Host != nil {
parent = parent.AsJSDocTemplateTag().Host
}
}
return parent
}

// Walks up the parents of a node to find the containing SourceFile
func GetSourceFileOfNode(node *Node) *SourceFile {
for node != nil {
Expand Down Expand Up @@ -1466,6 +1455,8 @@ func getAssignedName(node *Node) *Node {
}
}
}
case KindCommonJSExport:
return parent.AsCommonJSExport().Name()
case KindVariableDeclaration:
name := parent.AsVariableDeclaration().Name()
if IsIdentifier(name) {
Expand Down
2 changes: 0 additions & 2 deletions internal/binder/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1651,8 +1651,6 @@ func (b *Binder) bindChildren(node *ast.Node) {
case ast.KindObjectLiteralExpression, ast.KindArrayLiteralExpression, ast.KindPropertyAssignment, ast.KindSpreadElement:
b.inAssignmentPattern = saveInAssignmentPattern
b.bindEachChild(node)
case ast.KindJSExportAssignment, ast.KindCommonJSExport:
// Reparsed nodes do not double-bind children, which are not reparsed
default:
b.bindEachChild(node)
}
Expand Down
5 changes: 1 addition & 4 deletions internal/binder/nameresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ loop:
lastSelfReferenceLocation = location
}
lastLocation = location
location = ast.GetEffectiveTypeParent(location.Parent)
location = location.Parent
}
// We just climbed up parents looking for the name, meaning that we started in a descendant node of `lastLocation`.
// If `result === lastSelfReferenceLocation.symbol`, that means that we are somewhere inside `lastSelfReferenceLocation` looking up a name, and resolving to `lastLocation` itself.
Expand Down Expand Up @@ -483,9 +483,6 @@ func isTypeParameterSymbolDeclaredInContainer(symbol *ast.Symbol, container *ast
for _, decl := range symbol.Declarations {
if decl.Kind == ast.KindTypeParameter {
parent := decl.Parent
if parent.Kind == ast.KindJSDocTemplateTag {
parent = parent.AsJSDocTemplateTag().Host
}
if parent == container {
return true
}
Expand Down
20 changes: 4 additions & 16 deletions internal/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -2903,7 +2903,7 @@ func (c *Checker) checkTypePredicate(node *ast.Node) {
}

func (c *Checker) getTypePredicateParent(node *ast.Node) *ast.SignatureDeclaration {
parent := ast.GetEffectiveTypeParent(node.Parent)
parent := node.Parent
switch parent.Kind {
case ast.KindArrowFunction, ast.KindCallSignature, ast.KindFunctionDeclaration, ast.KindFunctionExpression, ast.KindFunctionType,
ast.KindMethodDeclaration, ast.KindMethodSignature:
Expand Down Expand Up @@ -13971,9 +13971,6 @@ func (c *Checker) getTargetOfImportSpecifier(node *ast.Node, dontResolveAlias bo
}
}
root := node.Parent.Parent.Parent // ImportDeclaration
if root.Kind == ast.KindJSDocImportTag {
root = root.AsJSDocImportTag().JSImportDeclaration.AsNode()
}
if ast.IsBindingElement(node) {
root = ast.GetRootDeclaration(node)
}
Expand Down Expand Up @@ -14344,8 +14341,6 @@ func (c *Checker) getModuleSpecifierForImportOrExport(node *ast.Node) *ast.Node

func getModuleSpecifierFromNode(node *ast.Node) *ast.Node {
switch node.Kind {
case ast.KindJSDocImportTag:
return node.AsJSDocImportTag().JSImportDeclaration.ModuleSpecifier
case ast.KindImportDeclaration, ast.KindJSImportDeclaration:
return node.AsImportDeclaration().ModuleSpecifier
case ast.KindExportDeclaration:
Expand Down Expand Up @@ -14449,12 +14444,6 @@ func (c *Checker) resolveExternalModule(location *ast.Node, moduleReference stri
contextSpecifier = location.AsVariableDeclaration().Initializer.AsCallExpression().Arguments.Nodes[0]
} else {
var ancestor *ast.Node
if location.Flags&ast.NodeFlagsJSDoc != 0 {
ancestor = ast.FindAncestor(location, ast.IsJSDocImportTag)
if ancestor != nil {
contextSpecifier = ancestor.AsJSDocImportTag().JSImportDeclaration.ModuleSpecifier
}
}
if ancestor == nil {
ancestor = ast.FindAncestor(location, ast.IsImportCall)
if ancestor != nil {
Expand Down Expand Up @@ -22039,7 +22028,6 @@ func (c *Checker) getTypeFromTypeOperatorNode(node *ast.Node) *Type {
}

func (c *Checker) getESSymbolLikeTypeForNode(node *ast.Node) *Type {
node = ast.GetEffectiveTypeParent(node)
if isValidESSymbolDeclaration(node) {
symbol := c.getSymbolOfNode(node)
if symbol != nil {
Expand Down Expand Up @@ -22710,9 +22698,9 @@ func (c *Checker) getAliasForTypeNode(node *ast.Node) *TypeAlias {
}

func (c *Checker) getAliasSymbolForTypeNode(node *ast.Node) *ast.Symbol {
host := ast.GetEffectiveTypeParent(node.Parent)
host := node.Parent
for ast.IsParenthesizedTypeNode(host) || ast.IsTypeOperatorNode(host) && host.AsTypeOperatorNode().Operator == ast.KindReadonlyKeyword {
host = ast.GetEffectiveTypeParent(host.Parent)
host = host.Parent
}
if isTypeAlias(host) {
return c.getSymbolOfDeclaration(host)
Expand Down Expand Up @@ -22748,7 +22736,7 @@ func (c *Checker) getOuterTypeParametersOfClassOrInterface(symbol *ast.Symbol) [
// Return the outer type parameters of a node or undefined if the node has no outer type parameters.
func (c *Checker) getOuterTypeParameters(node *ast.Node, includeThisTypes bool) []*Type {
for {
node = ast.GetEffectiveTypeParent(node.Parent)
node = node.Parent
if node == nil {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/checker/grammarchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -1394,7 +1394,7 @@ func (c *Checker) checkGrammarTypeOperatorNode(node *ast.TypeOperatorNode) bool
if innerType.Kind != ast.KindSymbolKeyword {
return c.grammarErrorOnNode(innerType, diagnostics.X_0_expected, scanner.TokenToString(ast.KindSymbolKeyword))
}
parent := ast.GetEffectiveTypeParent(ast.WalkUpParenthesizedTypes(node.Parent))
parent := ast.WalkUpParenthesizedTypes(node.Parent)
switch parent.Kind {
case ast.KindVariableDeclaration:
decl := parent.AsVariableDeclaration()
Expand Down
3 changes: 0 additions & 3 deletions internal/checker/utilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -1842,9 +1842,6 @@ func getAnyImportSyntax(node *ast.Node) *ast.Node {
default:
return nil
}
if importNode.Kind == ast.KindJSDocImportTag {
return importNode.AsJSDocImportTag().JSImportDeclaration.AsNode()
}
return importNode
}

Expand Down
86 changes: 86 additions & 0 deletions internal/ls/hover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,92 @@ function myFunction() {
},
},
},
{
title: "JSDocParamHoverFunctionDeclaration",
input: `
// @filename: index.js
/**
* @param {string} param - the greatest of days
*/
function /*marker*/myFunction(param) {
return "test" + param;
}

myFunction();`,
expected: map[string]*lsproto.Hover{
"marker": {
Contents: lsproto.MarkupContentOrMarkedStringOrMarkedStrings{
MarkupContent: &lsproto.MarkupContent{
Kind: lsproto.MarkupKindMarkdown,
Value: "```tsx\nfunction myFunction(param: string): string\n```\n\n\n*@param* `param` - the greatest of days\n",
},
},
},
},
},
{
title: "JSDocParamHoverFunctionCall",
input: `
// @filename: index.js
/**
* @param {string} param - the greatest of days
*/
function myFunction(param) {
return "test" + param;
}

/*marker*/myFunction();`,
expected: map[string]*lsproto.Hover{
"marker": {
Contents: lsproto.MarkupContentOrMarkedStringOrMarkedStrings{
MarkupContent: &lsproto.MarkupContent{
Kind: lsproto.MarkupKindMarkdown,
Value: "```tsx\nfunction myFunction(param: string): string\n```\n\n\n*@param* `param` - the greatest of days\n",
},
},
},
},
},
{
title: "JSDocParamHoverParameter",
input: `
// @filename: index.js
/**
* @param {string} param - the greatest of days
*/
function myFunction(/*marker*/param) {
return "test" + param;
}

myFunction();`,
expected: map[string]*lsproto.Hover{
"marker": {
Contents: lsproto.MarkupContentOrMarkedStringOrMarkedStrings{
MarkupContent: &lsproto.MarkupContent{
Kind: lsproto.MarkupKindMarkdown,
Value: "```tsx\n(parameter) param: string\n```\n- the greatest of days\n",
},
},
},
},
},
{
title: "JSDocParamHoverTagIdentifier",
input: `
// @filename: index.js
/**
* @param {string} /*marker*/param - the greatest of days
*/
function myFunction(param) {
return "test" + param;
}

myFunction();`,
expected: map[string]*lsproto.Hover{
// TODO: Should have same result as hovering on the parameter itself.
"marker": nil,
},
},
}

for _, testCase := range testCases {
Expand Down
Loading
Loading