From 9a5ac47331faf0f6030bff75ad8ef8dab2f8d2d9 Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Mon, 30 Nov 2020 11:16:48 -0800 Subject: [PATCH] feat(language-service): initial implementation for `findRenameLocations` (#40140) This commit lays the groundwork for potentially providing rename locations from the Ivy native LS. The approach is very similar to what was done with the feature to find references. One difference, however, is that we did not require the references to be fully "correct". That is, the exact text spans did not matter so much, as long as we provide a location that logically includes the referenced item. An example of a necessary difference between rename locations and references is directives. The entire element in the template is a "reference" of the directive's class. However, it's not a valid location to be renamed. The same goes for aliased inputs/outputs. The locations in the template directly map to the class property, which is correct for references, but would not be correct for rename locations, which should instead map to the string node fo the alias. As an initial approach to address the aforementioned issues with rename locations, we check that all the rename location nodes have the same text. If _any_ node has text that differs from the request, we do not return any rename locations. This works as a way to prevent renames that could break the the program by missing some required nodes in the rename action, but allowing other nodes to be renamed. PR Close #40140 --- .../language-service/ivy/language_service.ts | 14 +- packages/language-service/ivy/references.ts | 328 ++++- .../ivy/test/definitions_spec.ts | 6 +- .../ivy/test/references_spec.ts | 1291 ++++++++++++----- .../language-service/ivy/test/test_utils.ts | 48 +- .../ivy/test/type_definitions_spec.ts | 7 +- packages/language-service/ivy/ts_plugin.ts | 7 +- 7 files changed, 1205 insertions(+), 496 deletions(-) diff --git a/packages/language-service/ivy/language_service.ts b/packages/language-service/ivy/language_service.ts index 138a0acc92..1557a1ac8d 100644 --- a/packages/language-service/ivy/language_service.ts +++ b/packages/language-service/ivy/language_service.ts @@ -18,7 +18,7 @@ import {CompilerFactory} from './compiler_factory'; import {CompletionBuilder, CompletionNodeContext} from './completions'; import {DefinitionBuilder} from './definitions'; import {QuickInfoBuilder} from './quick_info'; -import {ReferenceBuilder} from './references'; +import {ReferencesAndRenameBuilder} from './references'; import {getTargetAtPosition, TargetContext, TargetNodeKind} from './template_target'; import {getTemplateInfoAtPosition, isTypeScriptFile} from './utils'; @@ -107,8 +107,16 @@ export class LanguageService { getReferencesAtPosition(fileName: string, position: number): ts.ReferenceEntry[]|undefined { const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName); - const results = - new ReferenceBuilder(this.strategy, this.tsLS, compiler).get(fileName, position); + const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler) + .getReferencesAtPosition(fileName, position); + this.compilerFactory.registerLastKnownProgram(); + return results; + } + + findRenameLocations(fileName: string, position: number): readonly ts.RenameLocation[]|undefined { + const compiler = this.compilerFactory.getOrCreateWithChangedFile(fileName); + const results = new ReferencesAndRenameBuilder(this.strategy, this.tsLS, compiler) + .findRenameLocations(fileName, position); this.compilerFactory.registerLastKnownProgram(); return results; } diff --git a/packages/language-service/ivy/references.ts b/packages/language-service/ivy/references.ts index 265a8c27eb..dfd76da857 100644 --- a/packages/language-service/ivy/references.ts +++ b/packages/language-service/ivy/references.ts @@ -5,52 +5,213 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {TmplAstBoundAttribute, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, LiteralPrimitive, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstNode, TmplAstReference, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system'; -import {DirectiveSymbol, SymbolKind, TemplateTypeChecker, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; +import {DirectiveSymbol, ShimLocation, SymbolKind, TemplateTypeChecker, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import {ExpressionIdentifier, hasExpressionIdentifier} from '@angular/compiler-cli/src/ngtsc/typecheck/src/comments'; import * as ts from 'typescript'; import {getTargetAtPosition, TargetNodeKind} from './template_target'; import {findTightestNode} from './ts_utils'; -import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, isWithin, TemplateInfo, toTextSpan} from './utils'; +import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, isTemplateNode, isWithin, TemplateInfo, toTextSpan} from './utils'; -export class ReferenceBuilder { +interface FilePosition { + fileName: string; + position: number; +} + +function toFilePosition(shimLocation: ShimLocation): FilePosition { + return {fileName: shimLocation.shimPath, position: shimLocation.positionInShimFile}; +} + +enum RequestKind { + Template, + TypeScript, +} + +interface TemplateRequest { + kind: RequestKind.Template; + requestNode: TmplAstNode|AST; + position: number; +} + +interface TypeScriptRequest { + kind: RequestKind.TypeScript; + requestNode: ts.Node; +} + +type RequestOrigin = TemplateRequest|TypeScriptRequest; + +interface TemplateLocationDetails { + /** + * A target node in a template. + */ + templateTarget: TmplAstNode|AST; + + /** + * TypeScript locations which the template node maps to. A given template node might map to + * several TS nodes. For example, a template node for an attribute might resolve to several + * directives or a directive and one of its inputs. + */ + typescriptLocations: FilePosition[]; +} + +export class ReferencesAndRenameBuilder { private readonly ttc = this.compiler.getTemplateTypeChecker(); constructor( private readonly strategy: TypeCheckingProgramStrategy, private readonly tsLS: ts.LanguageService, private readonly compiler: NgCompiler) {} - get(filePath: string, position: number): ts.ReferenceEntry[]|undefined { + findRenameLocations(filePath: string, position: number): readonly ts.RenameLocation[]|undefined { this.ttc.generateAllTypeCheckBlocks(); const templateInfo = getTemplateInfoAtPosition(filePath, position, this.compiler); - return templateInfo !== undefined ? - this.getReferencesAtTemplatePosition(templateInfo, position) : - this.getReferencesAtTypescriptPosition(filePath, position); + // We could not get a template at position so we assume the request is came from outside the + // template. + if (templateInfo === undefined) { + const requestNode = this.getTsNodeAtPosition(filePath, position); + if (requestNode === null) { + return undefined; + } + const requestOrigin: TypeScriptRequest = {kind: RequestKind.TypeScript, requestNode}; + return this.findRenameLocationsAtTypescriptPosition(filePath, position, requestOrigin); + } + + return this.findRenameLocationsAtTemplatePosition(templateInfo, position); } - private getReferencesAtTemplatePosition({template, component}: TemplateInfo, position: number): + private findRenameLocationsAtTemplatePosition(templateInfo: TemplateInfo, position: number): + readonly ts.RenameLocation[]|undefined { + const allTargetDetails = this.getTargetDetailsAtTemplatePosition(templateInfo, position); + if (allTargetDetails === null) { + return undefined; + } + + const allRenameLocations: ts.RenameLocation[] = []; + for (const targetDetails of allTargetDetails) { + const requestOrigin: TemplateRequest = { + kind: RequestKind.Template, + requestNode: targetDetails.templateTarget, + position, + }; + + for (const location of targetDetails.typescriptLocations) { + const locations = this.findRenameLocationsAtTypescriptPosition( + location.fileName, location.position, requestOrigin); + // If we couldn't find rename locations for _any_ result, we should not allow renaming to + // proceed instead of having a partially complete rename. + if (locations === undefined) { + return undefined; + } + allRenameLocations.push(...locations); + } + } + return allRenameLocations.length > 0 ? allRenameLocations : undefined; + } + + private getTsNodeAtPosition(filePath: string, position: number): ts.Node|null { + const sf = this.strategy.getProgram().getSourceFile(filePath); + if (!sf) { + return null; + } + return findTightestNode(sf, position) ?? null; + } + + findRenameLocationsAtTypescriptPosition( + filePath: string, position: number, + requestOrigin: RequestOrigin): readonly ts.RenameLocation[]|undefined { + let originalNodeText: string; + if (requestOrigin.kind === RequestKind.TypeScript) { + originalNodeText = requestOrigin.requestNode.getText(); + } else { + const templateNodeText = + getTemplateNodeRenameTextAtPosition(requestOrigin.requestNode, requestOrigin.position); + if (templateNodeText === null) { + return undefined; + } + originalNodeText = templateNodeText; + } + + const locations = this.tsLS.findRenameLocations( + filePath, position, /*findInStrings*/ false, /*findInComments*/ false); + if (locations === undefined) { + return undefined; + } + + const entries: ts.RenameLocation[] = []; + for (const location of locations) { + // TODO(atscott): Determine if a file is a shim file in a more robust way and make the API + // available in an appropriate location. + if (this.ttc.isTrackedTypeCheckFile(absoluteFrom(location.fileName))) { + const entry = this.convertToTemplateDocumentSpan(location, this.ttc, originalNodeText); + // There is no template node whose text matches the original rename request. Bail on + // renaming completely rather than providing incomplete results. + if (entry === null) { + return undefined; + } + entries.push(entry); + } else { + // Ensure we only allow renaming a TS result with matching text + const refNode = this.getTsNodeAtPosition(location.fileName, location.textSpan.start); + if (refNode === null || refNode.getText() !== originalNodeText) { + return undefined; + } + entries.push(location); + } + } + return entries; + } + + getReferencesAtPosition(filePath: string, position: number): ts.ReferenceEntry[]|undefined { + this.ttc.generateAllTypeCheckBlocks(); + const templateInfo = getTemplateInfoAtPosition(filePath, position, this.compiler); + if (templateInfo === undefined) { + return this.getReferencesAtTypescriptPosition(filePath, position); + } + return this.getReferencesAtTemplatePosition(templateInfo, position); + } + + private getReferencesAtTemplatePosition(templateInfo: TemplateInfo, position: number): ts.ReferenceEntry[]|undefined { + const allTargetDetails = this.getTargetDetailsAtTemplatePosition(templateInfo, position); + if (allTargetDetails === null) { + return undefined; + } + const allReferences: ts.ReferenceEntry[] = []; + for (const targetDetails of allTargetDetails) { + for (const location of targetDetails.typescriptLocations) { + const refs = this.getReferencesAtTypescriptPosition(location.fileName, location.position); + if (refs !== undefined) { + allReferences.push(...refs); + } + } + } + return allReferences.length > 0 ? allReferences : undefined; + } + + private getTargetDetailsAtTemplatePosition({template, component}: TemplateInfo, position: number): + TemplateLocationDetails[]|null { // Find the AST node in the template at the position. const positionDetails = getTargetAtPosition(template, position); if (positionDetails === null) { - return undefined; + return null; } const nodes = positionDetails.context.kind === TargetNodeKind.TwoWayBindingContext ? positionDetails.context.nodes : [positionDetails.context.node]; - const references: ts.ReferenceEntry[] = []; + const details: TemplateLocationDetails[] = []; + for (const node of nodes) { // Get the information about the TCB at the template position. const symbol = this.ttc.getSymbolOfNode(node, component); + const templateTarget = node; + if (symbol === null) { continue; } - switch (symbol.kind) { case SymbolKind.Directive: case SymbolKind.Template: @@ -59,7 +220,8 @@ export class ReferenceBuilder { break; case SymbolKind.Element: { const matches = getDirectiveMatchesForElementTag(symbol.templateNode, symbol.directives); - references.push(...this.getReferencesForDirectives(matches) ?? []); + details.push( + {typescriptLocations: this.getPositionsForDirectives(matches), templateTarget}); break; } case SymbolKind.DomBinding: { @@ -67,69 +229,64 @@ export class ReferenceBuilder { // 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; + return null; } const directives = getDirectiveMatchesForAttribute( node.name, symbol.host.templateNode, symbol.host.directives); - references.push(...this.getReferencesForDirectives(directives) ?? []); + details.push( + {typescriptLocations: this.getPositionsForDirectives(directives), templateTarget}); break; } case SymbolKind.Reference: { - const {shimPath, positionInShimFile} = symbol.referenceVarLocation; - references.push( - ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []); + details.push( + {typescriptLocations: [toFilePosition(symbol.referenceVarLocation)], templateTarget}); 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)) { + if ((templateTarget instanceof TmplAstVariable)) { + if (templateTarget.valueSpan !== undefined && + isWithin(position, templateTarget.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)) { + details.push({ + typescriptLocations: [toFilePosition(symbol.initializerLocation)], + templateTarget, + }); + } else if (isWithin(position, templateTarget.keySpan)) { // In the keySpan of the variable, we want to get the reference of the local variable. - references.push( - ...this.getReferencesAtTypescriptPosition(shimPath, localVarPosition) ?? []); + details.push( + {typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget}); } } 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) ?? []); + // If the templateNode is not the `TmplAstVariable`, it must be a usage of the + // variable somewhere in the template. + details.push( + {typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget}); } - 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) ?? []); + details.push({ + typescriptLocations: + symbol.bindings.map(binding => toFilePosition(binding.shimLocation)), + templateTarget, + }); break; } case SymbolKind.Pipe: case SymbolKind.Expression: { - const {shimPath, positionInShimFile} = symbol.shimLocation; - references.push( - ...this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile) ?? []); + details.push( + {typescriptLocations: [toFilePosition(symbol.shimLocation)], templateTarget}); break; } } } - if (references.length === 0) { - return undefined; - } - return references; + return details.length > 0 ? details : null; } - private getReferencesForDirectives(directives: Set): - ts.ReferenceEntry[]|undefined { - const allDirectiveRefs: ts.ReferenceEntry[] = []; + private getPositionsForDirectives(directives: Set): FilePosition[] { + const allDirectives: FilePosition[] = []; for (const dir of directives.values()) { const dirClass = dir.tsSymbol.valueDeclaration; if (dirClass === undefined || !ts.isClassDeclaration(dirClass) || @@ -137,15 +294,12 @@ export class ReferenceBuilder { continue; } - const dirFile = dirClass.getSourceFile().fileName; - const dirPosition = dirClass.name.getStart(); - const directiveRefs = this.getReferencesAtTypescriptPosition(dirFile, dirPosition); - if (directiveRefs !== undefined) { - allDirectiveRefs.push(...directiveRefs); - } + const {fileName} = dirClass.getSourceFile(); + const position = dirClass.name.getStart(); + allDirectives.push({fileName, position}); } - return allDirectiveRefs.length > 0 ? allDirectiveRefs : undefined; + return allDirectives; } private getReferencesAtTypescriptPosition(fileName: string, position: number): @@ -158,7 +312,7 @@ export class ReferenceBuilder { const entries: ts.ReferenceEntry[] = []; for (const ref of refs) { if (this.ttc.isTrackedTypeCheckFile(absoluteFrom(ref.fileName))) { - const entry = this.convertToTemplateReferenceEntry(ref, this.ttc); + const entry = this.convertToTemplateDocumentSpan(ref, this.ttc); if (entry !== null) { entries.push(entry); } @@ -169,27 +323,27 @@ export class ReferenceBuilder { return entries; } - private convertToTemplateReferenceEntry( - shimReferenceEntry: ts.ReferenceEntry, - templateTypeChecker: TemplateTypeChecker): ts.ReferenceEntry|null { - const sf = this.strategy.getProgram().getSourceFile(shimReferenceEntry.fileName); + private convertToTemplateDocumentSpan( + shimDocumentSpan: T, templateTypeChecker: TemplateTypeChecker, requiredNodeText?: string): T + |null { + const sf = this.strategy.getProgram().getSourceFile(shimDocumentSpan.fileName); if (sf === undefined) { return null; } - const tcbNode = findTightestNode(sf, shimReferenceEntry.textSpan.start); + const tcbNode = findTightestNode(sf, shimDocumentSpan.textSpan.start); if (tcbNode === undefined || hasExpressionIdentifier(sf, tcbNode, ExpressionIdentifier.EVENT_PARAMETER)) { - // If the reference result is the $event parameter in the subscribe/addEventListener function - // in the TCB, we want to filter this result out of the references. We really only want to - // return references to the parameter in the template itself. + // If the reference result is the $event parameter in the subscribe/addEventListener + // function in the TCB, we want to filter this result out of the references. We really only + // want to return references to the parameter in the template itself. return null; } - - // TODO(atscott): Determine how to consistently resolve paths. i.e. with the project serverHost - // or LSParseConfigHost in the adapter. We should have a better defined way to normalize paths. + // TODO(atscott): Determine how to consistently resolve paths. i.e. with the project + // serverHost or LSParseConfigHost in the adapter. We should have a better defined way to + // normalize paths. const mapping = templateTypeChecker.getTemplateMappingAtShimLocation({ - shimPath: absoluteFrom(shimReferenceEntry.fileName), - positionInShimFile: shimReferenceEntry.textSpan.start, + shimPath: absoluteFrom(shimDocumentSpan.fileName), + positionInShimFile: shimDocumentSpan.textSpan.start, }); if (mapping === null) { return null; @@ -202,16 +356,46 @@ export class ReferenceBuilder { } else if (templateSourceMapping.type === 'external') { templateUrl = absoluteFrom(templateSourceMapping.templateUrl); } else { - // This includes indirect mappings, which are difficult to map directly to the code location. - // Diagnostics similarly return a synthetic template string for this case rather than a real - // location. + // This includes indirect mappings, which are difficult to map directly to the code + // location. Diagnostics similarly return a synthetic template string for this case rather + // than a real location. + return null; + } + + if (requiredNodeText !== undefined && span.toString() !== requiredNodeText) { return null; } return { - ...shimReferenceEntry, + ...shimDocumentSpan, fileName: templateUrl, textSpan: toTextSpan(span), }; } } + +function getTemplateNodeRenameTextAtPosition(node: TmplAstNode|AST, position: number): string|null { + if (node instanceof TmplAstBoundAttribute || node instanceof TmplAstTextAttribute || + node instanceof TmplAstBoundEvent) { + return node.name; + } else if (node instanceof TmplAstVariable || node instanceof TmplAstReference) { + if (isWithin(position, node.keySpan)) { + return node.keySpan.toString(); + } else if (node.valueSpan && isWithin(position, node.valueSpan)) { + return node.valueSpan.toString(); + } + } + + if (node instanceof BindingPipe) { + // TODO(atscott): Add support for renaming pipes + return null; + } + if (node instanceof PropertyRead || node instanceof MethodCall || node instanceof PropertyWrite || + node instanceof SafePropertyRead || node instanceof SafeMethodCall) { + return node.name; + } else if (node instanceof LiteralPrimitive) { + return node.value; + } + + return null; +} diff --git a/packages/language-service/ivy/test/definitions_spec.ts b/packages/language-service/ivy/test/definitions_spec.ts index a1fcef7a11..52f1f82983 100644 --- a/packages/language-service/ivy/test/definitions_spec.ts +++ b/packages/language-service/ivy/test/definitions_spec.ts @@ -10,7 +10,7 @@ import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import {extractCursorInfo, LanguageServiceTestEnvironment} from './env'; -import {assertFileNames, createModuleWithDeclarations, humanizeDefinitionInfo} from './test_utils'; +import {assertFileNames, createModuleWithDeclarations, humanizeDocumentSpanLike} from './test_utils'; describe('definitions', () => { it('returns the pipe class as definition when checkTypeOfPipes is false', () => { @@ -27,8 +27,8 @@ describe('definitions', () => { export class AppCmp {} `, }; - const env = createModuleWithDeclarations([appFile], [templateFile]); // checkTypeOfPipes is set to false when strict templates is false + const env = createModuleWithDeclarations([appFile], [templateFile], {strictTemplates: false}); const {textSpan, definitions} = getDefinitionsAndAssertBoundSpan(env, absoluteFrom('/app.html'), cursor); expect(text.substr(textSpan.start, textSpan.length)).toEqual('date'); @@ -143,6 +143,6 @@ describe('definitions', () => { const definitionAndBoundSpan = env.ngLS.getDefinitionAndBoundSpan(fileName, cursor); const {textSpan, definitions} = definitionAndBoundSpan!; expect(definitions).toBeTruthy(); - return {textSpan, definitions: definitions!.map(d => humanizeDefinitionInfo(d, env.host))}; + return {textSpan, definitions: definitions!.map(d => humanizeDocumentSpanLike(d, env))}; } }); diff --git a/packages/language-service/ivy/test/references_spec.ts b/packages/language-service/ivy/test/references_spec.ts index 07d0dd51d7..715f27087c 100644 --- a/packages/language-service/ivy/test/references_spec.ts +++ b/packages/language-service/ivy/test/references_spec.ts @@ -8,90 +8,127 @@ import {absoluteFrom, absoluteFrom as _} from '@angular/compiler-cli/src/ngtsc/file_system'; import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; -import * as ts from 'typescript/lib/tsserverlibrary'; import {extractCursorInfo, LanguageServiceTestEnvironment} from './env'; -import {assertFileNames, assertTextSpans, createModuleWithDeclarations, getText} from './test_utils'; +import {assertFileNames, assertTextSpans, createModuleWithDeclarations, humanizeDocumentSpanLike} from './test_utils'; -describe('find references', () => { +describe('find references and rename locations', () => { let env: LanguageServiceTestEnvironment; beforeEach(() => { initMockFileSystem('Native'); }); - it('gets component member references from TS file', () => { - const {text, cursor} = extractCursorInfo(` + afterEach(() => { + // Clear env so it's not accidentally carried over to the next test. + env = undefined!; + }); + + describe('cursor is on binding in component class', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({templateUrl: './app.html'}) export class AppCmp { myP¦rop!: string; }`); - const appFile = {name: _('/app.ts'), contents: text}; - const templateFile = {name: _('/app.html'), contents: '{{myProp}}'}; - env = createModuleWithDeclarations([appFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.html', 'app.ts']); - assertTextSpans(refs, ['myProp']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + const templateFile = {name: _('/app.html'), contents: '{{myProp}}'}; + env = createModuleWithDeclarations([appFile], [templateFile]); + }); + + it('gets component member references from TS file and external template', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.html', 'app.ts']); + assertTextSpans(refs, ['myProp']); + }); + + it('gets rename locations from TS file and external template', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.html', 'app.ts']); + assertTextSpans(renameLocations, ['myProp']); + }); }); - it('gets component member references from TS file and inline template', () => { - const {text, cursor} = extractCursorInfo(` - import {Component} from '@angular/core'; + describe('when cursor is on binding in an external template', () => { + let cursor: number; - @Component({template: '{{myProp}}'}) - export class AppCmp { - myP¦rop!: string; - }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['myProp']); - }); - - it('gets component member references from template', () => { - const appFile = { - name: _('/app.ts'), - contents: ` + beforeEach(() => { + const appFile = { + name: _('/app.ts'), + contents: ` import {Component} from '@angular/core'; @Component({templateUrl: './app.html'}) export class AppCmp { myProp = ''; }`, - }; - const {text, cursor} = extractCursorInfo('{{myP¦rop}}'); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.html', 'app.ts']); - assertTextSpans(refs, ['myProp']); + }; + const cursorInfo = extractCursorInfo('{{myP¦rop}}'); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile], [templateFile]); + }); + + it('gets references', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.html', 'app.ts']); + assertTextSpans(refs, ['myProp']); + }); + + it('gets rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.html', 'app.ts']); + assertTextSpans(renameLocations, ['myProp']); + }); }); - it('should work for method calls', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on function call in external template', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { setTitle(s: number) {} }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['setTitle']); + it('gets component member reference in ts file', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['setTitle']); + }); + + it('gets rename location in ts file', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['setTitle']); + }); }); - it('should work for method call arguments', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor in on argument to a function call in an external template', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
'}) @@ -99,54 +136,97 @@ describe('find references', () => { title = ''; setTitle(s: string) {} }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); - assertTextSpans(refs, ['title']); + it('gets member reference in ts file', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + + assertTextSpans(refs, ['title']); + }); + + it('finds rename location in ts file', () => { + const refs = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + + assertTextSpans(refs, ['title']); + }); }); - it('should work for $event in method call arguments', () => { - const {text, cursor} = extractCursorInfo(` - import {Component} from '@angular/core'; + describe('when cursor is on $event in method call arguments', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` + import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { setTitle(s: any) {} }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(1); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); - assertTextSpans(refs, ['$event']); + it('find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(1); + + assertTextSpans(refs, ['$event']); + }); + + it('gets no rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + }); }); - it('should work for property writes', () => { - const appFile = { - name: _('/app.ts'), - contents: ` + describe('when cursor in on LHS of property write in external template', () => { + let cursor: number; + + beforeEach(() => { + const appFile = { + name: _('/app.ts'), + contents: ` import {Component} from '@angular/core'; @Component({templateUrl: './app.html' }) export class AppCmp { title = ''; }`, - }; - const templateFileWithCursor = `
`; - const {text, cursor} = extractCursorInfo(templateFileWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); + }; + const templateFileWithCursor = `
`; + const cursorInfo = extractCursorInfo(templateFileWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile], [templateFile]); + }); - assertFileNames(refs, ['app.ts', 'app.html']); - assertTextSpans(refs, ['title']); + it('gets member reference in ts file', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + + assertFileNames(refs, ['app.ts', 'app.html']); + assertTextSpans(refs, ['title']); + }); + + it('gets rename location in ts file', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + + assertFileNames(renameLocations, ['app.ts', 'app.html']); + assertTextSpans(renameLocations, ['title']); + }); }); - it('should work for RHS of property writes', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor in on RHS of property write in external template', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
' }) @@ -154,48 +234,85 @@ describe('find references', () => { title = ''; otherTitle = ''; }`); - const appFile = { - name: _('/app.ts'), - contents: text, - }; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); + cursor = cursorInfo.cursor; + const appFile = { + name: _('/app.ts'), + contents: cursorInfo.text, + }; + env = createModuleWithDeclarations([appFile]); + }); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['otherTitle']); + it('get reference to member in ts file', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['otherTitle']); + }); + + it('finds rename location in ts file', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['otherTitle']); + }); }); - it('should work for keyed reads', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor in on a keyed read', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '{{hero["na¦me"]}}' }) export class AppCmp { hero: {name: string} = {name: 'Superman'}; }`); - const appFile = { - name: _('/app.ts'), - contents: text, - }; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - // 3 references: the type definition, the value assignment, and the read in the template - expect(refs.length).toBe(3); + cursor = cursorInfo.cursor; + const appFile = { + name: _('/app.ts'), + contents: cursorInfo.text, + }; + env = createModuleWithDeclarations([appFile]); + }); - assertFileNames(refs, ['app.ts']); - // TODO(atscott): investigate if we can make the template keyed read be just the 'name' part. - // The TypeScript implementation specifically adjusts the span to accommodate string literals: - // https://sourcegraph.com/github.com/microsoft/TypeScript@d5779c75d3dd19565b60b9e2960b8aac36d4d635/-/blob/src/services/findAllReferences.ts#L508-512 - // One possible solution would be to extend `FullTemplateMapping` to include the matched TCB - // node and then do the same thing that TS does: if the node is a string, adjust the span. - assertTextSpans(refs, ['name', '"name"']); + it('gets reference to member type definition and initialization in component class', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + // 3 references: the type definition, the value assignment, and the read in the template + expect(refs.length).toBe(3); + + assertFileNames(refs, ['app.ts']); + // TODO(atscott): investigate if we can make the template keyed read be just the 'name' part. + // The TypeScript implementation specifically adjusts the span to accommodate string literals: + // https://sourcegraph.com/github.com/microsoft/TypeScript@d5779c75d3dd19565b60b9e2960b8aac36d4d635/-/blob/src/services/findAllReferences.ts#L508-512 + // One possible solution would be to extend `FullTemplateMapping` to include the matched TCB + // node and then do the same thing that TS does: if the node is a string, adjust the span. + assertTextSpans(refs, ['name', '"name"']); + }); + + it('gets rename locations in component class', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + + // TODO(atscott): We should handle this case. The fix requires us to fix the result span as + // described above. + // 3 references: the type definition, the value assignment, and the read in the template + // expect(renameLocations.length).toBe(3); + // + // assertFileNames(renameLocations, ['app.ts']); + // assertTextSpans(renameLocations, ['name']); + }); }); - it('should work for keyed writes', () => { - const appFile = { - name: _('/app.ts'), - contents: ` + describe('when cursor in on RHS of keyed write in a template', () => { + let cursor: number; + + beforeEach(() => { + const appFile = { + name: _('/app.ts'), + contents: ` import {Component} from '@angular/core'; @Component({templateUrl: './app.html' }) @@ -203,40 +320,70 @@ describe('find references', () => { hero: {name: string} = {name: 'Superman'}; batman = 'batman'; }`, - }; - const templateFileWithCursor = `
`; - const {text, cursor} = extractCursorInfo(templateFileWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); + }; + const templateFileWithCursor = `
`; + const cursorInfo = extractCursorInfo(templateFileWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile], [templateFile]); + }); - assertFileNames(refs, ['app.ts', 'app.html']); - assertTextSpans(refs, ['batman']); + it('get references in ts file', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + + assertFileNames(refs, ['app.ts', 'app.html']); + assertTextSpans(refs, ['batman']); + }); + + it('finds rename location in ts file', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + + assertFileNames(renameLocations, ['app.ts', 'app.html']); + assertTextSpans(renameLocations, ['batman']); + }); }); - describe('references', () => { - it('should work for element references', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor in on an element reference', () => { + let cursor: number; + + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: ' {{ myIn¦put.value }}'}) export class AppCmp { title = ''; }`); - const appFile = {name: _('/app.ts'), contents: text}; + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; env = createModuleWithDeclarations([appFile]); + }); + + it('get reference to declaration in template', () => { const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); assertTextSpans(refs, ['myInput']); - const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.ts'), cursor)!; // Get the declaration by finding the reference that appears first in the template - originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); - expect(originalRefs[0].isDefinition).toBe(true); + refs.sort((a, b) => a.textSpan.start - b.textSpan.start); + expect(refs[0].isDefinition).toBe(true); }); - it('should work for template references', () => { + it('finds rename location in template', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + + expect(renameLocations.length).toBe(2); + assertTextSpans(renameLocations, ['myInput']); + }); + }); + + describe('when cursor in on a template reference', () => { + let cursor: number; + + beforeEach(() => { const templateWithCursor = ` bla `; @@ -250,21 +397,33 @@ describe('find references', () => { title = ''; }`, }; - const {text, cursor} = extractCursorInfo(templateWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; + const cursorInfo = extractCursorInfo(templateWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; env = createModuleWithDeclarations([appFile], [templateFile]); + }); + + it('gets reference to declaration', () => { const refs = getReferencesAtPosition(_('/app.html'), cursor)!; expect(refs.length).toBe(2); assertTextSpans(refs, ['myTemplate']); assertFileNames(refs, ['app.html']); - const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.html'), cursor)!; // Get the declaration by finding the reference that appears first in the template - originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); - expect(originalRefs[0].isDefinition).toBe(true); + refs.sort((a, b) => a.textSpan.start - b.textSpan.start); + expect(refs[0].isDefinition).toBe(true); }); - describe('directive references', () => { + it('finds rename location in template', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertTextSpans(renameLocations, ['myTemplate']); + assertFileNames(renameLocations, ['app.html']); + }); + }); + + describe('template references', () => { + describe('directives', () => { let appFile: TestFile; let dirFile: TestFile; @@ -286,97 +445,185 @@ describe('find references', () => { dirFile = {name: _('/dir.ts'), contents: dirFileContents}; }); - it('should work for usage of reference in template', () => { - const templateWithCursor = '
{{ dirR¦ef }}'; - const {text, cursor} = extractCursorInfo(templateWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.html']); - assertTextSpans(refs, ['dirRef']); + describe('when cursor is on usage of template reference', () => { + let cursor: number; + beforeEach(() => { + const templateWithCursor = '
{{ dirR¦ef }}'; + const cursorInfo = extractCursorInfo(templateWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); + }); + + it('should get references', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.html']); + assertTextSpans(refs, ['dirRef']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.html']); + assertTextSpans(renameLocations, ['dirRef']); + }); }); - it('should work for prop reads of directive references', () => { - const fileWithCursor = '
{{ dirRef.dirV¦alue }}'; - const {text, cursor} = extractCursorInfo(fileWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['dir.ts', 'app.html']); - assertTextSpans(refs, ['dirValue']); + describe('when cursor is on a property read of directive reference', () => { + let cursor: number; + beforeEach(() => { + const fileWithCursor = '
{{ dirRef.dirV¦alue }}'; + const cursorInfo = extractCursorInfo(fileWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); + }); + + it('should get references', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['dir.ts', 'app.html']); + assertTextSpans(refs, ['dirValue']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['dir.ts', 'app.html']); + assertTextSpans(renameLocations, ['dirValue']); + }); }); - it('should work for safe prop reads', () => { - const fileWithCursor = '
{{ dirRef?.dirV¦alue }}'; - const {text, cursor} = extractCursorInfo(fileWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['dir.ts', 'app.html']); - assertTextSpans(refs, ['dirValue']); + describe('when cursor is on a safe prop read', () => { + let cursor: number; + beforeEach(() => { + const fileWithCursor = '
{{ dirRef?.dirV¦alue }}'; + const cursorInfo = extractCursorInfo(fileWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); + }); + + + it('should get references', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['dir.ts', 'app.html']); + assertTextSpans(refs, ['dirValue']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['dir.ts', 'app.html']); + assertTextSpans(renameLocations, ['dirValue']); + }); }); - it('should work for safe method calls', () => { - const fileWithCursor = '
{{ dirRef?.doSometh¦ing() }}'; - const {text, cursor} = extractCursorInfo(fileWithCursor); - const templateFile = {name: _('/app.html'), contents: text}; - env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); - const refs = getReferencesAtPosition(_('/app.html'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['dir.ts', 'app.html']); - assertTextSpans(refs, ['doSomething']); + describe('when cursor is on safe method call', () => { + let cursor: number; + beforeEach(() => { + const fileWithCursor = '
{{ dirRef?.doSometh¦ing() }}'; + const cursorInfo = extractCursorInfo(fileWithCursor); + cursor = cursorInfo.cursor; + const templateFile = {name: _('/app.html'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, dirFile], [templateFile]); + }); + + + it('should get references', () => { + const refs = getReferencesAtPosition(_('/app.html'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['dir.ts', 'app.html']); + assertTextSpans(refs, ['doSomething']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.html'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['dir.ts', 'app.html']); + assertTextSpans(renameLocations, ['doSomething']); + }); }); }); }); - describe('variables', () => { - it('should work for variable initialized implicitly', () => { - const {text, cursor} = extractCursorInfo(` + describe('template variables', () => { + describe('when cursor is on variable which was initialized implicitly', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
{{her¦o}}
'}) export class AppCmp { heroes: string[] = []; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['hero']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); - const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.ts'), cursor)!; - // Get the declaration by finding the reference that appears first in the template - originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); - expect(originalRefs[0].isDefinition).toBe(true); + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['hero']); + + const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.ts'), cursor)!; + // Get the declaration by finding the reference that appears first in the template + originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); + expect(originalRefs[0].isDefinition).toBe(true); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['hero']); + }); }); - it('should work for renamed variables', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on renamed variable', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
{{iR¦ef}}
'}) export class AppCmp { heroes: string[] = []; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['iRef']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); - const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.ts'), cursor)!; - // Get the declaration by finding the reference that appears first in the template - originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); - expect(originalRefs[0].isDefinition).toBe(true); + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['iRef']); + + const originalRefs = env.ngLS.getReferencesAtPosition(_('/app.ts'), cursor)!; + // Get the declaration by finding the reference that appears first in the template + originalRefs.sort((a, b) => a.textSpan.start - b.textSpan.start); + expect(originalRefs[0].isDefinition).toBe(true); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['iRef']); + }); }); - it('should work for initializer of variable', () => { - const dirFile = ` + describe('when cursor is on initializer of variable', () => { + let cursor: number; + beforeEach(() => { + const dirFile = ` import {Directive, Input} from '@angular/core'; export class ExampleContext { @@ -391,7 +638,7 @@ describe('find references', () => { return true; } }`; - const fileWithCursor = ` + const fileWithCursor = ` import {Component, NgModule} from '@angular/core'; import {ExampleDirective} from './example-directive'; @@ -402,33 +649,59 @@ describe('find references', () => { @NgModule({declarations: [AppCmp, ExampleDirective]}) export class AppModule {}`; - const {text, cursor} = extractCursorInfo(fileWithCursor); - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: text, isRoot: true}, - {name: _('/example-directive.ts'), contents: dirFile}, - ]); - env.expectNoSourceDiagnostics(); - env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp'); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts', 'example-directive.ts']); - assertTextSpans(refs, ['identifier']); + const cursorInfo = extractCursorInfo(fileWithCursor); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: cursorInfo.text, isRoot: true}, + {name: _('/example-directive.ts'), contents: dirFile}, + ]); + env.expectNoSourceDiagnostics(); + env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp'); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.ts', 'example-directive.ts']); + assertTextSpans(refs, ['identifier']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.ts', 'example-directive.ts']); + assertTextSpans(renameLocations, ['identifier']); + }); }); - it('should work for prop reads of variables', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on property read of variable', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
{{hero.na¦me}}
'}) export class AppCmp { heroes: Array<{name: string}> = []; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['name']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['name']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['name']); + }); }); }); @@ -449,8 +722,10 @@ describe('find references', () => { prefixPipeFile = {name: _('/prefix-pipe.ts'), contents: prefixPipe}; }); - it('should work for pipe names', () => { - const appContentsWithCursor = ` + describe('when cursor is on pipe name', () => { + let cursor: number; + beforeEach(() => { + const appContentsWithCursor = ` import {Component} from '@angular/core'; @Component({template: '{{birthday | prefi¦xPipe: "MM/dd/yy"}}'}) @@ -458,17 +733,34 @@ describe('find references', () => { birthday = ''; } `; - const {text, cursor} = extractCursorInfo(appContentsWithCursor); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile, prefixPipeFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(5); - assertFileNames(refs, ['index.d.ts', 'prefix-pipe.ts', 'app.ts']); - assertTextSpans(refs, ['transform', 'prefixPipe']); + const cursorInfo = extractCursorInfo(appContentsWithCursor); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, prefixPipeFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(5); + assertFileNames(refs, ['index.d.ts', 'prefix-pipe.ts', 'app.ts']); + assertTextSpans(refs, ['transform', 'prefixPipe']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + + // TODO(atscott): Add support for renaming the pipe 'name' + // expect(renameLocations.length).toBe(2); + // assertFileNames(renameLocations, ['prefix-pipe.ts', 'app.ts']); + // assertTextSpans(renameLocations, ['prefixPipe']); + }); }); - it('should work for pipe arguments', () => { - const appContentsWithCursor = ` + describe('when cursor is on pipe argument', () => { + let cursor: number; + beforeEach(() => { + const appContentsWithCursor = ` import {Component} from '@angular/core'; @Component({template: '{{birthday | prefixPipe: pr¦efix}}'}) @@ -477,13 +769,25 @@ describe('find references', () => { prefix = ''; } `; - const {text, cursor} = extractCursorInfo(appContentsWithCursor); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile, prefixPipeFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(2); - assertFileNames(refs, ['app.ts']); - assertTextSpans(refs, ['prefix']); + const cursorInfo = extractCursorInfo(appContentsWithCursor); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, prefixPipeFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(2); + assertFileNames(refs, ['app.ts']); + assertTextSpans(refs, ['prefix']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['app.ts']); + assertTextSpans(renameLocations, ['prefix']); + }); }); }); @@ -496,69 +800,162 @@ describe('find references', () => { @Input() model!: string; @Input('alias') aliasedModel!: string; }`; - it('should work from the template', () => { - const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on the input in the template', () => { + let cursor: number; + beforeEach(() => { + const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { title = 'title'; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile, stringModelTestFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertFileNames(refs, ['string-model.ts', 'app.ts']); - assertTextSpans(refs, ['model']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, stringModelTestFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertFileNames(refs, ['string-model.ts', 'app.ts']); + assertTextSpans(refs, ['model']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toEqual(2); + assertFileNames(renameLocations, ['string-model.ts', 'app.ts']); + assertTextSpans(renameLocations, ['model']); + }); }); - it('should work for text attributes', () => { - const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on an input that maps to multiple directives', () => { + let cursor: number; + beforeEach(() => { + const otherDirFile = { + name: _('/other-dir.ts'), + contents: ` + import {Directive, Input} from '@angular/core'; + + @Directive({selector: '[string-model]'}) + export class OtherDir { + @Input('model') model!: any; + } + ` + }; + const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; + const cursorInfo = extractCursorInfo(` + import {Component} from '@angular/core'; + + @Component({template: '
'}) + export class AppCmp { + title = 'title'; + }`); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, stringModelTestFile, otherDirFile]); + }); + + // TODO(atscott): This test does not pass because the template symbol builder only returns one + // binding. + xit('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(3); + assertFileNames(refs, ['string-model.ts', 'app.ts', 'other-dir']); + assertTextSpans(refs, ['model', 'otherDirAliasedInput']); + }); + + // TODO(atscott): This test fails because template symbol builder only returns one binding. + // The result is that rather than returning `undefined` because we don't handle alias inputs, + // we return the rename locations for the first binding. + xit('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): + // expect(renameLocations.length).toEqual(3); + // assertFileNames(renameLocations, ['string-model.ts', 'app.ts', 'other-dir']); + // assertTextSpans(renameLocations, ['model']); + }); + }); + + describe('should work when cursor is on text attribute input', () => { + let cursor: number; + beforeEach(() => { + const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { title = 'title'; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile, stringModelTestFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertFileNames(refs, ['string-model.ts', 'app.ts']); - assertTextSpans(refs, ['model']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, stringModelTestFile]); + }); + + it('should work for text attributes', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertFileNames(refs, ['string-model.ts', 'app.ts']); + assertTextSpans(refs, ['model']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toEqual(2); + assertFileNames(renameLocations, ['string-model.ts', 'app.ts']); + assertTextSpans(renameLocations, ['model']); + }); }); - it('should work from the TS input declaration', () => { - const dirFileWithCursor = ` + describe('when cursor is on the class member input', () => { + let cursor: number; + beforeEach(() => { + const dirFileWithCursor = ` import {Directive, Input} from '@angular/core'; @Directive({selector: '[string-model]'}) export class StringModel { @Input() mod¦el!: string; }`; - const {text, cursor} = extractCursorInfo(dirFileWithCursor); - const stringModelTestFile = {name: _('/string-model.ts'), contents: text}; - const appFile = { - name: _('/app.ts'), - contents: ` + const cursorInfo = extractCursorInfo(dirFileWithCursor); + cursor = cursorInfo.cursor; + const stringModelTestFile = {name: _('/string-model.ts'), contents: cursorInfo.text}; + const appFile = { + name: _('/app.ts'), + contents: ` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { title = 'title'; }`, - }; - env = createModuleWithDeclarations([appFile, stringModelTestFile]); - const refs = getReferencesAtPosition(_('/string-model.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertFileNames(refs, ['app.ts', 'string-model.ts']); - assertTextSpans(refs, ['model']); + }; + env = createModuleWithDeclarations([appFile, stringModelTestFile]); + }); + + it('should work from the TS input declaration', () => { + const refs = getReferencesAtPosition(_('/string-model.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertFileNames(refs, ['app.ts', 'string-model.ts']); + assertTextSpans(refs, ['model']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/string-model.ts'), cursor)!; + expect(renameLocations.length).toEqual(2); + assertFileNames(renameLocations, ['app.ts', 'string-model.ts']); + assertTextSpans(renameLocations, ['model']); + }); }); - it('should work for inputs referenced from some other place', () => { - const otherDirContents = ` + describe('when cursor is on input referenced somewhere in the class functions', () => { + let cursor: number; + beforeEach(() => { + const otherDirContents = ` import {Directive, Input} from '@angular/core'; import {StringModel} from './string-model'; @@ -570,50 +967,78 @@ describe('find references', () => { console.log(this.stringModelRef.mod¦el); } }`; - const {text, cursor} = extractCursorInfo(otherDirContents); - const otherDirFile = {name: _('/other-dir.ts'), contents: text}; - const stringModelTestFile = { - name: _('/string-model.ts'), - contents: ` + const cursorInfo = extractCursorInfo(otherDirContents); + cursor = cursorInfo.cursor; + const otherDirFile = {name: _('/other-dir.ts'), contents: cursorInfo.text}; + const stringModelTestFile = { + name: _('/string-model.ts'), + contents: ` import {Directive, Input} from '@angular/core'; @Directive({selector: '[string-model]'}) export class StringModel { @Input() model!: string; }`, - }; - const appFile = { - name: _('/app.ts'), - contents: ` + }; + const appFile = { + name: _('/app.ts'), + contents: ` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { title = 'title'; }`, - }; - env = createModuleWithDeclarations([appFile, stringModelTestFile, otherDirFile]); - const refs = getReferencesAtPosition(_('/other-dir.ts'), cursor)!; - expect(refs.length).toEqual(3); - assertFileNames(refs, ['app.ts', 'string-model.ts', 'other-dir.ts']); - assertTextSpans(refs, ['model']); + }; + env = createModuleWithDeclarations([appFile, stringModelTestFile, otherDirFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/other-dir.ts'), cursor)!; + expect(refs.length).toEqual(3); + assertFileNames(refs, ['app.ts', 'string-model.ts', 'other-dir.ts']); + assertTextSpans(refs, ['model']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/other-dir.ts'), cursor)!; + expect(renameLocations.length).toEqual(3); + assertFileNames(renameLocations, ['app.ts', 'string-model.ts', 'other-dir.ts']); + assertTextSpans(renameLocations, ['model']); + }); }); - it('should work with aliases', () => { - const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on an aliased input', () => { + let cursor: number; + beforeEach(() => { + const stringModelTestFile = {name: _('/string-model.ts'), contents: dirFileContents}; + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({template: '
'}) export class AppCmp { title = 'title'; }`); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile, stringModelTestFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertFileNames(refs, ['string-model.ts', 'app.ts']); - assertTextSpans(refs, ['aliasedModel', 'alias']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile, stringModelTestFile]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertFileNames(refs, ['string-model.ts', 'app.ts']); + assertTextSpans(refs, ['aliasedModel', 'alias']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): add support for renaming alias outputs + // expect(renameLocations.length).toEqual(2); + // assertFileNames(renameLocations, ['string-model.ts', 'app.ts']); + // assertTextSpans(renameLocations, ['alias']); + }); }); }); @@ -641,28 +1066,56 @@ describe('find references', () => { export class AppModule {}`; } - it('should work', () => { - const {text, cursor} = extractCursorInfo( - generateAppFile(`
`)); - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: text, isRoot: true}, - {name: _('/string-model.ts'), contents: dirFile}, - ]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertTextSpans(refs, ['modelChange']); + describe('when cursor is on output key in template', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo( + generateAppFile(`
`)); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: cursorInfo.text, isRoot: true}, + {name: _('/string-model.ts'), contents: dirFile}, + ]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertTextSpans(refs, ['modelChange']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations.length).toEqual(2); + assertTextSpans(renameLocations, ['modelChange']); + }); }); - it('should work with aliases', () => { - const {text, cursor} = extractCursorInfo( - generateAppFile(`
`)); - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: text, isRoot: true}, - {name: _('/string-model.ts'), contents: dirFile}, - ]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toEqual(2); - assertTextSpans(refs, ['aliasedModelChange', 'alias']); + describe('when cursor is on alias output key', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo( + generateAppFile(`
`)); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: cursorInfo.text, isRoot: true}, + {name: _('/string-model.ts'), contents: dirFile}, + ]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toEqual(2); + assertTextSpans(refs, ['aliasedModelChange', 'alias']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): add support for renaming alias outputs + // expect(renameLocations.length).toEqual(2); + // assertTextSpans(renameLocations, ['alias']); + }); }); }); @@ -698,13 +1151,16 @@ describe('find references', () => { }); describe('directives', () => { - it('works for directive classes', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on the directive class', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Directive} from '@angular/core'; @Directive({selector: '[dir]'}) export class Di¦r {}`); - const appFile = ` + cursor = cursorInfo.cursor; + const appFile = ` import {Component, NgModule} from '@angular/core'; import {Dir} from './dir'; @@ -715,30 +1171,45 @@ describe('find references', () => { @NgModule({declarations: [AppCmp, Dir]}) export class AppModule {} `; - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: appFile, isRoot: true}, - {name: _('/dir.ts'), contents: text}, - ]); - const refs = getReferencesAtPosition(_('/dir.ts'), cursor)!; - // 4 references are: class declaration, template usage, app import and use in declarations - // list. - expect(refs.length).toBe(4); - assertTextSpans(refs, ['
', 'Dir']); - assertFileNames(refs, ['app.ts', 'dir.ts']); + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: appFile, isRoot: true}, + {name: _('/dir.ts'), contents: cursorInfo.text}, + ]); + }); + + it('should find references', () => { + const refs = getReferencesAtPosition(_('/dir.ts'), cursor)!; + // 4 references are: class declaration, template usage, app import and use in declarations + // list. + expect(refs.length).toBe(4); + assertTextSpans(refs, ['
', 'Dir']); + assertFileNames(refs, ['app.ts', 'dir.ts']); + }); + + it('should find rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/dir.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): We should handle this case, but exclude the template results + // expect(renameLocations.length).toBe(3); + // assertTextSpans(renameLocations, ['Dir']); + // assertFileNames(renameLocations, ['app.ts', 'dir.ts']); + }); }); - it('gets references to all matching directives when cursor is on an attribute', () => { - const dirFile = ` + describe('when cursor is on an attribute', () => { + let cursor: number; + beforeEach(() => { + const dirFile = ` import {Directive} from '@angular/core'; @Directive({selector: '[dir]'}) export class Dir {}`; - const dirFile2 = ` + const dirFile2 = ` import {Directive} from '@angular/core'; @Directive({selector: '[dir]'}) export class Dir2 {}`; - const {text, cursor} = extractCursorInfo(` + const cursorInfo = extractCursorInfo(` import {Component, NgModule} from '@angular/core'; import {Dir} from './dir'; import {Dir2} from './dir2'; @@ -750,19 +1221,35 @@ describe('find references', () => { @NgModule({declarations: [AppCmp, Dir, Dir2]}) export class AppModule {} `); - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: text, isRoot: true}, - {name: _('/dir.ts'), contents: dirFile}, - {name: _('/dir2.ts'), contents: dirFile2}, - ]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(8); - assertTextSpans(refs, ['
', 'Dir', 'Dir2']); - assertFileNames(refs, ['app.ts', 'dir.ts', 'dir2.ts']); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: cursorInfo.text, isRoot: true}, + {name: _('/dir.ts'), contents: dirFile}, + {name: _('/dir2.ts'), contents: dirFile2}, + ]); + }); + + it('gets references to all matching directives', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(8); + assertTextSpans(refs, ['
', 'Dir', 'Dir2']); + assertFileNames(refs, ['app.ts', 'dir.ts', 'dir2.ts']); + }); + + it('finds rename locations for all matching directives', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): We could consider supporting rename for directive selectors in the future + // expect(renameLocations.length).toBe(3); + // assertTextSpans(renameLocations, ['dir']); + // assertFileNames(renameLocations, ['app.ts', 'dir.ts', 'dir2.ts']); + }); }); - it('should be able to request references for generic directives', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on generic directive selector in template', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component, NgModule} from '@angular/core'; @Component({template: '
'}) @@ -770,23 +1257,35 @@ describe('find references', () => { items = []; } `); - const appFile = {name: _('/app.ts'), contents: text}; - env = createModuleWithDeclarations([appFile]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - expect(refs.length).toBe(6); - assertTextSpans(refs, ['
', 'NgForOf']); - assertFileNames(refs, ['index.d.ts', 'app.ts']); + cursor = cursorInfo.cursor; + const appFile = {name: _('/app.ts'), contents: cursorInfo.text}; + env = createModuleWithDeclarations([appFile]); + }); + + it('should be able to request references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + expect(refs.length).toBe(6); + assertTextSpans(refs, ['
', 'NgForOf']); + assertFileNames(refs, ['index.d.ts', 'app.ts']); + }); + + it('should not support rename if directive is in a dts file', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor); + expect(renameLocations).toBeUndefined(); + }); }); }); describe('components', () => { - it('works for component classes', () => { - const {text, cursor} = extractCursorInfo(` + describe('when cursor is on component class', () => { + let cursor: number; + beforeEach(() => { + const cursorInfo = extractCursorInfo(` import {Component} from '@angular/core'; @Component({selector: 'my-comp', template: ''}) export class MyCo¦mp {}`); - const appFile = ` + const appFile = ` import {Component, NgModule} from '@angular/core'; import {MyComp} from './comp'; @@ -797,25 +1296,42 @@ describe('find references', () => { @NgModule({declarations: [AppCmp, MyComp]}) export class AppModule {} `; - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: appFile, isRoot: true}, - {name: _('/comp.ts'), contents: text}, - ]); - const refs = getReferencesAtPosition(_('/comp.ts'), cursor)!; - // 4 references are: class declaration, template usage, app import and use in declarations - // list. - expect(refs.length).toBe(4); - assertTextSpans(refs, ['', 'MyComp']); - assertFileNames(refs, ['app.ts', 'comp.ts']); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: appFile, isRoot: true}, + {name: _('/comp.ts'), contents: cursorInfo.text}, + ]); + }); + + it('finds references', () => { + const refs = getReferencesAtPosition(_('/comp.ts'), cursor)!; + // 4 references are: class declaration, template usage, app import and use in declarations + // list. + expect(refs.length).toBe(4); + assertTextSpans(refs, ['', 'MyComp']); + assertFileNames(refs, ['app.ts', 'comp.ts']); + }); + + it('gets rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/comp.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): If we register as an exclusive provider for TS, we may need to return + // results here and should exclude the template results. + // expect(renameLocations.length).toBe(3); + // assertTextSpans(renameLocations, ['MyComp']); + // assertFileNames(renameLocations, ['app.ts', 'comp.ts']); + }); }); - it('gets works when cursor is on element tag', () => { - const compFile = ` + describe('when cursor is on the element tag', () => { + let cursor: number; + beforeEach(() => { + const compFile = ` import {Component} from '@angular/core'; @Component({selector: 'my-comp', template: ''}) export class MyComp {}`; - const {text, cursor} = extractCursorInfo(` + const cursorInfo = extractCursorInfo(` import {Component, NgModule} from '@angular/core'; import {MyComp} from './comp'; @@ -826,43 +1342,42 @@ describe('find references', () => { @NgModule({declarations: [AppCmp, MyComp]}) export class AppModule {} `); - env = LanguageServiceTestEnvironment.setup([ - {name: _('/app.ts'), contents: text, isRoot: true}, - {name: _('/comp.ts'), contents: compFile}, - ]); - const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; - // 4 references are: class declaration, template usage, app import and use in declarations - // list. - expect(refs.length).toBe(4); - assertTextSpans(refs, ['', 'MyComp']); - assertFileNames(refs, ['app.ts', 'comp.ts']); + cursor = cursorInfo.cursor; + env = LanguageServiceTestEnvironment.setup([ + {name: _('/app.ts'), contents: cursorInfo.text, isRoot: true}, + {name: _('/comp.ts'), contents: compFile}, + ]); + }); + + it('gets references', () => { + const refs = getReferencesAtPosition(_('/app.ts'), cursor)!; + // 4 references are: class declaration, template usage, app import and use in declarations + // list. + expect(refs.length).toBe(4); + assertTextSpans(refs, ['', 'MyComp']); + assertFileNames(refs, ['app.ts', 'comp.ts']); + }); + + it('finds rename locations', () => { + const renameLocations = getRenameLocationsAtPosition(_('/app.ts'), cursor)!; + expect(renameLocations).toBeUndefined(); + // TODO(atscott): We may consider supporting rename of component selector in the future + // expect(renameLocations.length).toBe(2); + // assertTextSpans(renameLocations, ['my-comp']); + // assertFileNames(renameLocations, ['app.ts', 'comp.ts']); + }); }); }); function getReferencesAtPosition(fileName: string, position: number) { env.expectNoSourceDiagnostics(); const result = env.ngLS.getReferencesAtPosition(fileName, position); - return result?.map(humanizeReferenceEntry); + return result?.map((item) => humanizeDocumentSpanLike(item, env)); } - function humanizeReferenceEntry(entry: ts.ReferenceEntry): Stringy& - Pick { - const fileContents = env.host.readFile(entry.fileName); - if (!fileContents) { - throw new Error('Could not read file ${entry.fileName}'); - } - return { - ...entry, - textSpan: getText(fileContents, entry.textSpan), - contextSpan: entry.contextSpan ? getText(fileContents, entry.contextSpan) : undefined, - originalTextSpan: entry.originalTextSpan ? getText(fileContents, entry.originalTextSpan) : - undefined, - originalContextSpan: - entry.originalContextSpan ? getText(fileContents, entry.originalContextSpan) : undefined, - }; + function getRenameLocationsAtPosition(fileName: string, position: number) { + env.expectNoSourceDiagnostics(); + const result = env.ngLS.findRenameLocations(fileName, position); + return result?.map((item) => humanizeDocumentSpanLike(item, env)); } }); - -type Stringy = { - [P in keyof T]: string; -}; diff --git a/packages/language-service/ivy/test/test_utils.ts b/packages/language-service/ivy/test/test_utils.ts index 371fbd2bbb..ccc0408862 100644 --- a/packages/language-service/ivy/test/test_utils.ts +++ b/packages/language-service/ivy/test/test_utils.ts @@ -5,12 +5,11 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ - import {absoluteFrom as _} from '@angular/compiler-cli/src/ngtsc/file_system'; import {TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; -import {LanguageServiceTestEnvironment} from '@angular/language-service/ivy/test/env'; +import {LanguageServiceTestEnvironment, TestableOptions} from '@angular/language-service/ivy/test/env'; +import * as ts from 'typescript/lib/tsserverlibrary'; -import {MockServerHost} from './mock_host'; export function getText(contents: string, textSpan: ts.TextSpan) { return contents.substr(textSpan.start, textSpan.length); @@ -29,8 +28,8 @@ function getFirstClassDeclaration(declaration: string) { } export function createModuleWithDeclarations( - filesWithClassDeclarations: TestFile[], - externalResourceFiles: TestFile[] = []): LanguageServiceTestEnvironment { + filesWithClassDeclarations: TestFile[], externalResourceFiles: TestFile[] = [], + options: TestableOptions = {}): LanguageServiceTestEnvironment { const externalClasses = filesWithClassDeclarations.map(file => getFirstClassDeclaration(file.contents)); const externalImports = filesWithClassDeclarations.map(file => { @@ -51,30 +50,31 @@ export function createModuleWithDeclarations( `; const moduleFile = {name: _('/app-module.ts'), contents, isRoot: true}; return LanguageServiceTestEnvironment.setup( - [moduleFile, ...filesWithClassDeclarations, ...externalResourceFiles]); + [moduleFile, ...filesWithClassDeclarations, ...externalResourceFiles], options); } -export interface HumanizedDefinitionInfo { - fileName: string; - textSpan: string; - contextSpan: string|undefined; -} - -export function humanizeDefinitionInfo( - def: ts.DefinitionInfo, host: MockServerHost, - overrides: Map = new Map()): HumanizedDefinitionInfo { - const contents = (overrides.get(def.fileName) !== undefined ? overrides.get(def.fileName) : - host.readFile(def.fileName)) ?? +export function humanizeDocumentSpanLike( + item: T, env: LanguageServiceTestEnvironment, overrides: Map = new Map()): T& + Stringy { + const fileContents = (overrides.has(item.fileName) ? overrides.get(item.fileName) : + env.host.readFile(item.fileName)) ?? ''; - + if (!fileContents) { + throw new Error('Could not read file ${entry.fileName}'); + } return { - fileName: def.fileName, - textSpan: contents.substr(def.textSpan.start, def.textSpan.start + def.textSpan.length), - contextSpan: def.contextSpan ? - contents.substr(def.contextSpan.start, def.contextSpan.start + def.contextSpan.length) : - undefined, + ...item, + textSpan: getText(fileContents, item.textSpan), + contextSpan: item.contextSpan ? getText(fileContents, item.contextSpan) : undefined, + originalTextSpan: item.originalTextSpan ? getText(fileContents, item.originalTextSpan) : + undefined, + originalContextSpan: + item.originalContextSpan ? getText(fileContents, item.originalContextSpan) : undefined, }; } +type Stringy = { + [P in keyof T]: string; +}; export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNames: string[]) { const actualPaths = refs.map(r => r.fileName); @@ -85,4 +85,4 @@ export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNam export function assertTextSpans(items: Array<{textSpan: string}>, expectedTextSpans: string[]) { const actualSpans = items.map(item => item.textSpan); expect(new Set(actualSpans)).toEqual(new Set(expectedTextSpans)); -} \ No newline at end of file +} diff --git a/packages/language-service/ivy/test/type_definitions_spec.ts b/packages/language-service/ivy/test/type_definitions_spec.ts index 0fbe43830c..5822586745 100644 --- a/packages/language-service/ivy/test/type_definitions_spec.ts +++ b/packages/language-service/ivy/test/type_definitions_spec.ts @@ -10,7 +10,7 @@ import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import {LanguageServiceTestEnvironment} from './env'; -import {HumanizedDefinitionInfo, humanizeDefinitionInfo} from './test_utils'; +import {humanizeDocumentSpanLike} from './test_utils'; describe('type definitions', () => { let env: LanguageServiceTestEnvironment; @@ -48,8 +48,7 @@ describe('type definitions', () => { expect(def.contextSpan).toContain('DatePipe'); }); - function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}): - HumanizedDefinitionInfo[] { + function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}) { const {cursor, text} = env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride); env.expectNoSourceDiagnostics(); @@ -58,6 +57,6 @@ describe('type definitions', () => { expect(defs).toBeTruthy(); const overrides = new Map(); overrides.set(absoluteFrom('/app.html'), text); - return defs!.map(d => humanizeDefinitionInfo(d, env.host, overrides)); + return defs!.map(d => humanizeDocumentSpanLike(d, env, overrides)); } }); diff --git a/packages/language-service/ivy/ts_plugin.ts b/packages/language-service/ivy/ts_plugin.ts index b48adc87bd..64a3b654b9 100644 --- a/packages/language-service/ivy/ts_plugin.ts +++ b/packages/language-service/ivy/ts_plugin.ts @@ -64,8 +64,11 @@ export function create(info: ts.server.PluginCreateInfo): ts.LanguageService { function findRenameLocations( fileName: string, position: number, findInStrings: boolean, findInComments: boolean, providePrefixAndSuffixTextForRename?: boolean): readonly ts.RenameLocation[]|undefined { - // TODO(atscott): implement - return undefined; + // Most operations combine results from all extensions. However, rename locations are exclusive + // (results from only one extension are used) so our rename locations are a superset of the TS + // rename locations. As a result, we do not check the `angularOnly` flag here because we always + // want to include results at TS locations as well as locations in templates. + return ngLS.findRenameLocations(fileName, position); } function getCompletionsAtPosition(