From 874792dc43a70dc55c9b926807ca6bbcb5cafb9e Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 4 Jul 2020 01:52:40 +0200 Subject: [PATCH] feat(compiler): support unary operators for more accurate type checking (#37918) Prior to this change, the unary + and - operators would be parsed as `x - 0` and `0 - x` respectively. The runtime semantics of these expressions are equivalent, however they may introduce inaccurate template type checking errors as the literal type is lost, for example: ```ts @Component({ template: `` }) export class Example { isAdjacent(direction: -1 | 1): boolean { return false; } } ``` would incorrectly report a type-check error: > error TS2345: Argument of type 'number' is not assignable to parameter of type '-1 | 1'. Additionally, the translated expression for the unary + operator would be considered as arithmetic expression with an incompatible left-hand side: > error TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type. To resolve this issues, the implicit transformation should be avoided. This commit adds a new unary AST node to represent these expressions, allowing for more accurate type-checking. Fixes #20845 Fixes #36178 PR Close #37918 --- .../src/ngtsc/translator/src/translator.ts | 19 ++++- .../src/ngtsc/typecheck/src/expression.ts | 25 +++++- .../ngtsc/typecheck/test/diagnostics_spec.ts | 11 +++ .../typecheck/test/span_comments_spec.ts | 4 + .../typecheck/test/type_check_block_spec.ts | 5 ++ .../src/transformers/node_emitter.ts | 19 ++++- .../src/compiler_util/expression_converter.ts | 26 ++++++ packages/compiler/src/constant_pool.ts | 1 + .../compiler/src/expression_parser/ast.ts | 80 +++++++++++++++++++ .../compiler/src/expression_parser/parser.ts | 14 ++-- .../compiler/src/output/abstract_emitter.ts | 19 +++++ packages/compiler/src/output/output_ast.ts | 45 +++++++++++ .../compiler/src/output/output_interpreter.ts | 12 +++ .../test/expression_parser/parser_spec.ts | 10 +-- .../test/expression_parser/utils/unparser.ts | 7 +- .../test/expression_parser/utils/validator.ts | 6 +- .../compiler/test/output/js_emitter_spec.ts | 2 + .../compiler/test/output/ts_emitter_spec.ts | 2 + .../template_parser/template_parser_spec.ts | 2 +- .../test/template_parser/util/expression.ts | 4 + .../language-service/src/expression_type.ts | 19 ++++- packages/language-service/src/expressions.ts | 2 + .../test/expression_diagnostics_spec.ts | 2 + 23 files changed, 313 insertions(+), 23 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index c91a6bdf59..9e672d2f38 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -7,7 +7,7 @@ */ import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeofExpr, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; -import {LocalizedString} from '@angular/compiler/src/output/output_ast'; +import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast'; import * as ts from 'typescript'; import {DefaultImportRecorder, ImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports'; @@ -24,6 +24,11 @@ export class Context { } } +const UNARY_OPERATORS = new Map([ + [UnaryOperator.Minus, ts.SyntaxKind.MinusToken], + [UnaryOperator.Plus, ts.SyntaxKind.PlusToken], +]); + const BINARY_OPERATORS = new Map([ [BinaryOperator.And, ts.SyntaxKind.AmpersandAmpersandToken], [BinaryOperator.Bigger, ts.SyntaxKind.GreaterThanToken], @@ -361,6 +366,14 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor undefined, ts.createBlock(ast.statements.map(stmt => stmt.visitStatement(this, context)))); } + visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: Context): ts.Expression { + if (!UNARY_OPERATORS.has(ast.operator)) { + throw new Error(`Unknown unary operator: ${UnaryOperator[ast.operator]}`); + } + return ts.createPrefix( + UNARY_OPERATORS.get(ast.operator)!, ast.expr.visitExpression(this, context)); + } + visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: Context): ts.Expression { if (!BINARY_OPERATORS.has(ast.operator)) { throw new Error(`Unknown binary operator: ${BinaryOperator[ast.operator]}`); @@ -567,6 +580,10 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { throw new Error('Method not implemented.'); } + visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: Context) { + throw new Error('Method not implemented.'); + } + visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: Context) { throw new Error('Method not implemented.'); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 81fb098be2..708ac522a1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, ASTWithSource, 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 {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeCheckingConfig} from '../api'; @@ -17,7 +17,12 @@ export const NULL_AS_ANY = ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); const UNDEFINED = ts.createIdentifier('undefined'); -const BINARY_OPS = new Map([ +const UNARY_OPS = new Map([ + ['+', ts.SyntaxKind.PlusToken], + ['-', ts.SyntaxKind.MinusToken], +]); + +const BINARY_OPS = new Map([ ['+', ts.SyntaxKind.PlusToken], ['-', ts.SyntaxKind.MinusToken], ['<', ts.SyntaxKind.LessThanToken], @@ -74,6 +79,17 @@ class AstTranslator implements AstVisitor { return ast.visit(this); } + visitUnary(ast: Unary): ts.Expression { + const expr = this.translate(ast.expr); + const op = UNARY_OPS.get(ast.operator); + if (op === undefined) { + throw new Error(`Unsupported Unary.operator: ${ast.operator}`); + } + const node = wrapForDiagnostics(ts.createPrefix(op, expr)); + addParseSpanInfo(node, ast.sourceSpan); + return node; + } + visitBinary(ast: Binary): ts.Expression { const lhs = wrapForDiagnostics(this.translate(ast.left)); const rhs = wrapForDiagnostics(this.translate(ast.right)); @@ -81,7 +97,7 @@ class AstTranslator implements AstVisitor { if (op === undefined) { throw new Error(`Unsupported Binary.operation: ${ast.operation}`); } - const node = ts.createBinary(lhs, op as any, rhs); + const node = ts.createBinary(lhs, op, rhs); addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -314,6 +330,9 @@ class VeSafeLhsInferenceBugDetector implements AstVisitor { return ast.receiver.visit(VeSafeLhsInferenceBugDetector.SINGLETON); } + visitUnary(ast: Unary): boolean { + return ast.expr.visit(this); + } visitBinary(ast: Binary): boolean { return ast.left.visit(this) || ast.right.visit(this); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 2c278a3d68..4a19284ea1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -248,6 +248,17 @@ runInEachFileSystem(() => { expect(messages).toEqual([]); }); + it('should treat unary operators as literal types', () => { + const messages = diagnose(`{{ test(-1) + test(+1) + test(-2) }}`, ` + class TestComponent { + test(value: -1 | 1): number { return value; } + }`); + + expect(messages).toEqual([ + `TestComponent.html(1, 31): Argument of type '-2' is not assignable to parameter of type '1 | -1'.`, + ]); + }); + describe('outputs', () => { it('should produce a diagnostic for directive outputs', () => { const messages = diagnose( 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 index c15b1bc608..1a87da6987 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts @@ -10,6 +10,10 @@ import {tcb, TestDeclaration} from './test_utils'; describe('type check blocks diagnostics', () => { describe('parse spans', () => { + it('should annotate unary ops', () => { + expect(tcbWithSpans('{{ -a }}')).toContain('(-((ctx).a /*4,5*/) /*4,5*/) /*3,5*/'); + }); + it('should annotate binary ops', () => { expect(tcbWithSpans('{{ a + b }}')) .toContain('(((ctx).a /*3,4*/) /*3,4*/) + (((ctx).b /*7,8*/) /*7,8*/) /*3,8*/'); 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 9dafff8bde..fb168922b4 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 @@ -31,6 +31,11 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE)).toContain('((((ctx).a))!);'); }); + it('should handle unary - operator', () => { + const TEMPLATE = `{{-1}}`; + expect(tcb(TEMPLATE)).toContain('(-1);'); + }); + it('should handle keyed property access', () => { const TEMPLATE = `{{a[b]}}`; expect(tcb(TEMPLATE)).toContain('(((ctx).a))[((ctx).b)];'); diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index 4a791b27bd..5ad9e0b2e3 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -7,7 +7,7 @@ */ import {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ParseSourceFile, ParseSourceSpan, PartialModule, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; -import {LocalizedString} from '@angular/compiler/src/output/output_ast'; +import {LocalizedString, UnaryOperator, UnaryOperatorExpr} from '@angular/compiler/src/output/output_ast'; import * as ts from 'typescript'; import {error} from './util'; @@ -622,6 +622,23 @@ export class NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { /* type */ undefined, this._visitStatements(expr.statements))); } + visitUnaryOperatorExpr(expr: UnaryOperatorExpr): + RecordedNode { + let unaryOperator: ts.BinaryOperator; + switch (expr.operator) { + case UnaryOperator.Minus: + unaryOperator = ts.SyntaxKind.MinusToken; + break; + case UnaryOperator.Plus: + unaryOperator = ts.SyntaxKind.PlusToken; + break; + default: + throw new Error(`Unknown operator: ${expr.operator}`); + } + const binary = ts.createPrefix(unaryOperator, expr.expr.visitExpression(this, null)); + return this.record(expr, expr.parens ? ts.createParen(binary) : binary); + } + visitBinaryOperatorExpr(expr: BinaryOperatorExpr): RecordedNode { let binaryOperator: ts.BinaryOperator; diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index 240c35b66f..1bae07ce32 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -330,6 +330,26 @@ class _AstToIrVisitor implements cdAst.AstVisitor { private bindingId: string, private interpolationFunction: InterpolationFunction|undefined, private baseSourceSpan?: ParseSourceSpan, private implicitReceiverAccesses?: Set) {} + visitUnary(ast: cdAst.Unary, mode: _Mode): any { + let op: o.UnaryOperator; + switch (ast.operator) { + case '+': + op = o.UnaryOperator.Plus; + break; + case '-': + op = o.UnaryOperator.Minus; + break; + default: + throw new Error(`Unsupported operator ${ast.operator}`); + } + + return convertToStatementIfNeeded( + mode, + new o.UnaryOperatorExpr( + op, this._visit(ast.expr, _Mode.Expression), undefined, + this.convertSourceSpan(ast.span))); + } + visitBinary(ast: cdAst.Binary, mode: _Mode): any { let op: o.BinaryOperator; switch (ast.operation) { @@ -710,6 +730,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return (this._nodeMap.get(ast) || ast).visit(visitor); }; return ast.visit({ + visitUnary(ast: cdAst.Unary) { + return null; + }, visitBinary(ast: cdAst.Binary) { return null; }, @@ -784,6 +807,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return ast.some(ast => visit(visitor, ast)); }; return ast.visit({ + visitUnary(ast: cdAst.Unary): boolean { + return visit(this, ast.expr); + }, visitBinary(ast: cdAst.Binary): boolean { return visit(this, ast.left) || visit(this, ast.right); }, diff --git a/packages/compiler/src/constant_pool.ts b/packages/compiler/src/constant_pool.ts index ae9f7c3009..0046f7cc87 100644 --- a/packages/compiler/src/constant_pool.ts +++ b/packages/compiler/src/constant_pool.ts @@ -330,6 +330,7 @@ class KeyVisitor implements o.ExpressionVisitor { visitAssertNotNullExpr = invalid; visitCastExpr = invalid; visitFunctionExpr = invalid; + visitUnaryOperatorExpr = invalid; visitBinaryOperatorExpr = invalid; visitReadPropExpr = invalid; visitReadKeyExpr = invalid; diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 18ab08e533..6568d6fafb 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -227,6 +227,52 @@ export class Binary extends AST { } } +/** + * For backwards compatibility reasons, `Unary` inherits from `Binary` and mimics the binary AST + * node that was originally used. This inheritance relation can be deleted in some future major, + * after consumers have been given a chance to fully support Unary. + */ +export class Unary extends Binary { + // Redeclare the properties that are inherited from `Binary` as `never`, as consumers should not + // depend on these fields when operating on `Unary`. + left: never; + right: never; + operation: never; + + /** + * Creates a unary minus expression "-x", represented as `Binary` using "0 - x". + */ + static createMinus(span: ParseSpan, sourceSpan: AbsoluteSourceSpan, expr: AST): Unary { + return new Unary( + span, sourceSpan, '-', expr, '-', new LiteralPrimitive(span, sourceSpan, 0), expr); + } + + /** + * Creates a unary plus expression "+x", represented as `Binary` using "x - 0". + */ + static createPlus(span: ParseSpan, sourceSpan: AbsoluteSourceSpan, expr: AST): Unary { + return new Unary( + span, sourceSpan, '+', expr, '-', expr, new LiteralPrimitive(span, sourceSpan, 0)); + } + + /** + * During the deprecation period this constructor is private, to avoid consumers from creating + * a `Unary` with the fallback properties for `Binary`. + */ + private constructor( + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public operator: string, public expr: AST, + binaryOp: string, binaryLeft: AST, binaryRight: AST) { + super(span, sourceSpan, binaryOp, binaryLeft, binaryRight); + } + + visit(visitor: AstVisitor, context: any = null): any { + if (visitor.visitUnary !== undefined) { + return visitor.visitUnary(this, context); + } + return visitor.visitBinary(this, context); + } +} + export class PrefixNot extends AST { constructor(span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public expression: AST) { super(span, sourceSpan); @@ -361,6 +407,11 @@ export interface TemplateBindingIdentifier { } export interface AstVisitor { + /** + * The `visitUnary` method is declared as optional for backwards compatibility. In an upcoming + * major release, this method will be made required. + */ + visitUnary?(ast: Unary, context: any): any; visitBinary(ast: Binary, context: any): any; visitChain(ast: Chain, context: any): any; visitConditional(ast: Conditional, context: any): any; @@ -398,6 +449,9 @@ export class RecursiveAstVisitor implements AstVisitor { // to selectively visit the specified node. ast.visit(this, context); } + visitUnary(ast: Unary, context: any): any { + this.visit(ast.expr, context); + } visitBinary(ast: Binary, context: any): any { this.visit(ast.left, context); this.visit(ast.right, context); @@ -527,6 +581,17 @@ export class AstTransformer implements AstVisitor { return new LiteralMap(ast.span, ast.sourceSpan, ast.keys, this.visitAll(ast.values)); } + visitUnary(ast: Unary, context: any): AST { + switch (ast.operator) { + case '+': + return Unary.createPlus(ast.span, ast.sourceSpan, ast.expr.visit(this)); + case '-': + return Unary.createMinus(ast.span, ast.sourceSpan, ast.expr.visit(this)); + default: + throw new Error(`Unknown unary operator ${ast.operator}`); + } + } + visitBinary(ast: Binary, context: any): AST { return new Binary( ast.span, ast.sourceSpan, ast.operation, ast.left.visit(this), ast.right.visit(this)); @@ -665,6 +730,21 @@ export class AstMemoryEfficientTransformer implements AstVisitor { return ast; } + visitUnary(ast: Unary, context: any): AST { + const expr = ast.expr.visit(this); + if (expr !== ast.expr) { + switch (ast.operator) { + case '+': + return Unary.createPlus(ast.span, ast.sourceSpan, expr); + case '-': + return Unary.createMinus(ast.span, ast.sourceSpan, expr); + default: + throw new Error(`Unknown unary operator ${ast.operator}`); + } + } + return ast; + } + visitBinary(ast: Binary, context: any): AST { const left = ast.left.visit(this); const right = ast.right.visit(this); diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 8f97b71ff3..f71fa40f57 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -10,7 +10,7 @@ import * as chars from '../chars'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; import {escapeRegExp} from '../util'; -import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, VariableBinding} from './ast'; +import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, Unary, VariableBinding} from './ast'; import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer'; export class SplitInterpolation { @@ -591,22 +591,16 @@ export class _ParseAST { if (this.next.type == TokenType.Operator) { const start = this.inputIndex; const operator = this.next.strValue; - const literalSpan = new ParseSpan(start, start); - const literalSourceSpan = literalSpan.toAbsolute(this.absoluteOffset); let result: AST; switch (operator) { case '+': this.advance(); result = this.parsePrefix(); - return new Binary( - this.span(start), this.sourceSpan(start), '-', result, - new LiteralPrimitive(literalSpan, literalSourceSpan, 0)); + return Unary.createPlus(this.span(start), this.sourceSpan(start), result); case '-': this.advance(); result = this.parsePrefix(); - return new Binary( - this.span(start), this.sourceSpan(start), operator, - new LiteralPrimitive(literalSpan, literalSourceSpan, 0), result); + return Unary.createMinus(this.span(start), this.sourceSpan(start), result); case '!': this.advance(); result = this.parsePrefix(); @@ -1059,6 +1053,8 @@ class SimpleExpressionChecker implements AstVisitor { this.visitAll(ast.values, context); } + visitUnary(ast: Unary, context: any) {} + visitBinary(ast: Binary, context: any) {} visitPrefixNot(ast: PrefixNot, context: any) {} diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index 412ab31960..486e075358 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -420,6 +420,25 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex abstract visitFunctionExpr(ast: o.FunctionExpr, ctx: EmitterVisitorContext): any; abstract visitDeclareFunctionStmt(stmt: o.DeclareFunctionStmt, context: any): any; + visitUnaryOperatorExpr(ast: o.UnaryOperatorExpr, ctx: EmitterVisitorContext): any { + let opStr: string; + switch (ast.operator) { + case o.UnaryOperator.Plus: + opStr = '+'; + break; + case o.UnaryOperator.Minus: + opStr = '-'; + break; + default: + throw new Error(`Unknown operator ${ast.operator}`); + } + if (ast.parens) ctx.print(ast, `(`); + ctx.print(ast, opStr); + ast.expr.visitExpression(this, ctx); + if (ast.parens) ctx.print(ast, `)`); + return null; + } + visitBinaryOperatorExpr(ast: o.BinaryOperatorExpr, ctx: EmitterVisitorContext): any { let opStr: string; switch (ast.operator) { diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index dfac81e170..f718565ce3 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -100,6 +100,11 @@ export interface TypeVisitor { ///// Expressions +export enum UnaryOperator { + Minus, + Plus, +} + export enum BinaryOperator { Equals, NotEquals, @@ -753,6 +758,28 @@ export class FunctionExpr extends Expression { } +export class UnaryOperatorExpr extends Expression { + constructor( + public operator: UnaryOperator, public expr: Expression, type?: Type|null, + sourceSpan?: ParseSourceSpan|null, public parens: boolean = true) { + super(type || NUMBER_TYPE, sourceSpan); + } + + isEquivalent(e: Expression): boolean { + return e instanceof UnaryOperatorExpr && this.operator === e.operator && + this.expr.isEquivalent(e.expr); + } + + isConstant() { + return false; + } + + visitExpression(visitor: ExpressionVisitor, context: any): any { + return visitor.visitUnaryOperatorExpr(this, context); + } +} + + export class BinaryOperatorExpr extends Expression { public lhs: Expression; constructor( @@ -912,6 +939,7 @@ export interface ExpressionVisitor { visitAssertNotNullExpr(ast: AssertNotNull, context: any): any; visitCastExpr(ast: CastExpr, context: any): any; visitFunctionExpr(ast: FunctionExpr, context: any): any; + visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: any): any; visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: any): any; visitReadPropExpr(ast: ReadPropExpr, context: any): any; visitReadKeyExpr(ast: ReadKeyExpr, context: any): any; @@ -1292,6 +1320,13 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { context); } + visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: any): any { + return this.transformExpr( + new UnaryOperatorExpr( + ast.operator, ast.expr.visitExpression(this, context), ast.type, ast.sourceSpan), + context); + } + visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: any): any { return this.transformExpr( new BinaryOperatorExpr( @@ -1517,6 +1552,10 @@ export class RecursiveAstVisitor implements StatementVisitor, ExpressionVisitor this.visitAllStatements(ast.statements, context); return this.visitExpression(ast, context); } + visitUnaryOperatorExpr(ast: UnaryOperatorExpr, context: any): any { + ast.expr.visitExpression(this, context); + return this.visitExpression(ast, context); + } visitBinaryOperatorExpr(ast: BinaryOperatorExpr, context: any): any { ast.lhs.visitExpression(this, context); ast.rhs.visitExpression(this, context); @@ -1730,6 +1769,12 @@ export function literalMap( values.map(e => new LiteralMapEntry(e.key, e.value, e.quoted)), type, null); } +export function unary( + operator: UnaryOperator, expr: Expression, type?: Type, + sourceSpan?: ParseSourceSpan|null): UnaryOperatorExpr { + return new UnaryOperatorExpr(operator, expr, type, sourceSpan); +} + export function not(expr: Expression, sourceSpan?: ParseSourceSpan|null): NotExpr { return new NotExpr(expr, sourceSpan); } diff --git a/packages/compiler/src/output/output_interpreter.ts b/packages/compiler/src/output/output_interpreter.ts index 7aa35963aa..b45444469d 100644 --- a/packages/compiler/src/output/output_interpreter.ts +++ b/packages/compiler/src/output/output_interpreter.ts @@ -282,6 +282,18 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor { } return null; } + visitUnaryOperatorExpr(ast: o.UnaryOperatorExpr, ctx: _ExecutionContext): any { + const rhs = () => ast.expr.visitExpression(this, ctx); + + switch (ast.operator) { + case o.UnaryOperator.Plus: + return +rhs(); + case o.UnaryOperator.Minus: + return -rhs(); + default: + throw new Error(`Unknown operator ${ast.operator}`); + } + } visitBinaryOperatorExpr(ast: o.BinaryOperatorExpr, ctx: _ExecutionContext): any { const lhs = () => ast.lhs.visitExpression(this, ctx); const rhs = () => ast.rhs.visitExpression(this, ctx); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index e58f3a7efe..bbc58d16f7 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -34,11 +34,11 @@ describe('parser', () => { checkAction('undefined'); }); - it('should parse unary - expressions', () => { - checkAction('-1', '0 - 1'); - checkAction('+1', '1 - 0'); - checkAction(`-'1'`, `0 - "1"`); - checkAction(`+'1'`, `"1" - 0`); + it('should parse unary - and + expressions', () => { + checkAction('-1', '-1'); + checkAction('+1', '+1'); + checkAction(`-'1'`, `-"1"`); + checkAction(`+'1'`, `+"1"`); }); it('should parse unary ! expressions', () => { diff --git a/packages/compiler/test/expression_parser/utils/unparser.ts b/packages/compiler/test/expression_parser/utils/unparser.ts index 01a330e204..c16dae0d51 100644 --- a/packages/compiler/test/expression_parser/utils/unparser.ts +++ b/packages/compiler/test/expression_parser/utils/unparser.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead} from '../../../src/expression_parser/ast'; +import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, Unary} from '../../../src/expression_parser/ast'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; class Unparser implements AstVisitor { @@ -35,6 +35,11 @@ class Unparser implements AstVisitor { this._visit(ast.value); } + visitUnary(ast: Unary, context: any) { + this._expression += ast.operator; + this._visit(ast.expr); + } + visitBinary(ast: Binary, context: any) { this._visit(ast.left); this._expression += ` ${ast.operation} `; diff --git a/packages/compiler/test/expression_parser/utils/validator.ts b/packages/compiler/test/expression_parser/utils/validator.ts index 53082b0e64..0023fa77dd 100644 --- a/packages/compiler/test/expression_parser/utils/validator.ts +++ b/packages/compiler/test/expression_parser/utils/validator.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead} from '../../../src/expression_parser/ast'; +import {AST, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, Unary} from '../../../src/expression_parser/ast'; import {unparse} from './unparser'; @@ -34,6 +34,10 @@ class ASTValidator extends RecursiveAstVisitor { this.parentSpan = oldParent; } + visitUnary(ast: Unary, context: any): any { + this.validate(ast, () => super.visitUnary(ast, context)); + } + visitBinary(ast: Binary, context: any): any { this.validate(ast, () => super.visitBinary(ast, context)); } diff --git a/packages/compiler/test/output/js_emitter_spec.ts b/packages/compiler/test/output/js_emitter_spec.ts index 9ab5e9d5a8..c21297c4ee 100644 --- a/packages/compiler/test/output/js_emitter_spec.ts +++ b/packages/compiler/test/output/js_emitter_spec.ts @@ -138,6 +138,8 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some const lhs = o.variable('lhs'); const rhs = o.variable('rhs'); expect(emitStmt(o.not(someVar).toStmt())).toEqual('!someVar;'); + expect(emitStmt(o.unary(o.UnaryOperator.Minus, someVar).toStmt())).toEqual('(-someVar);'); + expect(emitStmt(o.unary(o.UnaryOperator.Plus, someVar).toStmt())).toEqual('(+someVar);'); expect(emitStmt(o.assertNotNull(someVar).toStmt())).toEqual('someVar;'); expect( emitStmt(someVar.conditional(o.variable('trueCase'), o.variable('falseCase')).toStmt())) diff --git a/packages/compiler/test/output/ts_emitter_spec.ts b/packages/compiler/test/output/ts_emitter_spec.ts index 810d24cbc2..78da4b697f 100644 --- a/packages/compiler/test/output/ts_emitter_spec.ts +++ b/packages/compiler/test/output/ts_emitter_spec.ts @@ -191,6 +191,8 @@ const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'some const rhs = o.variable('rhs'); expect(emitStmt(someVar.cast(o.INT_TYPE).toStmt())).toEqual('(someVar);'); expect(emitStmt(o.not(someVar).toStmt())).toEqual('!someVar;'); + expect(emitStmt(o.unary(o.UnaryOperator.Minus, someVar).toStmt())).toEqual('(-someVar);'); + expect(emitStmt(o.unary(o.UnaryOperator.Plus, someVar).toStmt())).toEqual('(+someVar);'); expect(emitStmt(o.assertNotNull(someVar).toStmt())).toEqual('someVar!;'); expect( emitStmt(someVar.conditional(o.variable('trueCase'), o.variable('falseCase')).toStmt())) diff --git a/packages/compiler/test/template_parser/template_parser_spec.ts b/packages/compiler/test/template_parser/template_parser_spec.ts index cf8729125f..fe620e0e6f 100644 --- a/packages/compiler/test/template_parser/template_parser_spec.ts +++ b/packages/compiler/test/template_parser/template_parser_spec.ts @@ -1656,7 +1656,7 @@ Reference "#a" is defined several times ("
]#a>
expect(humanizeTplAst(parse('
', [ngIf]))).toEqual([ [EmbeddedTemplateAst], [DirectiveAst, ngIf], - [BoundDirectivePropertyAst, 'ngIf', '0 - 1'], + [BoundDirectivePropertyAst, 'ngIf', '-1'], [ElementAst, 'div'], ]); }); diff --git a/packages/compiler/test/template_parser/util/expression.ts b/packages/compiler/test/template_parser/util/expression.ts index e5ce110a5e..bad905d9ff 100644 --- a/packages/compiler/test/template_parser/util/expression.ts +++ b/packages/compiler/test/template_parser/util/expression.ts @@ -30,6 +30,10 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Templ this.recordAst(ast); this.visitAll([ast.ast], null); } + visitUnary(ast: e.Unary) { + this.recordAst(ast); + super.visitUnary(ast, null); + } visitBinary(ast: e.Binary) { this.recordAst(ast); super.visitBinary(ast, null); diff --git a/packages/language-service/src/expression_type.ts b/packages/language-service/src/expression_type.ts index 50b179d2af..099ff99fcc 100644 --- a/packages/language-service/src/expression_type.ts +++ b/packages/language-service/src/expression_type.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, ASTWithName, 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, AstVisitor, ASTWithName, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler'; import {createDiagnostic, Diagnostic} from './diagnostic_messages'; import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols'; @@ -38,6 +38,23 @@ export class AstType implements AstVisitor { return this.diagnostics; } + visitUnary(ast: Unary): Symbol { + // Visit the child to produce diagnostics. + ast.expr.visit(this); + + // The unary plus and minus operator are always of type number. + // https://github.com/Microsoft/TypeScript/blob/v1.8.10/doc/spec.md#4.18 + switch (ast.operator) { + case '-': + case '+': + return this.query.getBuiltinType(BuiltinType.Number); + } + + this.diagnostics.push( + createDiagnostic(refinedSpan(ast), Diagnostic.unrecognized_operator, ast.operator)); + return this.anyType; + } + visitBinary(ast: Binary): Symbol { const getType = (ast: AST, operation: string): Symbol => { const type = this.getType(ast); diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index f6cebc40d4..ac0eb18ab4 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -63,6 +63,7 @@ export function getExpressionCompletions( // (that is the scope of the implicit receiver) is the right scope as the user is typing the // beginning of an expression. tail.visit({ + visitUnary(_ast) {}, visitBinary(_ast) {}, visitChain(_ast) {}, visitConditional(_ast) {}, @@ -157,6 +158,7 @@ export function getExpressionSymbol( // (that is the scope of the implicit receiver) is the right scope as the user is typing the // beginning of an expression. tail.visit({ + visitUnary(_ast) {}, visitBinary(_ast) {}, visitChain(_ast) {}, visitConditional(_ast) {}, diff --git a/packages/language-service/test/expression_diagnostics_spec.ts b/packages/language-service/test/expression_diagnostics_spec.ts index 31eedf8608..cab7a9a690 100644 --- a/packages/language-service/test/expression_diagnostics_spec.ts +++ b/packages/language-service/test/expression_diagnostics_spec.ts @@ -102,6 +102,8 @@ describe('expression diagnostics', () => { it('should reject *ngIf of misspelled identifier in PrefixNot node', () => reject('
', 'Identifier \'persson\' is not defined')); + it('should reject misspelled field in unary operator expression', + () => reject('{{ +persson }}', `Identifier 'persson' is not defined`)); it('should accept an *ngFor', () => accept(`
{{p.name.first}} {{p.name.last}}