From 8c6468a02541697c24634d6a65ff23959494a5f1 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 15 Dec 2019 14:28:51 +0100 Subject: [PATCH] refactor(ivy): use absolute source spans in type checker (#34417) Previously, the type checker would compute an absolute source span by combining an expression AST node's `ParseSpan` (relative to the start of the expression) together with the absolute offset of the expression as represented in a `ParseSourceSpan`, to arrive at a span relative to the start of the file. This information is now directly available on an expression AST node in the `AST.sourceSpan` property, which can be used instead. PR Close #34417 --- .../src/ngtsc/typecheck/src/diagnostics.ts | 51 ++++----------- .../src/ngtsc/typecheck/src/expression.ts | 45 +++++++------ .../src/ngtsc/typecheck/src/oob.ts | 17 ++--- .../ngtsc/typecheck/src/template_semantics.ts | 13 ++-- .../ngtsc/typecheck/src/type_check_block.ts | 64 ++++++++----------- .../compiler/src/expression_parser/ast.ts | 4 +- .../compiler/src/expression_parser/parser.ts | 2 +- 7 files changed, 75 insertions(+), 121 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 8d9dd5a266..c1cc9df1f7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -5,7 +5,7 @@ * 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 {AbsoluteSourceSpan, ParseSourceSpan, ParseSpan, Position} from '@angular/compiler'; +import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler'; import * as ts from 'typescript'; import {getTokenAtPosition} from '../../util/src/typescript'; @@ -35,26 +35,6 @@ export interface TcbSourceResolver { sourceLocationToSpan(location: SourceLocation): ParseSourceSpan|null; } -/** - * An `AbsoluteSpan` is the result of translating the `ParseSpan` of `AST` template expression nodes - * to their absolute positions, as the `ParseSpan` is always relative to the start of the - * expression, not the full template. - */ -export interface AbsoluteSpan { - __brand__: 'AbsoluteSpan'; - start: number; - end: number; -} - -/** - * Translates a `ParseSpan` into an `AbsoluteSpan` by incorporating the location information that - * the `ParseSourceSpan` represents. - */ -export function toAbsoluteSpan(span: ParseSpan, sourceSpan: ParseSourceSpan): AbsoluteSpan { - const offset = sourceSpan.start.offset; - return {start: span.start + offset, end: span.end + offset}; -} - export function absoluteSourceSpanToSourceLocation( id: string, span: AbsoluteSourceSpan): SourceLocation { return {id, ...span}; @@ -78,20 +58,15 @@ export function wrapForDiagnostics(expr: ts.Expression): ts.Expression { * Adds a synthetic comment to the expression that represents the parse span of the provided node. * This comment can later be retrieved as trivia of a node to recover original source locations. */ -export function addParseSpanInfo(node: ts.Node, span: AbsoluteSpan | ParseSourceSpan): void { +export function addParseSpanInfo(node: ts.Node, span: AbsoluteSourceSpan | ParseSourceSpan): void { let commentText: string; - if (isAbsoluteSpan(span)) { + if (span instanceof AbsoluteSourceSpan) { commentText = `${span.start},${span.end}`; } else { commentText = `${span.start.offset},${span.end.offset}`; } ts.addSyntheticTrailingComment( - node, ts.SyntaxKind.MultiLineCommentTrivia, commentText, - /* hasTrailingNewLine */ false); -} - -function isAbsoluteSpan(span: AbsoluteSpan | ParseSourceSpan): span is AbsoluteSpan { - return typeof span.start === 'number'; + node, ts.SyntaxKind.MultiLineCommentTrivia, commentText, /* hasTrailingNewLine */ false); } /** @@ -245,18 +220,18 @@ export function makeTemplateDiagnostic( function findSourceLocation(node: ts.Node, sourceFile: ts.SourceFile): SourceLocation|null { // Search for comments until the TCB's function declaration is encountered. while (node !== undefined && !ts.isFunctionDeclaration(node)) { - const parseSpan = + const sourceSpan = ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { return null; } const commentText = sourceFile.text.substring(pos, end); - return parseParseSpanComment(commentText); + return parseSourceSpanComment(commentText); }) || null; - if (parseSpan !== null) { + if (sourceSpan !== null) { // Once the positional information has been extracted, search further up the TCB to extract // the file information that is attached with the TCB's function declaration. - return toSourceLocation(parseSpan, node, sourceFile); + return toSourceLocation(sourceSpan, node, sourceFile); } node = node.parent; @@ -266,7 +241,7 @@ function findSourceLocation(node: ts.Node, sourceFile: ts.SourceFile): SourceLoc } function toSourceLocation( - parseSpan: ParseSpan, node: ts.Node, sourceFile: ts.SourceFile): SourceLocation|null { + sourceSpan: AbsoluteSourceSpan, node: ts.Node, sourceFile: ts.SourceFile): SourceLocation|null { // Walk up to the function declaration of the TCB, the file information is attached there. let tcb = node; while (!ts.isFunctionDeclaration(tcb)) { @@ -292,18 +267,18 @@ function toSourceLocation( return { id, - start: parseSpan.start, - end: parseSpan.end, + start: sourceSpan.start, + end: sourceSpan.end, }; } const parseSpanComment = /^\/\*(\d+),(\d+)\*\/$/; -function parseParseSpanComment(commentText: string): ParseSpan|null { +function parseSourceSpanComment(commentText: string): AbsoluteSourceSpan|null { const match = commentText.match(parseSpanComment); if (match === null) { return null; } - return new ParseSpan(+match[1], +match[2]); + return new AbsoluteSourceSpan(+match[1], +match[2]); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 7b42cef1f3..99a6365175 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -6,11 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, ASTWithSource, AstVisitor, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; +import {AST, ASTWithSource, AstVisitor, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeCheckingConfig} from './api'; -import {AbsoluteSpan, addParseSpanInfo, wrapForDiagnostics} from './diagnostics'; +import {addParseSpanInfo, wrapForDiagnostics} from './diagnostics'; export const NULL_AS_ANY = ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); @@ -41,17 +41,16 @@ const BINARY_OPS = new Map([ * AST. */ export function astToTypescript( - ast: AST, maybeResolve: (ast: AST) => (ts.Expression | null), config: TypeCheckingConfig, - translateSpan: (span: ParseSpan) => AbsoluteSpan): ts.Expression { - const translator = new AstTranslator(maybeResolve, config, translateSpan); + ast: AST, maybeResolve: (ast: AST) => (ts.Expression | null), + config: TypeCheckingConfig): ts.Expression { + const translator = new AstTranslator(maybeResolve, config); return translator.translate(ast); } class AstTranslator implements AstVisitor { constructor( private maybeResolve: (ast: AST) => (ts.Expression | null), - private config: TypeCheckingConfig, - private translateSpan: (span: ParseSpan) => AbsoluteSpan) {} + private config: TypeCheckingConfig) {} translate(ast: AST): ts.Expression { // Skip over an `ASTWithSource` as its `visit` method calls directly into its ast's `visit`, @@ -82,14 +81,14 @@ class AstTranslator implements AstVisitor { throw new Error(`Unsupported Binary.operation: ${ast.operation}`); } const node = ts.createBinary(lhs, op as any, rhs); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } visitChain(ast: Chain): ts.Expression { const elements = ast.expressions.map(expr => this.translate(expr)); const node = wrapForDiagnostics(ts.createCommaList(elements)); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -98,7 +97,7 @@ class AstTranslator implements AstVisitor { const trueExpr = this.translate(ast.trueExp); const falseExpr = this.translate(ast.falseExp); const node = ts.createParen(ts.createConditional(condExpr, trueExpr, falseExpr)); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -106,7 +105,7 @@ class AstTranslator implements AstVisitor { const receiver = wrapForDiagnostics(this.translate(ast.target !)); const args = ast.args.map(expr => this.translate(expr)); const node = ts.createCall(receiver, undefined, args); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -127,7 +126,7 @@ class AstTranslator implements AstVisitor { const receiver = wrapForDiagnostics(this.translate(ast.obj)); const key = this.translate(ast.key); const node = ts.createElementAccess(receiver, key); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -138,14 +137,14 @@ class AstTranslator implements AstVisitor { // available on `ast`. const right = this.translate(ast.value); const node = wrapForDiagnostics(ts.createBinary(left, ts.SyntaxKind.EqualsToken, right)); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } visitLiteralArray(ast: LiteralArray): ts.Expression { const elements = ast.expressions.map(expr => this.translate(expr)); const node = ts.createArrayLiteral(elements); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -155,7 +154,7 @@ class AstTranslator implements AstVisitor { return ts.createPropertyAssignment(ts.createStringLiteral(key), value); }); const node = ts.createObjectLiteral(properties, true); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -168,7 +167,7 @@ class AstTranslator implements AstVisitor { } else { node = ts.createLiteral(ast.value); } - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -177,14 +176,14 @@ class AstTranslator implements AstVisitor { const method = ts.createPropertyAccess(receiver, ast.name); const args = ast.args.map(expr => this.translate(expr)); const node = ts.createCall(method, undefined, args); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } visitNonNullAssert(ast: NonNullAssert): ts.Expression { const expr = wrapForDiagnostics(this.translate(ast.expression)); const node = ts.createNonNullExpression(expr); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -193,7 +192,7 @@ class AstTranslator implements AstVisitor { visitPrefixNot(ast: PrefixNot): ts.Expression { const expression = wrapForDiagnostics(this.translate(ast.expression)); const node = ts.createLogicalNot(expression); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -202,7 +201,7 @@ class AstTranslator implements AstVisitor { // TypeScript expression to read the property. const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const node = ts.createPropertyAccess(receiver, ast.name); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -213,7 +212,7 @@ class AstTranslator implements AstVisitor { // available on `ast`. const right = this.translate(ast.value); const node = wrapForDiagnostics(ts.createBinary(left, ts.SyntaxKind.EqualsToken, right)); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -228,7 +227,7 @@ class AstTranslator implements AstVisitor { const expr = ts.createCall(method, undefined, args); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; const node = safeTernary(receiver, expr, whenNull); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -241,7 +240,7 @@ class AstTranslator implements AstVisitor { const expr = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; const node = safeTernary(receiver, expr, whenNull); - addParseSpanInfo(node, this.translateSpan(ast.span)); + addParseSpanInfo(node, ast.sourceSpan); return node; } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index 3cd5b21fa1..8b22433bbb 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -44,15 +44,11 @@ export interface OutOfBandDiagnosticRecorder { * @param templateId the template type-checking ID of the template which contains the unknown * pipe. * @param ast the `BindingPipe` invocation of the pipe which could not be found. - * @param sourceSpan the `AbsoluteSourceSpan` of the pipe invocation (ideally, this should be the - * source span of the pipe's name). This depends on the source span of the `BindingPipe` itself - * plus span of the larger expression context. */ - missingPipe(templateId: string, ast: BindingPipe, sourceSpan: AbsoluteSourceSpan): void; + missingPipe(templateId: string, ast: BindingPipe): void; illegalAssignmentToTemplateVar( - templateId: string, assignment: PropertyWrite, assignmentSpan: AbsoluteSourceSpan, - target: TmplAstVariable): void; + templateId: string, assignment: PropertyWrite, target: TmplAstVariable): void; } export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder { @@ -72,11 +68,11 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg)); } - missingPipe(templateId: string, ast: BindingPipe, absSpan: AbsoluteSourceSpan): void { + missingPipe(templateId: string, ast: BindingPipe): void { const mapping = this.resolver.getSourceMapping(templateId); const errorMsg = `No pipe found with name '${ast.name}'.`; - const location = absoluteSourceSpanToSourceLocation(templateId, absSpan); + const location = absoluteSourceSpanToSourceLocation(templateId, ast.nameSpan); const sourceSpan = this.resolver.sourceLocationToSpan(location); if (sourceSpan === null) { throw new Error( @@ -88,13 +84,12 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor } illegalAssignmentToTemplateVar( - templateId: string, assignment: PropertyWrite, assignmentSpan: AbsoluteSourceSpan, - target: TmplAstVariable): void { + templateId: string, assignment: PropertyWrite, target: TmplAstVariable): void { const mapping = this.resolver.getSourceMapping(templateId); const errorMsg = `Cannot use variable '${assignment.name}' as the left-hand side of an assignment expression. Template variables are read-only.`; - const location = absoluteSourceSpanToSourceLocation(templateId, assignmentSpan); + const location = absoluteSourceSpanToSourceLocation(templateId, assignment.sourceSpan); const sourceSpan = this.resolver.sourceLocationToSpan(location); if (sourceSpan === null) { throw new Error(`Assertion failure: no SourceLocation found for property binding.`); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts index 24a294bca3..b9c0235a60 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_semantics.ts @@ -6,9 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BoundTarget, ImplicitReceiver, ParseSourceSpan, PropertyWrite, RecursiveAstVisitor, TmplAstVariable} from '@angular/compiler'; +import {AST, BoundTarget, ImplicitReceiver, PropertyWrite, RecursiveAstVisitor, TmplAstVariable} from '@angular/compiler'; -import {toAbsoluteSpan} from './diagnostics'; import {OutOfBandDiagnosticRecorder} from './oob'; /** @@ -17,7 +16,7 @@ import {OutOfBandDiagnosticRecorder} from './oob'; export class ExpressionSemanticVisitor extends RecursiveAstVisitor { constructor( private templateId: string, private boundTarget: BoundTarget, - private oob: OutOfBandDiagnosticRecorder, private sourceSpan: ParseSourceSpan) { + private oob: OutOfBandDiagnosticRecorder) { super(); } @@ -31,14 +30,12 @@ export class ExpressionSemanticVisitor extends RecursiveAstVisitor { const target = this.boundTarget.getExpressionTarget(ast); if (target instanceof TmplAstVariable) { // Template variables are read-only. - const astSpan = toAbsoluteSpan(ast.span, this.sourceSpan); - this.oob.illegalAssignmentToTemplateVar(this.templateId, ast, astSpan, target); + this.oob.illegalAssignmentToTemplateVar(this.templateId, ast, target); } } static visit( - ast: AST, sourceSpan: ParseSourceSpan, id: string, boundTarget: BoundTarget, - oob: OutOfBandDiagnosticRecorder): void { - ast.visit(new ExpressionSemanticVisitor(id, boundTarget, oob, sourceSpan)); + ast: AST, id: string, boundTarget: BoundTarget, oob: OutOfBandDiagnosticRecorder): void { + ast.visit(new ExpressionSemanticVisitor(id, boundTarget, oob)); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 802dc61b12..6d52d5afbd 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, ParsedEventType, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParseSourceSpan, ParsedEventType, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api'; -import {addParseSpanInfo, addSourceId, toAbsoluteSpan, wrapForDiagnostics} from './diagnostics'; +import {addParseSpanInfo, addSourceId, wrapForDiagnostics} from './diagnostics'; import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {NULL_AS_ANY, astToTypescript} from './expression'; @@ -215,9 +215,7 @@ class TcbTemplateBodyOp extends TcbOp { i instanceof TmplAstBoundAttribute && i.name === guard.inputName); if (boundInput !== undefined) { // If there is such a binding, generate an expression for it. - const expr = tcbExpression( - boundInput.value, this.tcb, this.scope, - boundInput.valueSpan || boundInput.sourceSpan); + const expr = tcbExpression(boundInput.value, this.tcb, this.scope); if (guard.type === 'binding') { // Use the binding expression itself as guard. @@ -229,8 +227,7 @@ class TcbTemplateBodyOp extends TcbOp { dirInstId, expr, ]); - addParseSpanInfo( - guardInvoke, toAbsoluteSpan(boundInput.value.span, boundInput.sourceSpan)); + addParseSpanInfo(guardInvoke, boundInput.value.sourceSpan); directiveGuards.push(guardInvoke); } } @@ -281,7 +278,7 @@ class TcbTextInterpolationOp extends TcbOp { } execute(): null { - const expr = tcbExpression(this.binding.value, this.tcb, this.scope, this.binding.sourceSpan); + const expr = tcbExpression(this.binding.value, this.tcb, this.scope); this.scope.addStatement(ts.createExpressionStatement(expr)); return null; } @@ -400,8 +397,7 @@ class TcbUnclaimedInputsOp extends TcbOp { continue; } - let expr = tcbExpression( - binding.value, this.tcb, this.scope, binding.valueSpan || binding.sourceSpan); + let expr = tcbExpression(binding.value, this.tcb, this.scope); if (!this.tcb.env.config.checkTypeOfInputBindings) { // If checking the type of bindings is disabled, cast the resulting expression to 'any' // before the assignment. @@ -494,8 +490,7 @@ class TcbDirectiveOutputsOp extends TcbOp { } ExpressionSemanticVisitor.visit( - output.handler, output.handlerSpan, this.tcb.id, this.tcb.boundTarget, - this.tcb.oobRecorder); + output.handler, this.tcb.id, this.tcb.boundTarget, this.tcb.oobRecorder); } return null; @@ -556,8 +551,7 @@ class TcbUnclaimedOutputsOp extends TcbOp { } ExpressionSemanticVisitor.visit( - output.handler, output.handlerSpan, this.tcb.id, this.tcb.boundTarget, - this.tcb.oobRecorder); + output.handler, this.tcb.id, this.tcb.boundTarget, this.tcb.oobRecorder); } return null; @@ -949,23 +943,19 @@ function tcbCtxParam( * Process an `AST` expression and convert it into a `ts.Expression`, generating references to the * correct identifiers in the current scope. */ -function tcbExpression( - ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression { - const translator = new TcbExpressionTranslator(tcb, scope, sourceSpan); +function tcbExpression(ast: AST, tcb: Context, scope: Scope): ts.Expression { + const translator = new TcbExpressionTranslator(tcb, scope); return translator.translate(ast); } class TcbExpressionTranslator { - constructor( - protected tcb: Context, protected scope: Scope, protected sourceSpan: ParseSourceSpan) {} + constructor(protected tcb: Context, protected scope: Scope) {} translate(ast: AST): ts.Expression { // `astToTypescript` actually does the conversion. A special resolver `tcbResolve` is passed // which interprets specific expression nodes that interact with the `ImplicitReceiver`. These // nodes actually refer to identifiers within the current scope. - return astToTypescript( - ast, ast => this.resolve(ast), this.tcb.env.config, - (span: ParseSpan) => toAbsoluteSpan(span, this.sourceSpan)); + return astToTypescript(ast, ast => this.resolve(ast), this.tcb.env.config); } /** @@ -989,7 +979,7 @@ class TcbExpressionTranslator { const expr = this.translate(ast.value); const result = ts.createParen(ts.createBinary(target, ts.SyntaxKind.EqualsToken, expr)); - addParseSpanInfo(result, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(result, ast.sourceSpan); return result; } else if (ast instanceof ImplicitReceiver) { // AST instances representing variables and references look very similar to property reads @@ -1012,8 +1002,7 @@ class TcbExpressionTranslator { pipe = this.tcb.getPipeByName(ast.name); if (pipe === null) { // No pipe by that name exists in scope. Record this as an error. - const nameAbsoluteSpan = toAbsoluteSpan(ast.nameSpan, this.sourceSpan); - this.tcb.oobRecorder.missingPipe(this.tcb.id, ast, nameAbsoluteSpan); + this.tcb.oobRecorder.missingPipe(this.tcb.id, ast); // Return an 'any' value to at least allow the rest of the expression to be checked. pipe = NULL_AS_ANY; @@ -1024,7 +1013,7 @@ class TcbExpressionTranslator { } const args = ast.args.map(arg => this.translate(arg)); const result = tsCallMethod(pipe, 'transform', [expr, ...args]); - addParseSpanInfo(result, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(result, ast.sourceSpan); return result; } else if (ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver) { // Resolve the special `$any(expr)` syntax to insert a cast of the argument to type `any`. @@ -1033,7 +1022,7 @@ class TcbExpressionTranslator { const exprAsAny = ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); const result = ts.createParen(exprAsAny); - addParseSpanInfo(result, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(result, ast.sourceSpan); return result; } @@ -1049,7 +1038,7 @@ class TcbExpressionTranslator { const method = ts.createPropertyAccess(wrapForDiagnostics(receiver), ast.name); const args = ast.args.map(arg => this.translate(arg)); const node = ts.createCall(method, undefined, args); - addParseSpanInfo(node, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(node, ast.sourceSpan); return node; } else { // This AST isn't special after all. @@ -1071,7 +1060,7 @@ class TcbExpressionTranslator { // This expression has a binding to some variable or reference in the template. Resolve it. if (binding instanceof TmplAstVariable) { const expr = ts.getMutableClone(this.scope.resolve(binding)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(expr, ast.sourceSpan); return expr; } else if (binding instanceof TmplAstReference) { const target = this.tcb.boundTarget.getReferenceTarget(binding); @@ -1092,7 +1081,7 @@ class TcbExpressionTranslator { } const expr = ts.getMutableClone(this.scope.resolve(target)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(expr, ast.sourceSpan); return expr; } else if (target instanceof TmplAstTemplate) { if (!this.tcb.env.config.checkTypeOfNonDomReferences) { @@ -1109,7 +1098,7 @@ class TcbExpressionTranslator { value, this.tcb.env.referenceExternalType('@angular/core', 'TemplateRef', [DYNAMIC_TYPE])); value = ts.createParen(value); - addParseSpanInfo(value, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(value, ast.sourceSpan); return value; } else { if (!this.tcb.env.config.checkTypeOfNonDomReferences) { @@ -1118,7 +1107,7 @@ class TcbExpressionTranslator { } const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive)); - addParseSpanInfo(expr, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(expr, ast.sourceSpan); return expr; } } else { @@ -1236,7 +1225,7 @@ function tcbGetDirectiveInputs( let expr: ts.Expression; if (attr instanceof TmplAstBoundAttribute) { // Produce an expression representing the value of the binding. - expr = tcbExpression(attr.value, tcb, scope, attr.valueSpan || attr.sourceSpan); + expr = tcbExpression(attr.value, tcb, scope); } else { // For regular attributes with a static string value, use the represented string literal. expr = ts.createStringLiteral(attr.value); @@ -1275,7 +1264,7 @@ const enum EventParamType { function tcbCreateEventHandler( event: TmplAstBoundEvent, tcb: Context, scope: Scope, eventType: EventParamType | ts.TypeNode): ts.ArrowFunction { - const handler = tcbEventHandlerExpression(event.handler, tcb, scope, event.handlerSpan); + const handler = tcbEventHandlerExpression(event.handler, tcb, scope); let eventParamType: ts.TypeNode|undefined; if (eventType === EventParamType.Infer) { @@ -1307,9 +1296,8 @@ function tcbCreateEventHandler( * `ts.Expression`, with special handling of the `$event` variable that can be used within event * bindings. */ -function tcbEventHandlerExpression( - ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression { - const translator = new TcbEventHandlerTranslator(tcb, scope, sourceSpan); +function tcbEventHandlerExpression(ast: AST, tcb: Context, scope: Scope): ts.Expression { + const translator = new TcbEventHandlerTranslator(tcb, scope); return translator.translate(ast); } @@ -1322,7 +1310,7 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator { if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver && ast.name === EVENT_PARAMETER) { const event = ts.createIdentifier(EVENT_PARAMETER); - addParseSpanInfo(event, toAbsoluteSpan(ast.span, this.sourceSpan)); + addParseSpanInfo(event, ast.sourceSpan); return event; } diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 2b7052e791..f5ea683083 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -30,7 +30,7 @@ export class AST { /** * Absolute location of the expression AST in a source code file. */ - public sourceSpan: Readonly) {} + public sourceSpan: AbsoluteSourceSpan) {} visit(visitor: AstVisitor, context: any = null): any { return null; } toString(): string { return 'AST'; } } @@ -145,7 +145,7 @@ export class KeyedWrite extends AST { export class BindingPipe extends AST { constructor( span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string, - public args: any[], public nameSpan: ParseSpan) { + public args: any[], public nameSpan: AbsoluteSourceSpan) { super(span, sourceSpan); } visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPipe(this, context); } diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 9e0205bd9e..23d804d19b 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -359,7 +359,7 @@ export class _ParseAST { do { const nameStart = this.inputIndex; const name = this.expectIdentifierOrKeyword(); - const nameSpan = this.span(nameStart); + const nameSpan = this.sourceSpan(nameStart); const args: AST[] = []; while (this.optionalCharacter(chars.$COLON)) { args.push(this.parseExpression());