Merge pull request #3852 from mixonic/get

[BREAKING BUGFIX] Do not assume null Ember.get targets are all globals

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Ember Changelog
 
+### Canary
+
+- [#3852](https://github.com/emberjs/ember.js/pull/3852) [BREAKING BUGFIX] Do not assume null Ember.get targets always refer to a global
+
 ### 1.11.0-beta.1 (February 06, 2015)
 
 - [#10160](https://github.com/emberjs/ember.js/pull/10160) [FEATURE] Add index as an optional parameter to #each blocks [@tim-evans](https://github.com/tim-evans)

packages/ember-metal/lib/path_cache.js

@@ -1,7 +1,7 @@
 import Cache from 'ember-metal/cache';
 
-var IS_GLOBAL      = /^([A-Z$]|([0-9][A-Z$]))/;
-var IS_GLOBAL_PATH = /^([A-Z$]|([0-9][A-Z$])).*[\.]/;
+var IS_GLOBAL      = /^[A-Z$]/;
+var IS_GLOBAL_PATH = /^[A-Z$].*[\.]/;
 var HAS_THIS       = 'this.';
 
 var isGlobalCache       = new Cache(1000, function(key) { return IS_GLOBAL.test(key);          });

packages/ember-metal/lib/property_get.js

@@ -5,7 +5,7 @@
 import Ember from "ember-metal/core";
 import EmberError from "ember-metal/error";
 import {
-  isGlobalPath,
+  isGlobal as detectIsGlobal,
   isPath,
   hasThis as pathHasThis
 } from "ember-metal/path_cache";
@@ -52,19 +52,14 @@ export function get(obj, keyName) {
 
   if (!keyName && 'string' === typeof obj) {
     keyName = obj;
-    obj = null;
+    obj = Ember.lookup;
   }
 
   Ember.assert("Cannot call get with "+ keyName +" key.", !!keyName);
   Ember.assert("Cannot call get with '"+ keyName +"' on an undefined object.", obj !== undefined);
 
-  if (obj === null) {
-    var value = _getPath(obj, keyName);
-    Ember.deprecate(
-      "Ember.get fetched '"+keyName+"' from the global context. This behavior will change in the future (issue #3852)",
-      !value || (obj && obj !== Ember.lookup) || isPath(keyName) || isGlobalPath(keyName+".") // Add a . to ensure simple paths are matched.
-    );
-    return value;
+  if (!obj) {
+    return _getPath(obj, keyName);
   }
 
   var meta = obj['__ember_meta__'];
@@ -113,46 +108,42 @@ export function get(obj, keyName) {
 */
 export function normalizeTuple(target, path) {
   var hasThis  = pathHasThis(path);
-  var isGlobal = !hasThis && isGlobalPath(path);
+  var isGlobal = !hasThis && detectIsGlobal(path);
   var key;
 
-  if (!target || isGlobal) {
-    target = Ember.lookup;
+  if (!target && !isGlobal) {
+    return [undefined, ''];
   }
 
   if (hasThis) {
     path = path.slice(5);
   }
 
-  Ember.deprecate(
-    "normalizeTuple will return '"+path+"' as a non-global. This behavior will change in the future (issue #3852)",
-    target === Ember.lookup || !target || hasThis || isGlobal || !isGlobalPath(path+'.')
-  );
+  if (!target || isGlobal) {
+    target = Ember.lookup;
+  }
 
-  if (target === Ember.lookup) {
+  if (isGlobal && isPath(path)) {
     key = path.match(FIRST_KEY)[0];
     target = get(target, key);
     path   = path.slice(key.length+1);
   }
 
   // must return some kind of path to be valid else other things will break.
-  if (!path || path.length===0) {
-    throw new EmberError('Path cannot be empty');
-  }
+  validateIsPath(path);
 
   return [target, path];
 }
 
+function validateIsPath(path) {
+  if (!path || path.length===0) {
+    throw new EmberError('Object in path '+path+' could not be found or was destroyed.');
+  }
+}
+
 export function _getPath(root, path) {
   var hasThis, parts, tuple, idx, len;
 
-  // If there is no root and path is a key name, return that
-  // property from the global object.
-  // E.g. get('Ember') -> Ember
-  if (root === null && !isPath(path)) {
-    return get(Ember.lookup, path);
-  }
-
   // detect complicated paths and normalize them
   hasThis = pathHasThis(path);
 

packages/ember-metal/lib/property_set.js

@@ -7,12 +7,11 @@ import {
 import { defineProperty } from "ember-metal/properties";
 import EmberError from "ember-metal/error";
 import {
-  isPath
+  isPath,
+  isGlobalPath
 } from "ember-metal/path_cache";
 import { hasPropertyAccessors } from "ember-metal/platform/define_property";
 
-var IS_GLOBAL = /^([A-Z$]|([0-9][A-Z$]))/;
-
 /**
   Sets the value of a property on an object, respecting computed properties
   and notifying observers and other listeners of the change. If the
@@ -28,25 +27,27 @@ var IS_GLOBAL = /^([A-Z$]|([0-9][A-Z$]))/;
 */
 export function set(obj, keyName, value, tolerant) {
   if (typeof obj === 'string') {
-    Ember.assert("Path '" + obj + "' must be global if no obj is given.", IS_GLOBAL.test(obj));
+    Ember.assert("Path '" + obj + "' must be global if no obj is given.", isGlobalPath(obj));
     value = keyName;
     keyName = obj;
-    obj = null;
+    obj = Ember.lookup;
   }
 
   Ember.assert("Cannot call set with "+ keyName +" key.", !!keyName);
 
-  if (!obj) {
+  if (obj === Ember.lookup) {
     return setPath(obj, keyName, value, tolerant);
   }
 
-  var meta = obj['__ember_meta__'];
-  var possibleDesc = obj[keyName];
-  var desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;
+  var meta, possibleDesc, desc;
+  if (obj) {
+    meta = obj['__ember_meta__'];
+    possibleDesc = obj[keyName];
+    desc = (possibleDesc !== null && typeof possibleDesc === 'object' && possibleDesc.isDescriptor) ? possibleDesc : undefined;
+  }
 
   var isUnknown, currentValue;
-
-  if (desc === undefined && isPath(keyName)) {
+  if ((!obj || desc === undefined) && isPath(keyName)) {
     return setPath(obj, keyName, value, tolerant);
   }
 
@@ -57,7 +58,7 @@ export function set(obj, keyName, value, tolerant) {
     desc.set(obj, keyName, value);
   } else {
 
-    if (typeof obj === 'object' && obj !== null && value !== undefined && obj[keyName] === value) {
+    if (obj !== null && value !== undefined && typeof obj === 'object' && obj[keyName] === value) {
       return value;
     }
 

packages/ember-metal/tests/accessors/get_path_test.js

@@ -2,13 +2,6 @@
 
 import { get } from 'ember-metal/property_get';
 
-function expectGlobalContextDeprecation(assertion) {
-  expectDeprecation(
-    assertion,
-    "Ember.get fetched 'localPathGlobal' from the global context. This behavior will change in the future (issue #3852)"
-  );
-}
-
 var obj;
 var moduleOpts = {
   setup: function() {
@@ -23,7 +16,10 @@ var moduleOpts = {
           baz: { biff: 'BIFF' }
         }
       },
-      falseValue: false
+      falseValue: false,
+      Wuz: {
+        nar: 'foo'
+      }
     };
 
     window.Foo = {
@@ -32,20 +28,20 @@ var moduleOpts = {
       }
     };
 
+    window.aProp = 'aPropy';
+
     window.$foo = {
       bar: {
         baz: { biff: '$FOOBIFF' }
       }
     };
-
-    window.localPathGlobal = 5;
   },
 
   teardown: function() {
     obj = undefined;
     window.Foo = undefined;
+    window.aProp = undefined;
     window.$foo = undefined;
-    window.localPathGlobal = undefined;
   }
 };
 
@@ -75,39 +71,66 @@ QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
   deepEqual(get(obj, 'this.foo.bar'), obj.foo.bar);
 });
 
-QUnit.test('[obj, this.Foo.bar] -> (null)', function() {
-  deepEqual(get(obj, 'this.Foo.bar'), undefined);
+QUnit.test('[obj, this.Foo.bar] -> (undefined)', function() {
+  equal(get(obj, 'this.Foo.bar'), undefined);
 });
 
-QUnit.test('[obj, falseValue.notDefined] -> (null)', function() {
-  deepEqual(get(obj, 'falseValue.notDefined'), undefined);
+QUnit.test('[obj, falseValue.notDefined] -> (undefined)', function() {
+  equal(get(obj, 'falseValue.notDefined'), undefined);
 });
 
 // ..........................................................
-// LOCAL PATHS WITH NO TARGET DEPRECATION
+// GLOBAL PATHS TREATED LOCAL WITH GET
 //
 
-QUnit.test('[null, length] returning data is deprecated', function() {
-  expectGlobalContextDeprecation(function() {
-    equal(5, get(null, 'localPathGlobal'));
-  });
+QUnit.test('[obj, Wuz] -> obj.Wuz', function() {
+  deepEqual(get(obj, 'Wuz'), obj.Wuz);
 });
 
-QUnit.test('[length] returning data is deprecated', function() {
-  expectGlobalContextDeprecation(function() {
-    equal(5, get('localPathGlobal'));
-  });
+QUnit.test('[obj, Wuz.nar] -> obj.Wuz.nar', function() {
+  deepEqual(get(obj, 'Wuz.nar'), obj.Wuz.nar);
+});
+
+QUnit.test('[obj, Foo] -> (undefined)', function() {
+  equal(get(obj, 'Foo'), undefined);
+});
+
+QUnit.test('[obj, Foo.bar] -> (undefined)', function() {
+  equal(get(obj, 'Foo.bar'), undefined);
 });
 
 // ..........................................................
-// NO TARGET
+// NULL TARGET
 //
 
 QUnit.test('[null, Foo] -> Foo', function() {
-  deepEqual(get('Foo'), Foo);
+  equal(get(null, 'Foo'), Foo);
 });
 
 QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
-  deepEqual(get('Foo.bar'), Foo.bar);
+  deepEqual(get(null, 'Foo.bar'), Foo.bar);
+});
+
+QUnit.test('[null, $foo] -> $foo', function() {
+  equal(get(null, '$foo'), window.$foo);
 });
 
+QUnit.test('[null, aProp] -> null', function() {
+  equal(get(null, 'aProp'), null);
+});
+
+// ..........................................................
+// NO TARGET
+//
+
+QUnit.test('[Foo] -> Foo', function() {
+  deepEqual(get('Foo'), Foo);
+});
+
+QUnit.test('[aProp] -> aProp', function() {
+  deepEqual(get('aProp'), window.aProp);
+});
+
+QUnit.test('[Foo.bar] -> Foo.bar', function() {
+  deepEqual(get('Foo.bar'), Foo.bar);
+});

packages/ember-metal/tests/accessors/get_test.js

@@ -60,6 +60,14 @@ QUnit.test('warn on attempts to get a property path of undefined', function() {
   }, /Cannot call get with 'aProperty.on.aPath' on an undefined object/);
 });
 
+QUnit.test('returns null when fetching a complex local path on a null context', function() {
+  equal(get(null, 'aProperty.on.aPath'), null);
+});
+
+QUnit.test('returns null when fetching a simple local path on a null context', function() {
+  equal(get(null, 'aProperty'), null);
+});
+
 QUnit.test('warn on attempts to get a falsy property', function() {
   var obj = {};
   expectAssertion(function() {

packages/ember-metal/tests/accessors/normalize_tuple_test.js

@@ -74,10 +74,8 @@ QUnit.test('[obj, this.Foo.bar] -> [obj, Foo.bar]', function() {
 // GLOBAL PATHS
 //
 
-QUnit.test('[obj, Foo] -> [obj, Foo]', function() {
-  expectDeprecation(function() {
-    deepEqual(normalizeTuple(obj, 'Foo'), [obj, 'Foo']);
-  }, "normalizeTuple will return 'Foo' as a non-global. This behavior will change in the future (issue #3852)");
+QUnit.test('[obj, Foo] -> [Ember.lookup, Foo]', function() {
+  deepEqual(normalizeTuple(obj, 'Foo'), [Ember.lookup, 'Foo']);
 });
 
 QUnit.test('[obj, Foo.bar] -> [Foo, bar]', function() {
@@ -92,12 +90,26 @@ QUnit.test('[obj, $foo.bar.baz] -> [$foo, bar.baz]', function() {
 // NO TARGET
 //
 
-QUnit.test('[null, Foo] -> EXCEPTION', function() {
-  throws(function() {
-    normalizeTuple(null, 'Foo');
-  }, Error);
+QUnit.test('[null, Foo] -> [Ember.lookup, Foo]', function() {
+  deepEqual(normalizeTuple(null, 'Foo'), [Ember.lookup, 'Foo']);
 });
 
 QUnit.test('[null, Foo.bar] -> [Foo, bar]', function() {
   deepEqual(normalizeTuple(null, 'Foo.bar'), [Foo, 'bar']);
 });
+
+QUnit.test("[null, foo] -> [undefined, '']", function() {
+  deepEqual(normalizeTuple(null, 'foo'), [undefined, '']);
+});
+
+QUnit.test("[null, foo.bar] -> [undefined, '']", function() {
+  deepEqual(normalizeTuple(null, 'foo'), [undefined, '']);
+});
+
+QUnit.test('[null, $foo] -> [Ember.lookup, $foo]', function() {
+  deepEqual(normalizeTuple(null, '$foo'), [Ember.lookup, '$foo']);
+});
+
+QUnit.test('[null, $foo.bar] -> [$foo, bar]', function() {
+  deepEqual(normalizeTuple(null, '$foo.bar'), [$foo, 'bar']);
+});

packages/ember-metal/tests/accessors/set_path_test.js

@@ -91,12 +91,9 @@ QUnit.module("set with path - deprecated", {
 });
 
 QUnit.test('[null, bla] gives a proper exception message', function() {
-  var exceptionMessage = 'Property set failed: object in path \"bla\" could not be found or was destroyed.';
-  try {
+  expectAssertion(function() {
     set(null, 'bla', "BAM");
-  } catch(ex) {
-    equal(ex.message, exceptionMessage);
-  }
+  }, /You need to provide an object and key to `set`/);
 });
 
 QUnit.test('[obj, bla.bla] gives a proper exception message', function() {