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

* Changes globals to only mean properties starting with $ or a capital letter.
* Changes normalizeTuple(obj, 'Foo') to resolve Foo as a global. This
  makes the behavior consistent with normalizeTuple(obj, 'Foo.bar') which
  already resolves Foo as a global (and looks up bar).
* Changes normalizeTuple(null, 'Foo') to resolve Foo as a global.
* Specs normalizeTuple('null', 'foo') to resolve to [undefined, '']. This
  is unfortunate but needed to keep getPath happy.

This code is old, and tricky to modify due to perf optimizations and
edge case behavior. It would be great to refactor more heavily at some
other date. I've spec'd out more of the current behavior and edge cases
in this commit.

Fixes #3760

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() {