Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(language-service): Provide ability to rename pipes #40523

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
}
}
Expand Up @@ -430,19 +430,14 @@ export class SymbolBuilder {
}

private getSymbolOfPipe(expression: BindingPipe): PipeSymbol|null {
const node = findFirstMatchingNode(
this.typeCheckBlock, {withSpan: expression.sourceSpan, filter: ts.isCallExpression});
if (node === null || !ts.isPropertyAccessExpression(node.expression)) {
const methodAccess = findFirstMatchingNode(
this.typeCheckBlock,
{withSpan: expression.nameSpan, filter: ts.isPropertyAccessExpression});
if (methodAccess === null) {
return null;
}

const methodAccess = node.expression;
// Find the node for the pipe variable from the transform property access. This will be one of
// two forms: `_pipe1.transform` or `(_pipe1 as any).transform`.
const pipeVariableNode = ts.isParenthesizedExpression(methodAccess.expression) &&
ts.isAsExpression(methodAccess.expression.expression) ?
methodAccess.expression.expression.expression :
methodAccess.expression;
const pipeVariableNode = methodAccess.expression;
const pipeDeclaration = this.getTypeChecker().getSymbolAtLocation(pipeVariableNode);
if (pipeDeclaration === undefined || pipeDeclaration.valueDeclaration === undefined) {
return null;
Expand Down
Expand Up @@ -1712,17 +1712,19 @@ class TcbExpressionTranslator {

// Use an 'any' value to at least allow the rest of the expression to be checked.
pipe = NULL_AS_ANY;
} else if (this.tcb.env.config.checkTypeOfPipes) {
} else {
// Use a variable declared as the pipe's type.
pipe = this.tcb.env.pipeInst(pipeRef);
} else {
// Use an 'any' value when not checking the type of the pipe.
pipe = ts.createAsExpression(
this.tcb.env.pipeInst(pipeRef), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
}
const args = ast.args.map(arg => this.translate(arg));
const methodAccess = ts.createPropertyAccess(pipe, 'transform');
let methodAccess: ts.Expression =
ts.factory.createPropertyAccessExpression(pipe, 'transform');
addParseSpanInfo(methodAccess, ast.nameSpan);
if (!this.tcb.env.config.checkTypeOfPipes) {
methodAccess = ts.factory.createAsExpression(
methodAccess, ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
}

const result = ts.createCall(
/* expression */ methodAccess,
/* typeArguments */ undefined,
Expand Down
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
Expand Up @@ -992,7 +992,7 @@ describe('type check blocks', () => {
const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false};
const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG);
expect(block).toContain('var _pipe1: i0.TestPipe = null!;');
expect(block).toContain('((_pipe1 as any).transform(((ctx).a), ((ctx).b), ((ctx).c)));');
expect(block).toContain('((_pipe1.transform as any)(((ctx).a), ((ctx).b), ((ctx).c))');
});
});

Expand Down
Expand Up @@ -722,29 +722,19 @@ runInEachFileSystem(() => {
BindingPipe;
}

it('should get symbol for pipe', () => {
setupPipesTest();
const pipeSymbol = templateTypeChecker.getSymbolOfNode(binding, cmp)!;
assertPipeSymbol(pipeSymbol);
expect(program.getTypeChecker().symbolToString(pipeSymbol.tsSymbol!))
.toEqual('transform');
expect(program.getTypeChecker().symbolToString(pipeSymbol.classSymbol.tsSymbol))
.toEqual('TestPipe');
expect(program.getTypeChecker().typeToString(pipeSymbol.tsType!))
.toEqual('(value: string, repeat: number, commaSeparate: boolean) => string[]');
});

it('should get symbol for pipe, checkTypeOfPipes: false', () => {
setupPipesTest(false);
const pipeSymbol = templateTypeChecker.getSymbolOfNode(binding, cmp)! as PipeSymbol;
assertPipeSymbol(pipeSymbol);
expect(pipeSymbol.tsSymbol).toBeNull();
expect(program.getTypeChecker().typeToString(pipeSymbol.tsType!)).toEqual('any');
expect(program.getTypeChecker().symbolToString(pipeSymbol.classSymbol.tsSymbol))
.toEqual('TestPipe');
expect(program.getTypeChecker().typeToString(pipeSymbol.classSymbol.tsType))
.toEqual('TestPipe');
});
for (const checkTypeOfPipes of [true, false]) {
it(`should get symbol for pipe, checkTypeOfPipes: ${checkTypeOfPipes}`, () => {
setupPipesTest(checkTypeOfPipes);
const pipeSymbol = templateTypeChecker.getSymbolOfNode(binding, cmp)!;
assertPipeSymbol(pipeSymbol);
expect(program.getTypeChecker().symbolToString(pipeSymbol.tsSymbol!))
.toEqual('transform');
expect(program.getTypeChecker().symbolToString(pipeSymbol.classSymbol.tsSymbol))
.toEqual('TestPipe');
expect(program.getTypeChecker().typeToString(pipeSymbol.tsType!))
.toEqual('(value: string, repeat: number, commaSeparate: boolean) => string[]');
});
}

it('should get symbols for pipe expression and args', () => {
setupPipesTest(false);
Expand Down
Expand Up @@ -71,6 +71,7 @@ i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDE
}]
}] });
class MyForwardPipe {
transform() { }
}
MyForwardPipe.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyForwardPipe, deps: [], target: i0.ɵɵFactoryTarget.Pipe });
MyForwardPipe.ɵpipe = i0.ɵɵngDeclarePipe({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyForwardPipe, name: "my_forward_pipe" });
Expand Down
Expand Up @@ -11,6 +11,7 @@ export class HostBindingComp {

@Pipe({name: 'my_forward_pipe'})
class MyForwardPipe {
transform() {}
}

@NgModule({declarations: [HostBindingComp, MyForwardPipe]})
Expand Down
Expand Up @@ -1361,7 +1361,9 @@ runInEachFileSystem(() => {
@Pipe({
name: 'dep',
})
export class DepB {}
export class DepB {
transform() {}
}
`);
env.write('module.ts', `
import {NgModule} from '@angular/core';
Expand All @@ -1385,7 +1387,9 @@ runInEachFileSystem(() => {
@Pipe({
name: 'dep',
})
export class DepA {}
export class DepA {
transform() {}
}
@Directive({
selector: 'dep',
Expand Down