no-multi-assign option only when declaring variables.

[zepumph]

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"; // acceptable

What 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."
          } );
        }
      }
    };

  }
};

[kaicataldo]

Thanks for the proposal. Could you clarify what problem you’re trying to solve?

[zepumph]

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:

 var x = someObject.y = 'string value';

Where this would be allowed:

x = {};
y = {};
x.something = y.something = 'string value';

Currently there is no way to make the eslint rule no-multi-assign differentiate 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]

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]

Whoops, sorry, didn't mean to close this 😅

[zepumph]

Brainstorming a few more possibilities here:

• What about white list options that default to true? Since currently the rule supports two "modes" of sorts, failing out on assignment in declaration, and out of declaration, we could have two options that default to true, like:
  • {boolean} prohibitOnDeclaration=true
  • {boolean} prohibitOnAssignment=true
    Then you can opt out of either. That feels pretty robust, and could allow for future adaptation.
• (I think this is a worse idea) What about a single string option like {'onDeclaration'|'onAssignment'|'all'} specificationMode='all'. This would be more prohibitive and less future proof.

[kaicataldo]

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):

{
  VariableDeclaration: boolean;
  AssignmentExpression: boolean;
}

[kaicataldo]

I'll champion this. Let's see what the rest of the team has to say.

[mdjermanovic]

This rule is in the Stylistic Issues category. In my opinion:

• a = b = 1; is mostly a stylistic issue.
• var a = b = 1; is more a possible error than a stylistic issue, as it looks like the intention to declare b as well.

An error on b will be usually reported by no-undef (and no-implicit-globals if enabled). But if it happens that b already exists in an outer scope, no-multi-assign would be the only rule to report this.

So I'm 👍 for an option to disable this check in statements as a user's preference because it allows the user to keep the rule enabled to report possible errors in declarations.

[zepumph]

Thanks for looking into it you two!

[anikethsaha]

+1 for ignoreNonDeclaration

[anikethsaha]

Marking this as accepted as there are 3 👍 from team members.

From above comments, marking this with ignoreNonDeclaration as an option name.

[anikethsaha]

@zepumph you can submit a PR if you want else let me know.

[t-mangoe]

@anikethsaha @zepumph
Hi. I'm working on this issue.
If you like, can I send pull request?

I added ignoreNonDeclaration option, and made it possible to accept multiple assignments if the variable has already been declared.

[t-mangoe]

I will try sending a pull request.