Refactored toThrowMatching to facilitate adding more strategies

* Extracted sub-matchers for the two major existing strategies
  (matching all errors, and matching by type and/or message)
* Reduced the use of mutable state

lib/jasmine-core/jasmine.js

@@ -3702,58 +3702,30 @@ getJasmineRequireObj().toThrowError = function(j$) {
   function toThrowError () {
     return {
       compare: function(actual) {
-        var threw = false,
+        var errorMatcher = getMatcher.apply(null, arguments),
-          pass = {pass: true},
-          fail = {pass: false},
           thrown;
 
         if (typeof actual != 'function') {
           throw new Error(getErrorMsg('Actual is not a Function'));
         }
 
-        var errorMatcher = getMatcher.apply(null, arguments);
-
         try {
           actual();
+          return fail('Expected function to throw an Error.');
         } catch (e) {
-          threw = true;
           thrown = e;
         }
 
-        if (!threw) {
-          fail.message = 'Expected function to throw an Error.';
-          return fail;
-        }
-
-        // Get Error constructor of thrown
         if (!isErrorObject(thrown)) {
-          fail.message = function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; };
+          return fail(function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; });
-          return fail;
         }
 
-        if (errorMatcher.hasNoSpecifics()) {
+        return errorMatcher.match(thrown);
-          pass.message = 'Expected function not to throw an Error, but it threw ' + j$.fnNameFor(thrown) + '.';
-          return pass;
-        }
-
-        if (errorMatcher.matches(thrown)) {
-          pass.message = function() {
-            return 'Expected function not to throw ' + errorMatcher.errorTypeDescription + errorMatcher.messageDescription() + '.';
-          };
-          return pass;
-        } else {
-          fail.message = function() {
-            return 'Expected function to throw ' + errorMatcher.errorTypeDescription + errorMatcher.messageDescription() +
-              ', but it threw ' + errorMatcher.thrownDescription(thrown) + '.';
-          };
-          return fail;
-        }
       }
     };
 
     function getMatcher() {
-      var expected = null,
+      var expected, errorType;
-          errorType = null;
 
       if (arguments[2]) {
         errorType = arguments[1];
@@ -3761,14 +3733,29 @@ getJasmineRequireObj().toThrowError = function(j$) {
         if (!isAnErrorType(errorType)) {
           throw new Error(getErrorMsg('Expected error type is not an Error.'));
         }
+
+        return conditionalMatcher(expected, errorType);
       } else if (arguments[1]) {
         expected = arguments[1];
-        if (isAnErrorType(expected)) {
+        if (isAnErrorType(arguments[1])) {
-          errorType = expected;
+          return conditionalMatcher(null, arguments[1]);
-          expected = null;
+        } else {
+          return conditionalMatcher(arguments[1], null);
         }
+      } else {
+        return anyMatcher();
       }
+    }
 
+    function anyMatcher() {
+      return {
+        match: function(error) {
+          return pass('Expected function not to throw an Error, but it threw ' + j$.fnNameFor(error) + '.');
+        }
+      };
+    }
+
+    function conditionalMatcher(expected, errorType) {
       if (expected && !isStringOrRegExp(expected)) {
         if (errorType) {
           throw new Error(getErrorMsg('Expected error message is not a string or RegExp.'));
@@ -3785,33 +3772,46 @@ getJasmineRequireObj().toThrowError = function(j$) {
         }
       }
 
+      var errorTypeDescription = errorType ? j$.fnNameFor(errorType) : 'an exception';
+
+      function thrownDescription(thrown) {
+        var thrownName = errorType ? j$.fnNameFor(thrown.constructor) : 'an exception',
+            thrownMessage = '';
+
+        if (expected) {
+          thrownMessage = ' with message ' + j$.pp(thrown.message);
+        }
+
+        return thrownName + thrownMessage;
+      }
+
+      function messageDescription() {
+        if (expected === null) {
+          return '';
+        } else if (expected instanceof RegExp) {
+          return ' with a message matching ' + j$.pp(expected);
+        } else {
+          return ' with message ' + j$.pp(expected);
+        }
+      }
+
+      function matches(error) {
+        return (errorType === null || error instanceof errorType) &&
+          (expected === null || messageMatch(error.message));
+      }
+
       return {
-        errorTypeDescription: errorType ? j$.fnNameFor(errorType) : 'an exception',
+        match: function(thrown) {
-        thrownDescription: function(thrown) {
+          if (matches(thrown)) {
-          var thrownName = errorType ? j$.fnNameFor(thrown.constructor) : 'an exception',
+            return pass(function() {
-              thrownMessage = '';
+              return 'Expected function not to throw ' + errorTypeDescription + messageDescription() + '.';
-
+            });
-          if (expected) {
-            thrownMessage = ' with message ' + j$.pp(thrown.message);
-          }
-
-          return thrownName + thrownMessage;
-        },
-        messageDescription: function() {
-          if (expected === null) {
-            return '';
-          } else if (expected instanceof RegExp) {
-            return ' with a message matching ' + j$.pp(expected);
           } else {
-            return ' with message ' + j$.pp(expected);
+            return fail(function() {
+              return 'Expected function to throw ' + errorTypeDescription + messageDescription() +
+                ', but it threw ' + thrownDescription(thrown) + '.';
+            });
           }
-        },
-        hasNoSpecifics: function() {
-          return expected === null && errorType === null;
-        },
-        matches: function(error) {
-          return (errorType === null || error instanceof errorType) &&
-            (expected === null || messageMatch(error.message));
         }
       };
     }
@@ -3842,6 +3842,20 @@ getJasmineRequireObj().toThrowError = function(j$) {
     }
   }
 
+  function pass(message) {
+    return {
+      pass: true,
+      message: message
+    };
+  }
+
+  function fail(message) {
+    return {
+      pass: false,
+      message: message
+    };
+  }
+
   return toThrowError;
 };
 

src/core/matchers/toThrowError.js

@@ -18,58 +18,30 @@ getJasmineRequireObj().toThrowError = function(j$) {
   function toThrowError () {
     return {
       compare: function(actual) {
-        var threw = false,
+        var errorMatcher = getMatcher.apply(null, arguments),
-          pass = {pass: true},
-          fail = {pass: false},
           thrown;
 
         if (typeof actual != 'function') {
           throw new Error(getErrorMsg('Actual is not a Function'));
         }
 
-        var errorMatcher = getMatcher.apply(null, arguments);
-
         try {
           actual();
+          return fail('Expected function to throw an Error.');
         } catch (e) {
-          threw = true;
           thrown = e;
         }
 
-        if (!threw) {
-          fail.message = 'Expected function to throw an Error.';
-          return fail;
-        }
-
-        // Get Error constructor of thrown
         if (!isErrorObject(thrown)) {
-          fail.message = function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; };
+          return fail(function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; });
-          return fail;
         }
 
-        if (errorMatcher.hasNoSpecifics()) {
+        return errorMatcher.match(thrown);
-          pass.message = 'Expected function not to throw an Error, but it threw ' + j$.fnNameFor(thrown) + '.';
-          return pass;
-        }
-
-        if (errorMatcher.matches(thrown)) {
-          pass.message = function() {
-            return 'Expected function not to throw ' + errorMatcher.errorTypeDescription + errorMatcher.messageDescription() + '.';
-          };
-          return pass;
-        } else {
-          fail.message = function() {
-            return 'Expected function to throw ' + errorMatcher.errorTypeDescription + errorMatcher.messageDescription() +
-              ', but it threw ' + errorMatcher.thrownDescription(thrown) + '.';
-          };
-          return fail;
-        }
       }
     };
 
     function getMatcher() {
-      var expected = null,
+      var expected, errorType;
-          errorType = null;
 
       if (arguments[2]) {
         errorType = arguments[1];
@@ -77,14 +49,29 @@ getJasmineRequireObj().toThrowError = function(j$) {
         if (!isAnErrorType(errorType)) {
           throw new Error(getErrorMsg('Expected error type is not an Error.'));
         }
+
+        return conditionalMatcher(expected, errorType);
       } else if (arguments[1]) {
         expected = arguments[1];
-        if (isAnErrorType(expected)) {
+        if (isAnErrorType(arguments[1])) {
-          errorType = expected;
+          return conditionalMatcher(null, arguments[1]);
-          expected = null;
+        } else {
+          return conditionalMatcher(arguments[1], null);
         }
+      } else {
+        return anyMatcher();
       }
+    }
 
+    function anyMatcher() {
+      return {
+        match: function(error) {
+          return pass('Expected function not to throw an Error, but it threw ' + j$.fnNameFor(error) + '.');
+        }
+      };
+    }
+
+    function conditionalMatcher(expected, errorType) {
       if (expected && !isStringOrRegExp(expected)) {
         if (errorType) {
           throw new Error(getErrorMsg('Expected error message is not a string or RegExp.'));
@@ -101,33 +88,46 @@ getJasmineRequireObj().toThrowError = function(j$) {
         }
       }
 
+      var errorTypeDescription = errorType ? j$.fnNameFor(errorType) : 'an exception';
+
+      function thrownDescription(thrown) {
+        var thrownName = errorType ? j$.fnNameFor(thrown.constructor) : 'an exception',
+            thrownMessage = '';
+
+        if (expected) {
+          thrownMessage = ' with message ' + j$.pp(thrown.message);
+        }
+
+        return thrownName + thrownMessage;
+      }
+
+      function messageDescription() {
+        if (expected === null) {
+          return '';
+        } else if (expected instanceof RegExp) {
+          return ' with a message matching ' + j$.pp(expected);
+        } else {
+          return ' with message ' + j$.pp(expected);
+        }
+      }
+
+      function matches(error) {
+        return (errorType === null || error instanceof errorType) &&
+          (expected === null || messageMatch(error.message));
+      }
+
       return {
-        errorTypeDescription: errorType ? j$.fnNameFor(errorType) : 'an exception',
+        match: function(thrown) {
-        thrownDescription: function(thrown) {
+          if (matches(thrown)) {
-          var thrownName = errorType ? j$.fnNameFor(thrown.constructor) : 'an exception',
+            return pass(function() {
-              thrownMessage = '';
+              return 'Expected function not to throw ' + errorTypeDescription + messageDescription() + '.';
-
+            });
-          if (expected) {
-            thrownMessage = ' with message ' + j$.pp(thrown.message);
-          }
-
-          return thrownName + thrownMessage;
-        },
-        messageDescription: function() {
-          if (expected === null) {
-            return '';
-          } else if (expected instanceof RegExp) {
-            return ' with a message matching ' + j$.pp(expected);
           } else {
-            return ' with message ' + j$.pp(expected);
+            return fail(function() {
+              return 'Expected function to throw ' + errorTypeDescription + messageDescription() +
+                ', but it threw ' + thrownDescription(thrown) + '.';
+            });
           }
-        },
-        hasNoSpecifics: function() {
-          return expected === null && errorType === null;
-        },
-        matches: function(error) {
-          return (errorType === null || error instanceof errorType) &&
-            (expected === null || messageMatch(error.message));
         }
       };
     }
@@ -158,5 +158,19 @@ getJasmineRequireObj().toThrowError = function(j$) {
     }
   }
 
+  function pass(message) {
+    return {
+      pass: true,
+      message: message
+    };
+  }
+
+  function fail(message) {
+    return {
+      pass: false,
+      message: message
+    };
+  }
+
   return toThrowError;
 };