From 70056455925dadc7908149cfceef050eb6990781 Mon Sep 17 00:00:00 2001 From: ivanwonder Date: Fri, 3 Jan 2020 18:32:30 +0800 Subject: [PATCH] fix(language-service): break the hover/definitions for two-way binding (#34564) PR Close #34564 --- .../language-service/src/locate_symbol.ts | 27 +++++++++++--- .../language-service/test/definitions_spec.ts | 36 +++++++++++++++++++ packages/language-service/test/hover_spec.ts | 16 +++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/packages/language-service/src/locate_symbol.ts b/packages/language-service/src/locate_symbol.ts index ca2d4a7b0b..ba123ee0af 100644 --- a/packages/language-service/src/locate_symbol.ts +++ b/packages/language-service/src/locate_symbol.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, SelectorMatcher, TemplateAstPath, tokenReference} from '@angular/compiler'; +import {AST, Attribute, BoundDirectivePropertyAst, BoundEventAst, CompileTypeSummary, CssSelector, DirectiveAst, ElementAst, RecursiveTemplateAstVisitor, SelectorMatcher, TemplateAst, TemplateAstPath, templateVisitAll, tokenReference} from '@angular/compiler'; import {AstResult} from './common'; import {getExpressionScope} from './expression_diagnostics'; @@ -148,14 +148,31 @@ function findAttribute(info: AstResult, position: number): Attribute|undefined { return path.first(Attribute); } +function findParentOfDirectivePropertyAst(ast: TemplateAst[], binding: BoundDirectivePropertyAst) { + let res: DirectiveAst|undefined; + const visitor = new class extends RecursiveTemplateAstVisitor { + visitDirective(ast: DirectiveAst) { + const result = this.visitChildren(ast, visit => { visit(ast.inputs); }); + return result; + } + visitDirectiveProperty(ast: BoundDirectivePropertyAst, context: any) { + if (ast === binding) { + res = context; + } + } + }; + templateVisitAll(visitor, ast); + return res; +} + function findInputBinding( info: AstResult, path: TemplateAstPath, binding: BoundDirectivePropertyAst): Symbol|undefined { - const directive = path.parentOf(path.tail); - if (directive instanceof DirectiveAst) { - const invertedInput = invertMap(directive.directive.inputs); + const directiveAst = findParentOfDirectivePropertyAst(info.templateAst, binding); + if (directiveAst) { + const invertedInput = invertMap(directiveAst.directive.inputs); const fieldName = invertedInput[binding.templateName]; if (fieldName) { - const classSymbol = info.template.query.getTypeSymbol(directive.directive.type.reference); + const classSymbol = info.template.query.getTypeSymbol(directiveAst.directive.type.reference); if (classSymbol) { return classSymbol.members().get(fieldName); } diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 41d5ff7ea6..5564d44d7f 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -291,6 +291,42 @@ describe('definitions', () => { // Not asserting the textSpan of definition because it's external file }); + it('should be able to find a two-way binding', () => { + const fileName = mockHost.addCode(` + @Component({ + template: '' + }) + export class MyComponent { + test = ""; + }`); + // Get the marker for «model» in the code added above. + const marker = mockHost.getReferenceMarkerFor(fileName, 'model'); + + const result = ngService.getDefinitionAt(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); + + // There should be exactly 1 definition + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + const refFileName = '/app/parsing-cases.ts'; + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('model'); + expect(def.kind).toBe('property'); + const content = mockHost.readFile(refFileName) !; + const ref = `@Input() model: string = 'model';`; + expect(def.textSpan).toEqual({ + start: content.indexOf(ref), + length: ref.length, + }); + }); + it('should be able to find a template from a url', () => { const fileName = mockHost.addCode(` @Component({ diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index c08bb865cc..914be473ec 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -162,6 +162,22 @@ describe('hover', () => { expect(toText(displayParts)).toBe('(property) NgIf.ngIf: T'); }); + it('should be able to find a reference to a two-way binding', () => { + const fileName = mockHost.addCode(` + @Component({ + template: '' + }) + export class MyComponent { + test = ""; + }`); + const marker = mockHost.getDefinitionMarkerFor(fileName, 'model'); + const quickInfo = ngLS.getHoverAt(fileName, marker.start); + expect(quickInfo).toBeTruthy(); + const {textSpan, displayParts} = quickInfo !; + expect(textSpan).toEqual(marker); + expect(toText(displayParts)).toBe('(property) StringModel.model: string'); + }); + it('should be able to ignore a reference declaration', () => { const fileName = mockHost.addCode(` @Component({