diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts new file mode 100644 index 0000000000..e28c774e2b --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright Google Inc. 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 {ParseSourceSpan, ParseSpan} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {ClassDeclaration} from '../../reflection'; +import {getSourceFile} from '../../util/src/typescript'; + +/** + * 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}; +} + +/** + * Wraps the node in parenthesis such that inserted span comments become attached to the proper + * node. This is an alias for `ts.createParen` with the benefit that it signifies that the + * inserted parenthesis are for diagnostic purposes, not for correctness of the rendered TCB code. + * + * Note that it is important that nodes and its attached comment are not wrapped into parenthesis + * by default, as it prevents correct translation of e.g. diagnostics produced for incorrect method + * arguments. Such diagnostics would then be produced for the parenthesised node whereas the + * positional comment would be located within that node, resulting in a mismatch. + */ +export function wrapForDiagnostics(expr: ts.Expression): ts.Expression { + return ts.createParen(expr); +} + +/** + * 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 { + let commentText: string; + if (typeof span.start === 'number') { + commentText = `${span.start},${span.end}`; + } else { + const {start, end} = span as ParseSourceSpan; + commentText = `${start.offset},${end.offset}`; + } + ts.addSyntheticTrailingComment( + node, ts.SyntaxKind.MultiLineCommentTrivia, commentText, + /* hasTrailingNewLine */ false); +} + +/** + * Adds a synthetic comment to the function declaration that contains the source location + * of the class declaration. + */ +export function addSourceInfo( + tcb: ts.FunctionDeclaration, source: ClassDeclaration): void { + const fileName = getSourceFile(source).fileName; + const commentText = `${fileName}#${source.name.text}`; + ts.addSyntheticLeadingComment(tcb, ts.SyntaxKind.MultiLineCommentTrivia, commentText, true); +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 9228824a4a..e9b3973012 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -6,10 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, ASTWithSource, AstVisitor, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; +import {AST, ASTWithSource, AstVisitor, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeCheckingConfig} from './api'; +import {AbsoluteSpan, addParseSpanInfo, wrapForDiagnostics} from './diagnostics'; const NULL_AS_ANY = ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); @@ -40,16 +41,17 @@ const BINARY_OPS = new Map([ * AST. */ export function astToTypescript( - ast: AST, maybeResolve: (ast: AST) => ts.Expression | null, - config: TypeCheckingConfig): ts.Expression { - const translator = new AstTranslator(maybeResolve, config); + ast: AST, maybeResolve: (ast: AST) => (ts.Expression | null), config: TypeCheckingConfig, + translateSpan: (span: ParseSpan) => AbsoluteSpan): ts.Expression { + const translator = new AstTranslator(maybeResolve, config, translateSpan); return translator.translate(ast); } class AstTranslator implements AstVisitor { constructor( - private maybeResolve: (ast: AST) => ts.Expression | null, - private config: TypeCheckingConfig) {} + private maybeResolve: (ast: AST) => (ts.Expression | null), + private config: TypeCheckingConfig, + private translateSpan: (span: ParseSpan) => AbsoluteSpan) {} translate(ast: AST): ts.Expression { // Skip over an `ASTWithSource` as its `visit` method calls directly into its ast's `visit`, @@ -68,13 +70,15 @@ class AstTranslator implements AstVisitor { } visitBinary(ast: Binary): ts.Expression { - const lhs = this.translate(ast.left); - const rhs = this.translate(ast.right); + const lhs = wrapForDiagnostics(this.translate(ast.left)); + const rhs = wrapForDiagnostics(this.translate(ast.right)); const op = BINARY_OPS.get(ast.operation); if (op === undefined) { throw new Error(`Unsupported Binary.operation: ${ast.operation}`); } - return ts.createBinary(lhs, op as any, rhs); + const node = ts.createBinary(lhs, op as any, rhs); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitChain(ast: Chain): never { throw new Error('Method not implemented.'); } @@ -83,7 +87,9 @@ class AstTranslator implements AstVisitor { const condExpr = this.translate(ast.condition); const trueExpr = this.translate(ast.trueExp); const falseExpr = this.translate(ast.falseExp); - return ts.createParen(ts.createConditional(condExpr, trueExpr, falseExpr)); + const node = ts.createParen(ts.createConditional(condExpr, trueExpr, falseExpr)); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitFunctionCall(ast: FunctionCall): never { throw new Error('Method not implemented.'); } @@ -102,16 +108,20 @@ class AstTranslator implements AstVisitor { } visitKeyedRead(ast: KeyedRead): ts.Expression { - const receiver = this.translate(ast.obj); + const receiver = wrapForDiagnostics(this.translate(ast.obj)); const key = this.translate(ast.key); - return ts.createElementAccess(receiver, key); + const node = ts.createElementAccess(receiver, key); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitKeyedWrite(ast: KeyedWrite): never { throw new Error('Method not implemented.'); } visitLiteralArray(ast: LiteralArray): ts.Expression { const elements = ast.expressions.map(expr => this.translate(expr)); - return ts.createArrayLiteral(elements); + const node = ts.createArrayLiteral(elements); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitLiteralMap(ast: LiteralMap): ts.Expression { @@ -119,42 +129,56 @@ class AstTranslator implements AstVisitor { const value = this.translate(ast.values[idx]); return ts.createPropertyAssignment(ts.createStringLiteral(key), value); }); - return ts.createObjectLiteral(properties, true); + const node = ts.createObjectLiteral(properties, true); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitLiteralPrimitive(ast: LiteralPrimitive): ts.Expression { + let node: ts.Expression; if (ast.value === undefined) { - return ts.createIdentifier('undefined'); + node = ts.createIdentifier('undefined'); } else if (ast.value === null) { - return ts.createNull(); + node = ts.createNull(); } else { - return ts.createLiteral(ast.value); + node = ts.createLiteral(ast.value); } + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitMethodCall(ast: MethodCall): ts.Expression { - const receiver = this.translate(ast.receiver); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const method = ts.createPropertyAccess(receiver, ast.name); const args = ast.args.map(expr => this.translate(expr)); - return ts.createCall(method, undefined, args); + const node = ts.createCall(method, undefined, args); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitNonNullAssert(ast: NonNullAssert): ts.Expression { - const expr = this.translate(ast.expression); - return ts.createNonNullExpression(expr); + const expr = wrapForDiagnostics(this.translate(ast.expression)); + const node = ts.createNonNullExpression(expr); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitPipe(ast: BindingPipe): never { throw new Error('Method not implemented.'); } visitPrefixNot(ast: PrefixNot): ts.Expression { - return ts.createLogicalNot(this.translate(ast.expression)); + const expression = wrapForDiagnostics(this.translate(ast.expression)); + const node = ts.createLogicalNot(expression); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitPropertyRead(ast: PropertyRead): ts.Expression { // This is a normal property read - convert the receiver to an expression and emit the correct // TypeScript expression to read the property. - const receiver = this.translate(ast.receiver); - return ts.createPropertyAccess(receiver, ast.name); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); + const node = ts.createPropertyAccess(receiver, ast.name); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitPropertyWrite(ast: PropertyWrite): never { throw new Error('Method not implemented.'); } @@ -164,12 +188,14 @@ class AstTranslator implements AstVisitor { visitSafeMethodCall(ast: SafeMethodCall): ts.Expression { // See the comment in SafePropertyRead above for an explanation of the need for the non-null // assertion here. - const receiver = this.translate(ast.receiver); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const args = ast.args.map(expr => this.translate(expr)); const expr = ts.createCall(method, undefined, args); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; - return safeTernary(receiver, expr, whenNull); + const node = safeTernary(receiver, expr, whenNull); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } visitSafePropertyRead(ast: SafePropertyRead): ts.Expression { @@ -177,10 +203,12 @@ class AstTranslator implements AstVisitor { // whenNull is either of type 'any' or or 'undefined' depending on strictness. The non-null // assertion is necessary because in practice 'a' may be a method call expression, which won't // have a narrowed type when repeated in the ternary true branch. - const receiver = this.translate(ast.receiver); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const expr = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; - return safeTernary(receiver, expr, whenNull); + const node = safeTernary(receiver, expr, whenNull); + addParseSpanInfo(node, this.translateSpan(ast.span)); + return node; } } 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 cb568f9911..af70c38c12 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,13 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, MethodCall, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, PropertyRead, TmplAstBoundAttribute, 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, addSourceInfo, toAbsoluteSpan, wrapForDiagnostics} from './diagnostics'; import {Environment} from './environment'; import {astToTypescript} from './expression'; import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util'; @@ -50,16 +51,17 @@ export function generateTypeCheckBlock( // Wrap the body in an "if (true)" expression. This is unnecessary but has the effect of causing // the `ts.Printer` to format the type-check block nicely. const body = ts.createBlock([ts.createIf(ts.createTrue(), innerBody, undefined)]); - - return ts.createFunctionDeclaration( - /* decorators */ undefined, - /* modifiers */ undefined, - /* asteriskToken */ undefined, - /* name */ name, - /* typeParameters */ ref.node.typeParameters, - /* parameters */ paramList, - /* type */ undefined, - /* body */ body); + const fnDecl = ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* asteriskToken */ undefined, + /* name */ name, + /* typeParameters */ ref.node.typeParameters, + /* parameters */ paramList, + /* type */ undefined, + /* body */ body); + addSourceInfo(fnDecl, ref.node); + return fnDecl; } /** @@ -95,7 +97,9 @@ class TcbElementOp extends TcbOp { execute(): ts.Identifier { const id = this.tcb.allocateId(); // Add the declaration of the element using document.createElement. - this.scope.addStatement(tsCreateVariable(id, tsCreateElement(this.element.name))); + const initializer = tsCreateElement(this.element.name); + addParseSpanInfo(initializer, this.element.startSourceSpan || this.element.sourceSpan); + this.scope.addStatement(tsCreateVariable(id, initializer)); return id; } } @@ -123,6 +127,7 @@ class TcbVariableOp extends TcbOp { const initializer = ts.createPropertyAccess( /* expression */ ctx, /* name */ this.variable.value || '$implicit'); + addParseSpanInfo(initializer, this.variable.sourceSpan); // Declare the variable, and return its identifier. this.scope.addStatement(tsCreateVariable(id, initializer)); @@ -193,7 +198,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); + const expr = tcbExpression(boundInput.value, this.tcb, this.scope, boundInput.sourceSpan); if (guard.type === 'binding') { // Use the binding expression itself as guard. @@ -205,6 +210,8 @@ class TcbTemplateBodyOp extends TcbOp { dirInstId, expr, ]); + addParseSpanInfo( + guardInvoke, toAbsoluteSpan(boundInput.value.span, boundInput.sourceSpan)); directiveGuards.push(guardInvoke); } } @@ -215,6 +222,7 @@ class TcbTemplateBodyOp extends TcbOp { if (dir.hasNgTemplateContextGuard && this.tcb.env.config.applyTemplateContextGuards) { const ctx = this.scope.resolve(this.template); const guardInvoke = tsCallMethod(dirId, 'ngTemplateContextGuard', [dirInstId, ctx]); + addParseSpanInfo(guardInvoke, this.template.sourceSpan); directiveGuards.push(guardInvoke); } } @@ -254,7 +262,7 @@ class TcbTextInterpolationOp extends TcbOp { } execute(): null { - const expr = tcbExpression(this.binding.value, this.tcb, this.scope); + const expr = tcbExpression(this.binding.value, this.tcb, this.scope, this.binding.sourceSpan); this.scope.addStatement(ts.createExpressionStatement(expr)); return null; } @@ -282,6 +290,7 @@ class TcbDirectiveOp extends TcbOp { // Call the type constructor of the directive to infer a type, and assign the directive // instance. const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, bindings); + addParseSpanInfo(typeCtor, this.node.sourceSpan); this.scope.addStatement(tsCreateVariable(id, typeCtor)); return id; } @@ -326,7 +335,7 @@ class TcbUnclaimedInputsOp extends TcbOp { continue; } - let expr = tcbExpression(binding.value, this.tcb, this.scope); + let expr = tcbExpression(binding.value, this.tcb, this.scope, binding.sourceSpan); // If checking the type of bindings is disabled, cast the resulting expression to 'any' before // the assignment. @@ -339,8 +348,9 @@ class TcbUnclaimedInputsOp extends TcbOp { // A direct binding to a property. const propertyName = ATTR_TO_PROP[binding.name] || binding.name; const prop = ts.createPropertyAccess(elId, propertyName); - const assign = ts.createBinary(prop, ts.SyntaxKind.EqualsToken, expr); - this.scope.addStatement(ts.createStatement(assign)); + const stmt = ts.createBinary(prop, ts.SyntaxKind.EqualsToken, wrapForDiagnostics(expr)); + addParseSpanInfo(stmt, binding.sourceSpan); + this.scope.addStatement(ts.createExpressionStatement(stmt)); } else { this.scope.addStatement(ts.createExpressionStatement(expr)); } @@ -686,11 +696,15 @@ 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): ts.Expression { +function tcbExpression( + ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression { + const translateSpan = (span: ParseSpan) => toAbsoluteSpan(span, sourceSpan); + // `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) => tcbResolve(ast, tcb, scope), tcb.env.config); + return astToTypescript( + ast, (ast) => tcbResolve(ast, tcb, scope, sourceSpan), tcb.env.config, translateSpan); } /** @@ -703,11 +717,13 @@ function tcbCallTypeCtor( // Construct an array of `ts.PropertyAssignment`s for each input of the directive that has a // matching binding. - const members = bindings.map(({field, expression}) => { + const members = bindings.map(({field, expression, sourceSpan}) => { if (!tcb.env.config.checkTypeOfBindings) { expression = tsCastToAny(expression); } - return ts.createPropertyAssignment(field, expression); + const assignment = ts.createPropertyAssignment(field, wrapForDiagnostics(expression)); + addParseSpanInfo(assignment, sourceSpan); + return assignment; }); // Call the `ngTypeCtor` method on the directive class, with an object literal argument created @@ -722,6 +738,7 @@ interface TcbBinding { field: string; property: string; expression: ts.Expression; + sourceSpan: ParseSourceSpan; } function tcbGetInputBindingExpressions( @@ -751,12 +768,13 @@ function tcbGetInputBindingExpressions( function processAttribute(attr: TmplAstBoundAttribute | TmplAstTextAttribute): void { if (attr instanceof TmplAstBoundAttribute && propMatch.has(attr.name)) { // Produce an expression representing the value of the binding. - const expr = tcbExpression(attr.value, tcb, scope); + const expr = tcbExpression(attr.value, tcb, scope, attr.sourceSpan); // Call the callback. bindings.push({ property: attr.name, field: propMatch.get(attr.name) !, expression: expr, + sourceSpan: attr.sourceSpan, }); } } @@ -768,7 +786,8 @@ function tcbGetInputBindingExpressions( * Some `AST` expressions refer to top-level concepts (references, variables, the component * context). This method assists in resolving those. */ -function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { +function tcbResolve( + ast: AST, tcb: Context, scope: Scope, sourceSpan: ParseSourceSpan): ts.Expression|null { if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver) { // Check whether the template metadata has bound a target for this expression. If so, then // resolve that target. If not, then the expression is referencing the top-level component @@ -777,7 +796,9 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { if (binding !== null) { // This expression has a binding to some variable or reference in the template. Resolve it. if (binding instanceof TmplAstVariable) { - return scope.resolve(binding); + const expr = ts.getMutableClone(scope.resolve(binding)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); + return expr; } else if (binding instanceof TmplAstReference) { const target = tcb.boundTarget.getReferenceTarget(binding); if (target === null) { @@ -788,7 +809,9 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { // element or template. if (target instanceof TmplAstElement) { - return scope.resolve(target); + const expr = ts.getMutableClone(scope.resolve(target)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); + return expr; } else if (target instanceof TmplAstTemplate) { // Direct references to an node simply require a value of type // `TemplateRef`. To get this, an expression of the form @@ -797,9 +820,12 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { value = ts.createAsExpression(value, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); value = ts.createAsExpression(value, tcb.env.referenceCoreType('TemplateRef', 1)); value = ts.createParen(value); + addParseSpanInfo(value, toAbsoluteSpan(ast.span, sourceSpan)); return value; } else { - return scope.resolve(target.node, target.directive); + const expr = ts.getMutableClone(scope.resolve(target.node, target.directive)); + addParseSpanInfo(expr, toAbsoluteSpan(ast.span, sourceSpan)); + return expr; } } else { throw new Error(`Unreachable: ${binding}`); @@ -823,7 +849,7 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { // the component context itself. return ts.createIdentifier('ctx'); } else if (ast instanceof BindingPipe) { - const expr = tcbExpression(ast.exp, tcb, scope); + const expr = tcbExpression(ast.exp, tcb, scope, sourceSpan); let pipe: ts.Expression; if (tcb.env.config.checkTypeOfPipes) { pipe = tcb.getPipeByName(ast.name); @@ -831,15 +857,19 @@ function tcbResolve(ast: AST, tcb: Context, scope: Scope): ts.Expression|null { pipe = ts.createParen(ts.createAsExpression( ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword))); } - const args = ast.args.map(arg => tcbExpression(arg, tcb, scope)); - return tsCallMethod(pipe, 'transform', [expr, ...args]); + const args = ast.args.map(arg => tcbExpression(arg, tcb, scope, sourceSpan)); + const result = tsCallMethod(pipe, 'transform', [expr, ...args]); + addParseSpanInfo(result, toAbsoluteSpan(ast.span, sourceSpan)); + return result; } else if ( ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver && ast.name === '$any' && ast.args.length === 1) { - const expr = tcbExpression(ast.args[0], tcb, scope); + const expr = tcbExpression(ast.args[0], tcb, scope, sourceSpan); const exprAsAny = ts.createAsExpression(expr, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); - return ts.createParen(exprAsAny); + const result = ts.createParen(exprAsAny); + addParseSpanInfo(result, toAbsoluteSpan(ast.span, sourceSpan)); + return result; } else { // This AST isn't special after all. return null; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts new file mode 100644 index 0000000000..863f37d2e6 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts @@ -0,0 +1,128 @@ +/** + * @license + * Copyright Google Inc. 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 {TestDeclaration, tcb} from './test_utils'; + +describe('type check blocks diagnostics', () => { + describe('parse spans', () => { + it('should annotate binary ops', () => { + expect(tcbWithSpans('{{ a + b }}')) + .toContain('"" + (((ctx).a /*3,5*/) + ((ctx).b /*7,9*/) /*3,9*/);'); + }); + + it('should annotate conditions', () => { + expect(tcbWithSpans('{{ a ? b : c }}')) + .toContain('((ctx).a /*3,5*/ ? (ctx).b /*7,9*/ : (ctx).c /*11,13*/) /*3,13*/;'); + }); + + it('should annotate interpolations', () => { + expect(tcbWithSpans('{{ hello }} {{ world }}')) + .toContain('"" + (ctx).hello /*3,9*/ + (ctx).world /*15,21*/;'); + }); + + it('should annotate literal map expressions', () => { + // The additional method call is present to avoid that the object literal is emitted as + // statement, which would wrap it into parenthesis that clutter the expected output. + const TEMPLATE = '{{ m({foo: a, bar: b}) }}'; + expect(tcbWithSpans(TEMPLATE)) + .toContain('m({ "foo": (ctx).a /*11,12*/, "bar": (ctx).b /*19,20*/ } /*5,21*/)'); + }); + + it('should annotate literal array expressions', () => { + const TEMPLATE = '{{ [a, b] }}'; + expect(tcbWithSpans(TEMPLATE)).toContain('[(ctx).a /*4,5*/, (ctx).b /*7,8*/] /*3,10*/;'); + }); + + it('should annotate literals', () => { + const TEMPLATE = '{{ 123 }}'; + expect(tcbWithSpans(TEMPLATE)).toContain('123 /*3,7*/;'); + }); + + it('should annotate non-null assertions', () => { + const TEMPLATE = `{{ a! }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/)! /*3,6*/);'); + }); + + it('should annotate prefix not', () => { + const TEMPLATE = `{{ !a }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('!((ctx).a /*4,6*/) /*3,6*/;'); + }); + + it('should annotate method calls', () => { + const TEMPLATE = `{{ method(a, b) }}`; + expect(tcbWithSpans(TEMPLATE)) + .toContain('(ctx).method((ctx).a /*10,11*/, (ctx).b /*13,14*/) /*3,16*/;'); + }); + + it('should annotate property access', () => { + const TEMPLATE = `{{ a.b.c }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('(((ctx).a /*3,4*/).b /*3,6*/).c /*3,9*/;'); + }); + + it('should annotate keyed property access', () => { + const TEMPLATE = `{{ a[b] }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*3,4*/)[(ctx).b /*5,6*/] /*3,8*/;'); + }); + + it('should annotate safe property access', () => { + const TEMPLATE = `{{ a?.b }}`; + expect(tcbWithSpans(TEMPLATE)) + .toContain('(((ctx).a /*3,4*/) != null ? ((ctx).a /*3,4*/)!.b : undefined) /*3,8*/;'); + }); + + it('should annotate safe method calls', () => { + const TEMPLATE = `{{ a?.method(b) }}`; + expect(tcbWithSpans(TEMPLATE)) + .toContain( + '(((ctx).a /*3,4*/) != null ? ((ctx).a /*3,4*/)!.method((ctx).b /*13,14*/) : undefined) /*3,16*/;'); + }); + + it('should annotate $any casts', () => { + const TEMPLATE = `{{ $any(a) }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('((ctx).a /*8,9*/ as any) /*3,11*/;'); + }); + + it('should annotate pipe usages', () => { + const TEMPLATE = `{{ a | test:b }}`; + const PIPES: TestDeclaration[] = [{ + type: 'pipe', + name: 'TestPipe', + pipeName: 'test', + }]; + const block = tcbWithSpans(TEMPLATE, PIPES); + expect(block).toContain( + '(null as TestPipe).transform((ctx).a /*3,5*/, (ctx).b /*12,14*/) /*3,14*/;'); + }); + + describe('attaching multiple comments for multiple references', () => { + it('should be correct for element refs', () => { + const TEMPLATE = `{{ a || a }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('((_t1 /*19,21*/) || (_t1 /*24,26*/) /*19,26*/);'); + }); + it('should be correct for template vars', () => { + const TEMPLATE = `{{ a || a }}`; + expect(tcbWithSpans(TEMPLATE)).toContain('((_t2 /*26,28*/) || (_t2 /*31,33*/) /*26,33*/);'); + }); + it('should be correct for directive refs', () => { + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'MyComponent', + selector: 'my-cmp', + isComponent: true, + }]; + const TEMPLATE = `{{ a || a }}`; + expect(tcbWithSpans(TEMPLATE, DIRECTIVES)) + .toContain('((_t2 /*23,25*/) || (_t2 /*28,30*/) /*23,30*/);'); + }); + }); + }); +}); + +function tcbWithSpans(template: string, declarations: TestDeclaration[] = []): string { + return tcb(template, declarations, undefined, {emitSpans: true}); +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index 20ce5b77aa..3ab0df0c4e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -28,7 +28,8 @@ export type TestPipe = { export type TestDeclaration = TestDirective | TestPipe; export function tcb( - template: string, declarations: TestDeclaration[] = [], config?: TypeCheckingConfig): string { + template: string, declarations: TestDeclaration[] = [], config?: TypeCheckingConfig, + options?: {emitSpans?: boolean}): string { const classes = ['Test', ...declarations.map(decl => decl.name)]; const code = classes.map(name => `class ${name} {}`).join('\n'); @@ -76,11 +77,15 @@ export function tcb( checkTemplateBodies: true, strictSafeNavigationTypes: true, }; + options = options || { + emitSpans: false, + }; const tcb = generateTypeCheckBlock( FakeEnvironment.newFake(config), new Reference(clazz), ts.createIdentifier('Test_TCB'), meta); - const res = ts.createPrinter().printNode(ts.EmitHint.Unspecified, tcb, sf); + const removeComments = !options.emitSpans; + const res = ts.createPrinter({removeComments}).printNode(ts.EmitHint.Unspecified, tcb, sf); return res.replace(/\s+/g, ' '); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 971c806f2e..1d6848e6c3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -7,37 +7,36 @@ */ import {TypeCheckingConfig} from '../src/api'; - import {TestDeclaration, TestDirective, tcb} from './test_utils'; describe('type check blocks', () => { it('should generate a basic block for a binding', - () => { expect(tcb('{{hello}} {{world}}')).toContain('"" + ctx.hello + ctx.world;'); }); + () => { expect(tcb('{{hello}} {{world}}')).toContain('"" + (ctx).hello + (ctx).world;'); }); it('should generate literal map expressions', () => { const TEMPLATE = '{{ method({foo: a, bar: b}) }}'; - expect(tcb(TEMPLATE)).toContain('ctx.method({ "foo": ctx.a, "bar": ctx.b });'); + expect(tcb(TEMPLATE)).toContain('(ctx).method({ "foo": (ctx).a, "bar": (ctx).b });'); }); it('should generate literal array expressions', () => { const TEMPLATE = '{{ method([a, b]) }}'; - expect(tcb(TEMPLATE)).toContain('ctx.method([ctx.a, ctx.b]);'); + expect(tcb(TEMPLATE)).toContain('(ctx).method([(ctx).a, (ctx).b]);'); }); it('should handle non-null assertions', () => { const TEMPLATE = `{{a!}}`; - expect(tcb(TEMPLATE)).toContain('(ctx.a!);'); + expect(tcb(TEMPLATE)).toContain('(((ctx).a)!);'); }); it('should handle keyed property access', () => { const TEMPLATE = `{{a[b]}}`; - expect(tcb(TEMPLATE)).toContain('ctx.a[ctx.b];'); + expect(tcb(TEMPLATE)).toContain('((ctx).a)[(ctx).b];'); }); it('should translate unclaimed bindings to their property equivalent', () => { const TEMPLATE = ``; - expect(tcb(TEMPLATE)).toContain('_t1.htmlFor = "test";'); + expect(tcb(TEMPLATE)).toContain('_t1.htmlFor = ("test");'); }); it('should handle implicit vars on ng-template', () => { @@ -55,7 +54,7 @@ describe('type check blocks', () => { {{ i.value }} `; - expect(tcb(TEMPLATE)).toContain('var _t1 = document.createElement("input"); "" + _t1.value;'); + expect(tcb(TEMPLATE)).toContain('var _t1 = document.createElement("input"); "" + (_t1).value;'); }); it('should generate a forward directive reference correctly', () => { @@ -71,7 +70,7 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t1 = Dir.ngTypeCtor({}); "" + _t1.value; var _t2 = document.createElement("div");'); + 'var _t1 = Dir.ngTypeCtor({}); "" + (_t1).value; var _t2 = document.createElement("div");'); }); it('should handle style and class bindings specially', () => { @@ -79,17 +78,58 @@ describe('type check blocks', () => {
`; const block = tcb(TEMPLATE); - expect(block).toContain('ctx.a; ctx.b;'); + expect(block).toContain('(ctx).a; (ctx).b;'); // There should be no assignments to the class or style properties. expect(block).not.toContain('.class = '); expect(block).not.toContain('.style = '); }); + it('should generate a circular directive reference correctly', () => { + const TEMPLATE = ` +
+ `; + const DIRECTIVES: TestDirective[] = [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + exportAs: ['dir'], + inputs: {input: 'input'}, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t2 = Dir.ngTypeCtor({ input: (null!) });'); + }); + + it('should generate circular references between two directives correctly', () => { + const TEMPLATE = ` +
A
+
B
+`; + const DIRECTIVES: TestDirective[] = [ + { + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + exportAs: ['dirA'], + inputs: {inputA: 'inputA'}, + }, + { + type: 'directive', + name: 'DirB', + selector: '[dir-b]', + exportAs: ['dirB'], + inputs: {inputA: 'inputB'}, + } + ]; + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t3 = DirB.ngTypeCtor({ inputA: (null!) }); ' + + 'var _t2 = DirA.ngTypeCtor({ inputA: (_t3) });'); + }); + it('should handle $any casts', () => { const TEMPLATE = `{{$any(a)}}`; const block = tcb(TEMPLATE); - expect(block).toContain('(ctx.a as any);'); + expect(block).toContain('((ctx).a as any);'); }); describe('template guards', () => { @@ -106,7 +146,7 @@ describe('type check blocks', () => { }]; const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('if (NgIf.ngTemplateGuard_ngIf(_t1, ctx.person))'); + expect(block).toContain('if (NgIf.ngTemplateGuard_ngIf(_t1, (ctx).person))'); }); it('should emit binding guards', () => { @@ -122,7 +162,7 @@ describe('type check blocks', () => { }]; const TEMPLATE = `
`; const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('if (ctx.person !== null)'); + expect(block).toContain('if (((ctx).person) !== (null))'); }); }); @@ -164,12 +204,12 @@ describe('type check blocks', () => { it('should descend into template bodies when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('ctx.a;'); + expect(block).toContain('(ctx).a;'); }); it('should not descend into template bodies when disabled', () => { const DISABLED_CONFIG = {...BASE_CONFIG, checkTemplateBodies: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).not.toContain('ctx.a;'); + expect(block).not.toContain('(ctx).a;'); }); }); @@ -178,14 +218,14 @@ describe('type check blocks', () => { it('should check types of bindings when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('Dir.ngTypeCtor({ dirInput: ctx.a })'); - expect(block).toContain('.nonDirInput = ctx.a;'); + expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })'); + expect(block).toContain('.nonDirInput = ((ctx).a);'); }); it('should not check types of bindings when disabled', () => { const DISABLED_CONFIG = {...BASE_CONFIG, checkTypeOfBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('Dir.ngTypeCtor({ dirInput: (ctx.a as any) })'); - expect(block).toContain('.nonDirInput = (ctx.a as any);'); + expect(block).toContain('Dir.ngTypeCtor({ dirInput: (((ctx).a as any)) })'); + expect(block).toContain('.nonDirInput = (((ctx).a as any));'); }); }); @@ -200,12 +240,12 @@ describe('type check blocks', () => { it('should check types of pipes when enabled', () => { const block = tcb(TEMPLATE, PIPES); - expect(block).toContain('(null as TestPipe).transform(ctx.a, ctx.b, ctx.c);'); + expect(block).toContain('(null as TestPipe).transform((ctx).a, (ctx).b, (ctx).c);'); }); it('should not check types of pipes when disabled', () => { const DISABLED_CONFIG = {...BASE_CONFIG, checkTypeOfPipes: false}; const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG); - expect(block).toContain('(null as any).transform(ctx.a, ctx.b, ctx.c);'); + expect(block).toContain('(null as any).transform((ctx).a, (ctx).b, (ctx).c);'); }); }); @@ -214,56 +254,15 @@ describe('type check blocks', () => { it('should use undefined for safe navigation operations when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('(ctx.a != null ? ctx.a!.method() : undefined)'); - expect(block).toContain('(ctx.a != null ? ctx.a!.b : undefined)'); + expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.method() : undefined)'); + expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.b : undefined)'); }); it('should use an \'any\' type for safe navigation operations when disabled', () => { const DISABLED_CONFIG = {...BASE_CONFIG, strictSafeNavigationTypes: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('(ctx.a != null ? ctx.a!.method() : null as any)'); - expect(block).toContain('(ctx.a != null ? ctx.a!.b : null as any)'); + expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.method() : null as any)'); + expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.b : null as any)'); }); }); }); }); - -it('should generate a circular directive reference correctly', () => { - const TEMPLATE = ` -
-`; - const DIRECTIVES: TestDirective[] = [{ - type: 'directive', - name: 'Dir', - selector: '[dir]', - exportAs: ['dir'], - inputs: {input: 'input'}, - }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t2 = Dir.ngTypeCtor({ input: (null!) });'); -}); - -it('should generate circular references between two directives correctly', () => { - const TEMPLATE = ` -
A
-
B
-`; - const DIRECTIVES: TestDirective[] = [ - { - type: 'directive', - name: 'DirA', - selector: '[dir-a]', - exportAs: ['dirA'], - inputs: {inputA: 'inputA'}, - }, - { - type: 'directive', - name: 'DirB', - selector: '[dir-b]', - exportAs: ['dirB'], - inputs: {inputA: 'inputB'}, - } - ]; - expect(tcb(TEMPLATE, DIRECTIVES)) - .toContain( - 'var _t3 = DirB.ngTypeCtor({ inputA: (null!) }); ' + - 'var _t2 = DirA.ngTypeCtor({ inputA: _t3 });'); -});