refactor(compiler-cli): Adjust generated TCB when checkTypeOfPipes is false (#40523)
When `checkTypeOfPipes` is set to `false`, our TCB currently generates the a statement like the following when pipes appear in the template: `(_pipe1 as any).transform(args)` This did enable us to get _some_ information from the Language Service about pipes in this case because we still had access to the pipe instance. However, because it is immediately cast to `any`, we cannot get type information about the transform access. That means actions like "go to definition", "find references", "quick info", etc. will return incomplete information or fail altogether. Instead, this commit changes the TCB to generate `(_pipe1.transform as any)(args)`. This gives us the ability to get complete information for the LS operations listed above. PR Close #40523
This commit is contained in:
parent
a86ca4fe04
commit
459af57a31
|
@ -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;
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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))');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -722,8 +722,9 @@ runInEachFileSystem(() => {
|
|||
BindingPipe;
|
||||
}
|
||||
|
||||
it('should get symbol for pipe', () => {
|
||||
setupPipesTest();
|
||||
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!))
|
||||
|
@ -733,18 +734,7 @@ runInEachFileSystem(() => {
|
|||
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');
|
||||
});
|
||||
}
|
||||
|
||||
it('should get symbols for pipe expression and args', () => {
|
||||
setupPipesTest(false);
|
||||
|
|
|
@ -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" });
|
||||
|
|
|
@ -11,6 +11,7 @@ export class HostBindingComp {
|
|||
|
||||
@Pipe({name: 'my_forward_pipe'})
|
||||
class MyForwardPipe {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@NgModule({declarations: [HostBindingComp, MyForwardPipe]})
|
||||
|
|
|
@ -1361,7 +1361,9 @@ runInEachFileSystem(() => {
|
|||
@Pipe({
|
||||
name: 'dep',
|
||||
})
|
||||
export class DepB {}
|
||||
export class DepB {
|
||||
transform() {}
|
||||
}
|
||||
`);
|
||||
env.write('module.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
|
@ -1385,7 +1387,9 @@ runInEachFileSystem(() => {
|
|||
@Pipe({
|
||||
name: 'dep',
|
||||
})
|
||||
export class DepA {}
|
||||
export class DepA {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@Directive({
|
||||
selector: 'dep',
|
||||
|
|
|
@ -200,7 +200,9 @@ runInEachFileSystem(() => {
|
|||
import {Pipe} from '@angular/core';
|
||||
|
||||
@Pipe({name: 'myPipe'})
|
||||
export class MyPipe {}
|
||||
export class MyPipe {
|
||||
transform() {}
|
||||
}
|
||||
`);
|
||||
env.write('module.ts', `
|
||||
import {NgModule, NO_ERRORS_SCHEMA} from '@angular/core';
|
||||
|
@ -238,7 +240,9 @@ runInEachFileSystem(() => {
|
|||
import {Pipe} from '@angular/core';
|
||||
|
||||
@Pipe({name: 'foo_changed'})
|
||||
export class FooPipe {}
|
||||
export class FooPipe {
|
||||
transform() {}
|
||||
}
|
||||
`);
|
||||
env.driveMain();
|
||||
const written = env.getFilesWrittenSinceLastFlush();
|
||||
|
@ -910,7 +914,9 @@ runInEachFileSystem(() => {
|
|||
import {Pipe} from '@angular/core';
|
||||
|
||||
@Pipe({name: 'foo'})
|
||||
export class FooPipe {}
|
||||
export class FooPipe {
|
||||
transform() {}
|
||||
}
|
||||
`);
|
||||
env.write('foo_module.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
|
@ -940,7 +946,9 @@ runInEachFileSystem(() => {
|
|||
import {Pipe} from '@angular/core';
|
||||
|
||||
@Pipe({name: 'foo'})
|
||||
export class BarPipe {}
|
||||
export class BarPipe {
|
||||
transform() {}
|
||||
}
|
||||
`);
|
||||
env.write('bar_module.ts', `
|
||||
import {NgModule} from '@angular/core';
|
||||
|
|
|
@ -757,7 +757,9 @@ function allTests(os: string) {
|
|||
// ModuleA classes
|
||||
|
||||
@Pipe({name: 'number'})
|
||||
class PipeA {}
|
||||
class PipeA {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@NgModule({
|
||||
declarations: [PipeA],
|
||||
|
@ -768,7 +770,9 @@ function allTests(os: string) {
|
|||
// ModuleB classes
|
||||
|
||||
@Pipe({name: 'number'})
|
||||
class PipeB {}
|
||||
class PipeB {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@Component({
|
||||
selector: 'app',
|
||||
|
@ -800,7 +804,9 @@ function allTests(os: string) {
|
|||
// ModuleA classes
|
||||
|
||||
@Pipe({name: 'number'})
|
||||
class PipeA {}
|
||||
class PipeA {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@NgModule({
|
||||
declarations: [PipeA],
|
||||
|
@ -811,7 +817,9 @@ function allTests(os: string) {
|
|||
// ModuleB classes
|
||||
|
||||
@Pipe({name: 'number'})
|
||||
class PipeB {}
|
||||
class PipeB {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@NgModule({
|
||||
declarations: [PipeB],
|
||||
|
@ -1647,7 +1655,9 @@ function allTests(os: string) {
|
|||
import {Component, NgModule, Pipe} from '@angular/core';
|
||||
|
||||
@Pipe({name: 'test'})
|
||||
export class TestPipe {}
|
||||
export class TestPipe {
|
||||
transform() {}
|
||||
}
|
||||
|
||||
@Component({selector: 'test-cmp', template: '{{value | test}}'})
|
||||
export class TestCmp {
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
|
||||
import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
|
||||
|
||||
import {assertFileNames, createModuleAndProjectWithDeclarations, humanizeDocumentSpanLike, LanguageServiceTestEnv, OpenBuffer} from '../testing';
|
||||
import {assertFileNames, assertTextSpans, createModuleAndProjectWithDeclarations, humanizeDocumentSpanLike, LanguageServiceTestEnv, OpenBuffer} from '../testing';
|
||||
|
||||
describe('definitions', () => {
|
||||
it('gets definition for template reference in overridden template', () => {
|
||||
|
@ -35,7 +35,7 @@ describe('definitions', () => {
|
|||
assertFileNames(Array.from(definitions!), ['app.html']);
|
||||
});
|
||||
|
||||
it('returns the pipe class as definition when checkTypeOfPipes is false', () => {
|
||||
it('returns the pipe definitions when checkTypeOfPipes is false', () => {
|
||||
initMockFileSystem('Native');
|
||||
const files = {
|
||||
'app.ts': `
|
||||
|
@ -57,10 +57,9 @@ describe('definitions', () => {
|
|||
|
||||
const {textSpan, definitions} = getDefinitionsAndAssertBoundSpan(env, template);
|
||||
expect(template.contents.substr(textSpan.start, textSpan.length)).toEqual('date');
|
||||
expect(definitions!.length).toEqual(1);
|
||||
const [def] = definitions!;
|
||||
expect(def.textSpan).toContain('DatePipe');
|
||||
expect(def.contextSpan).toContain('DatePipe');
|
||||
expect(definitions.length).toEqual(3);
|
||||
assertTextSpans(definitions, ['transform']);
|
||||
assertFileNames(definitions, ['index.d.ts']);
|
||||
});
|
||||
|
||||
it('gets definitions for all inputs when attribute matches more than one', () => {
|
||||
|
|
|
@ -559,8 +559,13 @@ describe('quick info', () => {
|
|||
// checkTypeOfPipes is set to false when strict templates is false
|
||||
project = env.addProject('test', quickInfoSkeleton(), {strictTemplates: false});
|
||||
const templateOverride = `<p>The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}</p>`;
|
||||
expectQuickInfo(
|
||||
{templateOverride, expectedSpanText: 'date', expectedDisplayString: '(pipe) DatePipe'});
|
||||
expectQuickInfo({
|
||||
templateOverride,
|
||||
expectedSpanText: 'date',
|
||||
expectedDisplayString:
|
||||
'(pipe) DatePipe.transform(value: string | number | Date, format?: string | undefined, timezone?: ' +
|
||||
'string | undefined, locale?: string | undefined): string | null (+2 overloads)'
|
||||
});
|
||||
});
|
||||
|
||||
it('should still get quick info if there is an invalid css resource', () => {
|
||||
|
|
|
@ -754,7 +754,8 @@ describe('find references and rename locations', () => {
|
|||
}
|
||||
}`;
|
||||
|
||||
describe('when cursor is on pipe name', () => {
|
||||
for (const checkTypeOfPipes of [true, false]) {
|
||||
describe(`when cursor is on pipe name, checkTypeOfPipes: ${checkTypeOfPipes}`, () => {
|
||||
let file: OpenBuffer;
|
||||
beforeEach(() => {
|
||||
const files = {
|
||||
|
@ -795,6 +796,7 @@ describe('find references and rename locations', () => {
|
|||
expect(result.displayName).toEqual('prefixPipe');
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
describe('when cursor is on pipe name expression', () => {
|
||||
it('finds rename locations and rename info', () => {
|
||||
|
|
|
@ -1,3 +1,11 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright Google LLC All Rights Reserved.
|
||||
*
|
||||
* 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 {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
|
||||
import * as ts from 'typescript';
|
||||
import {LanguageServiceTestEnv, OpenBuffer, Project} from '../testing';
|
||||
|
@ -7,7 +15,7 @@ describe('ts utils', () => {
|
|||
describe('collectMemberMethods', () => {
|
||||
beforeEach(() => {
|
||||
initMockFileSystem('Native');
|
||||
})
|
||||
});
|
||||
|
||||
it('gets only methods in class, not getters, setters, or properties', () => {
|
||||
const files = {
|
||||
|
@ -69,7 +77,7 @@ describe('ts utils', () => {
|
|||
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)
|
||||
expect(ts.isClassDeclaration(node.parent)).toBe(true);
|
||||
return collectMemberMethods(node.parent as ts.ClassDeclaration, project.getTypeChecker())
|
||||
.map(m => m.name.getText())
|
||||
.sort();
|
||||
|
|
|
@ -8,7 +8,7 @@
|
|||
|
||||
import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
|
||||
|
||||
import {humanizeDocumentSpanLike, LanguageServiceTestEnv, Project} from '../testing';
|
||||
import {assertFileNames, assertTextSpans, humanizeDocumentSpanLike, LanguageServiceTestEnv, Project} from '../testing';
|
||||
|
||||
describe('type definitions', () => {
|
||||
let env: LanguageServiceTestEnv;
|
||||
|
@ -33,11 +33,10 @@ describe('type definitions', () => {
|
|||
const project = env.addProject('test', files, {strictTemplates: false});
|
||||
const definitions =
|
||||
getTypeDefinitionsAndAssertBoundSpan(project, {templateOverride: '{{"1/1/2020" | dat¦e}}'});
|
||||
expect(definitions!.length).toEqual(1);
|
||||
expect(definitions!.length).toEqual(3);
|
||||
|
||||
const [def] = definitions;
|
||||
expect(def.textSpan).toContain('DatePipe');
|
||||
expect(def.contextSpan).toContain('DatePipe');
|
||||
assertTextSpans(definitions, ['transform']);
|
||||
assertFileNames(definitions, ['index.d.ts']);
|
||||
});
|
||||
|
||||
function getTypeDefinitionsAndAssertBoundSpan(
|
||||
|
|
Loading…
Reference in New Issue