Skip to content

Commit

Permalink
feat(language-service): Append symbol type to hover tooltip (#34515)
Browse files Browse the repository at this point in the history
Now that #34177 fixed the `TypeWrapper`
to have a proper name, we have the information needed to show the type
name in a hover tooltip.

PR Close #34515
  • Loading branch information
kyliau authored and alxhub committed Dec 20, 2019
1 parent ca8b584 commit 381b895
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 14 deletions.
2 changes: 1 addition & 1 deletion integration/language_service_plugin/goldens/quickinfo.json
Expand Up @@ -15,7 +15,7 @@
"line": 5, "line": 5,
"offset": 30 "offset": 30
}, },
"displayString": "(property) AppComponent.name", "displayString": "(property) AppComponent.name: string",
"documentation": "", "documentation": "",
"tags": [] "tags": []
} }
Expand Down
20 changes: 15 additions & 5 deletions packages/language-service/src/hover.ts
Expand Up @@ -18,6 +18,7 @@ const SYMBOL_SPACE = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.space];
const SYMBOL_PUNC = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.punctuation]; const SYMBOL_PUNC = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.punctuation];
const SYMBOL_CLASS = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.className]; const SYMBOL_CLASS = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.className];
const SYMBOL_TEXT = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.text]; const SYMBOL_TEXT = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.text];
const SYMBOL_INTERFACE = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.interfaceName];


/** /**
* Traverse the template AST and look for the symbol located at `position`, then * Traverse the template AST and look for the symbol located at `position`, then
Expand Down Expand Up @@ -45,19 +46,28 @@ export function getHover(info: AstResult, position: number, host: Readonly<TypeS
{text: '.', kind: SYMBOL_PUNC}, {text: '.', kind: SYMBOL_PUNC},
] : ] :
[]; [];
const typeDisplayParts: ts.SymbolDisplayPart[] = symbol.type ?
[
{text: ':', kind: SYMBOL_PUNC},
{text: ' ', kind: SYMBOL_SPACE},
{text: symbol.type.name, kind: SYMBOL_INTERFACE},
] :
[];
return { return {
kind: symbol.kind as ts.ScriptElementKind, kind: symbol.kind as ts.ScriptElementKind,
kindModifiers: '', // kindModifier info not available on 'ng.Symbol' kindModifiers: '', // kindModifier info not available on 'ng.Symbol'
textSpan, textSpan,
// this would generate a string like '(property) ClassX.propY' // this would generate a string like '(property) ClassX.propY: type'
// 'kind' in displayParts does not really matter because it's dropped when // 'kind' in displayParts does not really matter because it's dropped when
// displayParts get converted to string. // displayParts get converted to string.
displayParts: [ displayParts: [
{text: '(', kind: SYMBOL_PUNC}, {text: symbol.kind, kind: symbol.kind}, {text: '(', kind: SYMBOL_PUNC},
{text: ')', kind: SYMBOL_PUNC}, {text: ' ', kind: SYMBOL_SPACE}, ...containerDisplayParts, {text: symbol.kind, kind: symbol.kind},
{text: ')', kind: SYMBOL_PUNC},
{text: ' ', kind: SYMBOL_SPACE},
...containerDisplayParts,
{text: symbol.name, kind: symbol.kind}, {text: symbol.name, kind: symbol.kind},
// TODO: Append type info as well, but Symbol doesn't expose that! ...typeDisplayParts,
// Ideally hover text should be like '(property) ClassX.propY: string'
], ],
}; };
} }
Expand Down
16 changes: 8 additions & 8 deletions packages/language-service/test/hover_spec.ts
Expand Up @@ -36,7 +36,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) MyComponent.name'); expect(toText(displayParts)).toBe('(property) MyComponent.name: string');
}); });


it('should be able to find a field in a attribute reference', () => { it('should be able to find a field in a attribute reference', () => {
Expand All @@ -52,7 +52,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) MyComponent.name'); expect(toText(displayParts)).toBe('(property) MyComponent.name: string');
}); });


it('should be able to find a method from a call', () => { it('should be able to find a method from a call', () => {
Expand All @@ -69,7 +69,7 @@ describe('hover', () => {
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(textSpan.length).toBe('myClick()'.length); expect(textSpan.length).toBe('myClick()'.length);
expect(toText(displayParts)).toBe('(method) MyComponent.myClick'); expect(toText(displayParts)).toBe('(method) MyComponent.myClick: () => void');
}); });


it('should be able to find a field reference in an *ngIf', () => { it('should be able to find a field reference in an *ngIf', () => {
Expand All @@ -85,7 +85,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) MyComponent.include'); expect(toText(displayParts)).toBe('(property) MyComponent.include: boolean');
}); });


it('should be able to find a reference to a component', () => { it('should be able to find a reference to a component', () => {
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(directive) StringModel'); expect(toText(displayParts)).toBe('(directive) StringModel: typeof StringModel');
}); });


it('should be able to find an event provider', () => { it('should be able to find an event provider', () => {
Expand All @@ -129,7 +129,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(event) TestComponent.testEvent'); expect(toText(displayParts)).toBe('(event) TestComponent.testEvent: EventEmitter<any>');
}); });


it('should be able to find an input provider', () => { it('should be able to find an input provider', () => {
Expand All @@ -145,7 +145,7 @@ describe('hover', () => {
expect(quickInfo).toBeTruthy(); expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !; const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker); expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) TestComponent.name'); expect(toText(displayParts)).toBe('(property) TestComponent.name: string');
}); });


it('should be able to ignore a reference declaration', () => { it('should be able to ignore a reference declaration', () => {
Expand Down Expand Up @@ -203,7 +203,7 @@ describe('hover', () => {
start: position, start: position,
length: '$any(title)'.length, length: '$any(title)'.length,
}); });
expect(toText(displayParts)).toBe('(method) $any'); expect(toText(displayParts)).toBe('(method) $any: $any');
}); });
}); });


Expand Down

0 comments on commit 381b895

Please sign in to comment.