From a9be2ebf1bb9813aa754673dbe107eaba86c3547 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Tue, 26 May 2015 10:19:47 +0200 Subject: [PATCH] feat: add support for the safe navigation (aka Elvis) operator fixes #791 --- .../change_detection_jit_generator.ts | 12 +++++- .../change_detection/change_detection_util.ts | 2 + .../dynamic_change_detector.ts | 16 +++++++- .../src/change_detection/parser/ast.ts | 40 +++++++++++++++++++ .../src/change_detection/parser/lexer.ts | 7 ++-- .../src/change_detection/parser/parser.ts | 17 +++++--- .../change_detection/proto_change_detector.ts | 19 ++++++++- .../src/change_detection/proto_record.ts | 2 + .../change_detection/change_detector_spec.ts | 16 +++++++- .../change_detection/parser/lexer_spec.ts | 11 +++-- .../change_detection/parser/parser_spec.ts | 29 +++++++++++++- 11 files changed, 153 insertions(+), 18 deletions(-) diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/change_detection/change_detection_jit_generator.ts index a1a5ad2b3c..233ce2682f 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -17,7 +17,9 @@ import { RECORD_TYPE_KEYED_ACCESS, RECORD_TYPE_PIPE, RECORD_TYPE_BINDING_PIPE, - RECORD_TYPE_INTERPOLATE + RECORD_TYPE_INTERPOLATE, + RECORD_TYPE_SAFE_PROPERTY, + RECORD_TYPE_SAFE_INVOKE_METHOD } from './proto_record'; @@ -295,6 +297,10 @@ export class ChangeDetectorJITGenerator { rhs = `${context}.${r.name}`; break; + case RECORD_TYPE_SAFE_PROPERTY: + rhs = `${UTIL}.isValueBlank(${context}) ? null : ${context}.${r.name}`; + break; + case RECORD_TYPE_LOCAL: rhs = `${LOCALS_ACCESSOR}.get('${r.name}')`; break; @@ -303,6 +309,10 @@ export class ChangeDetectorJITGenerator { rhs = `${context}.${r.name}(${argString})`; break; + case RECORD_TYPE_SAFE_INVOKE_METHOD: + rhs = `${UTIL}.isValueBlank(${context}) ? null : ${context}.${r.name}(${argString})`; + break; + case RECORD_TYPE_INVOKE_CLOSURE: rhs = `${context}(${argString})`; break; diff --git a/modules/angular2/src/change_detection/change_detection_util.ts b/modules/angular2/src/change_detection/change_detection_util.ts index 7e1db5ff25..ea85820d98 100644 --- a/modules/angular2/src/change_detection/change_detection_util.ts +++ b/modules/angular2/src/change_detection/change_detection_util.ts @@ -142,4 +142,6 @@ export class ChangeDetectionUtil { changes[propertyName] = change; return changes; } + + static isValueBlank(value: any): boolean { return isBlank(value); } } diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts index 67ff46c897..12d6e342b8 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/change_detection/dynamic_change_detector.ts @@ -19,7 +19,9 @@ import { RECORD_TYPE_KEYED_ACCESS, RECORD_TYPE_PIPE, RECORD_TYPE_BINDING_PIPE, - RECORD_TYPE_INTERPOLATE + RECORD_TYPE_INTERPOLATE, + RECORD_TYPE_SAFE_PROPERTY, + RECORD_TYPE_SAFE_INVOKE_METHOD } from './proto_record'; import {ExpressionChangedAfterItHasBeenChecked, ChangeDetectionError} from './exceptions'; @@ -192,6 +194,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var context = this._readContext(proto); return proto.funcOrValue(context); + case RECORD_TYPE_SAFE_PROPERTY: + var context = this._readContext(proto); + return isBlank(context) ? null : proto.funcOrValue(context); + case RECORD_TYPE_LOCAL: return this.locals.get(proto.name); @@ -200,6 +206,14 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var args = this._readArgs(proto); return proto.funcOrValue(context, args); + case RECORD_TYPE_SAFE_INVOKE_METHOD: + var context = this._readContext(proto); + if (isBlank(context)) { + return null; + } + var args = this._readArgs(proto); + return proto.funcOrValue(context, args); + case RECORD_TYPE_KEYED_ACCESS: var arg = this._readArgs(proto)[0]; return this._readContext(proto)[arg]; diff --git a/modules/angular2/src/change_detection/parser/ast.ts b/modules/angular2/src/change_detection/parser/ast.ts index 891e46507b..b6374179e0 100644 --- a/modules/angular2/src/change_detection/parser/ast.ts +++ b/modules/angular2/src/change_detection/parser/ast.ts @@ -91,6 +91,20 @@ export class AccessMember extends AST { visit(visitor) { return visitor.visitAccessMember(this); } } +export class SafeAccessMember extends AST { + constructor(public receiver: AST, public name: string, public getter: Function, + public setter: Function) { + super(); + } + + eval(context, locals) { + var evaluatedReceiver = this.receiver.eval(context, locals); + return isBlank(evaluatedReceiver) ? null : this.getter(evaluatedReceiver); + } + + visit(visitor) { return visitor.visitSafeAccessMember(this); } +} + export class KeyedAccess extends AST { constructor(public obj: AST, public key: AST) { super(); } @@ -251,6 +265,22 @@ export class MethodCall extends AST { visit(visitor) { return visitor.visitMethodCall(this); } } +export class SafeMethodCall extends AST { + constructor(public receiver: AST, public name: string, public fn: Function, + public args: List) { + super(); + } + + eval(context, locals) { + var evaluatedReceiver = this.receiver.eval(context, locals); + if (isBlank(evaluatedReceiver)) return null; + var evaluatedArgs = evalList(context, locals, this.args); + return this.fn(evaluatedReceiver, evaluatedArgs); + } + + visit(visitor) { return visitor.visitSafeMethodCall(this); } +} + export class FunctionCall extends AST { constructor(public target: AST, public args: List) { super(); } @@ -300,6 +330,8 @@ export class AstVisitor { visitLiteralPrimitive(ast: LiteralPrimitive) {} visitMethodCall(ast: MethodCall) {} visitPrefixNot(ast: PrefixNot) {} + visitSafeAccessMember(ast: SafeAccessMember) {} + visitSafeMethodCall(ast: SafeMethodCall) {} } export class AstTransformer { @@ -315,10 +347,18 @@ export class AstTransformer { return new AccessMember(ast.receiver.visit(this), ast.name, ast.getter, ast.setter); } + visitSafeAccessMember(ast: SafeAccessMember) { + return new SafeAccessMember(ast.receiver.visit(this), ast.name, ast.getter, ast.setter); + } + visitMethodCall(ast: MethodCall) { return new MethodCall(ast.receiver.visit(this), ast.name, ast.fn, this.visitAll(ast.args)); } + visitSafeMethodCall(ast: SafeMethodCall) { + return new SafeMethodCall(ast.receiver.visit(this), ast.name, ast.fn, this.visitAll(ast.args)); + } + visitFunctionCall(ast: FunctionCall) { return new FunctionCall(ast.target.visit(this), this.visitAll(ast.args)); } diff --git a/modules/angular2/src/change_detection/parser/lexer.ts b/modules/angular2/src/change_detection/parser/lexer.ts index 0a8784572e..1596c11a78 100644 --- a/modules/angular2/src/change_detection/parser/lexer.ts +++ b/modules/angular2/src/change_detection/parser/lexer.ts @@ -219,15 +219,15 @@ class _Scanner { case $DQ: return this.scanString(); case $HASH: - return this.scanOperator(start, StringWrapper.fromCharCode(peek)); case $PLUS: case $MINUS: case $STAR: case $SLASH: case $PERCENT: case $CARET: - case $QUESTION: return this.scanOperator(start, StringWrapper.fromCharCode(peek)); + case $QUESTION: + return this.scanComplexOperator(start, $PERIOD, '?', '.'); case $LT: case $GT: case $BANG: @@ -434,7 +434,8 @@ var OPERATORS = SetWrapper.createFromList([ '|', '!', '?', - '#' + '#', + '?.' ]); diff --git a/modules/angular2/src/change_detection/parser/parser.ts b/modules/angular2/src/change_detection/parser/parser.ts index edded16539..3099f3ae46 100644 --- a/modules/angular2/src/change_detection/parser/parser.ts +++ b/modules/angular2/src/change_detection/parser/parser.ts @@ -28,6 +28,7 @@ import { EmptyExpr, ImplicitReceiver, AccessMember, + SafeAccessMember, LiteralPrimitive, Binary, PrefixNot, @@ -40,6 +41,7 @@ import { LiteralMap, Interpolation, MethodCall, + SafeMethodCall, FunctionCall, TemplateBinding, ASTWithSource @@ -360,7 +362,10 @@ class _ParseAST { var result = this.parsePrimary(); while (true) { if (this.optionalCharacter($PERIOD)) { - result = this.parseAccessMemberOrMethodCall(result); + result = this.parseAccessMemberOrMethodCall(result, false); + + } else if (this.optionalOperator('?.')) { + result = this.parseAccessMemberOrMethodCall(result, true); } else if (this.optionalCharacter($LBRACKET)) { var key = this.parseExpression(); @@ -405,7 +410,7 @@ class _ParseAST { return this.parseLiteralMap(); } else if (this.next.isIdentifier()) { - return this.parseAccessMemberOrMethodCall(_implicitReceiver); + return this.parseAccessMemberOrMethodCall(_implicitReceiver, false); } else if (this.next.isNumber()) { var value = this.next.toNumber(); @@ -451,19 +456,21 @@ class _ParseAST { return new LiteralMap(keys, values); } - parseAccessMemberOrMethodCall(receiver): AST { + parseAccessMemberOrMethodCall(receiver, isSafe: boolean = false): AST { var id = this.expectIdentifierOrKeyword(); if (this.optionalCharacter($LPAREN)) { var args = this.parseCallArguments(); this.expectCharacter($RPAREN); var fn = this.reflector.method(id); - return new MethodCall(receiver, id, fn, args); + return isSafe ? new SafeMethodCall(receiver, id, fn, args) : + new MethodCall(receiver, id, fn, args); } else { var getter = this.reflector.getter(id); var setter = this.reflector.setter(id); - var am = new AccessMember(receiver, id, getter, setter); + var am = isSafe ? new SafeAccessMember(receiver, id, getter, setter) : + new AccessMember(receiver, id, getter, setter); if (this.optionalOperator("|")) { return this.parseInlinedPipe(am); diff --git a/modules/angular2/src/change_detection/proto_change_detector.ts b/modules/angular2/src/change_detection/proto_change_detector.ts index 79e16afa34..9b0dd76e97 100644 --- a/modules/angular2/src/change_detection/proto_change_detector.ts +++ b/modules/angular2/src/change_detection/proto_change_detector.ts @@ -19,7 +19,9 @@ import { LiteralMap, LiteralPrimitive, MethodCall, - PrefixNot + PrefixNot, + SafeAccessMember, + SafeMethodCall } from './parser/ast'; import { @@ -49,7 +51,9 @@ import { RECORD_TYPE_KEYED_ACCESS, RECORD_TYPE_PIPE, RECORD_TYPE_BINDING_PIPE, - RECORD_TYPE_INTERPOLATE + RECORD_TYPE_INTERPOLATE, + RECORD_TYPE_SAFE_PROPERTY, + RECORD_TYPE_SAFE_INVOKE_METHOD } from './proto_record'; export class DynamicProtoChangeDetector extends ProtoChangeDetector { @@ -149,6 +153,11 @@ class _ConvertAstIntoProtoRecords { } } + visitSafeAccessMember(ast: SafeAccessMember) { + var receiver = ast.receiver.visit(this); + return this._addRecord(RECORD_TYPE_SAFE_PROPERTY, ast.name, ast.getter, [], null, receiver); + } + visitMethodCall(ast: MethodCall) { var receiver = ast.receiver.visit(this); var args = this._visitAll(ast.args); @@ -160,6 +169,12 @@ class _ConvertAstIntoProtoRecords { } } + visitSafeMethodCall(ast: SafeMethodCall) { + var receiver = ast.receiver.visit(this); + var args = this._visitAll(ast.args); + return this._addRecord(RECORD_TYPE_SAFE_INVOKE_METHOD, ast.name, ast.fn, args, null, receiver); + } + visitFunctionCall(ast: FunctionCall) { var target = ast.target.visit(this); var args = this._visitAll(ast.args); diff --git a/modules/angular2/src/change_detection/proto_record.ts b/modules/angular2/src/change_detection/proto_record.ts index 32d781bde8..68e2625ee4 100644 --- a/modules/angular2/src/change_detection/proto_record.ts +++ b/modules/angular2/src/change_detection/proto_record.ts @@ -13,6 +13,8 @@ export const RECORD_TYPE_KEYED_ACCESS = 7; export const RECORD_TYPE_PIPE = 8; export const RECORD_TYPE_BINDING_PIPE = 9; export const RECORD_TYPE_INTERPOLATE = 10; +export const RECORD_TYPE_SAFE_PROPERTY = 11; +export const RECORD_TYPE_SAFE_INVOKE_METHOD = 12; export class ProtoRecord { constructor(public mode: number, public name: string, public funcOrValue, public args: List, diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index 99d64c8c62..758be0e06f 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -151,7 +151,7 @@ export function main() { expect(executeWatch('const', '"a\n\nb"')).toEqual(['const=a\n\nb']); }); - it('simple chained property access', () => { + it('should support simple chained property access', () => { var address = new Address('Grenoble'); var person = new Person('Victor', address); @@ -159,6 +159,18 @@ export function main() { .toEqual(['address.city=Grenoble']); }); + it('should support the safe navigation operator', () => { + var person = new Person('Victor', null); + + expect(executeWatch('city', 'address?.city', person)).toEqual(['city=null']); + expect(executeWatch('city', 'address?.toString()', person)).toEqual(['city=null']); + + person.address = new Address('MTV'); + + expect(executeWatch('city', 'address?.city', person)).toEqual(['city=MTV']); + expect(executeWatch('city', 'address?.toString()', person)).toEqual(['city=MTV']); + }); + it("should support method calls", () => { var person = new Person('Victor'); expect(executeWatch('m', 'sayHi("Jim")', person)).toEqual(['m=Hi, Jim']); @@ -976,7 +988,7 @@ class Address { city: string; constructor(city: string) { this.city = city; } - toString(): string { return this.city; } + toString(): string { return isBlank(this.city) ? '-' : this.city } } class Uninitialized { diff --git a/modules/angular2/test/change_detection/parser/lexer_spec.ts b/modules/angular2/test/change_detection/parser/lexer_spec.ts index e73662f904..db3805681c 100644 --- a/modules/angular2/test/change_detection/parser/lexer_spec.ts +++ b/modules/angular2/test/change_detection/parser/lexer_spec.ts @@ -126,14 +126,14 @@ export function main() { expectIdentifierToken(tokens[1], 8, 'b'); }); - it('should tokenize quoted string', function() { + it('should tokenize quoted string', () => { var str = "['\\'', \"\\\"\"]"; var tokens: List = lex(str); expectStringToken(tokens[1], 1, "'"); expectStringToken(tokens[3], 7, '"'); }); - it('should tokenize escaped quoted string', function() { + it('should tokenize escaped quoted string', () => { var str = '"\\"\\n\\f\\r\\t\\v\\u00A0"'; var tokens: List = lex(str); expect(tokens.length).toEqual(1); @@ -203,7 +203,7 @@ export function main() { }); // NOTE(deboer): NOT A LEXER TEST - // it('should tokenize negative number', function() { + // it('should tokenize negative number', () => { // var tokens:List = lex("-0.5"); // expectNumberToken(tokens[0], 0, -0.5); // }); @@ -240,6 +240,11 @@ export function main() { expectOperatorToken(tokens[0], 0, '#'); }); + it('should tokenize ?. as operator', () => { + var tokens: List = lex('?.'); + expectOperatorToken(tokens[0], 0, '?.'); + }); + }); }); } diff --git a/modules/angular2/test/change_detection/parser/parser_spec.ts b/modules/angular2/test/change_detection/parser/parser_spec.ts index 88477f6238..082feb7b5e 100644 --- a/modules/angular2/test/change_detection/parser/parser_spec.ts +++ b/modules/angular2/test/change_detection/parser/parser_spec.ts @@ -1,4 +1,4 @@ -import {ddescribe, describe, it, xit, iit, expect, beforeEach} from 'angular2/test_lib'; +import {ddescribe, describe, it, xit, iit, expect, beforeEach, IS_DARTIUM} from 'angular2/test_lib'; import {BaseException, isBlank, isPresent} from 'angular2/src/facade/lang'; import {reflector} from 'angular2/src/reflection/reflection'; import {MapWrapper, ListWrapper} from 'angular2/src/facade/collection'; @@ -202,6 +202,33 @@ export function main() { }); }); + describe('safe navigation operator', () => { + it('should parse field access', () => { + expectEval('a?.a', td(td(999))).toEqual(999); + expectEval('a.a?.a', td(td(td(999)))).toEqual(999); + }); + + it('should return null when accessing a field on null', + () => { expect(() => { expectEval('null?.a', td()).toEqual(null); }).not.toThrow(); }); + + it('should have the same priority as .', () => { + expect(() => { expectEval('null?.a.a', td()).toEqual(null); }).toThrowError(); + }); + + if (!IS_DARTIUM) { + it('should return null when accessing a field on undefined', () => { + expect(() => { expectEval('_undefined?.a', td()).toEqual(null); }).not.toThrow(); + }); + } + + it('should evaluate method calls', + () => { expectEval('a?.add(1,2)', td(td())).toEqual(3); }); + + it('should return null when accessing a method on null', () => { + expect(() => { expectEval('null?.add(1, 2)', td()).toEqual(null); }).not.toThrow(); + }); + }); + describe("method calls", () => { it("should evaluate method calls", () => { expectEval("fn()", td(0, 0, "constant")).toEqual("constant");