Skip to content

Commit 3cee8d9

Browse files
gabrittogabritto
authored
Support Object.defineProperty in document symbols (microsoft#2592)
1 parent 12a1cfc commit 3cee8d9

2 files changed

Lines changed: 70 additions & 64 deletions

File tree

‎internal/ls/symbols.go‎

Lines changed: 62 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ func (l *LanguageService) getDocumentSymbolInformations(ctx context.Context, fil
7272
func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, node *ast.Node, file *ast.SourceFile) []*lsproto.DocumentSymbol {
7373
var symbols []*lsproto.DocumentSymbol
7474
expandoTargets := collections.Set[string]{}
75-
addSymbolForNode := func(node *ast.Node, children []*lsproto.DocumentSymbol) {
75+
addSymbolForNode := func(node *ast.Node, name *ast.Node, children []*lsproto.DocumentSymbol) {
7676
if node.Flags&ast.NodeFlagsReparsed == 0 || node.Kind == ast.KindJSExportAssignment {
77-
symbol := l.newDocumentSymbol(node, children)
77+
symbol := l.newDocumentSymbol(node, name, children)
7878
if symbol != nil {
7979
symbols = append(symbols, symbol)
8080
}
@@ -95,7 +95,7 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
9595
}
9696
return result
9797
}
98-
startNode := func(node *ast.Node) func() {
98+
startNode := func(node *ast.Node, name *ast.Node) func() {
9999
if node == nil {
100100
return func() {}
101101
}
@@ -107,7 +107,7 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
107107
result := symbols
108108
symbols = saveSymbols
109109
expandoTargets = saveExpandoTargets
110-
addSymbolForNode(node, result)
110+
addSymbolForNode(node, name, result)
111111
}
112112
}
113113
getSymbolsForNode := func(node *ast.Node) []*lsproto.DocumentSymbol {
@@ -131,7 +131,7 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
131131
if tagList := jsdoc.AsJSDoc().Tags; tagList != nil {
132132
for _, tag := range tagList.Nodes {
133133
if ast.IsJSDocTypedefTag(tag) || ast.IsJSDocCallbackTag(tag) {
134-
addSymbolForNode(tag, nil /*children*/)
134+
addSymbolForNode(tag, nil /*name*/, nil /*children*/)
135135
}
136136
}
137137
}
@@ -143,69 +143,89 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
143143
if ast.IsClassLike(node) && ast.GetDeclarationName(node) != "" {
144144
expandoTargets.Add(ast.GetDeclarationName(node))
145145
}
146-
addSymbolForNode(node, getSymbolsForChildren(node))
146+
addSymbolForNode(node, nil /*name*/, getSymbolsForChildren(node))
147147
case ast.KindModuleDeclaration:
148-
addSymbolForNode(node, getSymbolsForChildren(getInteriorModule(node)))
148+
addSymbolForNode(node, nil /*name*/, getSymbolsForChildren(getInteriorModule(node)))
149149
case ast.KindConstructor:
150-
addSymbolForNode(node, getSymbolsForChildren(node.Body()))
150+
addSymbolForNode(node, nil /*name*/, getSymbolsForChildren(node.Body()))
151151
for _, param := range node.Parameters() {
152152
if ast.IsParameterPropertyDeclaration(param, node) {
153-
addSymbolForNode(param, nil /*children*/)
153+
addSymbolForNode(param, nil /*name*/, nil /*children*/)
154154
}
155155
}
156156
case ast.KindFunctionDeclaration, ast.KindFunctionExpression, ast.KindArrowFunction, ast.KindMethodDeclaration, ast.KindGetAccessor,
157157
ast.KindSetAccessor:
158-
name := ast.GetDeclarationName(node)
159-
if name != "" {
160-
expandoTargets.Add(name)
158+
declName := ast.GetDeclarationName(node)
159+
if declName != "" {
160+
expandoTargets.Add(declName)
161161
}
162-
addSymbolForNode(node, getSymbolsForChildren(node.Body()))
162+
addSymbolForNode(node, nil /*name*/, getSymbolsForChildren(node.Body()))
163163
case ast.KindVariableDeclaration, ast.KindBindingElement, ast.KindPropertyAssignment, ast.KindPropertyDeclaration:
164-
name := node.Name()
165-
if name != nil {
166-
if ast.IsBindingPattern(name) {
167-
visit(name)
164+
nodeName := node.Name()
165+
if nodeName != nil {
166+
if ast.IsBindingPattern(nodeName) {
167+
visit(nodeName)
168168
} else {
169-
addSymbolForNode(node, getSymbolsForChildren(node.Initializer()))
169+
addSymbolForNode(node, nil /*name*/, getSymbolsForChildren(node.Initializer()))
170170
}
171171
}
172172
case ast.KindSpreadAssignment:
173-
addSymbolForNode(node, nil /*children*/)
173+
addSymbolForNode(node, node.Expression(), nil /*children*/)
174174
case ast.KindMethodSignature, ast.KindPropertySignature, ast.KindCallSignature, ast.KindConstructSignature, ast.KindIndexSignature,
175175
ast.KindEnumMember, ast.KindShorthandPropertyAssignment, ast.KindTypeAliasDeclaration, ast.KindImportEqualsDeclaration, ast.KindExportSpecifier:
176-
addSymbolForNode(node, nil)
176+
addSymbolForNode(node, nil /*name*/, nil /*children*/)
177177
case ast.KindImportClause:
178178
// Handle default import case e.g.:
179179
// import d from "mod";
180180
if node.Name() != nil {
181-
addSymbolForNode(node.Name(), nil /*children*/)
181+
addSymbolForNode(node.Name(), node.Name(), nil /*children*/)
182182
}
183183
// Handle named bindings in imports e.g.:
184184
// import * as NS from "mod";
185185
// import {a, b as B} from "mod";
186186
if namedBindings := node.AsImportClause().NamedBindings; namedBindings != nil {
187187
if namedBindings.Kind == ast.KindNamespaceImport {
188-
addSymbolForNode(namedBindings, nil /*children*/)
188+
addSymbolForNode(namedBindings, nil /*name*/, nil /*children*/)
189189
} else {
190190
for _, element := range namedBindings.Elements() {
191-
addSymbolForNode(element, nil /*children*/)
191+
addSymbolForNode(element, nil /*name*/, nil /*children*/)
192192
}
193193
}
194194
}
195-
case ast.KindBinaryExpression:
196-
binaryExpr := node.AsBinaryExpression()
195+
case ast.KindBinaryExpression, ast.KindCallExpression:
197196
assignmentKind := ast.GetAssignmentDeclarationKind(node)
198197
switch assignmentKind {
199198
// `module.exports = ...`` should be reparsed into a JSExportAssignment,
200199
// and `exports.a = ...`` into a CommonJSExport.
201200
case ast.JSDeclarationKindNone, ast.JSDeclarationKindThisProperty,
202-
ast.JSDeclarationKindModuleExports, ast.JSDeclarationKindExportsProperty:
201+
ast.JSDeclarationKindModuleExports, ast.JSDeclarationKindExportsProperty,
202+
ast.JSDeclarationKindObjectDefinePropertyExports:
203203
node.ForEachChild(visit)
204-
case ast.JSDeclarationKindProperty:
204+
case ast.JSDeclarationKindProperty, ast.JSDeclarationKindObjectDefinePropertyValue:
205+
var target *ast.Expression
206+
var targetFunction *ast.Expression
207+
var definition *ast.Node
208+
var propertyName *ast.Node
205209
// `A.b = ... ` or `A.prototype.b = ...`
206-
target := binaryExpr.Left
207-
targetFunction := target.Expression()
208-
if isPrototypeExpando(binaryExpr) {
210+
if ast.IsBinaryExpression(node) {
211+
binaryExpr := node.AsBinaryExpression()
212+
target = binaryExpr.Left
213+
targetFunction = target.Expression()
214+
definition = binaryExpr.Right
215+
// `A.b` or `A.prototype.b`
216+
if ast.IsPropertyAccessExpression(target) {
217+
propertyName = target.AsPropertyAccessExpression().Name()
218+
} else { // `A["b"]` or `A.prototype["b"]`
219+
propertyName = target.AsElementAccessExpression().ArgumentExpression
220+
}
221+
} else { // `Object.defineProperty(A, "b", {...})`
222+
args := node.Arguments()
223+
targetFunction = args[0]
224+
target = args[1]
225+
propertyName = target
226+
definition = args[2]
227+
}
228+
if isPrototypeExpando(targetFunction) {
209229
targetFunction = targetFunction.Expression()
210230
// If we see a prototype assignment, start tracking the target as an expando target.
211231
if ast.IsIdentifier(targetFunction) {
@@ -214,16 +234,16 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
214234
}
215235
if ast.IsIdentifier(targetFunction) &&
216236
expandoTargets.Has(targetFunction.Text()) {
217-
endNode := startNode(node)
218-
addSymbolForNode(target, getSymbolsForNode(binaryExpr.Right))
237+
endNode := startNode(node, targetFunction)
238+
addSymbolForNode(target, propertyName, getSymbolsForNode(definition))
219239
endNode()
220240
} else {
221241
node.ForEachChild(visit)
222242
}
223243
}
224244
case ast.KindExportAssignment, ast.KindJSExportAssignment:
225245
if node.AsExportAssignment().IsExportEquals {
226-
addSymbolForNode(node, getSymbolsForNode(node.Expression()))
246+
addSymbolForNode(node, nil /*name*/, getSymbolsForNode(node.Expression()))
227247
} else {
228248
node.ForEachChild(visit)
229249
}
@@ -236,9 +256,8 @@ func (l *LanguageService) getDocumentSymbolsForChildren(ctx context.Context, nod
236256
return mergeExpandos(symbols)
237257
}
238258

239-
// Binary expression is `f.prototype.prop`.
240-
func isPrototypeExpando(binaryExpr *ast.BinaryExpression) bool {
241-
target := binaryExpr.Left.Expression()
259+
// Target is `f.prototype`.
260+
func isPrototypeExpando(target *ast.Node) bool {
242261
if ast.IsAccessExpression(target) {
243262
accessName := ast.GetElementOrPropertyAccessName(target)
244263
return accessName != nil && accessName.Text() == "prototype"
@@ -248,29 +267,11 @@ func isPrototypeExpando(binaryExpr *ast.BinaryExpression) bool {
248267

249268
const maxLength = 150
250269

251-
func (l *LanguageService) newDocumentSymbol(node *ast.Node, children []*lsproto.DocumentSymbol) *lsproto.DocumentSymbol {
270+
func (l *LanguageService) newDocumentSymbol(node *ast.Node, name *ast.Node, children []*lsproto.DocumentSymbol) *lsproto.DocumentSymbol {
252271
result := new(lsproto.DocumentSymbol)
253272
file := ast.GetSourceFileOfNode(node)
254273
nodeStartPos := scanner.SkipTrivia(file.Text(), node.Pos())
255-
var name *ast.Node
256-
// Expando properties
257-
if ast.IsBinaryExpression(node) {
258-
if isPrototypeExpando(node.AsBinaryExpression()) { // `f.prototype.prop = ...`
259-
name = node.AsBinaryExpression().Left.Expression().Expression()
260-
} else { // `f[prop] = ...`
261-
name = node.AsBinaryExpression().Left.Expression()
262-
}
263-
} else if ast.IsAccessExpression(node) {
264-
if ast.IsPropertyAccessExpression(node) {
265-
name = node.AsPropertyAccessExpression().Name()
266-
} else if ast.IsElementAccessExpression(node) {
267-
name = node.AsElementAccessExpression().ArgumentExpression
268-
}
269-
} else if ast.IsIdentifier(node) || ast.IsPrivateIdentifier(node) {
270-
name = node
271-
} else if ast.IsSpreadAssignment(node) && ast.IsIdentifier(node.Expression()) {
272-
name = node.Expression()
273-
} else {
274+
if name == nil {
274275
name = ast.GetNameOfDeclaration(node)
275276
}
276277
var text string
@@ -673,12 +674,15 @@ func getSymbolKindFromNode(node *ast.Node) lsproto.SymbolKind {
673674
return lsproto.SymbolKindProperty
674675
}
675676
return lsproto.SymbolKindVariable
676-
case ast.KindBinaryExpression:
677+
case ast.KindBinaryExpression, ast.KindCallExpression:
677678
kind := ast.GetAssignmentDeclarationKind(node)
678679
switch kind {
679-
case ast.JSDeclarationKindThisProperty, ast.JSDeclarationKindProperty:
680+
case ast.JSDeclarationKindThisProperty, ast.JSDeclarationKindProperty, ast.JSDeclarationKindObjectDefinePropertyValue:
680681
return lsproto.SymbolKindProperty
681682
}
683+
case ast.KindStringLiteral, ast.KindNoSubstitutionTemplateLiteral, ast.KindNumericLiteral:
684+
// String literals used as property names (e.g., in Object.defineProperty)
685+
return lsproto.SymbolKindProperty
682686
}
683687
return lsproto.SymbolKindVariable
684688
}

‎testdata/baselines/reference/fourslash/documentSymbols/navigationBarFunctionPrototype.baseline‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
// <|f.prototype.[|{| name: x, kind: Variable |}x|]|> = 0;
55
// <|f.[|{| name: y, kind: Variable |}y|]|> = 0;
66
// <|f.prototype.[|{| name: method, kind: Variable |}method|]|> = <|[|{| name: method, kind: Function |}|]function () {}|>;
7-
// Object.defineProperty(f, 'staticProp', {
7+
// Object.defineProperty(f, <|[|{| name: "staticProp", kind: Property |}'staticProp'|]|>, {
88
// <|[|{| name: set, kind: Property |}set|]: function() {}|>,
99
// <|[|{| name: get, kind: Property |}get|]: function(){
1010
// }|>
1111
// });
12-
// Object.defineProperty(f.prototype, 'name', {
12+
// Object.defineProperty(f.prototype, <|[|{| name: "name", kind: Property |}'name'|]|>, {
1313
// <|[|{| name: set, kind: Property |}set|]: function() {}|>,
1414
// <|[|{| name: get, kind: Property |}get|]: function(){
1515
// }|>
@@ -21,7 +21,9 @@
2121
(Variable) y
2222
(Variable) method
2323
(Function) method
24-
(Property) set
25-
(Property) get
26-
(Property) set
27-
(Property) get
24+
(Property) "staticProp"
25+
(Property) set
26+
(Property) get
27+
(Property) "name"
28+
(Property) set
29+
(Property) get

0 commit comments

Comments
 (0)