Description
What rule do you want to change?
no-multi-assign
https://eslint.org/docs/rules/no-multi-assign
Does this change cause the rule to produce more or fewer warnings?
For this rule, it produces fewer.
How will the change be implemented? (New option, new default behavior, etc.)?
I think it should be an option, something like onlyForVariableDeclation of type {boolean}. When true, no-multi-assign will only trigger when multi-assign occurs when a variable is being declared. That said. I don't have a firm grasp on all the possible option schemas available, and I'm sure there is likely a better one for this case.
Please provide some example code that this change will affect:
// incorrect usage `no-multi-assign` with { "onlyForVariableDeclation": true }
const tree = { numberOfLeaves: 9 };
const numberOfLeaves = tree.numberOfLeaves = 10; // would error out
// correct usage `no-multi-assign` with { "onlyForVariableDeclation": true }
const x = {};
const y = {};
x.hi = y.hi = "salutations"; // acceptableWhat does the rule currently do for this code?
It errors out both when declaring variables in the statement as well as when no declaring.
What will the rule do after it's changed?
You will have the option to only error out when declaring variables
Are you willing to submit a pull request to implement this change?
I am, though this is likely a simple change, and I have not set up the eslint dev environment before. If someone else feels passionately, then likely I won't be the most efficient implementer.
Here is a copy of the rule, outfitted with a first pass at the changes, for review:
no-multi-assign.js with added option
/**
* @fileoverview Rule to check use of chained assignment expressions
* @author Stewart Rand
*/
"use strict";
//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
module.exports = {
meta: {
type: "suggestion",
docs: {
description: "disallow use of chained assignment expressions",
category: "Stylistic Issues",
recommended: false,
url: "https://eslint.org/docs/rules/no-multi-assign"
},
schema: [ {
type: "object",
onlyForVariableDeclation: {
type: "boolean",
default: false
}
}
]
},
create( context ) {
const options = context.options[ 0 ] || {};
const onlyForVariableDeclation = options.onlyForVariableDeclation;
const disallowedParents = [ "VariableDeclarator" ].concat( onlyForVariableDeclation ? [] : [ "AssignmentExpression" ] );
//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------
return {
AssignmentExpression( node ) {
if ( disallowedParents.indexOf( node.parent.type ) !== -1 ) {
context.report( {
node,
message: "Unexpected chained assignment."
} );
}
}
};
}
};
Activity
kaicataldo commentedon Nov 9, 2019
Thanks for the proposal. Could you clarify what problem you’re trying to solve?
zepumph commentedon Nov 11, 2019
Yes of course.
The project I work for called PhET Interactive Simulations (https://github.com/phetsims/) has decided on code style that allows multi-assign when when not declaring a variable, but disallows multi-assign when declaring.
So this would be flagged as an error:
Where this would be allowed:
Currently there is no way to make the eslint rule
no-multi-assigndifferentiate between those two, but when I looked at the rule source code, it looks like it could be straight forward to supply only one or the other (since each has a different node parent type), potentially based on a boolean option.kaicataldo commentedon Nov 14, 2019
Thanks for the explanation. This seems like a reasonable request to me, though I wonder if we can come up with an option name that's more along the lines of
ignoreNonDeclaration- i.e. adding it as a black list options rather than a white list. Implementation-wise, it doesn't make a difference, but it prevents us from backing ourselves into a corner if we add options in the future.kaicataldo commentedon Nov 14, 2019
Whoops, sorry, didn't mean to close this 😅
zepumph commentedon Nov 14, 2019
Brainstorming a few more possibilities here:
{boolean} prohibitOnDeclaration=true{boolean} prohibitOnAssignment=trueThen you can opt out of either. That feels pretty robust, and could allow for future adaptation.
{'onDeclaration'|'onAssignment'|'all'} specificationMode='all'. This would be more prohibitive and less future proof.kaicataldo commentedon Nov 15, 2019
I think that generally makes more sense and falls in line with some of other rules. We could create flags with the AST node type as the key (both defaulting to
true):22 remaining items