From 3f2232b5a181512fac23775b1df4a6ebda67d018 Mon Sep 17 00:00:00 2001
From: Brian Ford <btford@umich.edu>
Date: Tue, 1 Jul 2014 10:56:55 -0700
Subject: [PATCH] feat($controller): disable using global controller
 constructors

With the exception of simple demos, it is not helpful to use globals
for controller constructors. This adds a new method to `$controllerProvider`
to re-enable the old behavior, but disables this feature by default.

BREAKING CHANGE:
`$controller` will no longer look for controllers on `window`.
The old behavior of looking on `window` for controllers was originally intended
for use in examples, demos, and toy apps. We found that allowing global controller
functions encouraged poor practices, so we resolved to disable this behavior by
default.

To migrate, register your controllers with modules rather than exposing them
as globals:

Before:

```javascript
function MyController() {
  // ...
}
```

After:

```javascript
angular.module('myApp', []).controller('MyController', [function() {
  // ...
}]);
```

Although it's not recommended, you can re-enable the old behavior like this:

```javascript
angular.module('myModule').config(['$controllerProvider', function($controllerProvider) {
  // this option might be handy for migrating old apps, but please don't use it
  // in new ones!
  $controllerProvider.allowGlobals();
}]);
```
---
 src/ng/controller.js                  | 16 ++++++++++++++--
 test/ng/controllerSpec.js             | 23 +++++++++++++++++++----
 test/ng/directive/ngControllerSpec.js | 21 +++++++++++----------
 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/ng/controller.js b/src/ng/controller.js
index 35353cd1..0e215bdc 100644
--- a/src/ng/controller.js
+++ b/src/ng/controller.js
@@ -12,6 +12,7 @@
  */
 function $ControllerProvider() {
   var controllers = {},
+      globals = false,
       CNTRL_REG = /^(\S+)(\s+as\s+(\w+))?$/;
 
 
@@ -32,6 +33,15 @@ function $ControllerProvider() {
     }
   };
 
+  /**
+   * @ngdoc method
+   * @name $controllerProvider#allowGlobals
+   * @description If called, allows `$controller` to find controller constructors on `window`
+   */
+  this.allowGlobals = function() {
+    globals = true;
+  };
+
 
   this.$get = ['$injector', '$window', function($injector, $window) {
 
@@ -46,7 +56,8 @@ function $ControllerProvider() {
      *
      *    * check if a controller with given name is registered via `$controllerProvider`
      *    * check if evaluating the string on the current scope returns a constructor
-     *    * check `window[constructor]` on the global `window` object
+     *    * if $controllerProvider#allowGlobals, check `window[constructor]` on the global
+     *      `window` object (not recommended)
      *
      * @param {Object} locals Injection locals for Controller.
      * @return {Object} Instance of given controller.
@@ -66,7 +77,8 @@ function $ControllerProvider() {
         identifier = match[3];
         expression = controllers.hasOwnProperty(constructor)
             ? controllers[constructor]
-            : getter(locals.$scope, constructor, true) || getter($window, constructor, true);
+            : getter(locals.$scope, constructor, true) ||
+                (globals ? getter($window, constructor, true) : undefined);
 
         assertArgFn(expression, constructor, true);
       }
diff --git a/test/ng/controllerSpec.js b/test/ng/controllerSpec.js
index 8c091746..0ef8efbb 100644
--- a/test/ng/controllerSpec.js
+++ b/test/ng/controllerSpec.js
@@ -64,6 +64,21 @@ describe('$controller', function() {
         $controllerProvider.register('hasOwnProperty', function($scope) {});
       }).toThrowMinErr('ng', 'badname', "hasOwnProperty is not a valid controller name");
     });
+
+
+    it('should instantiate a controller defined on window if allowGlobals is set',
+      inject(function($window) {
+        var scope = {};
+        var Foo = function() {};
+
+        $controllerProvider.allowGlobals();
+
+        $window.a = {Foo: Foo};
+
+        var foo = $controller('a.Foo', {$scope: scope});
+        expect(foo).toBeDefined();
+        expect(foo instanceof Foo).toBe(true);
+      }));
   });
 
 
@@ -97,15 +112,15 @@ describe('$controller', function() {
   });
 
 
-  it('should instantiate controller defined on window', inject(function($window) {
+  it('should not instantiate a controller defined on window', inject(function($window) {
     var scope = {};
     var Foo = function() {};
 
     $window.a = {Foo: Foo};
 
-    var foo = $controller('a.Foo', {$scope: scope});
-    expect(foo).toBeDefined();
-    expect(foo instanceof Foo).toBe(true);
+    expect(function () {
+      $controller('a.Foo', {$scope: scope});
+    }).toThrow();
   }));
 
 
diff --git a/test/ng/directive/ngControllerSpec.js b/test/ng/directive/ngControllerSpec.js
index b604d443..bd6d3eee 100644
--- a/test/ng/directive/ngControllerSpec.js
+++ b/test/ng/directive/ngControllerSpec.js
@@ -7,9 +7,8 @@ describe('ngController', function() {
     $controllerProvider.register('PublicModule', function() {
       this.mark = 'works';
     });
-  }));
-  beforeEach(inject(function($window) {
-    $window.Greeter = function($scope) {
+
+    var Greeter = function($scope) {
       // private stuff (not exported to scope)
       this.prefix = 'Hello ';
 
@@ -22,20 +21,22 @@ describe('ngController', function() {
 
       $scope.protoGreet = bind(this, this.protoGreet);
     };
-    $window.Greeter.prototype = {
+    Greeter.prototype = {
       suffix: '!',
       protoGreet: function(name) {
         return this.prefix + name + this.suffix;
       }
     };
+    $controllerProvider.register('Greeter', Greeter);
 
-    $window.Child = function($scope) {
+    $controllerProvider.register('Child', function($scope) {
       $scope.name = 'Adam';
-    };
+    });
 
-    $window.Public = function() {
+    $controllerProvider.register('Public', function($scope) {
       this.mark = 'works';
-    };
+    });
+
   }));
 
   afterEach(function() {
@@ -77,11 +78,11 @@ describe('ngController', function() {
 
 
   it('should instantiate controller defined on scope', inject(function($compile, $rootScope) {
-    $rootScope.Greeter = function($scope) {
+    $rootScope.VojtaGreeter = function($scope) {
       $scope.name = 'Vojta';
     };
 
-    element = $compile('<div ng-controller="Greeter">{{name}}</div>')($rootScope);
+    element = $compile('<div ng-controller="VojtaGreeter">{{name}}</div>')($rootScope);
     $rootScope.$digest();
     expect(element.text()).toBe('Vojta');
   }));