Skip to content

Commit

Permalink
Merge pull request #5706 from isaacl/master
Browse files Browse the repository at this point in the history 
Improve no-param-reassign props: true msg
  • Loading branch information
@nzakas
nzakas committed Apr 1, 2016
2 parents 2bb8486 + f59d91d commit e54598a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 57 deletions.
4 changes: 3 additions & 1 deletion docs/rules/no-param-reassign.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

Assignment to variables declared as function parameters can be misleading and lead to confusing behavior, as modifying function parameters will also mutate the `arguments` object. Often, assignment to function parameters is unintended and indicative of a mistake or programmer error.

This rule can be also configured to fail when function parameters are modified. Side effects on parameters can cause counter-intuitive execution flow and make errors difficult to track down.

## Rule Details

This rule aims to prevent unintended behavior caused by overwriting function parameters.
This rule aims to prevent unintended behavior caused by modification or reassignment of function parameters.

Examples of **incorrect** code for this rule:

Expand Down
100 changes: 49 additions & 51 deletions lib/rules/no-param-reassign.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,60 +15,52 @@ module.exports = function(context) {
var props = context.options[0] && Boolean(context.options[0].props);

/**
* Checks whether or not a reference modifies its variable.
* If the `props` option is `true`, this checks whether or not the reference modifies properties of its variable also.
* Checks whether or not the reference modifies properties of its variable.
* @param {Reference} reference - A reference to check.
* @returns {boolean} Whether or not the reference modifies its variable.
* @returns {boolean} Whether or not the reference modifies properties of its variable.
*/
function isModifying(reference) {
if (reference.isWrite()) {
return true;
}
function isModifyingProp(reference) {
var node = reference.identifier;
var parent = node.parent;

// Checks whether its property is modified.
if (props) {
var node = reference.identifier;
var parent = node.parent;
while (parent && !stopNodePattern.test(parent.type)) {
switch (parent.type) {

while (parent && !stopNodePattern.test(parent.type)) {
switch (parent.type) {
// e.g. foo.a = 0;
case "AssignmentExpression":
return parent.left === node;

// e.g. foo.a = 0;
case "AssignmentExpression":
return parent.left === node;
// e.g. ++foo.a;
case "UpdateExpression":
return true;

// e.g. ++foo.a;
case "UpdateExpression":
// e.g. delete foo.a;
case "UnaryExpression":
if (parent.operator === "delete") {
return true;

// e.g. delete foo.a;
case "UnaryExpression":
if (parent.operator === "delete") {
return true;
}
break;

// EXCLUDES: e.g. cache.get(foo.a).b = 0;
case "CallExpression":
if (parent.callee !== node) {
return false;
}
break;

// EXCLUDES: e.g. cache[foo.a] = 0;
case "MemberExpression":
if (parent.property === node) {
return false;
}
break;

default:
break;
}

node = parent;
parent = parent.parent;
}
break;

// EXCLUDES: e.g. cache.get(foo.a).b = 0;
case "CallExpression":
if (parent.callee !== node) {
return false;
}
break;

// EXCLUDES: e.g. cache[foo.a] = 0;
case "MemberExpression":
if (parent.property === node) {
return false;
}
break;

default:
break;
}

node = parent;
parent = node.parent;
}

return false;
Expand All @@ -86,16 +78,22 @@ module.exports = function(context) {

if (identifier &&
!reference.init &&
isModifying(reference) &&

// Destructuring assignments can have multiple default value,
// so possibly there are multiple writeable references for the same identifier.
(index === 0 || references[index - 1].identifier !== identifier)
) {
context.report(
identifier,
"Assignment to function parameter '{{name}}'.",
{name: identifier.name});
if (reference.isWrite()) {
context.report(
identifier,
"Assignment to function parameter '{{name}}'.",
{name: identifier.name});
} else if (props && isModifyingProp(reference)) {
context.report(
identifier,
"Assignment to property of function parameter '{{name}}'.",
{name: identifier.name});
}
}
}

Expand Down
10 changes: 5 additions & 5 deletions tests/lib/rules/no-param-reassign.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,28 @@ ruleTester.run("no-param-reassign", rule, {
{
code: "function foo(bar) { bar.a = 0; }",
options: [{props: true}],
errors: [{ message: "Assignment to function parameter 'bar'." }]
errors: [{ message: "Assignment to property of function parameter 'bar'." }]
},
{
code: "function foo(bar) { bar.get(0).a = 0; }",
options: [{props: true}],
errors: [{ message: "Assignment to function parameter 'bar'." }]
errors: [{ message: "Assignment to property of function parameter 'bar'." }]
},
{
code: "function foo(bar) { delete bar.a; }",
options: [{props: true}],
errors: [{ message: "Assignment to function parameter 'bar'." }]
errors: [{ message: "Assignment to property of function parameter 'bar'." }]
},
{
code: "function foo(bar) { ++bar.a; }",
options: [{props: true}],
errors: [{ message: "Assignment to function parameter 'bar'." }]
errors: [{ message: "Assignment to property of function parameter 'bar'." }]
},
{
code: "function foo(bar) { [bar.a] = []; }",
parserOptions: { ecmaVersion: 6 },
options: [{props: true}],
errors: [{ message: "Assignment to function parameter 'bar'." }]
errors: [{ message: "Assignment to property of function parameter 'bar'." }]
}
]
});

0 comments on commit e54598a

Please sign in to comment.