diff --git a/src/ng/parse.js b/src/ng/parse.js index bd3aa048df44..5184631d5329 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -1,6 +1,7 @@ 'use strict'; var $parseMinErr = minErr('$parse'); +var log; var promiseWarningCache = {}; var promiseWarning; @@ -313,7 +314,7 @@ Lexer.prototype = { } else { var getter = getterFn(ident, this.options, this.text); token.fn = extend(function(self, locals) { - return (getter(self, locals)); + return (getter(self, locals, log)); }, { assign: function(self, value) { return setter(self, ident, value, parser.text, parser.options); @@ -707,7 +708,7 @@ Parser.prototype = { var getter = getterFn(field, this.options, this.text); return extend(function(scope, locals, self) { - return getter(self || object(scope, locals)); + return getter(self || object(scope, locals, log)); }, { assign: function(scope, value, locals) { return setter(object(scope, locals), field, value, parser.text, parser.options); @@ -875,6 +876,17 @@ function setter(obj, path, setValue, fullExp, options) { var getterFnCache = {}; +/** + * Logs a warning when a key cannot be resolved in an expression because the parent object is undefined. + * @param key + * @param fullExp + * @returns {undefined} + */ +function logAndReturnUndefined(key, fullExp){ + log.warn('[$parse] Key `' + key + '` cannot be resolved in expression `' + fullExp + '` because parent object is null.'); + return undefined; +} + /** * Implementation of the "Black Hole" variant from: * - http://jsperf.com/angularjs-parse-getter/4 @@ -893,21 +905,32 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { if (pathVal == null) return pathVal; pathVal = pathVal[key0]; + if (pathVal == null) { + return logAndReturnUndefined(key0, fullExp); + } if (!key1) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key1, fullExp); + } pathVal = pathVal[key1]; if (!key2) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key2, fullExp); + } pathVal = pathVal[key2]; if (!key3) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key3, fullExp); + } pathVal = pathVal[key3]; if (!key4) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key4, fullExp); + } pathVal = pathVal[key4]; return pathVal; @@ -916,7 +939,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { var pathVal = (locals && locals.hasOwnProperty(key0)) ? locals : scope, promise; - if (pathVal == null) return pathVal; + if (pathVal == null) { + return logAndReturnUndefined(key0, fullExp); + } pathVal = pathVal[key0]; if (pathVal && pathVal.then) { @@ -930,7 +955,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } if (!key1) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key1, fullExp); + } pathVal = pathVal[key1]; if (pathVal && pathVal.then) { promiseWarning(fullExp); @@ -943,7 +970,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } if (!key2) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key2, fullExp); + } pathVal = pathVal[key2]; if (pathVal && pathVal.then) { promiseWarning(fullExp); @@ -956,7 +985,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } if (!key3) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key3, fullExp); + } pathVal = pathVal[key3]; if (pathVal && pathVal.then) { promiseWarning(fullExp); @@ -969,7 +1000,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) { } if (!key4) return pathVal; - if (pathVal == null) return undefined; + if (pathVal == null) { + return logAndReturnUndefined(key4, fullExp); + } pathVal = pathVal[key4]; if (pathVal && pathVal.then) { promiseWarning(fullExp); @@ -988,8 +1021,10 @@ function simpleGetterFn1(key0, fullExp) { ensureSafeMemberName(key0, fullExp); return function simpleGetterFn1(scope, locals) { - if (scope == null) return undefined; - return ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; + if (scope == null) { + return logAndReturnUndefined(key0, fullExp); + } + return((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; }; } @@ -998,9 +1033,14 @@ function simpleGetterFn2(key0, key1, fullExp) { ensureSafeMemberName(key1, fullExp); return function simpleGetterFn2(scope, locals) { - if (scope == null) return undefined; - scope = ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; - return scope == null ? undefined : scope[key1]; + if (scope == null) { + return logAndReturnUndefined(key0, fullExp); + } + var pathVal = ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0]; + if (pathVal == null) { + return logAndReturnUndefined(key1, fullExp); + } + return pathVal[key1]; }; } @@ -1043,7 +1083,10 @@ function getterFn(path, options, fullExp) { var code = 'var p;\n'; forEach(pathKeys, function(key, index) { ensureSafeMemberName(key, fullExp); - code += 'if(s == null) return undefined;\n' + + code += 'if(s == null) {\n' + + ' l.warn("[$parse] Key `' + key + '` cannot be resolved in expression `' + fullExp.replace(/(["\r\n])/g, '\\$1') + '` because parent object is null.");\n' + + ' return undefined;\n' + + '}\n' + 's='+ (index // we simply dereference 's' on any .dot notation ? 's' @@ -1064,11 +1107,11 @@ function getterFn(path, options, fullExp) { code += 'return s;'; /* jshint -W054 */ - var evaledFnGetter = new Function('s', 'k', 'pw', code); // s=scope, k=locals, pw=promiseWarning + var evaledFnGetter = new Function('s', 'k', 'l', 'pw', code); // s=scope, k=locals, l=log, pw=promiseWarning /* jshint +W054 */ evaledFnGetter.toString = valueFn(code); fn = options.unwrapPromises ? function(scope, locals) { - return evaledFnGetter(scope, locals, promiseWarning); + return evaledFnGetter(scope, locals, log, promiseWarning); } : evaledFnGetter; } @@ -1226,6 +1269,8 @@ function $ParseProvider() { this.$get = ['$filter', '$sniffer', '$log', function($filter, $sniffer, $log) { $parseOptions.csp = $sniffer.csp; + log = $log; + promiseWarning = function promiseWarningFn(fullExp) { if (!$parseOptions.logPromiseWarnings || promiseWarningCache.hasOwnProperty(fullExp)) return; promiseWarningCache[fullExp] = true; diff --git a/test/BinderSpec.js b/test/BinderSpec.js index b553c68dcfd0..6cdc9e4768ca 100644 --- a/test/BinderSpec.js +++ b/test/BinderSpec.js @@ -195,8 +195,10 @@ describe('Binder', function() { module(function($exceptionHandlerProvider){ $exceptionHandlerProvider.mode('log'); }); - inject(function($rootScope, $exceptionHandler, $compile) { + inject(function($rootScope, $exceptionHandler, $compile, $log) { $compile('
', null, true)($rootScope); + expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./); + expect($log.warn.logs).toEqual([]); var errorLogs = $exceptionHandler.errors; var count = 0; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 98b1650f7706..21746b44cdfa 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -3039,10 +3039,13 @@ describe('$compile', function() { }; }); }); - inject(function($templateCache, $compile, $rootScope) { + inject(function($templateCache, $compile, $rootScope, $log) { $templateCache.put('main.html', 'template:{{mainCtrl.name}}
'); element = $compile('
transclude:{{mainCtrl.name}}
')($rootScope); $rootScope.$apply(); + expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./); + expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./); + expect($log.warn.logs).toEqual([]); expect(element.text()).toBe('template:lucas transclude:'); }); }); diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js index 606123296ddc..3dcea685df3a 100644 --- a/test/ng/interpolateSpec.js +++ b/test/ng/interpolateSpec.js @@ -2,6 +2,8 @@ describe('$interpolate', function() { + var UNDEFINED_WARNING_REGEXP = /\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./; + it('should return a function when there are no bindings and textOnly is undefined', inject(function($interpolate) { expect(typeof $interpolate('some text')).toBe('function'); @@ -13,11 +15,13 @@ describe('$interpolate', function() { expect($interpolate('some text', true)).toBeUndefined(); })); - it('should suppress falsy objects', inject(function($interpolate) { + it('should suppress falsy objects', inject(function($interpolate, $log) { expect($interpolate('{{undefined}}')()).toEqual(''); expect($interpolate('{{undefined+undefined}}')()).toEqual(''); expect($interpolate('{{null}}')()).toEqual(''); expect($interpolate('{{a.b}}')()).toEqual(''); + expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./); + expect($log.warn.logs).toEqual([]); })); it('should jsonify objects', inject(function($interpolate) { @@ -56,8 +60,10 @@ describe('$interpolate', function() { })); - it('should ignore undefined model', inject(function($interpolate) { + it('should ignore undefined model', inject(function($interpolate, $log) { expect($interpolate("Hello {{'World' + foo}}")()).toEqual('Hello World'); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); })); @@ -215,8 +221,10 @@ describe('$interpolate', function() { "when a trusted value is required. See http://docs.angularjs.org/api/ng.$sce"); })); - it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) { + it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate, $log) { expect($interpolate('some/{{id}}')()).toEqual('some/'); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); expect($interpolate('some/{{id}}')({id: 1})).toEqual('some/1'); expect($interpolate('{{foo}}{{bar}}')({foo: 1, bar: 2})).toEqual('12'); })); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 466be755d523..da0a00390abe 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -2,6 +2,8 @@ describe('parser', function() { + var UNDEFINED_WARNING_REGEXP = /\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./; + beforeEach(function() { // clear caches getterFnCache = {}; @@ -204,6 +206,7 @@ describe('parser', function() { describe('csp: ' + cspEnabled + ", unwrapPromises: " + unwrapPromisesEnabled, function() { + var $log; var originalSecurityPolicy; @@ -220,8 +223,9 @@ describe('parser', function() { $parseProvider.unwrapPromises(unwrapPromisesEnabled); })); - beforeEach(inject(function ($rootScope) { + beforeEach(inject(function ($rootScope, _$log_) { scope = $rootScope; + $log = _$log_; })); it('should parse expressions', function() { @@ -348,6 +352,8 @@ describe('parser', function() { expect(scope.$eval("a", scope)).toEqual(123); expect(scope.$eval("b.c", scope)).toEqual(456); expect(scope.$eval("x.y.z", scope)).not.toBeDefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); }); it('should resolve deeply nested paths (important for CSP mode)', function() { @@ -379,8 +385,12 @@ describe('parser', function() { it('should be forgiving', function() { scope.a = {b: 23}; expect(scope.$eval('b')).toBeUndefined(); + expect($log.warn.logs).toEqual([]); expect(scope.$eval('a.x')).toBeUndefined(); + expect($log.warn.logs).toEqual([]); expect(scope.$eval('a.b.c.d')).toBeUndefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); }); it('should support property names that collide with native object properties', function() { @@ -960,6 +970,8 @@ describe('parser', function() { expect($parse('a.b')({a: {b: 0}}, {a: {b:1}})).toEqual(1); expect($parse('a.b')({a: null}, {a: {b:1}})).toEqual(1); expect($parse('a.b')({a: {b: 0}}, {a: null})).toEqual(undefined); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); expect($parse('a.b.c')({a: null}, {a: {b: {c: 1}}})).toEqual(1); })); @@ -1053,11 +1065,15 @@ describe('parser', function() { // simpleGetterFn2 it('should return undefined for properties of `null` constant', inject(function($rootScope) { expect($rootScope.$eval('null.a')).toBeUndefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); })); it('should return undefined for properties of `null` values', inject(function($rootScope) { $rootScope.a = null; expect($rootScope.$eval('a.b')).toBeUndefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); })); it('should return null for `a.b` where `b` is null', inject(function($rootScope) { @@ -1074,6 +1090,8 @@ describe('parser', function() { it('should return undefined for `a.b.c.d.e` where `d` is null', inject(function($rootScope) { $rootScope.a = { b: { c: { d: null } } }; expect($rootScope.$eval('a.b.c.d.e')).toBeUndefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); })); // cspSafeGetter || pathKeys.length > 6 @@ -1085,6 +1103,8 @@ describe('parser', function() { it('should return undefined for `a.b.c.d.e.f.g` where `f` is null', inject(function($rootScope) { $rootScope.a = { b: { c: { d: { e: { f: null } } } } }; expect($rootScope.$eval('a.b.c.d.e.f.g')).toBeUndefined(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); })); }); }); @@ -1169,7 +1189,9 @@ describe('parser', function() { it('should log warnings for deep promises', function() { scope.car = {wheel: {disc: promise}}; scope.$eval('car.wheel.disc.pad'); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); expect($log.warn.logs.pop()).toMatch(PROMISE_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); }); @@ -1177,6 +1199,7 @@ describe('parser', function() { scope.person = promise; scope.$eval('person.name = "Bubu"'); expect($log.warn.logs.pop()).toMatch(PROMISE_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); }); @@ -1292,12 +1315,14 @@ describe('parser', function() { describe('assignment into promises', function() { // This behavior is analogous to assignments to non-promise values // that are lazily set on the scope. - it('should evaluate a resolved object promise and set its value', inject(function($parse) { + it('should evaluate a resolved object promise and set its value', inject(function($parse, $log) { scope.person = promise; deferred.resolve({'name': 'Bill Gates'}); var getter = $parse('person.name', { unwrapPromises: true }); expect(getter(scope)).toBe(undefined); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); scope.$digest(); expect(getter(scope)).toBe('Bill Gates'); @@ -1321,14 +1346,18 @@ describe('parser', function() { })); - it('should evaluate an unresolved promise and set and remember its value', inject(function($parse) { + it('should evaluate an unresolved promise and set and remember its value', inject(function($parse, $log) { scope.person = promise; var getter = $parse('person.name', { unwrapPromises: true }); expect(getter(scope)).toBe(undefined); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); scope.$digest(); expect(getter(scope)).toBe(undefined); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); getter.assign(scope, 'Bonjour'); scope.$digest(); @@ -1345,6 +1374,8 @@ describe('parser', function() { // Set another property on the person.A.B var c2Getter = $parse('person.A.B.C2', { unwrapPromises: true }); scope.$digest(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); expect(c2Getter(scope)).toBe(undefined); c2Getter.assign(scope, 'c2_value'); scope.$digest(); @@ -1356,7 +1387,7 @@ describe('parser', function() { })); - it('should evaluate a resolved promise and overwrite the previous set value in the absense of the getter', + it('should evaluate a resolved promise and overwrite the previous set value in the absence of the getter', inject(function($parse) { scope.person = promise; var c1Getter = $parse('person.A.B.C1', { unwrapPromises: true }); @@ -1370,20 +1401,25 @@ describe('parser', function() { }); describe('dereferencing', function() { - it('should evaluate and dereference properties leading to and from a promise', function() { + it('should evaluate and dereference properties leading to and from a promise', inject(function($log) { scope.obj = {greeting: promise}; expect(scope.$eval('obj.greeting')).toBe(undefined); expect(scope.$eval('obj.greeting.polite')).toBe(undefined); scope.$digest(); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); + expect(scope.$eval('obj.greeting')).toBe(undefined); expect(scope.$eval('obj.greeting.polite')).toBe(undefined); + expect($log.warn.logs.pop()).toMatch(UNDEFINED_WARNING_REGEXP); + expect($log.warn.logs).toEqual([]); deferred.resolve({polite: 'Good morning!'}); scope.$digest(); expect(scope.$eval('obj.greeting')).toEqual({polite: 'Good morning!'}); expect(scope.$eval('obj.greeting.polite')).toBe('Good morning!'); - }); + })); it('should evaluate and dereference properties leading to and from a promise via bracket ' + 'notation', function() { diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index f9cf9412c605..af0c50f0c191 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -90,10 +90,12 @@ describe('Scope', function() { })); - it('should watch and fire on expression change', inject(function($rootScope) { + it('should watch and fire on expression change', inject(function($rootScope, $log) { var spy = jasmine.createSpy(); $rootScope.$watch('name.first', spy); $rootScope.$digest(); + expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./); + expect($log.warn.logs).toEqual([]); spy.reset(); $rootScope.name = {}; diff --git a/test/ngScenario/dslSpec.js b/test/ngScenario/dslSpec.js index 2553fadd65b1..b4cf8828ebb4 100644 --- a/test/ngScenario/dslSpec.js +++ b/test/ngScenario/dslSpec.js @@ -617,11 +617,13 @@ describe("angular.scenario.dsl", function() { expect($root.futureResult.toLowerCase()).toEqual('some value'); }); - it('should select binding in template by name', function() { + it('should select binding in template by name', inject(function($log) { compile('
', 'bar');
+        expect($log.warn.logs.pop()).toMatch(/\[\$parse\] Key `.+` cannot be resolved in expression `.+` because parent object is null\./);
+        expect($log.warn.logs).toEqual([]);
         $root.dsl.binding('foo.bar');
         expect($root.futureResult).toEqual('bar');
-      });
+      }));
 
       it('should match bindings by substring match', function() {
         compile('
', 'binding value');