Repository: unexpectedjs/unexpected
Pull request #372 (2017-02-16 01:11:08)

addAssertion: Fail when the handler takes too many parameters
As discussed on #339


Patch:
diff --git a/lib/Unexpected.js b/lib/Unexpected.js
index 62ca2ee4..8343fe59 100644
--- a/lib/Unexpected.js
+++ b/lib/Unexpected.js
@@ -575,6 +575,7 @@ Unexpected.prototype.addAssertion = function (patternOrPatterns, handler) {
 
     var defaultValueByFlag = {};
     var assertionHandlers = [];
+    var maxNumberOfArgs = 0;
     patterns.forEach(function (pattern) {
         var assertionDeclarations = that.parseAssertion(pattern);
         assertionDeclarations.forEach(function (assertionDeclaration) {
@@ -584,7 +585,9 @@ Unexpected.prototype.addAssertion = function (patternOrPatterns, handler) {
                 Object.keys(expandedAssertion.flags).forEach(function (flag) {
                     defaultValueByFlag[flag] = false;
                 });
-
+                maxNumberOfArgs = Math.max(maxNumberOfArgs, assertionDeclaration.args.reduce(function (previous, argDefinition) {
+                    return previous + (argDefinition.maximum === null ? Infinity : argDefinition.maximum);
+                }, 0));
                 assertionHandlers.push({
                     handler: handler,
                     alternations: expandedAssertion.alternations,
@@ -597,6 +600,9 @@ Unexpected.prototype.addAssertion = function (patternOrPatterns, handler) {
             });
         });
     });
+    if (handler.length - 2 > maxNumberOfArgs) {
+        throw new Error('The provided assertion handler takes ' + (handler.length - 2) + ' parameters, but the type signature specifies a maximum of ' + maxNumberOfArgs);
+    }
 
     assertionHandlers.forEach(function (handler) {
         // Make sure that all flags are defined.
diff --git a/lib/assertions.js b/lib/assertions.js
index aec38df5..c0782ae1 100644
--- a/lib/assertions.js
+++ b/lib/assertions.js
@@ -618,7 +618,7 @@ module.exports = function (expect) {
         });
     });
 
-    expect.addAssertion('<function> not to error', function (expect, subject, arg) {
+    expect.addAssertion('<function> not to error', function (expect, subject) {
         var threw = false;
         return expect.promise(function () {
             try {
@@ -661,7 +661,7 @@ module.exports = function (expect) {
         }
     });
 
-    expect.addAssertion('<function> to (throw|throw error|throw exception)', function (expect, subject, arg) {
+    expect.addAssertion('<function> to (throw|throw error|throw exception)', function (expect, subject) {
         try {
             subject();
         } catch (e) {


Test patch:
diff --git a/test/api/addAssertion.spec.js b/test/api/addAssertion.spec.js
index 38e7b6ca..902bc0b6 100644
--- a/test/api/addAssertion.spec.js
+++ b/test/api/addAssertion.spec.js
@@ -23,6 +23,16 @@ describe('addAssertion', function () {
         }, 'to throw', 'expected [ 1, 2, 3 ] not to be sorted');
     });
 
+    it('throws when a handler takes more parameters than are specified in the type signature', function () {
+        expect(function () {
+            expect.addAssertion('<string> to foobar <number>', function (expect, subject, one, toomany) {});
+        }, 'to throw', 'The provided assertion handler takes 2 parameters, but the type signature specifies a maximum of 1');
+    });
+
+    it('allows a handler with many parameters when the type signature contains varargs', function () {
+        expect.clone().addAssertion('<string> to foobar <number+>', function (expect, subject, quite, a, lot) {});
+    });
+
     describe('when overriding an assertion', function () {
         it('uses the most specific version', function () {
             var clonedExpect = expect.clone()
diff --git a/test/api/fail.spec.js b/test/api/fail.spec.js
index 9ed7b672..9f0b180a 100644
--- a/test/api/fail.spec.js
+++ b/test/api/fail.spec.js
@@ -95,7 +95,7 @@ describe('fail assertion', function () {
     describe('with a diff function', function () {
         it('should generate the diff', function () {
             var clonedExpect = expect.clone();
-            clonedExpect.addAssertion('<any> to foo', function (expect, subject, value) {
+            clonedExpect.addAssertion('<any> to foo', function (expect, subject) {
                 expect.fail({
                     diff: function (output, diff, inspect, equal) {
                         return output.text('custom');
@@ -113,7 +113,7 @@ describe('fail assertion', function () {
 
         it('should support a diff function that uses the old API', function () {
             var clonedExpect = expect.clone();
-            clonedExpect.addAssertion('<any> to foo', function (expect, subject, value) {
+            clonedExpect.addAssertion('<any> to foo', function (expect, subject) {
                 expect.fail({
                     diff: function (output, diff, inspect, equal) {
                         return {
diff --git a/test/external.spec.js b/test/external.spec.js
index 31d1da01..959ff15f 100644
--- a/test/external.spec.js
+++ b/test/external.spec.js
@@ -114,7 +114,7 @@ if (typeof process === 'object') {
         });
 
         describe('executed through jasmine', function () {
-            expect.addAssertion('<string> executed through jasmine', function (expect, subject, value) {
+            expect.addAssertion('<string> executed through jasmine', function (expect, subject) {
                 return expect.promise(function (run) {
                     childProcess.execFile(pathModule.resolve(__dirname, '..', 'node_modules', '.bin', 'jasmine'), {
                         cwd: basePath,