From 406419bc0fd5075ea5be33dd974b9b7caa6571c8 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Mon, 9 Mar 2020 22:16:08 -0700 Subject: [PATCH] fix(language-service): fix calculation of pipe spans (#35986) This commit accomplishes two tasks: - Fixes the span of queried pipes to only be applied on pipe names - By consequence, fixes how pipes are located in arguments (previously, pipes with arguments could not be found because the span of a pipe uses a relative span, while the template position is absolute) The screenshots attached to the PR for this commit demonstrate the change. Closes https://github.com/angular/vscode-ng-language-service/issues/677 PR Close #35986 --- packages/language-service/src/expressions.ts | 25 ++++++++++---- .../language-service/test/definitions_spec.ts | 33 +++++++++++++++---- packages/language-service/test/hover_spec.ts | 13 ++++++++ .../test/project/app/parsing-cases.ts | 1 + 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index 570522ac73..03df9d4ba7 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -96,6 +96,14 @@ export function getExpressionCompletions( return result && result.values(); } +/** + * Retrieves the expression symbol at a particular position in a template. + * + * @param scope symbols in scope of the template + * @param ast template AST + * @param position absolute location in template to retrieve symbol at + * @param query type symbol query for the template scope + */ export function getExpressionSymbol( scope: SymbolTable, ast: AST, position: number, query: SymbolQuery): {symbol: Symbol, span: Span}|undefined { @@ -129,14 +137,19 @@ export function getExpressionSymbol( span = ast.span; }, visitPipe(ast) { - if (position >= ast.exp.span.end && - (!ast.args || !ast.args.length || position < (ast.args[0]).span.start)) { + if (inSpan(position, ast.nameSpan, /* exclusive */ true)) { // We are in a position a pipe name is expected. const pipes = query.getPipes(); - if (pipes) { - symbol = pipes.get(ast.name); - span = ast.span; - } + symbol = pipes.get(ast.name); + + // `nameSpan` is an absolute span, but the span expected by the result of this method is + // relative to the start of the expression. + // TODO(ayazhafiz): migrate to only using absolute spans + const offset = ast.sourceSpan.start - ast.span.start; + span = { + start: ast.nameSpan.start - offset, + end: ast.nameSpan.end - offset, + }; } }, visitPrefixNot(ast) {}, diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 6f989d03c9..43bdf3e561 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -234,22 +234,19 @@ describe('definitions', () => { it('should be able to find a pipe', () => { const fileName = mockHost.addCode(` @Component({ - template: '
' + template: '
' }) export class MyComponent { input: EventEmitter; }`); - // Get the marker for «test» in the code added above. + // Get the marker for «async» in the code added above. const marker = mockHost.getReferenceMarkerFor(fileName, 'async'); - const result = ngService.getDefinitionAndBoundSpan(fileName, marker.start); + expect(result).toBeDefined(); const {textSpan, definitions} = result !; - - // Get the marker for bounded text in the code added above - const boundedText = mockHost.getLocationMarkerFor(fileName, 'my'); - expect(textSpan).toEqual(boundedText); + expect(textSpan).toEqual(marker); expect(definitions).toBeDefined(); expect(definitions !.length).toBe(4); @@ -263,6 +260,28 @@ describe('definitions', () => { } }); + // https://github.com/angular/vscode-ng-language-service/issues/677 + it('should be able to find a pipe with arguments', () => { + mockHost.override(TEST_TEMPLATE, `{{birthday | «date»: "MM/dd/yy"}}`); + + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date'); + const result = ngService.getDefinitionAndBoundSpan(TEST_TEMPLATE, marker.start); + + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + expect(textSpan).toEqual(marker); + + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + + const refFileName = '/node_modules/@angular/common/common.d.ts'; + for (const def of definitions !) { + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('date'); + expect(def.kind).toBe('pipe'); + } + }); + describe('in structural directive', () => { it('should be able to find the directive', () => { mockHost.override( diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index 593fba3c62..9f144a6624 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -138,6 +138,19 @@ describe('hover', () => { expect(toText(displayParts)).toBe('(property) TemplateReference.heroes: Hero[]'); }); + it('should work for pipes', () => { + mockHost.override(TEST_TEMPLATE, ` +

The hero's birthday is {{birthday | «date»: "MM/dd/yy"}}

`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'date'); + const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); + expect(quickInfo).toBeTruthy(); + const {textSpan, displayParts} = quickInfo !; + expect(textSpan).toEqual(marker); + expect(toText(displayParts)) + .toBe( + '(pipe) date: (value: any, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined) => string | null'); + }); + it('should work for the $any() cast function', () => { const content = mockHost.override(TEST_TEMPLATE, '
{{$any(title)}}
'); const position = content.indexOf('$any'); diff --git a/packages/language-service/test/project/app/parsing-cases.ts b/packages/language-service/test/project/app/parsing-cases.ts index 988aa6c5a4..949c786621 100644 --- a/packages/language-service/test/project/app/parsing-cases.ts +++ b/packages/language-service/test/project/app/parsing-cases.ts @@ -171,6 +171,7 @@ export class TemplateReference { anyValue: any; optional?: string; myClick(event: any) {} + birthday = new Date(); } @Component({