From 0fd5ab58703d30ad5949682fde96024b7d277e10 Mon Sep 17 00:00:00 2001
From: Caitlin Potter <caitpotter88@gmail.com>
Date: Thu, 29 Jan 2015 12:13:40 -0500
Subject: [PATCH] fix($controller): throw better error when controller
 expression is bad

Previously, the error was a JS runtime error when trying to access a property of `null`. But, it's
a bit nicer to throw a real error and provide a description of how to fix it. Developer ergonomics
and all that.

Closes #10875
---
 docs/content/error/$controller/ctrlfmt.ngdoc | 46 ++++++++++++++++++++
 src/ng/controller.js                         |  9 +++-
 test/ng/controllerSpec.js                    | 43 +++++++++++++++++-
 3 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 docs/content/error/$controller/ctrlfmt.ngdoc

diff --git a/docs/content/error/$controller/ctrlfmt.ngdoc b/docs/content/error/$controller/ctrlfmt.ngdoc
new file mode 100644
index 000000000000..019598f7951e
--- /dev/null
+++ b/docs/content/error/$controller/ctrlfmt.ngdoc
@@ -0,0 +1,46 @@
+@ngdoc error
+@name $controller:ctrlfmt
+@fullName Badly formed controller string
+@description
+
+This error occurs when {@link ng.$controller $controller} service is called
+with a string that does not match the supported controller string formats.
+
+Supported formats:
+
+1. `__name__`
+2. `__name__ as __identifier__`
+
+N'either `__name__` or `__identifier__` may contain spaces.
+
+Example of incorrect usage that leads to this error:
+```html
+<!-- unclosed ng-controller attribute messes up the format -->
+<div ng-controller="myController>
+```
+
+or
+
+```js
+// does not match `__name__` or `__name__ as __identifier__`
+var myCtrl = $controller("mY contRoller", { $scope: newScope });
+```
+
+or
+
+```js
+directive("myDirective", function() {
+  return {
+    // does not match `__name__` or `__name__ as __identifier__`
+    controller: "mY contRoller",
+    link: function() {}
+  };
+});
+```
+
+To fix the examples above, ensure that the controller string matches the supported
+formats, and that any html attributes which are used as controller expressions are
+closed.
+
+
+Please consult the {@link ng.$controller $controller} service api docs to learn more.
diff --git a/src/ng/controller.js b/src/ng/controller.js
index 1b5828fadb41..cf7e964c2b14 100644
--- a/src/ng/controller.js
+++ b/src/ng/controller.js
@@ -1,5 +1,7 @@
 'use strict';
 
+var $controllerMinErr = minErr('$controller');
+
 /**
  * @ngdoc provider
  * @name $controllerProvider
@@ -87,7 +89,12 @@ function $ControllerProvider() {
       }
 
       if (isString(expression)) {
-        match = expression.match(CNTRL_REG),
+        match = expression.match(CNTRL_REG);
+        if (!match) {
+          throw new $controllerMinErr('ctrlfmt',
+            "Badly formed controller string '{0}'. " +
+            "Must match `__name__ as __id__` or `__name__`.", expression);
+        }
         constructor = match[1],
         identifier = identifier || match[3];
         expression = controllers.hasOwnProperty(constructor)
diff --git a/test/ng/controllerSpec.js b/test/ng/controllerSpec.js
index 8627bf88845f..839e3310c5e6 100644
--- a/test/ng/controllerSpec.js
+++ b/test/ng/controllerSpec.js
@@ -90,7 +90,16 @@ describe('$controller', function() {
         var foo = $controller('a.Foo', {$scope: scope});
         expect(foo).toBeDefined();
         expect(foo instanceof Foo).toBe(true);
-      }));
+    }));
+
+
+    it('should throw ctrlfmt if name contains spaces', function() {
+      expect(function() {
+        $controller('ctrl doom');
+      }).toThrowMinErr("$controller", "ctrlfmt",
+                       "Badly formed controller string 'ctrl doom'. " +
+                       "Must match `__name__ as __id__` or `__name__`.");
+    });
   });
 
 
@@ -168,5 +177,37 @@ describe('$controller', function() {
       }).toThrowMinErr("$controller", "noscp", "Cannot export controller 'a.b.FooCtrl' as 'foo'! No $scope object provided via `locals`.");
 
     });
+
+
+    it('should throw ctrlfmt if identifier contains non-ident characters', function() {
+      expect(function() {
+        $controller('ctrl as foo<bar');
+      }).toThrowMinErr("$controller", "ctrlfmt",
+                       "Badly formed controller string 'ctrl as foo<bar'. " +
+                       "Must match `__name__ as __id__` or `__name__`.");
+    });
+
+
+    it('should throw ctrlfmt if identifier contains spaces', function() {
+      expect(function() {
+        $controller('ctrl as foo bar');
+      }).toThrowMinErr("$controller", "ctrlfmt",
+                       "Badly formed controller string 'ctrl as foo bar'. " +
+                       "Must match `__name__ as __id__` or `__name__`.");
+    });
+
+
+    it('should throw ctrlfmt if identifier missing after " as "', function() {
+      expect(function() {
+        $controller('ctrl as ');
+      }).toThrowMinErr("$controller", "ctrlfmt",
+                       "Badly formed controller string 'ctrl as '. " +
+                       "Must match `__name__ as __id__` or `__name__`.");
+      expect(function() {
+        $controller('ctrl as');
+      }).toThrowMinErr("$controller", "ctrlfmt",
+                       "Badly formed controller string 'ctrl as'. " +
+                       "Must match `__name__ as __id__` or `__name__`.");
+    });
   });
 });