Skip to content

Commit

Permalink
feat(language-service): Enable renaming of pipes (#40523)
Browse files Browse the repository at this point in the history
This commit updates the logic in the LS renaming to handle renaming of
pipes, both from the name expression in the pipe metadata as well as
from the template.

The approach here is to introduce a new concept for renaming: an
"indirect" rename. In this type of rename, we find rename locations
in with the native TS Language Service using a different node than the
one we are renaming. Using pipes as an example, if we want to rename the
pipe name from the string literal expression, we use the transform
method to find rename locations rather than the string literal itself
(which will not return any results because it's just a string).

So the general approach is:
* Determine the details about the requested rename location, i.e. the
  targeted template node and symbol for a template rename, or the TS
  node for a rename outside a template.
* Using the details of the location, determine if the node is attempting
  to rename something that is an indirect rename (pipes, selectors,
  bindings). Other renames are considered "direct" and we use whatever
  results the native TSLS returns for the rename locations.
* In the case of indirect renames, we throw out results that do not
  appear in the templates (in this case, the shim files). These results will be
  for the "indirect" rename that we don't want to touch, but are only
  using to find template results.
* Create an additional rename result for the string literal expression
  that is used for the input/output alias, the pipe name, or the
  selector.

 Note that renaming is moving towards being much more accurate in its
 results than "find references". When the approach for renaming
 stabilizes, we may want to then port the changes back to being shared
 with the approach for retrieving references.

PR Close #40523
  • Loading branch information
atscott authored and alxhub committed May 6, 2021
1 parent c1bcbeb commit a86ca4f
Show file tree
Hide file tree
Showing 16 changed files with 503 additions and 94 deletions.
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts
Expand Up @@ -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';
Expand All @@ -27,6 +27,7 @@ import {compileResults, findAngularDecorator, getValidConstructorDependencies, m
export interface PipeHandlerData {
meta: R3PipeMetadata;
classMetadata: R3ClassMetadata|null;
pipeNameExpr: ts.Expression;
}

/**
Expand Down Expand Up @@ -134,6 +135,7 @@ export class PipeDecoratorHandler implements
pure,
},
classMetadata: extractClassMetadata(clazz, this.reflector, this.isCore),
pipeNameExpr,
},
};
}
Expand All @@ -144,7 +146,8 @@ export class PipeDecoratorHandler implements

register(node: ClassDeclaration, analysis: Readonly<PipeHandlerData>): 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);
}
Expand Down
15 changes: 14 additions & 1 deletion packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Expand Up @@ -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<ClassDeclaration>;
/**
* Unparsed selector of the directive, or null if the directive does not have a selector.
Expand Down Expand Up @@ -144,8 +151,10 @@ export interface TemplateGuardMeta {
* Metadata for a pipe within an NgModule's scope.
*/
export interface PipeMeta {
type: MetaType.Pipe;
ref: Reference<ClassDeclaration>;
name: string;
nameExpr: ts.Expression|null;
}

/**
Expand Down
10 changes: 8 additions & 2 deletions packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -232,6 +232,7 @@ describe('LocalModuleScopeRegistry', () => {
function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {
const name = ref.debugName!;
return {
type: MetaType.Directive,
ref,
name,
selector: `[${ref.debugName}]`,
Expand All @@ -255,7 +256,7 @@ function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {

function fakePipe(ref: Reference<ClassDeclaration>): PipeMeta {
const name = ref.debugName!;
return {ref, name};
return {type: MetaType.Pipe, ref, name, nameExpr: null};
}

class MockDtsModuleScopeResolver implements DtsModuleScopeResolver {
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts
Expand Up @@ -307,4 +307,4 @@ export interface ClassSymbol {

/** The position for the variable declaration for the class instance. */
shimLocation: ShimLocation;
}
}
5 changes: 4 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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,
});
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/language-service/ivy/language_service.ts
Expand Up @@ -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;
});
}

Expand Down

0 comments on commit a86ca4f

Please sign in to comment.