feat($injector): add support for non-string IDs

Previously, only strings could be used as identifiers for Angular services. This commit adds support
for using any value as identifier for an Angular service (e.g. used with `.provider()`,
`.factory()`, `.service()`, `.value()`, `.constant()`, `.decorator()`).

Identifiers for directives and filters are still restricted to string values, since they need to be
references in HTML (text).

--
As usual, if a service has a string ID, its provider's ID is constructed by appending `Provider` to
the service ID. For services with non-string IDs, their providers have the exact same ID (it is the
context that determines if a service or a provider should be injected).

E.g.:
```js
var bar = {};

angular.
  module('myModule', []).

  provider('foo' /* string ID     */, {$get: function () { return 'FOO'; }}).
  provider( bar  /* non-string ID */, {$get: function () { return 'BAR'; }}).

  config(['fooProvider', function (fooProvider) {
    // `foo` provider injected (because we are in config block)
  }]).
  run(['foo', function (foo) {
    // `foo` service injected (because we are in run block)
  }]).

  config([bar /* same ID */, function (barProvider) {
    // `bar` provider injected (because we are in config block)
  }]).
  run([bar /* same ID */, function (bar) {
    // `bar` service injected (because we are in run block)
  }]);
```

--
This change is backwards compatible (afaict).

Fixes angular#10347

Author: gkalpak

src/apis.js

@@ -36,14 +36,19 @@ function hashKey(obj, nextUidFn) {
 /**
  * HashMap which can use objects as keys
  */
-function HashMap(array, isolatedUid) {
+function HashMap(seedData, isolatedUid) {
   if (isolatedUid) {
     var uid = 0;
     this.nextUid = function() {
       return ++uid;
     };
   }
-  forEach(array, this.put, this);
+
+  if (seedData) {
+    var putFn = isArray(seedData) ?
+        this.put : reverseParams(this.put.bind(this));
+    forEach(seedData, putFn, this);
+  }
 }
 HashMap.prototype = {
   /**
@@ -63,6 +68,14 @@ HashMap.prototype = {
     return this[hashKey(key, this.nextUid)];
   },
 
+  /**
+   * @param key
+   * @returns {boolean} whether a value is stored under the specified key
+   */
+  has: function(key) {
+    return this.hasOwnProperty(hashKey(key, this.nextUid));
+  },
+
   /**
    * Remove the key/value pair
    * @param key

src/auto/injector.js

@@ -74,7 +74,7 @@ function stringifyFn(fn) {
   // Support: Chrome 50-51 only
   // Creating a new string by adding `' '` at the end, to hack around some bug in Chrome v50/51
   // (See https://github.com/angular/angular.js/issues/14487.)
-  // TODO (gkalpak): Remove workaround when Chrome v52 is released
+  // TODO(gkalpak): Remove workaround when Chrome v52 is released
   return Function.prototype.toString.call(fn) + ' ';
 }
 
@@ -129,6 +129,10 @@ function annotate(fn, strictDi, name) {
   return $inject;
 }
 
+function stringifyServiceId(id, suffix) {
+  return (isUndefined(id) || isString(id)) ? id : id + suffix;
+}
+
 ///////////////////////////////////////
 
 /**
@@ -650,34 +654,34 @@ function createInjector(modulesToLoad, strictDi) {
   var INSTANTIATING = {},
       providerSuffix = 'Provider',
       path = [],
-      loadedModules = new HashMap([], true),
-      providerCache = {
+      loadedModules = new HashMap(null, true),
+      providerCache = new HashMap({
         $provide: {
-            provider: supportObject(provider),
-            factory: supportObject(factory),
-            service: supportObject(service),
-            value: supportObject(value),
-            constant: supportObject(constant),
-            decorator: decorator
-          }
-      },
-      providerInjector = (providerCache.$injector =
-          createInternalInjector(providerCache, function(serviceName, caller) {
-            if (angular.isString(caller)) {
-              path.push(caller);
-            }
+          provider: supportObject(provider),
+          factory: supportObject(factory),
+          service: supportObject(service),
+          value: supportObject(value),
+          constant: supportObject(constant),
+          decorator: decorator
+        }
+      }, true),
+      instanceCache = new HashMap(null, true),
+      providerInjector =
+          createInternalInjector(providerCache, function(serviceName) {
             throw $injectorMinErr('unpr', "Unknown provider: {0}", path.join(' <- '));
-          })),
-      instanceCache = {},
+          }, ' (provider)'),
       protoInstanceInjector =
-          createInternalInjector(instanceCache, function(serviceName, caller) {
-            var provider = providerInjector.get(serviceName + providerSuffix, caller);
-            return instanceInjector.invoke(
-                provider.$get, provider, undefined, serviceName);
+          createInternalInjector(instanceCache, function(serviceName) {
+            var providerId = !isString(serviceName) ? serviceName : serviceName + providerSuffix;
+            var provider = providerInjector.get(providerId);
+
+            return instanceInjector.invoke(provider.$get, provider, undefined, serviceName);
           }),
       instanceInjector = protoInstanceInjector;
 
-  providerCache['$injector' + providerSuffix] = { $get: valueFn(protoInstanceInjector) };
+  providerCache.put('$injector', providerInjector);
+  providerCache.put('$injector' + providerSuffix, {$get: valueFn(protoInstanceInjector)});
+
   var runBlocks = loadModules(modulesToLoad);
   instanceInjector = protoInstanceInjector.get('$injector');
   instanceInjector.strictDi = strictDi;
@@ -691,7 +695,7 @@ function createInjector(modulesToLoad, strictDi) {
 
   function supportObject(delegate) {
     return function(key, value) {
-      if (isObject(key)) {
+      if ((arguments.length === 1) && isObject(key)) {
         forEach(key, reverseParams(delegate));
       } else {
         return delegate(key, value);
@@ -707,7 +711,11 @@ function createInjector(modulesToLoad, strictDi) {
     if (!provider_.$get) {
       throw $injectorMinErr('pget', "Provider '{0}' must define $get factory method.", name);
     }
-    return providerCache[name + providerSuffix] = provider_;
+
+    var providerId = !isString(name) ? name : name + providerSuffix;
+    providerCache.put(providerId, provider_);
+
+    return provider_;
   }
 
   function enforceReturnValue(name, factory) {
@@ -727,21 +735,24 @@ function createInjector(modulesToLoad, strictDi) {
   }
 
   function service(name, constructor) {
-    return factory(name, ['$injector', function($injector) {
-      return $injector.instantiate(constructor);
-    }]);
+    return factory(name, function() {
+      return instanceInjector.instantiate(constructor);
+    });
   }
 
-  function value(name, val) { return factory(name, valueFn(val), false); }
+  function value(name, val) {
+    return factory(name, valueFn(val), false);
+  }
 
   function constant(name, value) {
     assertNotHasOwnProperty(name, 'constant');
-    providerCache[name] = value;
-    instanceCache[name] = value;
+    providerCache.put(name, value);
+    instanceCache.put(name, value);
   }
 
   function decorator(serviceName, decorFn) {
-    var origProvider = providerInjector.get(serviceName + providerSuffix),
+    var providerId = !isString(serviceName) ? serviceName : serviceName + providerSuffix;
+    var origProvider = providerInjector.get(providerId),
         orig$get = origProvider.$get;
 
     origProvider.$get = function() {
@@ -776,9 +787,7 @@ function createInjector(modulesToLoad, strictDi) {
           runBlocks = runBlocks.concat(loadModules(moduleFn.requires)).concat(moduleFn._runBlocks);
           runInvokeQueue(moduleFn._invokeQueue);
           runInvokeQueue(moduleFn._configBlocks);
-        } else if (isFunction(module)) {
-            runBlocks.push(providerInjector.invoke(module));
-        } else if (isArray(module)) {
+        } else if (isFunction(module) || isArray(module)) {
             runBlocks.push(providerInjector.invoke(module));
         } else {
           assertArgFn(module, 'module');
@@ -806,28 +815,44 @@ function createInjector(modulesToLoad, strictDi) {
   // internal Injector
   ////////////////////////////////////
 
-  function createInternalInjector(cache, factory) {
+  function createInternalInjector(cache, factory, suffix) {
+    suffix = suffix || '';
 
     function getService(serviceName, caller) {
-      if (cache.hasOwnProperty(serviceName)) {
-        if (cache[serviceName] === INSTANTIATING) {
-          throw $injectorMinErr('cdep', 'Circular dependency found: {0}',
-                    serviceName + ' <- ' + path.join(' <- '));
-        }
-        return cache[serviceName];
-      } else {
-        try {
-          path.unshift(serviceName);
-          cache[serviceName] = INSTANTIATING;
-          return cache[serviceName] = factory(serviceName, caller);
-        } catch (err) {
-          if (cache[serviceName] === INSTANTIATING) {
-            delete cache[serviceName];
+      var instance;
+      var callerStr = stringifyServiceId(caller, suffix);
+      var hasCaller = callerStr && (path[0] !== callerStr);
+
+      if (hasCaller) path.unshift(callerStr);
+      path.unshift(stringifyServiceId(serviceName, suffix));
+
+      try {
+        if (cache.has(serviceName)) {
+          instance = cache.get(serviceName);
+
+          if (instance === INSTANTIATING) {
+            throw $injectorMinErr('cdep', 'Circular dependency found: {0}', path.join(' <- '));
+          }
+
+          return instance;
+        } else {
+          try {
+            cache.put(serviceName, INSTANTIATING);
+
+            instance = factory(serviceName);
+            cache.put(serviceName, instance);
+
+            return instance;
+          } catch (err) {
+            if (cache.get(serviceName) === INSTANTIATING) {
+              cache.remove(serviceName);
+            }
+            throw err;
           }
-          throw err;
-        } finally {
-          path.shift();
         }
+      } finally {
+        path.shift();
+        if (hasCaller) path.shift();
       }
     }
 
@@ -838,12 +863,13 @@ function createInjector(modulesToLoad, strictDi) {
 
       for (var i = 0, length = $inject.length; i < length; i++) {
         var key = $inject[i];
-        if (typeof key !== 'string') {
-          throw $injectorMinErr('itkn',
-                  'Incorrect injection token! Expected service name as string, got {0}', key);
-        }
-        args.push(locals && locals.hasOwnProperty(key) ? locals[key] :
-                                                         getService(key, serviceName));
+        // TODO(gkalpak): Remove this and the corresponding error (?)
+        // if (typeof key !== 'string') {
+        //   throw $injectorMinErr('itkn',
+        //           'Incorrect injection token! Expected service name as string, got {0}', key);
+        // }
+        var localsHasKey = locals && isString(key) && locals.hasOwnProperty(key);
+        args.push(localsHasKey ? locals[key] : getService(key, serviceName));
       }
       return args;
     }
@@ -901,7 +927,8 @@ function createInjector(modulesToLoad, strictDi) {
       get: getService,
       annotate: createInjector.$$annotate,
       has: function(name) {
-        return providerCache.hasOwnProperty(name + providerSuffix) || cache.hasOwnProperty(name);
+        return cache.has(name) ||
+               providerCache.has(!isString(name) ? name : name + providerSuffix);
       }
     };
   }

src/ng/directive/select.js

@@ -564,8 +564,11 @@ var selectDirective = function() {
         // Write value now needs to set the selected property of each matching option
         selectCtrl.writeValue = function writeMultipleValue(value) {
           var items = new HashMap(value);
+          var selectValueMap = selectCtrl.selectValueMap;
+
           forEach(element.find('option'), function(option) {
-            option.selected = isDefined(items.get(option.value)) || isDefined(items.get(selectCtrl.selectValueMap[option.value]));
+            var value = option.value;
+            option.selected = items.has(value) || items.has(selectValueMap[value]);
           });
         };
 

test/ApiSpecs.js

@@ -8,11 +8,16 @@ describe('api', function() {
       var key = {};
       var value1 = {};
       var value2 = {};
+
       map.put(key, value1);
       map.put(key, value2);
+
+      expect(map.has(key)).toBe(true);
+      expect(map.has({})).toBe(false);
       expect(map.get(key)).toBe(value2);
       expect(map.get({})).toBeUndefined();
       expect(map.remove(key)).toBe(value2);
+      expect(map.has(key)).toBe(false);
       expect(map.get(key)).toBeUndefined();
     });
 
@@ -23,6 +28,13 @@ describe('api', function() {
       expect(map.get('c')).toBeUndefined();
     });
 
+    it('should init from an object', function() {
+      var map = new HashMap({a: 'foo', b: 'bar'});
+      expect(map.get('a')).toBe('foo');
+      expect(map.get('b')).toBe('bar');
+      expect(map.get('c')).toBeUndefined();
+    });
+
     it('should maintain hashKey for object keys', function() {
       var map = new HashMap();
       var key = {};