From 9d021eede3e5ebffe2c98fce12293703586b491b Mon Sep 17 00:00:00 2001 From: Luke Page Date: Sat, 4 Jul 2015 06:49:36 +0100 Subject: [PATCH] [[FIX]] Param destructuring should not effect max params Fixes destructuring so that multiple destructured params do not count towards the maxparams limit. This is because destructuring is the solution to having alot of numbered params, since you would replace a hard to figure out list of params with an object, then use destructuring to pull out from that object. Fixes #2183 --- src/jshint.js | 32 +++++++++++-------- .../fixtures/max-parameters-per-function.js | 12 +++++++ tests/unit/options.js | 28 +++++++++++++--- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/jshint.js b/src/jshint.js index 3a4819248a..ca1982bf79 100644 --- a/src/jshint.js +++ b/src/jshint.js @@ -2741,7 +2741,7 @@ var JSHINT = (function() { * single-argument shorthand. * @param {bool} [options.parsedOpening] Whether the opening parenthesis has * already been parsed. - * @returns {Array.} array of param identifiers + * @returns {{ arity: number, params: Array.}} */ function functionparams(options) { var next; @@ -2751,11 +2751,12 @@ var JSHINT = (function() { var t; var pastDefault = false; var pastRest = false; + var arity = 0; var loneArg = options && options.loneArg; if (loneArg && loneArg.identifier === true) { state.funct["(scope)"].addParam(loneArg.value, loneArg); - return [ loneArg.value ]; + return { arity: 1, params: [ loneArg.value ] }; } next = state.tokens.next; @@ -2774,7 +2775,7 @@ var JSHINT = (function() { } for (;;) { - // store the current param(s) of this loop so we can evaluate the default argument before parameters + arity++; // are added to the param scope var currentParams = []; @@ -2824,7 +2825,7 @@ var JSHINT = (function() { comma(); } else { advance(")", next); - return paramsIds; + return { arity: arity, params: paramsIds }; } } } @@ -2980,10 +2981,15 @@ var JSHINT = (function() { // create the param scope (params added in functionparams) state.funct["(scope)"].stackParams(true); - var paramsIds = functionparams(options); + var paramsInfo = functionparams(options); - state.funct["(params)"] = paramsIds; - state.funct["(metrics)"].verifyMaxParametersPerFunction(paramsIds); + if (paramsInfo) { + state.funct["(params)"] = paramsInfo.params; + state.funct["(metrics)"].arity = paramsInfo.arity; + state.funct["(metrics)"].verifyMaxParametersPerFunction(); + } else { + state.funct["(metrics)"].arity = 0; + } if (isArrow) { if (!state.option.esnext) { @@ -3027,6 +3033,7 @@ var JSHINT = (function() { statementCount: 0, nestedBlockDepth: -1, ComplexityCount: 1, + arity: 0, verifyMaxStatementsPerFunction: function() { if (state.option.maxstatements && @@ -3035,11 +3042,10 @@ var JSHINT = (function() { } }, - verifyMaxParametersPerFunction: function(params) { - params = params || []; - - if (_.isNumber(state.option.maxparams) && params.length > state.option.maxparams) { - warning("W072", functionStartToken, params.length); + verifyMaxParametersPerFunction: function() { + if (_.isNumber(state.option.maxparams) && + this.arity > state.option.maxparams) { + warning("W072", functionStartToken, this.arity); } }, @@ -5285,7 +5291,7 @@ var JSHINT = (function() { fu.metrics = { complexity: f["(metrics)"].ComplexityCount, - parameters: (f["(params)"] || []).length, + parameters: f["(metrics)"].arity, statements: f["(metrics)"].statementCount }; diff --git a/tests/unit/fixtures/max-parameters-per-function.js b/tests/unit/fixtures/max-parameters-per-function.js index b9932514d1..5b175d4603 100644 --- a/tests/unit/fixtures/max-parameters-per-function.js +++ b/tests/unit/fixtures/max-parameters-per-function.js @@ -3,3 +3,15 @@ function functionWithNoParameters() { function functionWith3Parameters(param1, param2, param3) { } + +var a = () => {}; +var b = (a) => {}; +var c = a => {}; +var d = (a, b, c) => {}; +var e = ({a, b, c}) => {}; + +function f([a, b, c] = [], [d, e] = []) { +} + +function g({a}, {b}, {c}) { +} \ No newline at end of file diff --git a/tests/unit/options.js b/tests/unit/options.js index fc0d4b7c53..d52e18a009 100644 --- a/tests/unit/options.js +++ b/tests/unit/options.js @@ -2048,17 +2048,37 @@ exports.maxparams = function (test) { TestRun(test) .addError(4, "This function has too many parameters. (3)") - .test(src, { es3: true, maxparams: 2 }); + .addError(10, "This function has too many parameters. (3)") + .addError(16, "This function has too many parameters. (3)") + .test(src, { esnext: true, maxparams: 2 }); TestRun(test) - .test(src, { es3: true, maxparams: 3 }); + .test(src, { esnext: true, maxparams: 3 }); TestRun(test) .addError(4, "This function has too many parameters. (3)") - .test(src, {es3: true, maxparams: 0 }); + .addError(8, "This function has too many parameters. (1)") + .addError(9, "This function has too many parameters. (1)") + .addError(10, "This function has too many parameters. (3)") + .addError(11, "This function has too many parameters. (1)") + .addError(13, "This function has too many parameters. (2)") + .addError(16, "This function has too many parameters. (3)") + .test(src, {esnext: true, maxparams: 0 }); TestRun(test) - .test(src, { es3: true }); + .test(src, { esnext: true }); + + var functions = JSHINT.data().functions; + test.equal(functions.length, 9); + test.equal(functions[0].metrics.parameters, 0); + test.equal(functions[1].metrics.parameters, 3); + test.equal(functions[2].metrics.parameters, 0); + test.equal(functions[3].metrics.parameters, 1); + test.equal(functions[4].metrics.parameters, 1); + test.equal(functions[5].metrics.parameters, 3); + test.equal(functions[6].metrics.parameters, 1); + test.equal(functions[7].metrics.parameters, 2); + test.equal(functions[8].metrics.parameters, 3); test.done(); };