Skip to content

Commit

Permalink
feat(language-service): Enable renaming of pipes
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.
  • Loading branch information
atscott committed Jan 26, 2021
1 parent 31852c0 commit ae9f7c0
Show file tree
Hide file tree
Showing 14 changed files with 432 additions and 101 deletions.
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {absoluteFrom, relative} from '../../file_system';
import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
import {DependencyTracker} from '../../incremental/api';
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} from '../../partial_evaluator';
import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
import {ComponentScopeReader, LocalModuleScopeRegistry, TypeCheckScopeRegistry} from '../../scope';
Expand Down Expand Up @@ -361,6 +361,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
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder, Reference} from '../../imports';
import {ClassPropertyMapping, DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata';
import {ClassPropertyMapping, DirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaType} from '../../metadata';
import {extractDirectiveTypeCheckMeta} from '../../metadata/src/util';
import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
Expand Down Expand Up @@ -121,6 +121,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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder, Reference} from '../../imports';
import {InjectableClassRegistry, MetadataRegistry} from '../../metadata';
import {InjectableClassRegistry, MetadataRegistry, MetaType} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
import {LocalModuleScopeRegistry} from '../../scope';
Expand All @@ -25,6 +25,7 @@ import {findAngularDecorator, getValidConstructorDependencies, makeDuplicateDecl
export interface PipeHandlerData {
meta: R3PipeMetadata;
metadataStmt: Statement|null;
pipeNameExpr: ts.Expression;
}

export class PipeDecoratorHandler implements DecoratorHandler<Decorator, PipeHandlerData, unknown> {
Expand Down Expand Up @@ -110,13 +111,15 @@ export class PipeDecoratorHandler implements DecoratorHandler<Decorator, PipeHan
},
metadataStmt: generateSetClassMetadataCall(
clazz, this.reflector, this.defaultImportRecorder, this.isCore),
pipeNameExpr,
},
};
}

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
17 changes: 15 additions & 2 deletions packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import {LogicalFileSystem, resolve} from '../../file_system';
import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracker, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, PrivateExportAliasingHost, R3SymbolsImportRewriter, Reference, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesAliasingHost, UnifiedModulesStrategy} from '../../imports';
import {IncrementalBuildStrategy, IncrementalDriver} from '../../incremental';
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 {NOOP_PERF_RECORDER, PerfRecorder} from '../../perf';
import {DeclarationNode, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {ClassDeclaration, DeclarationNode, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {AdapterResourceLoader} from '../../resource';
import {entryPointKeyFor, NgModuleRouteAnalyzer} from '../../routing';
import {ComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../scope';
Expand Down Expand Up @@ -272,6 +272,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
Original file line number Diff line number Diff line change
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
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
Original file line number Diff line number Diff line change
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,7 @@ 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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} from '../../imports';
import {NOOP_INCREMENTAL_BUILD} from '../../incremental';
import {ClassPropertyMapping, CompoundMetadataReader} from '../../metadata';
import {ClassPropertyMapping, CompoundMetadataReader, MetaType} from '../../metadata';
import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {ComponentScopeReader, LocalModuleScope, ScopeData, TypeCheckScopeRegistry} from '../../scope';
import {makeProgram} from '../../testing';
Expand Down Expand Up @@ -549,6 +549,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 @@ -572,8 +573,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
2 changes: 1 addition & 1 deletion packages/language-service/ivy/language_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export class LanguageService {
const results = new RenameBuilder(this.strategy, this.tsLS, compiler)
.findRenameLocations(fileName, position);
this.compilerFactory.registerLastKnownProgram();
return results;
return results ?? undefined;
}

private getCompletionBuilder(fileName: string, position: number):
Expand Down

0 comments on commit ae9f7c0

Please sign in to comment.