From ebb7ac5979b09edf9c56c7f4b143c220af2f1e9d Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 17 Dec 2020 14:43:53 -0800 Subject: [PATCH] fix(language-service): Support 'find references' for two-way bindings (#40185) Rather than expecting that a position in a template only targets a single node, this commit simply adjusts the approach to account for two way bindings. Specifically, we attempt to get references for each targeted node and then return the combination of all results, or `undefined` if none of the target nodes had references. PR Close #40185 --- packages/language-service/ivy/references.ts | 144 ++++++++++-------- .../ivy/test/references_spec.ts | 31 ++++ 2 files changed, 113 insertions(+), 62 deletions(-) diff --git a/packages/language-service/ivy/references.ts b/packages/language-service/ivy/references.ts index 81ba73a9c5..265a8c27eb 100644 --- a/packages/language-service/ivy/references.ts +++ b/packages/language-service/ivy/references.ts @@ -39,72 +39,92 @@ export class ReferenceBuilder { return undefined; } - const node = positionDetails.context.kind === TargetNodeKind.TwoWayBindingContext ? - positionDetails.context.nodes[0] : - positionDetails.context.node; + const nodes = positionDetails.context.kind === TargetNodeKind.TwoWayBindingContext ? + positionDetails.context.nodes : + [positionDetails.context.node]; - // Get the information about the TCB at the template position. - const symbol = this.ttc.getSymbolOfNode(node, component); - if (symbol === null) { + const references: ts.ReferenceEntry[] = []; + for (const node of nodes) { + // Get the information about the TCB at the template position. + const symbol = this.ttc.getSymbolOfNode(node, component); + if (symbol === null) { + continue; + } + + switch (symbol.kind) { + case SymbolKind.Directive: + case SymbolKind.Template: + // References to elements, templates, and directives will be through template references + // (#ref). They shouldn't be used directly for a Language Service reference request. + break; + case SymbolKind.Element: { + const matches = getDirectiveMatchesForElementTag(symbol.templateNode, symbol.directives); + references.push(...this.getReferencesForDirectives(matches) ?? []); + break; + } + case SymbolKind.DomBinding: { + // Dom bindings aren't currently type-checked (see `checkTypeOfDomBindings`) so they don't + // have a shim location. This means we can't match dom bindings to their lib.dom + // reference, but we can still see if they match to a directive. + if (!(node instanceof TmplAstTextAttribute) && !(node instanceof TmplAstBoundAttribute)) { + break; + } + const directives = getDirectiveMatchesForAttribute( + node.name, symbol.host.templateNode, symbol.host.directives); + references.push(...this.getReferencesForDirectives(directives) ?? []); + break; + } + case SymbolKind.Reference: { + const {shimPath, positionInShimFile} = symbol.referenceVarLocation; + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []); + break; + } + case SymbolKind.Variable: { + const {positionInShimFile: initializerPosition, shimPath} = symbol.initializerLocation; + const localVarPosition = symbol.localVarLocation.positionInShimFile; + + if ((node instanceof TmplAstVariable)) { + if (node.valueSpan !== undefined && isWithin(position, node.valueSpan)) { + // In the valueSpan of the variable, we want to get the reference of the initializer. + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, initializerPosition) ?? []); + } else if (isWithin(position, node.keySpan)) { + // In the keySpan of the variable, we want to get the reference of the local variable. + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, localVarPosition) ?? []); + } + } else { + // If the templateNode is not the `TmplAstVariable`, it must be a usage of the variable + // somewhere in the template. + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, localVarPosition) ?? []); + } + + break; + } + case SymbolKind.Input: + case SymbolKind.Output: { + // TODO(atscott): Determine how to handle when the binding maps to several inputs/outputs + const {shimPath, positionInShimFile} = symbol.bindings[0].shimLocation; + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []); + break; + } + case SymbolKind.Pipe: + case SymbolKind.Expression: { + const {shimPath, positionInShimFile} = symbol.shimLocation; + references.push( + ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []); + break; + } + } + } + if (references.length === 0) { return undefined; } - switch (symbol.kind) { - case SymbolKind.Directive: - case SymbolKind.Template: - // References to elements, templates, and directives will be through template references - // (#ref). They shouldn't be used directly for a Language Service reference request. - return undefined; - case SymbolKind.Element: { - const matches = getDirectiveMatchesForElementTag(symbol.templateNode, symbol.directives); - return this.getReferencesForDirectives(matches); - } - case SymbolKind.DomBinding: { - // Dom bindings aren't currently type-checked (see `checkTypeOfDomBindings`) so they don't - // have a shim location. This means we can't match dom bindings to their lib.dom reference, - // but we can still see if they match to a directive. - if (!(node instanceof TmplAstTextAttribute) && !(node instanceof TmplAstBoundAttribute)) { - return undefined; - } - const directives = getDirectiveMatchesForAttribute( - node.name, symbol.host.templateNode, symbol.host.directives); - return this.getReferencesForDirectives(directives); - } - case SymbolKind.Reference: { - const {shimPath, positionInShimFile} = symbol.referenceVarLocation; - return this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile); - } - case SymbolKind.Variable: { - const {positionInShimFile: initializerPosition, shimPath} = symbol.initializerLocation; - const localVarPosition = symbol.localVarLocation.positionInShimFile; - if ((node instanceof TmplAstVariable)) { - if (node.valueSpan !== undefined && isWithin(position, node.valueSpan)) { - // In the valueSpan of the variable, we want to get the reference of the initializer. - return this.getReferencesAtTypescriptPosition(shimPath, initializerPosition); - } else if (isWithin(position, node.keySpan)) { - // In the keySpan of the variable, we want to get the reference of the local variable. - return this.getReferencesAtTypescriptPosition(shimPath, localVarPosition); - } else { - return undefined; - } - } - - // If the templateNode is not the `TmplAstVariable`, it must be a usage of the variable - // somewhere in the template. - return this.getReferencesAtTypescriptPosition(shimPath, localVarPosition); - } - case SymbolKind.Input: - case SymbolKind.Output: { - // TODO(atscott): Determine how to handle when the binding maps to several inputs/outputs - const {shimPath, positionInShimFile} = symbol.bindings[0].shimLocation; - return this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile); - } - case SymbolKind.Pipe: - case SymbolKind.Expression: { - const {shimPath, positionInShimFile} = symbol.shimLocation; - return this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile); - } - } + return references; } private getReferencesForDirectives(directives: Set): diff --git a/packages/language-service/ivy/test/references_spec.ts b/packages/language-service/ivy/test/references_spec.ts index 2908260168..07d0dd51d7 100644 --- a/packages/language-service/ivy/test/references_spec.ts +++ b/packages/language-service/ivy/test/references_spec.ts @@ -666,6 +666,37 @@ describe('find references', () => { }); }); + it('should get references to both input and output for two-way binding', () => { + const dirFile = { + name: _('/dir.ts'), + contents: ` + import {Directive, Input, Output} from '@angular/core'; + + @Directive({selector: '[string-model]'}) + export class StringModel { + @Input() model!: any; + @Output() modelChange!: any; + }` + }; + const {text, cursor} = extractCursorInfo(` + import {Component} from '@angular/core'; + + @Component({template: '
'}) + export class AppCmp { + title = 'title'; + }`); + const appFile = {name: _('/app.ts'), contents: text}; + env = createModuleWithDeclarations([appFile, dirFile]); + + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + // Note that this includes the 'model` twice from the template. As with other potential + // duplicates (like if another plugin returns the same span), we expect the LS clients to filter + // these out themselves. + expect(refs.length).toEqual(4); + assertFileNames(refs, ['dir.ts', 'app.ts']); + assertTextSpans(refs, ['model', 'modelChange']); + }); + describe('directives', () => { it('works for directive classes', () => { const {text, cursor} = extractCursorInfo(`