Skip to content

Commit

Permalink
[[FIX]] Param destructuring should not effect max params
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lukeapage authored and jugglinmike committed Jul 26, 2015
1 parent d138db8 commit 9d021ee
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 17 deletions.
32 changes: 19 additions & 13 deletions src/jshint.js
Expand Up @@ -2741,7 +2741,7 @@ var JSHINT = (function() {
* single-argument shorthand.
* @param {bool} [options.parsedOpening] Whether the opening parenthesis has
* already been parsed.
* @returns {Array.<string>} array of param identifiers
* @returns {{ arity: number, params: Array.<string>}}
*/
function functionparams(options) {
var next;
Expand All @@ -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;
Expand All @@ -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 = [];

Expand Down Expand Up @@ -2824,7 +2825,7 @@ var JSHINT = (function() {
comma();
} else {
advance(")", next);
return paramsIds;
return { arity: arity, params: paramsIds };
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -3027,6 +3033,7 @@ var JSHINT = (function() {
statementCount: 0,
nestedBlockDepth: -1,
ComplexityCount: 1,
arity: 0,

verifyMaxStatementsPerFunction: function() {
if (state.option.maxstatements &&
Expand All @@ -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);
}
},

Expand Down Expand Up @@ -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
};

Expand Down
12 changes: 12 additions & 0 deletions tests/unit/fixtures/max-parameters-per-function.js
Expand Up @@ -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}) {
}
28 changes: 24 additions & 4 deletions tests/unit/options.js
Expand Up @@ -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();
};
Expand Down

0 comments on commit 9d021ee

Please sign in to comment.