diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 58b4110543..9e00e0cc92 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -16,7 +16,7 @@ import {ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../i import {DependencyTracker} from '../../incremental/api'; import {extractSemanticTypeParameters, isArrayEqual, isReferenceEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; -import {ClassPropertyMapping, ComponentResources, DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, Resource, ResourceRegistry} from '../../metadata'; +import {ClassPropertyMapping, ComponentResources, DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaType, Resource, ResourceRegistry} from '../../metadata'; import {EnumValue, PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; @@ -524,6 +524,7 @@ export class ComponentDecoratorHandler implements // the information about the component is available during the compile() phase. const ref = new Reference(node); this.metaRegistry.registerDirectiveMetadata({ + type: MetaType.Directive, ref, name: node.name.text, selector: analysis.meta.selector, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 3893ad0725..c9a4f2a831 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -13,7 +13,7 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference} from '../../imports'; import {areTypeParametersEqual, extractSemanticTypeParameters, isArrayEqual, isSetEqual, isSymbolEqual, SemanticDepGraphUpdater, SemanticSymbol, SemanticTypeParameter} from '../../incremental/semantic_graph'; -import {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, TemplateGuardMeta} from '../../metadata'; +import {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaType, TemplateGuardMeta} from '../../metadata'; import {extractDirectiveTypeCheckMeta} from '../../metadata/src/util'; import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; @@ -256,6 +256,7 @@ export class DirectiveDecoratorHandler implements // the information about the directive is available during the compile() phase. const ref = new Reference(node); this.metaRegistry.registerDirectiveMetadata({ + type: MetaType.Directive, ref, name: node.name.text, selector: analysis.meta.selector, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index 1dd0d4b961..f623bfaf6a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference} from '../../imports'; import {SemanticSymbol} from '../../incremental/semantic_graph'; -import {InjectableClassRegistry, MetadataRegistry} from '../../metadata'; +import {InjectableClassRegistry, MetadataRegistry, MetaType} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; @@ -27,6 +27,7 @@ import {compileResults, findAngularDecorator, getValidConstructorDependencies, m export interface PipeHandlerData { meta: R3PipeMetadata; classMetadata: R3ClassMetadata|null; + pipeNameExpr: ts.Expression; } /** @@ -134,6 +135,7 @@ export class PipeDecoratorHandler implements pure, }, classMetadata: extractClassMetadata(clazz, this.reflector, this.isCore), + pipeNameExpr, }, }; } @@ -144,7 +146,8 @@ export class PipeDecoratorHandler implements register(node: ClassDeclaration, analysis: Readonly): void { const ref = new Reference(node); - this.metaRegistry.registerPipeMetadata({ref, name: analysis.meta.pipeName}); + this.metaRegistry.registerPipeMetadata( + {type: MetaType.Pipe, ref, name: analysis.meta.pipeName, nameExpr: analysis.pipeNameExpr}); this.injectableRegistry.registerInjectable(node); } diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 90352bf11c..f33a869162 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -18,7 +18,7 @@ import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracke import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from '../../incremental'; import {SemanticSymbol} from '../../incremental/semantic_graph'; import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer'; -import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, ResourceRegistry} from '../../metadata'; +import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata'; import {ModuleWithProvidersScanner} from '../../modulewithproviders'; import {PartialEvaluator} from '../../partial_evaluator'; import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf'; @@ -528,6 +528,19 @@ export class NgCompiler { return {styles, template}; } + getMeta(classDecl: DeclarationNode): PipeMeta|DirectiveMeta|null { + if (!isNamedClassDeclaration(classDecl)) { + return null; + } + const ref = new Reference(classDecl); + const {metaReader} = this.ensureAnalyzed(); + const meta = metaReader.getPipeMetadata(ref) ?? metaReader.getDirectiveMetadata(ref); + if (meta === null) { + return null; + } + return meta; + } + /** * Perform Angular's analysis step (as a precursor to `getDiagnostics` or `prepareEmit`) * asynchronously. diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index 0b48090de9..d686e2faab 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -80,10 +80,17 @@ export interface DirectiveTypeCheckMeta { isGeneric: boolean; } +export enum MetaType { + Pipe, + Directive, +} + /** * Metadata collected for a directive within an NgModule's scope. */ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta { + type: MetaType.Directive; + ref: Reference; /** * Unparsed selector of the directive, or null if the directive does not have a selector. @@ -144,8 +151,10 @@ export interface TemplateGuardMeta { * Metadata for a pipe within an NgModule's scope. */ export interface PipeMeta { + type: MetaType.Pipe; ref: Reference; name: string; + nameExpr: ts.Expression|null; } /** diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index d88bf1a296..497d2e900f 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost, TypeValueReferenceKind} from '../../reflection'; -import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api'; +import {DirectiveMeta, MetadataReader, MetaType, NgModuleMeta, PipeMeta} from './api'; import {ClassPropertyMapping} from './property_mapping'; import {extractDirectiveTypeCheckMeta, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util'; @@ -95,6 +95,7 @@ export class DtsMetadataReader implements MetadataReader { const outputs = ClassPropertyMapping.fromMappedObject(readStringMapType(def.type.typeArguments[4])); return { + type: MetaType.Directive, ref, name: clazz.name.text, isComponent, @@ -131,7 +132,12 @@ export class DtsMetadataReader implements MetadataReader { return null; } const name = type.literal.text; - return {ref, name}; + return { + type: MetaType.Pipe, + ref, + name, + nameExpr: null, + }; } } diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 38a8ad79dd..0aa1d0c49f 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {Reference, ReferenceEmitter} from '../../imports'; -import {ClassPropertyMapping, CompoundMetadataRegistry, DirectiveMeta, LocalMetadataRegistry, MetadataRegistry, PipeMeta} from '../../metadata'; +import {ClassPropertyMapping, CompoundMetadataRegistry, DirectiveMeta, LocalMetadataRegistry, MetadataRegistry, MetaType, PipeMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {ScopeData} from '../src/api'; import {DtsModuleScopeResolver} from '../src/dependency'; @@ -232,6 +232,7 @@ describe('LocalModuleScopeRegistry', () => { function fakeDirective(ref: Reference): DirectiveMeta { const name = ref.debugName!; return { + type: MetaType.Directive, ref, name, selector: `[${ref.debugName}]`, @@ -255,7 +256,7 @@ function fakeDirective(ref: Reference): DirectiveMeta { function fakePipe(ref: Reference): PipeMeta { const name = ref.debugName!; - return {ref, name}; + return {type: MetaType.Pipe, ref, name, nameExpr: null}; } class MockDtsModuleScopeResolver implements DtsModuleScopeResolver { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts index 0640ac103b..1481d6301c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts @@ -307,4 +307,4 @@ export interface ClassSymbol { /** The position for the variable declaration for the class instance. */ shimLocation: ShimLocation; -} +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 34bbc26023..ae6545a127 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -13,7 +13,7 @@ import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} f import {TestFile} from '../../file_system/testing'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, Reexport, Reference, ReferenceEmitter, RelativePathStrategy} from '../../imports'; import {NOOP_INCREMENTAL_BUILD} from '../../incremental'; -import {ClassPropertyMapping, CompoundMetadataReader} from '../../metadata'; +import {ClassPropertyMapping, CompoundMetadataReader, MetaType} from '../../metadata'; import {NOOP_PERF_RECORDER} from '../../perf'; import {TsCreateProgramDriver} from '../../program_driver'; import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; @@ -595,6 +595,7 @@ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaratio if (decl.type === 'directive') { scope.directives.push({ + type: MetaType.Directive, ref: new Reference(declClass), baseClass: null, name: decl.name, @@ -618,8 +619,10 @@ function makeScope(program: ts.Program, sf: ts.SourceFile, decls: TestDeclaratio }); } else if (decl.type === 'pipe') { scope.pipes.push({ + type: MetaType.Pipe, ref: new Reference(declClass), name: decl.pipeName, + nameExpr: null, }); } } diff --git a/packages/language-service/ivy/language_service.ts b/packages/language-service/ivy/language_service.ts index 26fa982920..79f731f83a 100644 --- a/packages/language-service/ivy/language_service.ts +++ b/packages/language-service/ivy/language_service.ts @@ -192,7 +192,8 @@ export class LanguageService { findRenameLocations(fileName: string, position: number): readonly ts.RenameLocation[]|undefined { return this.withCompilerAndPerfTracing(PerfPhase.LsReferencesAndRenames, (compiler) => { return new RenameBuilder(this.programDriver, this.tsLS, compiler) - .findRenameLocations(fileName, position); + .findRenameLocations(fileName, position) ?? + undefined; }); } diff --git a/packages/language-service/ivy/references_and_rename.ts b/packages/language-service/ivy/references_and_rename.ts index 4ad85509fe..c99fe80157 100644 --- a/packages/language-service/ivy/references_and_rename.ts +++ b/packages/language-service/ivy/references_and_rename.ts @@ -8,12 +8,14 @@ import {AST, TmplAstNode} from '@angular/compiler'; import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; +import {MetaType, PipeMeta} from '@angular/compiler-cli/src/ngtsc/metadata'; import {PerfPhase} from '@angular/compiler-cli/src/ngtsc/perf'; import {ProgramDriver} from '@angular/compiler-cli/src/ngtsc/program_driver'; +import {SymbolKind} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import * as ts from 'typescript'; -import {convertToTemplateDocumentSpan, createLocationKey, getRenameTextAndSpanAtPosition, getTargetDetailsAtTemplatePosition} from './references_and_rename_utils'; -import {findTightestNode} from './ts_utils'; +import {convertToTemplateDocumentSpan, createLocationKey, FilePosition, getParentClassMeta, getRenameTextAndSpanAtPosition, getTargetDetailsAtTemplatePosition, TemplateLocationDetails} from './references_and_rename_utils'; +import {collectMemberMethods, findTightestNode} from './ts_utils'; import {getTemplateInfoAtPosition, TemplateInfo} from './utils'; export class ReferencesBuilder { @@ -73,23 +75,70 @@ export class ReferencesBuilder { } enum RequestKind { - Template, - TypeScript, + DirectFromTemplate, + DirectFromTypeScript, + PipeName, + Selector, } -interface TemplateRequest { - kind: RequestKind.Template; - requestNode: TmplAstNode|AST; - position: number; +/** The context needed to perform a rename of a pipe name. */ +interface PipeRenameContext { + type: RequestKind.PipeName; + + /** The string literal for the pipe name that appears in the @Pipe meta */ + pipeNameExpr: ts.StringLiteral; + + /** + * The location to use for querying the native TS LS for rename positions. This will be the + * pipe's transform method. + */ + renamePosition: FilePosition; } -interface TypeScriptRequest { - kind: RequestKind.TypeScript; +/** The context needed to perform a rename of a directive/component selector. */ +interface SelectorRenameContext { + type: RequestKind.Selector; + + /** The string literal that appears in the directive/component metadata. */ + selectorExpr: ts.StringLiteral; + + /** + * The location to use for querying the native TS LS for rename positions. This will be the + * component/directive class itself. Doing so will allow us to find the location of the + * directive/component instantiations, which map to template elements. + */ + renamePosition: FilePosition; +} + +interface DirectFromTypescriptRenameContext { + type: RequestKind.DirectFromTypeScript; + + /** The node that is being renamed. */ requestNode: ts.Node; } -type RequestOrigin = TemplateRequest|TypeScriptRequest; +interface DirectFromTemplateRenameContext { + type: RequestKind.DirectFromTemplate; + /** The position in the TCB file to use as the request to the native TSLS for renaming. */ + renamePosition: FilePosition; + + /** The position in the template the request originated from. */ + templatePosition: number; + + /** The target node in the template AST that corresponds to the template position. */ + requestNode: AST|TmplAstNode; +} + +type IndirectRenameContext = PipeRenameContext|SelectorRenameContext; +type RenameRequest = + IndirectRenameContext|DirectFromTemplateRenameContext|DirectFromTypescriptRenameContext; + +function isDirectRenameContext(context: RenameRequest): context is DirectFromTemplateRenameContext| + DirectFromTypescriptRenameContext { + return context.type === RequestKind.DirectFromTemplate || + context.type === RequestKind.DirectFromTypeScript; +} export class RenameBuilder { private readonly ttc = this.compiler.getTemplateTypeChecker(); @@ -133,94 +182,86 @@ export class RenameBuilder { }); } - findRenameLocations(filePath: string, position: number): readonly ts.RenameLocation[]|undefined { + findRenameLocations(filePath: string, position: number): readonly ts.RenameLocation[]|null { this.ttc.generateAllTypeCheckBlocks(); return this.compiler.perfRecorder.inPhase(PerfPhase.LsReferencesAndRenames, () => { const templateInfo = getTemplateInfoAtPosition(filePath, position, this.compiler); // We could not get a template at position so we assume the request came from outside the // template. if (templateInfo === undefined) { - const requestNode = this.getTsNodeAtPosition(filePath, position); - if (requestNode === null) { - return undefined; + const renameRequest = this.buildRenameRequestAtTypescriptPosition(filePath, position); + if (renameRequest === null) { + return null; } - const requestOrigin: TypeScriptRequest = {kind: RequestKind.TypeScript, requestNode}; - return this.findRenameLocationsAtTypescriptPosition(filePath, position, requestOrigin); + return this.findRenameLocationsAtTypescriptPosition(renameRequest); } - return this.findRenameLocationsAtTemplatePosition(templateInfo, position); }); } private findRenameLocationsAtTemplatePosition(templateInfo: TemplateInfo, position: number): - readonly ts.RenameLocation[]|undefined { + readonly ts.RenameLocation[]|null { const allTargetDetails = getTargetDetailsAtTemplatePosition(templateInfo, position, this.ttc); if (allTargetDetails === null) { - return undefined; + return null; + } + const renameRequests = this.buildRenameRequestsFromTemplateDetails(allTargetDetails, position); + if (renameRequests === null) { + return null; } 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); + for (const renameRequest of renameRequests) { + const locations = this.findRenameLocationsAtTypescriptPosition(renameRequest); + // 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 === null) { + return null; } + allRenameLocations.push(...locations); } - return allRenameLocations.length > 0 ? allRenameLocations : undefined; + return allRenameLocations.length > 0 ? allRenameLocations : null; } - findRenameLocationsAtTypescriptPosition( - filePath: string, position: number, - requestOrigin: RequestOrigin): readonly ts.RenameLocation[]|undefined { + findRenameLocationsAtTypescriptPosition(renameRequest: RenameRequest): + readonly ts.RenameLocation[]|null { return this.compiler.perfRecorder.inPhase(PerfPhase.LsReferencesAndRenames, () => { - let originalNodeText: string; - if (requestOrigin.kind === RequestKind.TypeScript) { - originalNodeText = requestOrigin.requestNode.getText(); - } else { - const templateNodeText = - getRenameTextAndSpanAtPosition(requestOrigin.requestNode, requestOrigin.position); - if (templateNodeText === null) { - return undefined; - } - originalNodeText = templateNodeText.text; + const renameInfo = getExpectedRenameTextAndInitalRenameEntries(renameRequest); + if (renameInfo === null) { + return null; } - - const locations = this.tsLS.findRenameLocations( - filePath, position, /*findInStrings*/ false, /*findInComments*/ false); + const {entries, expectedRenameText} = renameInfo; + const {fileName, position} = getRenameRequestPosition(renameRequest); + const findInStrings = false; + const findInComments = false; + const locations = + this.tsLS.findRenameLocations(fileName, position, findInStrings, findInComments); if (locations === undefined) { - return undefined; + return null; } - const entries: Map = new Map(); 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 = convertToTemplateDocumentSpan( - location, this.ttc, this.driver.getProgram(), originalNodeText); + location, this.ttc, this.driver.getProgram(), expectedRenameText); // 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; + return null; } entries.set(createLocationKey(entry), entry); } else { + if (!isDirectRenameContext(renameRequest)) { + // Discard any non-template results for non-direct renames. We should only rename + // template results + the name/selector/alias `ts.Expression`. The other results + // will be the the `ts.Identifier` of the transform method (pipe rename) or the + // directive class (selector rename). + continue; + } // 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; + if (refNode === null || refNode.getText() !== expectedRenameText) { + return null; } entries.set(createLocationKey(location), location); } @@ -236,4 +277,118 @@ export class RenameBuilder { } return findTightestNode(sf, position) ?? null; } + + private buildRenameRequestsFromTemplateDetails( + allTargetDetails: TemplateLocationDetails[], templatePosition: number): RenameRequest[]|null { + const renameRequests: RenameRequest[] = []; + for (const targetDetails of allTargetDetails) { + for (const location of targetDetails.typescriptLocations) { + if (targetDetails.symbol.kind === SymbolKind.Pipe) { + const meta = + this.compiler.getMeta(targetDetails.symbol.classSymbol.tsSymbol.valueDeclaration); + if (meta === null || meta.type !== MetaType.Pipe) { + return null; + } + const renameRequest = this.buildPipeRenameRequest(meta); + if (renameRequest === null) { + return null; + } + renameRequests.push(renameRequest); + } else { + const renameRequest: RenameRequest = { + type: RequestKind.DirectFromTemplate, + templatePosition, + requestNode: targetDetails.templateTarget, + renamePosition: location + }; + renameRequests.push(renameRequest); + } + } + } + return renameRequests; + } + + private buildRenameRequestAtTypescriptPosition(filePath: string, position: number): RenameRequest + |null { + const requestNode = this.getTsNodeAtPosition(filePath, position); + if (requestNode === null) { + return null; + } + const meta = getParentClassMeta(requestNode, this.compiler); + if (meta !== null && meta.type === MetaType.Pipe && meta.nameExpr === requestNode) { + return this.buildPipeRenameRequest(meta); + } else { + return {type: RequestKind.DirectFromTypeScript, requestNode}; + } + } + + private buildPipeRenameRequest(meta: PipeMeta): PipeRenameContext|null { + if (!ts.isClassDeclaration(meta.ref.node) || meta.nameExpr === null || + !ts.isStringLiteral(meta.nameExpr)) { + return null; + } + const typeChecker = this.driver.getProgram().getTypeChecker(); + const memberMethods = collectMemberMethods(meta.ref.node, typeChecker) ?? []; + const pipeTransformNode: ts.MethodDeclaration|undefined = + memberMethods.find(m => m.name.getText() === 'transform'); + if (pipeTransformNode === undefined) { + return null; + } + return { + type: RequestKind.PipeName, + pipeNameExpr: meta.nameExpr, + renamePosition: { + fileName: pipeTransformNode.getSourceFile().fileName, + position: pipeTransformNode.getStart(), + } + }; + } +} + +/** + * From the provided `RenameRequest`, determines what text we should expect all produced + * `ts.RenameLocation`s to have and creates an initial entry for indirect renames (one which is + * required for the rename operation, but cannot be found by the native TS LS). + */ +function getExpectedRenameTextAndInitalRenameEntries(renameRequest: RenameRequest): + {expectedRenameText: string, entries: Map}|null { + let expectedRenameText: string; + const entries = new Map(); + if (renameRequest.type === RequestKind.DirectFromTypeScript) { + expectedRenameText = renameRequest.requestNode.getText(); + } else if (renameRequest.type === RequestKind.DirectFromTemplate) { + const templateNodeText = + getRenameTextAndSpanAtPosition(renameRequest.requestNode, renameRequest.templatePosition); + if (templateNodeText === null) { + return null; + } + expectedRenameText = templateNodeText.text; + } else if (renameRequest.type === RequestKind.PipeName) { + const {pipeNameExpr} = renameRequest; + expectedRenameText = pipeNameExpr.text; + const entry: ts.RenameLocation = { + fileName: renameRequest.pipeNameExpr.getSourceFile().fileName, + textSpan: {start: pipeNameExpr.getStart() + 1, length: pipeNameExpr.getText().length - 2}, + }; + entries.set(createLocationKey(entry), entry); + } else { + // TODO(atscott): Implement other types of special renames + return null; + } + + return {entries, expectedRenameText}; +} + +/** + * Given a `RenameRequest`, determines the `FilePosition` to use asking the native TS LS for rename + * locations. + */ +function getRenameRequestPosition(renameRequest: RenameRequest): FilePosition { + const fileName = renameRequest.type === RequestKind.DirectFromTypeScript ? + renameRequest.requestNode.getSourceFile().fileName : + renameRequest.renamePosition.fileName; + const position = renameRequest.type === RequestKind.DirectFromTypeScript ? + renameRequest.requestNode.getStart() : + renameRequest.renamePosition.position; + return {fileName, position}; } \ No newline at end of file diff --git a/packages/language-service/ivy/references_and_rename_utils.ts b/packages/language-service/ivy/references_and_rename_utils.ts index 0753ecef7e..274f89837e 100644 --- a/packages/language-service/ivy/references_and_rename_utils.ts +++ b/packages/language-service/ivy/references_and_rename_utils.ts @@ -6,20 +6,26 @@ * found in the LICENSE file at https://angular.io/license */ 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} from '@angular/compiler-cli/src/ngtsc/file_system'; -import {DirectiveSymbol, ShimLocation, SymbolKind, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; +import {DirectiveMeta, PipeMeta} from '@angular/compiler-cli/src/ngtsc/metadata'; +import {DirectiveSymbol, ShimLocation, Symbol, SymbolKind, TemplateTypeChecker} 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 {findTightestNode, getParentClassDeclaration} from './ts_utils'; import {getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateLocationFromShimLocation, isWithin, TemplateInfo, toTextSpan} from './utils'; -interface FilePosition { +/** Represents a location in a file. */ +export interface FilePosition { fileName: string; position: number; } +/** + * Converts a `ShimLocation` to a more genericly named `FilePosition`. + */ function toFilePosition(shimLocation: ShimLocation): FilePosition { return {fileName: shimLocation.shimPath, position: shimLocation.positionInShimFile}; } @@ -35,9 +41,18 @@ export interface TemplateLocationDetails { * directives or a directive and one of its inputs. */ typescriptLocations: FilePosition[]; + + /** + * The resolved Symbol for the template target. + */ + symbol: Symbol; } +/** + * Takes a position in a template and finds equivalent targets in TS files as well as details about + * the targeted template node. + */ export function getTargetDetailsAtTemplatePosition( {template, component}: TemplateInfo, position: number, templateTypeChecker: TemplateTypeChecker): TemplateLocationDetails[]|null { @@ -69,7 +84,11 @@ export function getTargetDetailsAtTemplatePosition( break; case SymbolKind.Element: { const matches = getDirectiveMatchesForElementTag(symbol.templateNode, symbol.directives); - details.push({typescriptLocations: getPositionsForDirectives(matches), templateTarget}); + details.push({ + typescriptLocations: getPositionsForDirectives(matches), + templateTarget, + symbol, + }); break; } case SymbolKind.DomBinding: { @@ -84,6 +103,7 @@ export function getTargetDetailsAtTemplatePosition( details.push({ typescriptLocations: getPositionsForDirectives(directives), templateTarget, + symbol, }); break; } @@ -91,6 +111,7 @@ export function getTargetDetailsAtTemplatePosition( details.push({ typescriptLocations: [toFilePosition(symbol.referenceVarLocation)], templateTarget, + symbol, }); break; } @@ -102,12 +123,14 @@ export function getTargetDetailsAtTemplatePosition( details.push({ typescriptLocations: [toFilePosition(symbol.initializerLocation)], templateTarget, + symbol, }); } else if (isWithin(position, templateTarget.keySpan)) { // In the keySpan of the variable, we want to get the reference of the local variable. details.push({ typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget, + symbol, }); } } else { @@ -116,6 +139,7 @@ export function getTargetDetailsAtTemplatePosition( details.push({ typescriptLocations: [toFilePosition(symbol.localVarLocation)], templateTarget, + symbol, }); } break; @@ -125,12 +149,17 @@ export function getTargetDetailsAtTemplatePosition( details.push({ typescriptLocations: symbol.bindings.map(binding => toFilePosition(binding.shimLocation)), templateTarget, + symbol, }); break; } case SymbolKind.Pipe: case SymbolKind.Expression: { - details.push({typescriptLocations: [toFilePosition(symbol.shimLocation)], templateTarget}); + details.push({ + typescriptLocations: [toFilePosition(symbol.shimLocation)], + templateTarget, + symbol, + }); break; } } @@ -139,6 +168,9 @@ export function getTargetDetailsAtTemplatePosition( return details.length > 0 ? details : null; } +/** + * Given a set of `DirectiveSymbol`s, finds the equivalent `FilePosition` of the class declaration. + */ function getPositionsForDirectives(directives: Set): FilePosition[] { const allDirectives: FilePosition[] = []; for (const dir of directives.values()) { @@ -164,6 +196,13 @@ export function createLocationKey(ds: ts.DocumentSpan) { return ds.fileName + ds.textSpan.start + ds.textSpan.length; } +/** + * Converts a given `ts.DocumentSpan` in a shim file to its equivalent `ts.DocumentSpan` in the + * template. + * + * You can optionally provide a `requiredNodeText` that ensures the equivalent template node's text + * matches. If it does not, this function will return `null`. + */ export function convertToTemplateDocumentSpan( shimDocumentSpan: T, templateTypeChecker: TemplateTypeChecker, program: ts.Program, requiredNodeText?: string): T|null { @@ -206,6 +245,9 @@ export function convertToTemplateDocumentSpan( }; } +/** + * Finds the text and `ts.TextSpan` for the node at a position in a template. + */ export function getRenameTextAndSpanAtPosition( node: TmplAstNode|AST, position: number): {text: string, span: ts.TextSpan}|null { if (node instanceof TmplAstBoundAttribute || node instanceof TmplAstTextAttribute || @@ -222,12 +264,9 @@ export function getRenameTextAndSpanAtPosition( } } - 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) { + node instanceof SafePropertyRead || node instanceof SafeMethodCall || + node instanceof BindingPipe) { return {text: node.name, span: toTextSpan(node.nameSpan)}; } else if (node instanceof LiteralPrimitive) { const span = toTextSpan(node.sourceSpan); @@ -241,4 +280,18 @@ export function getRenameTextAndSpanAtPosition( } return null; +} + +/** + * Retrives the `PipeMeta` or `DirectiveMeta` of the given `ts.Node`'s parent class. + * + * Returns `null` if the node has no parent class or there is no meta associated with the class. + */ +export function getParentClassMeta(requestNode: ts.Node, compiler: NgCompiler): PipeMeta| + DirectiveMeta|null { + const parentClass = getParentClassDeclaration(requestNode); + if (parentClass === undefined) { + return null; + } + return compiler.getMeta(parentClass); } \ No newline at end of file diff --git a/packages/language-service/ivy/test/references_and_rename_spec.ts b/packages/language-service/ivy/test/references_and_rename_spec.ts index 70295cac80..45bdf5f192 100644 --- a/packages/language-service/ivy/test/references_and_rename_spec.ts +++ b/packages/language-service/ivy/test/references_and_rename_spec.ts @@ -784,12 +784,74 @@ describe('find references and rename locations', () => { it('should find rename locations', () => { const renameLocations = getRenameLocationsAtPosition(file)!; - expect(renameLocations).toBeUndefined(); + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['prefix-pipe.ts', 'app.ts']); + assertTextSpans(renameLocations, ['prefixPipe']); + }); - // 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 get rename info', () => { + const result = file.getRenameInfo() as ts.RenameInfoSuccess; + expect(result.canRename).toEqual(true); + expect(result.displayName).toEqual('prefixPipe'); + }); + }); + + describe('when cursor is on pipe name expression', () => { + it('finds rename locations and rename info', () => { + const files = { + '/app.ts': ` + import {Component} from '@angular/core'; + + @Component({template: '{{birthday | prefixPipe: "MM/dd/yy"}}'}) + export class AppCmp { + birthday = ''; + } + `, + 'prefix-pipe.ts': prefixPipe + }; + env = LanguageServiceTestEnv.setup(); + const project = createModuleAndProjectWithDeclarations(env, 'test', files); + const file = project.openFile('app.ts'); + file.moveCursorToText('prefi¦xPipe:'); + const renameLocations = getRenameLocationsAtPosition(file)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['prefix-pipe.ts', 'app.ts']); + assertTextSpans(renameLocations, ['prefixPipe']); + + const result = file.getRenameInfo() as ts.RenameInfoSuccess; + expect(result.canRename).toEqual(true); + expect(result.displayName).toEqual('prefixPipe'); + }); + + it('finds rename locations in base class', () => { + const files = { + '/base_pipe.ts': ` + import {Pipe, PipeTransform} from '@angular/core'; + + @Pipe({ name: 'basePipe' }) + export class BasePipe implements PipeTransform { + transform(value: string, prefix: string): string; + transform(value: number, prefix: number): number; + transform(value: string|number, prefix: string|number): string|number { + return ''; + } + }`, + 'prefix_pipe.ts': prefixPipe, + 'app.ts': ` + import {Component} from '@angular/core'; + + @Component({template: '{{"a" | prefixPipe: "MM/dd/yy"}}'}) + export class AppCmp { } + ` + }; + env = LanguageServiceTestEnv.setup(); + const project = createModuleAndProjectWithDeclarations(env, 'test', files); + const file = project.openFile('prefix_pipe.ts'); + file.moveCursorToText(`'prefi¦xPipe'`); + const renameLocations = getRenameLocationsAtPosition(file)!; + expect(renameLocations.length).toBe(2); + assertFileNames(renameLocations, ['prefix_pipe.ts', 'app.ts']); + assertTextSpans(renameLocations, ['prefixPipe']); }); }); @@ -884,7 +946,7 @@ describe('find references and rename locations', () => { @Directive({selector: '[string-model]'}) export class OtherDir { - @Input('model') model!: any; + @Input('model') otherDirAliasedInput!: any; } `, 'string-model.ts': dirFileContents, @@ -903,22 +965,21 @@ describe('find references and rename locations', () => { file.moveCursorToText('[mod¦el]'); }); - // TODO(atscott): This test does not pass because the template symbol builder only returns one - // binding. + // TODO(atscott): Does not work because we don't fully de-duplicate xit('should find references', () => { const refs = getReferencesAtPosition(file)!; expect(refs.length).toEqual(3); - assertFileNames(refs, ['string-model.ts', 'app.ts', 'other-dir']); + assertFileNames(refs, ['string-model.ts', 'app.ts', 'other-dir.ts']); 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', () => { + it('should find rename locations', () => { const renameLocations = getRenameLocationsAtPosition(file)!; expect(renameLocations).toBeUndefined(); - // TODO(atscott): + // TODO(atscott): The below assertions are the correct ones if we were supporting aliases // expect(renameLocations.length).toEqual(3); // assertFileNames(renameLocations, ['string-model.ts', 'app.ts', 'other-dir']); // assertTextSpans(renameLocations, ['model']); diff --git a/packages/language-service/ivy/test/ts_utils_spec.ts b/packages/language-service/ivy/test/ts_utils_spec.ts new file mode 100644 index 0000000000..657516f8e4 --- /dev/null +++ b/packages/language-service/ivy/test/ts_utils_spec.ts @@ -0,0 +1,78 @@ +import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; +import * as ts from 'typescript'; +import {LanguageServiceTestEnv, OpenBuffer, Project} from '../testing'; +import {collectMemberMethods, findTightestNode} from '../ts_utils'; + +describe('ts utils', () => { + describe('collectMemberMethods', () => { + beforeEach(() => { + initMockFileSystem('Native'); + }) + + it('gets only methods in class, not getters, setters, or properties', () => { + const files = { + 'app.ts': ` + export class AppCmp { + prop!: string; + get myString(): string { + return ''; + } + set myString(v: string) { + } + + one() {} + two() {} + }`, + }; + const env = LanguageServiceTestEnv.setup(); + const project = env.addProject('test', files); + const appFile = project.openFile('app.ts'); + appFile.moveCursorToText('AppC¦mp'); + const memberMethods = getMemberMethodNames(project, appFile); + expect(memberMethods).toEqual(['one', 'two']); + }); + + it('gets inherited methods in class', () => { + const files = { + 'app.ts': ` + export class BaseClass { + baseMethod() {} + } + export class AppCmp extends BaseClass {}`, + }; + const env = LanguageServiceTestEnv.setup(); + const project = env.addProject('test', files); + const appFile = project.openFile('app.ts'); + appFile.moveCursorToText('AppC¦mp'); + const memberMethods = getMemberMethodNames(project, appFile); + expect(memberMethods).toEqual(['baseMethod']); + }); + + it('does not return duplicates if base method is overridden', () => { + const files = { + 'app.ts': ` + export class BaseClass { + baseMethod() {} + } + export class AppCmp extends BaseClass { + baseMethod() {} + }`, + }; + const env = LanguageServiceTestEnv.setup(); + const project = env.addProject('test', files); + const appFile = project.openFile('app.ts'); + appFile.moveCursorToText('AppC¦mp'); + const memberMethods = getMemberMethodNames(project, appFile); + expect(memberMethods).toEqual(['baseMethod']); + }); + + function getMemberMethodNames(project: Project, file: OpenBuffer): string[] { + const sf = project.getSourceFile('app.ts')!; + const node = findTightestNode(sf, file.cursor)!; + expect(ts.isClassDeclaration(node.parent)).toBe(true) + return collectMemberMethods(node.parent as ts.ClassDeclaration, project.getTypeChecker()) + .map(m => m.name.getText()) + .sort(); + } + }); +}); diff --git a/packages/language-service/ivy/testing/src/project.ts b/packages/language-service/ivy/testing/src/project.ts index 307eadf66d..b3dc513405 100644 --- a/packages/language-service/ivy/testing/src/project.ts +++ b/packages/language-service/ivy/testing/src/project.ts @@ -117,6 +117,15 @@ export class Project { return this.buffers.get(projectFileName)!; } + getSourceFile(projectFileName: string): ts.SourceFile|undefined { + const fileName = absoluteFrom(`/${this.name}/${projectFileName}`); + return this.tsProject.getSourceFile(this.projectService.toPath(fileName)); + } + + getTypeChecker(): ts.TypeChecker { + return this.ngLS.compilerFactory.getOrCreate().getCurrentProgram().getTypeChecker(); + } + getDiagnosticsForFile(projectFileName: string): ts.Diagnostic[] { const fileName = absoluteFrom(`/${this.name}/${projectFileName}`); const diagnostics: ts.Diagnostic[] = []; diff --git a/packages/language-service/ivy/ts_utils.ts b/packages/language-service/ivy/ts_utils.ts index ef52343337..688535247c 100644 --- a/packages/language-service/ivy/ts_utils.ts +++ b/packages/language-service/ivy/ts_utils.ts @@ -79,3 +79,18 @@ export function getClassDeclFromDecoratorProp(propAsgnNode: ts.PropertyAssignmen const classDeclNode = decorator.parent; return classDeclNode; } + +/** + * Collects all member methods, including those from base classes. + */ +export function collectMemberMethods( + clazz: ts.ClassDeclaration, typeChecker: ts.TypeChecker): ts.MethodDeclaration[] { + const members: ts.MethodDeclaration[] = []; + const apparentProps = typeChecker.getTypeAtLocation(clazz).getApparentProperties(); + for (const prop of apparentProps) { + if (ts.isMethodDeclaration(prop.valueDeclaration) && prop.valueDeclaration) { + members.push(prop.valueDeclaration); + } + } + return members; +}