Skip to content

Commit 2028c39

Browse files
crisbetothePunderWoman
crisbeto
authored and
thePunderWoman
committed
refactor(compiler): combine call ASTs (#42882)
Currently the compiler has three different classes to represent a "call to something": 1. `MethodCall` - `foo.bar()` 2. `SafeMethodCall` - `foo?.bar()`. 3. `FunctionCall` - Any calls that don't fit into the first two classes. E.g. `foo.bar()()`. There are a few problems with this approach: 1. It is inconistent with the TypeScript AST which only has one node: `CallExpression`. 2. It means that we have to maintain more code, because the various parts of the compiler need to know about three node types. 3. It doesn't allow us to easily implement some new JS features like safe calls (e.g. `foo.bar?.())`). These changes rework the compiler so that it produces only one node: `Call`. The new node behaves similarly to the TypeScript `CallExpression` whose `receiver` can be any expression. There was a similar situation in the output AST where we had an `InvokeMethodExpression` and `InvokeFunctionExpression`. I've combined both of them into `InvokeFunctionExpression`. PR Close #42882
1 parent 77bd253 commit 2028c39

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+442
-791
lines changed

‎packages/compiler-cli/linker/test/file_linker/emit_scopes/emit_scope_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('EmitScope', () => {
3232
const emitScope = new EmitScope<ts.Statement, ts.Expression>(ngImport, translator);
3333

3434
const coreImportRef = new o.ExternalReference('@angular/core', 'foo');
35-
const def = emitScope.translateDefinition(o.importExpr(coreImportRef).callMethod('bar', []));
35+
const def = emitScope.translateDefinition(o.importExpr(coreImportRef).prop('bar').callFn([]));
3636
expect(generate(def)).toEqual('core.foo.bar()');
3737
});
3838

‎packages/compiler-cli/linker/test/file_linker/emit_scopes/iief_emit_scope_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe('IifeEmitScope', () => {
3434
new IifeEmitScope<ts.Statement, ts.Expression>(ngImport, translator, factory);
3535

3636
const coreImportRef = new o.ExternalReference('@angular/core', 'foo');
37-
const def = emitScope.translateDefinition(o.importExpr(coreImportRef).callMethod('bar', []));
37+
const def = emitScope.translateDefinition(o.importExpr(coreImportRef).prop('bar').callFn([]));
3838
expect(generate(def)).toEqual('function () { return core.foo.bar(); }()');
3939
});
4040

‎packages/compiler-cli/src/ngtsc/indexer/src/api.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {ClassDeclaration, DeclarationNode} from '../../reflection';
1414
*/
1515
export enum IdentifierKind {
1616
Property,
17-
Method,
17+
Method, // TODO: No longer being used. To be removed together with `MethodIdentifier`.
1818
Element,
1919
Template,
2020
Attribute,
@@ -46,7 +46,10 @@ export interface PropertyIdentifier extends ExpressionIdentifier {
4646
kind: IdentifierKind.Property;
4747
}
4848

49-
/** Describes a method accessed in a template. */
49+
/**
50+
* Describes a method accessed in a template.
51+
* @deprecated No longer being used. To be removed.
52+
*/
5053
export interface MethodIdentifier extends ExpressionIdentifier {
5154
kind: IdentifierKind.Method;
5255
}
@@ -109,8 +112,8 @@ export interface VariableIdentifier extends TemplateIdentifier {
109112
* Identifiers recorded at the top level of the template, without any context about the HTML nodes
110113
* they were discovered in.
111114
*/
112-
export type TopLevelIdentifier = PropertyIdentifier|MethodIdentifier|ElementIdentifier|
113-
TemplateNodeIdentifier|ReferenceIdentifier|VariableIdentifier;
115+
export type TopLevelIdentifier = PropertyIdentifier|ElementIdentifier|TemplateNodeIdentifier|
116+
ReferenceIdentifier|VariableIdentifier|MethodIdentifier;
114117

115118
/**
116119
* Describes the absolute byte offsets of a text anchor in a source code.

‎packages/compiler-cli/src/ngtsc/indexer/src/template.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import {AST, ASTWithSource, BoundTarget, ImplicitReceiver, MethodCall, ParseSourceSpan, PropertyRead, PropertyWrite, RecursiveAstVisitor, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstRecursiveVisitor, TmplAstReference, TmplAstTemplate, TmplAstVariable} from '@angular/compiler';
8+
import {AST, ASTWithSource, BoundTarget, ImplicitReceiver, ParseSourceSpan, PropertyRead, PropertyWrite, RecursiveAstVisitor, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstRecursiveVisitor, TmplAstReference, TmplAstTemplate, TmplAstVariable} from '@angular/compiler';
99
import {AbsoluteSourceSpan, AttributeIdentifier, ElementIdentifier, IdentifierKind, MethodIdentifier, PropertyIdentifier, ReferenceIdentifier, TemplateNodeIdentifier, TopLevelIdentifier, VariableIdentifier} from './api';
1010
import {ComponentMeta} from './context';
1111

@@ -66,11 +66,6 @@ class ExpressionVisitor extends RecursiveAstVisitor {
6666
ast.visit(this);
6767
}
6868

69-
override visitMethodCall(ast: MethodCall, context: {}) {
70-
this.visitIdentifier(ast, IdentifierKind.Method);
71-
super.visitMethodCall(ast, context);
72-
}
73-
7469
override visitPropertyRead(ast: PropertyRead, context: {}) {
7570
this.visitIdentifier(ast, IdentifierKind.Property);
7671
super.visitPropertyRead(ast, context);

‎packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ runInEachFileSystem(() => {
277277
});
278278
});
279279

280-
describe('generates identifiers for MethodCalls', () => {
280+
describe('generates identifiers for method calls', () => {
281281
it('should discover component method calls', () => {
282282
const template = '{{foo()}}';
283283
const refs = getTemplateIdentifiers(bind(template));
@@ -286,7 +286,7 @@ runInEachFileSystem(() => {
286286
const [ref] = Array.from(refs);
287287
expect(ref).toEqual({
288288
name: 'foo',
289-
kind: IdentifierKind.Method,
289+
kind: IdentifierKind.Property,
290290
span: new AbsoluteSourceSpan(2, 5),
291291
target: null,
292292
});
@@ -299,7 +299,7 @@ runInEachFileSystem(() => {
299299
const refArr = Array.from(refs);
300300
expect(refArr).toContain({
301301
name: 'foo',
302-
kind: IdentifierKind.Method,
302+
kind: IdentifierKind.Property,
303303
span: new AbsoluteSourceSpan(13, 16),
304304
target: null,
305305
});
@@ -321,7 +321,7 @@ runInEachFileSystem(() => {
321321
const refArr = Array.from(refs);
322322
expect(refArr).toContain({
323323
name: 'bar',
324-
kind: IdentifierKind.Method,
324+
kind: IdentifierKind.Property,
325325
span: new AbsoluteSourceSpan(12, 15),
326326
target: null,
327327
});
@@ -334,7 +334,7 @@ runInEachFileSystem(() => {
334334
const refArr = Array.from(refs);
335335
expect(refArr).toContain({
336336
name: 'foos',
337-
kind: IdentifierKind.Method,
337+
kind: IdentifierKind.Property,
338338
span: new AbsoluteSourceSpan(24, 28),
339339
target: null,
340340
});

‎packages/compiler-cli/src/ngtsc/translator/src/translator.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,6 @@ export class ExpressionTranslatorVisitor<TStatement, TExpression> implements o.E
153153
return this.factory.createAssignment(target, expr.value.visitExpression(this, context));
154154
}
155155

156-
visitInvokeMethodExpr(ast: o.InvokeMethodExpr, context: Context): TExpression {
157-
const target = ast.receiver.visitExpression(this, context);
158-
return this.setSourceMapRange(
159-
this.factory.createCallExpression(
160-
ast.name !== null ? this.factory.createPropertyAccess(target, ast.name) : target,
161-
ast.args.map(arg => arg.visitExpression(this, context)),
162-
/* pure */ false),
163-
ast.sourceSpan);
164-
}
165-
166156
visitInvokeFunctionExpr(ast: o.InvokeFunctionExpr, context: Context): TExpression {
167157
return this.setSourceMapRange(
168158
this.factory.createCallExpression(

‎packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,6 @@ export class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor
9090
throw new Error('Method not implemented.');
9191
}
9292

93-
visitInvokeMethodExpr(ast: o.InvokeMethodExpr, context: Context): never {
94-
throw new Error('Method not implemented.');
95-
}
96-
9793
visitInvokeFunctionExpr(ast: o.InvokeFunctionExpr, context: Context): never {
9894
throw new Error('Method not implemented.');
9995
}

‎packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, LiteralPrimitive, MethodCall, ParseError, ParseSourceSpan, PropertyRead, SafeMethodCall, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstTemplate} from '@angular/compiler';
9+
import {AST, Call, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstTemplate} from '@angular/compiler';
1010
import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
1111
import {TextAttribute} from '@angular/compiler/src/render3/r3_ast';
1212
import * as ts from 'typescript';
@@ -117,8 +117,7 @@ export interface TemplateTypeChecker {
117117
* autocompletion at that point in the expression, if such a location exists.
118118
*/
119119
getExpressionCompletionLocation(
120-
expr: PropertyRead|SafePropertyRead|MethodCall|SafeMethodCall,
121-
component: ts.ClassDeclaration): ShimLocation|null;
120+
expr: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): ShimLocation|null;
122121

123122
/**
124123
* For the given node represents a `LiteralPrimitive`(the `TextAttribute` represents a string

‎packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AST, CssSelector, DomElementSchemaRegistry, LiteralPrimitive, MethodCall, ParseError, ParseSourceSpan, parseTemplate, PropertyRead, SafeMethodCall, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstVariable} from '@angular/compiler';
9+
import {AST, CssSelector, DomElementSchemaRegistry, LiteralPrimitive, ParseSourceSpan, PropertyRead, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate} from '@angular/compiler';
1010
import {TextAttribute} from '@angular/compiler/src/render3/r3_ast';
1111
import * as ts from 'typescript';
1212
import {ErrorCode, ngErrorCode} from '../../diagnostics';
@@ -271,8 +271,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
271271
}
272272

273273
getExpressionCompletionLocation(
274-
ast: PropertyRead|SafePropertyRead|MethodCall|SafeMethodCall,
275-
component: ts.ClassDeclaration): ShimLocation|null {
274+
ast: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): ShimLocation|null {
276275
const engine = this.getOrCreateCompletionEngine(component);
277276
if (engine === null) {
278277
return null;

‎packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {TmplAstReference, TmplAstTemplate} from '@angular/compiler';
10-
import {AST, EmptyExpr, ImplicitReceiver, LiteralPrimitive, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstNode} from '@angular/compiler/src/compiler';
10+
import {AST, EmptyExpr, ImplicitReceiver, LiteralPrimitive, PropertyRead, PropertyWrite, SafePropertyRead, TmplAstNode} from '@angular/compiler/src/compiler';
1111
import {TextAttribute} from '@angular/compiler/src/render3/r3_ast';
1212
import * as ts from 'typescript';
1313

@@ -33,9 +33,8 @@ export class CompletionEngine {
3333
private templateContextCache =
3434
new Map<TmplAstTemplate|null, Map<string, ReferenceCompletion|VariableCompletion>>();
3535

36-
private expressionCompletionCache = new Map<
37-
PropertyRead|SafePropertyRead|MethodCall|SafeMethodCall|LiteralPrimitive|TextAttribute,
38-
ShimLocation>();
36+
private expressionCompletionCache =
37+
new Map<PropertyRead|SafePropertyRead|LiteralPrimitive|TextAttribute, ShimLocation>();
3938

4039

4140
constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) {
@@ -111,22 +110,21 @@ export class CompletionEngine {
111110
};
112111
}
113112

114-
getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|MethodCall|
115-
SafeMethodCall): ShimLocation|null {
113+
getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|SafePropertyRead): ShimLocation
114+
|null {
116115
if (this.expressionCompletionCache.has(expr)) {
117116
return this.expressionCompletionCache.get(expr)!;
118117
}
119118

120119
// Completion works inside property reads and method calls.
121120
let tsExpr: ts.PropertyAccessExpression|null = null;
122-
if (expr instanceof PropertyRead || expr instanceof MethodCall ||
123-
expr instanceof PropertyWrite) {
121+
if (expr instanceof PropertyRead || expr instanceof PropertyWrite) {
124122
// Non-safe navigation operations are trivial: `foo.bar` or `foo.bar()`
125123
tsExpr = findFirstMatchingNode(this.tcb, {
126124
filter: ts.isPropertyAccessExpression,
127125
withSpan: expr.nameSpan,
128126
});
129-
} else if (expr instanceof SafePropertyRead || expr instanceof SafeMethodCall) {
127+
} else if (expr instanceof SafePropertyRead) {
130128
// Safe navigation operations are a little more complex, and involve a ternary. Completion
131129
// happens in the "true" case of the ternary.
132130
const ternaryExpr = findFirstMatchingNode(this.tcb, {
@@ -138,11 +136,10 @@ export class CompletionEngine {
138136
}
139137
const whenTrue = ternaryExpr.expression.whenTrue;
140138

141-
if (expr instanceof SafePropertyRead && ts.isPropertyAccessExpression(whenTrue)) {
139+
if (ts.isPropertyAccessExpression(whenTrue)) {
142140
tsExpr = whenTrue;
143141
} else if (
144-
expr instanceof SafeMethodCall && ts.isCallExpression(whenTrue) &&
145-
ts.isPropertyAccessExpression(whenTrue.expression)) {
142+
ts.isCallExpression(whenTrue) && ts.isPropertyAccessExpression(whenTrue.expression)) {
146143
tsExpr = whenTrue.expression;
147144
}
148145
}

0 commit comments

Comments
 (0)