Skip to content

Commit 82c3710

Browse files
Add definitions and references for constructors (#250)
* WIP - hacking on adding constructor symbols * Handle constructors properly * Address reviews, consolidate code * Format code * Fix constructorless classes * Fix yarn issues, mention yarn version --------- Co-authored-by: Ólafur Páll Geirsson <[email protected]>
1 parent fdd66f0 commit 82c3710

File tree

8 files changed

+193
-16
lines changed

8 files changed

+193
-16
lines changed

Development.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Developing scip-typescript
22

3+
Please note that the yarn version used by CI is `v1.22.19` - you should use this version as well to prevent lockfile conflicts.
4+
35
## References
46

57
- VS Code is a good reference point for the "correct" behavior
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
namespace Yay {
2+
export class SuperConstructor {
3+
constructor(public readonly property: number) {}
4+
}
5+
6+
export namespace Woo {
7+
export class MyClass {
8+
constructor() {}
9+
}
10+
}
11+
}
12+
13+
export class SuperConstructor2 {
14+
constructor(public readonly property: number) {}
15+
}
16+
17+
export function useConstructor(): Yay.SuperConstructor {
18+
return new Yay.SuperConstructor(10)
19+
}
20+
21+
export function useConstructor2(): SuperConstructor2 {
22+
return new SuperConstructor2(10)
23+
}
24+
25+
export function useConstructor3(): Yay.Woo.MyClass {
26+
return new Yay.Woo.MyClass()
27+
}
28+
29+
export class NoConstructor {
30+
property: number
31+
}
32+
33+
export function useNoConstructor() {
34+
return new NoConstructor()
35+
}

snapshots/output/syntax/src/class.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
// ^^^^^^^^^^^^^ definition syntax 1.0.0 src/`class.ts`/Class#classProperty.
88
// documentation ```ts\n(property) classProperty: string\n```
99
constructor(constructorParam: string) {
10+
// ^^^^^^^^^^^ definition syntax 1.0.0 src/`class.ts`/Class#`<constructor>`().
11+
// documentation ```ts\nconstructor(constructorParam: string): Class\n```
1012
// ^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`class.ts`/Class#`<constructor>`().(constructorParam)
1113
// documentation ```ts\n(parameter) constructorParam: string\n```
1214
this.classProperty = constructorParam
@@ -48,7 +50,7 @@
4850
const instance = new Class(param).classProperty
4951
// ^^^^^^^^ definition local 2
5052
// documentation ```ts\nvar instance: string\n```
51-
// ^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#
53+
// ^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#`<constructor>`().
5254
// ^^^^^ reference syntax 1.0.0 src/`class.ts`/newClass().(param)
5355
// ^^^^^^^^^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#classProperty.
5456
const instance2 = Class.staticMethod(param)
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
namespace Yay {
2+
// definition syntax 1.0.0 src/`constructor.ts`/
3+
//documentation ```ts\nmodule "constructor.ts"\n```
4+
// ^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/
5+
// documentation ```ts\nYay: typeof Yay\n```
6+
export class SuperConstructor {
7+
// ^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/SuperConstructor#
8+
// documentation ```ts\nclass SuperConstructor\n```
9+
constructor(public readonly property: number) {}
10+
// ^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/SuperConstructor#`<constructor>`().
11+
// documentation ```ts\nconstructor(property: number): SuperConstructor\n```
12+
// ^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/SuperConstructor#`<constructor>`().(property)
13+
// documentation ```ts\n(property) property: number\n```
14+
}
15+
16+
export namespace Woo {
17+
// ^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/Woo/
18+
// documentation ```ts\nWoo: typeof Woo\n```
19+
export class MyClass {
20+
// ^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/Woo/MyClass#
21+
// documentation ```ts\nclass MyClass\n```
22+
constructor() {}
23+
// ^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/Yay/Woo/MyClass#`<constructor>`().
24+
// documentation ```ts\nconstructor(): MyClass\n```
25+
}
26+
}
27+
}
28+
29+
export class SuperConstructor2 {
30+
// ^^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/SuperConstructor2#
31+
// documentation ```ts\nclass SuperConstructor2\n```
32+
constructor(public readonly property: number) {}
33+
// ^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/SuperConstructor2#`<constructor>`().
34+
// documentation ```ts\nconstructor(property: number): SuperConstructor2\n```
35+
// ^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/SuperConstructor2#`<constructor>`().(property)
36+
// documentation ```ts\n(property) property: number\n```
37+
}
38+
39+
export function useConstructor(): Yay.SuperConstructor {
40+
// ^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/useConstructor().
41+
// documentation ```ts\nfunction useConstructor(): SuperConstructor\n```
42+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/
43+
// ^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/SuperConstructor#
44+
return new Yay.SuperConstructor(10)
45+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/
46+
// ^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/SuperConstructor#`<constructor>`().
47+
}
48+
49+
export function useConstructor2(): SuperConstructor2 {
50+
// ^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/useConstructor2().
51+
// documentation ```ts\nfunction useConstructor2(): SuperConstructor2\n```
52+
// ^^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/SuperConstructor2#
53+
return new SuperConstructor2(10)
54+
// ^^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/SuperConstructor2#`<constructor>`().
55+
}
56+
57+
export function useConstructor3(): Yay.Woo.MyClass {
58+
// ^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/useConstructor3().
59+
// documentation ```ts\nfunction useConstructor3(): MyClass\n```
60+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/
61+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/Woo/
62+
// ^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/Woo/MyClass#
63+
return new Yay.Woo.MyClass()
64+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/
65+
// ^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/Woo/
66+
// ^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/Yay/Woo/MyClass#`<constructor>`().
67+
}
68+
69+
export class NoConstructor {
70+
// ^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/NoConstructor#
71+
// documentation ```ts\nclass NoConstructor\n```
72+
property: number
73+
// ^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/NoConstructor#property.
74+
// documentation ```ts\n(property) property: number\n```
75+
}
76+
77+
export function useNoConstructor() {
78+
// ^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`constructor.ts`/useNoConstructor().
79+
// documentation ```ts\nfunction useNoConstructor(): NoConstructor\n```
80+
return new NoConstructor()
81+
// ^^^^^^^^^^^^^ reference syntax 1.0.0 src/`constructor.ts`/NoConstructor#
82+
}
83+

snapshots/output/syntax/src/import.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
// documentation ```ts\nfunction useEverything(): string\n```
2323
return (
2424
new Class('a').classProperty +
25-
// ^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#
25+
// ^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#`<constructor>`().
2626
// ^^^^^^^^^^^^^ reference syntax 1.0.0 src/`class.ts`/Class#classProperty.
2727
renamedInterface().methodSignature('a') +
2828
// ^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`interface.ts`/newInterface().

src/FileIndexer.ts

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ export class FileIndexer {
3131
public readonly input: Input,
3232
public readonly document: scip.scip.Document,
3333
public readonly globalSymbolTable: Map<ts.Node, ScipSymbol>,
34+
public readonly globalConstructorTable: Map<ts.ClassDeclaration, boolean>,
3435
public readonly packages: Packages,
3536
public readonly sourceFile: ts.SourceFile
3637
) {
3738
this.workingDirectoryRegExp = new RegExp(options.cwd, 'g')
3839
}
3940
public index(): void {
4041
// Uncomment below if you want to skip certain files for local development.
41-
// if (!this.sourceFile.fileName.includes('infer-relationship')) {
42+
// if (!this.sourceFile.fileName.includes('constructor')) {
4243
// return
4344
// }
4445
this.emitSourceFileOccurrence()
@@ -67,6 +68,7 @@ export class FileIndexer {
6768
}
6869
private visit(node: ts.Node): void {
6970
if (
71+
ts.isConstructorDeclaration(node) ||
7072
ts.isIdentifier(node) ||
7173
ts.isPrivateIdentifier(node) ||
7274
ts.isStringLiteralLike(node)
@@ -76,6 +78,7 @@ export class FileIndexer {
7678
this.visitSymbolOccurrence(node, sym)
7779
}
7880
}
81+
7982
ts.forEachChild(node, node => this.visit(node))
8083
}
8184

@@ -84,7 +87,10 @@ export class FileIndexer {
8487
//
8588
// This code is directly based off src/services/goToDefinition.ts.
8689
private getTSSymbolAtLocation(node: ts.Node): ts.Symbol | undefined {
87-
const symbol = this.checker.getSymbolAtLocation(node)
90+
const rangeNode: ts.Node = ts.isConstructorDeclaration(node)
91+
? node.getFirstToken() ?? node
92+
: node
93+
const symbol = this.checker.getSymbolAtLocation(rangeNode)
8894

8995
// If this is an alias, and the request came at the declaration location
9096
// get the aliased symbol instead. This allows for goto def on an import e.g.
@@ -105,14 +111,33 @@ export class FileIndexer {
105111
return symbol
106112
}
107113

114+
private hasConstructor(classDeclaration: ts.ClassDeclaration): boolean {
115+
const cached = this.globalConstructorTable.get(classDeclaration)
116+
if (cached !== undefined) {
117+
return cached
118+
}
119+
120+
for (const member of classDeclaration.members) {
121+
if (ts.isConstructorDeclaration(member)) {
122+
this.globalConstructorTable.set(classDeclaration, true)
123+
return true
124+
}
125+
}
126+
127+
this.globalConstructorTable.set(classDeclaration, false)
128+
return false
129+
}
130+
108131
private visitSymbolOccurrence(node: ts.Node, sym: ts.Symbol): void {
109132
const range = Range.fromNode(node).toLsif()
110133
let role = 0
111134
const isDefinitionNode = isDefinition(node)
112135
if (isDefinitionNode) {
113136
role |= scip.scip.SymbolRole.Definition
114137
}
115-
const declarations = isDefinitionNode
138+
const declarations = ts.isConstructorDeclaration(node)
139+
? [node]
140+
: isDefinitionNode
116141
? // Don't emit ambiguous definition at definition-site. You can reproduce
117142
// ambiguous results by triggering "Go to definition" in VS Code on `Conflict`
118143
// in the example below:
@@ -123,7 +148,20 @@ export class FileIndexer {
123148
[node.parent]
124149
: sym?.declarations || []
125150
for (const declaration of declarations) {
126-
const scipSymbol = this.scipSymbol(declaration)
151+
let scipSymbol = this.scipSymbol(declaration)
152+
153+
if (
154+
((ts.isIdentifier(node) && ts.isNewExpression(node.parent)) ||
155+
(ts.isPropertyAccessExpression(node.parent) &&
156+
ts.isNewExpression(node.parent.parent))) &&
157+
ts.isClassDeclaration(declaration) &&
158+
this.hasConstructor(declaration)
159+
) {
160+
scipSymbol = ScipSymbol.global(
161+
scipSymbol,
162+
methodDescriptor('<constructor>')
163+
)
164+
}
127165

128166
if (scipSymbol.isEmpty()) {
129167
// Skip empty symbols
@@ -474,17 +512,24 @@ export class FileIndexer {
474512
const kind = scriptElementKind(node, sym)
475513
const type = (): string =>
476514
this.checker.typeToString(this.checker.getTypeAtLocation(node))
477-
const signature = (): string | undefined => {
515+
const asSignatureDeclaration = (
516+
node: ts.Node,
517+
sym: ts.Symbol
518+
): ts.SignatureDeclaration | undefined => {
478519
const declaration = sym.declarations?.[0]
479520
if (!declaration) {
480521
return undefined
481522
}
482-
const signatureDeclaration: ts.SignatureDeclaration | undefined =
483-
ts.isFunctionDeclaration(declaration)
484-
? declaration
485-
: ts.isMethodDeclaration(declaration)
486-
? declaration
487-
: undefined
523+
return ts.isConstructorDeclaration(node)
524+
? node
525+
: ts.isFunctionDeclaration(declaration)
526+
? declaration
527+
: ts.isMethodDeclaration(declaration)
528+
? declaration
529+
: undefined
530+
}
531+
const signature = (): string | undefined => {
532+
const signatureDeclaration = asSignatureDeclaration(node, sym)
488533
if (!signatureDeclaration) {
489534
return undefined
490535
}
@@ -508,6 +553,9 @@ export class FileIndexer {
508553
return 'type ' + node.getText()
509554
case ts.ScriptElementKind.classElement:
510555
case ts.ScriptElementKind.localClassElement:
556+
if (ts.isConstructorDeclaration(node)) {
557+
return 'constructor' + (signature() || '')
558+
}
511559
return 'class ' + node.getText()
512560
case ts.ScriptElementKind.interfaceElement:
513561
return 'interface ' + node.getText()
@@ -769,5 +817,7 @@ function declarationName(node: ts.Node): ts.Node | undefined {
769817
* ^^^^^^^^^^^^^^^^^^^^^ node.parent
770818
*/
771819
function isDefinition(node: ts.Node): boolean {
772-
return declarationName(node.parent) === node
820+
return (
821+
declarationName(node.parent) === node || ts.isConstructorDeclaration(node)
822+
)
773823
}

src/ProjectIndexer.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export class ProjectIndexer {
7272
private program: ts.Program
7373
private checker: ts.TypeChecker
7474
private symbolCache: Map<ts.Node, ScipSymbol> = new Map()
75+
private hasConstructor: Map<ts.ClassDeclaration, boolean> = new Map()
7576
private packages: Packages
7677
constructor(
7778
public readonly config: ts.ParsedCommandLine,
@@ -140,6 +141,7 @@ export class ProjectIndexer {
140141
input,
141142
document,
142143
this.symbolCache,
144+
this.hasConstructor,
143145
this.packages,
144146
sourceFile
145147
)

src/Range.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ export class Range {
3232
}
3333
public static fromNode(node: ts.Node): Range {
3434
const sourceFile = node.getSourceFile()
35-
const start = sourceFile.getLineAndCharacterOfPosition(node.getStart())
36-
const end = sourceFile.getLineAndCharacterOfPosition(node.getEnd())
35+
const rangeNode: ts.Node = ts.isConstructorDeclaration(node)
36+
? node.getFirstToken() ?? node
37+
: node
38+
const start = sourceFile.getLineAndCharacterOfPosition(rangeNode.getStart())
39+
const end = sourceFile.getLineAndCharacterOfPosition(rangeNode.getEnd())
3740
return new Range(
3841
new Position(start.line, start.character),
3942
new Position(end.line, end.character)

0 commit comments

Comments
 (0)