[no-magic-numbers] - false error report on array destructuring declaration

[avalanche1]

Tell us about your environment

Node version: v12.12.0
npm version: v6.9.0
Local ESLint version: v6.8.0 (Currently used)
Global ESLint version: Not found

What parser (default, Babel-ESLint, etc.) are you using?
typescript 3.8
Please show your full configuration:

Configuration

env:
  es2020: true
  browser: true
extends:
  - "eslint:recommended"
  - "plugin:@typescript-eslint/recommended"
  - "plugin:@typescript-eslint/eslint-recommended"
  - "plugin:functional/all"
  - "plugin:jest/recommended"
  - "plugin:react/recommended"
  - "plugin:no-unsanitized/DOM"
  - "prettier"
  - "prettier/@typescript-eslint"
parser: "@typescript-eslint/parser"
parserOptions:
  #  project: "./tsconfig.json"
  sourceType: module
plugins:
  - "@typescript-eslint/eslint-plugin"
  - functional
  - jest
  - no-unsanitized
  - promise
  - react
  - react-hooks
overrides:
  - files:
      - "*.test.js"
      - "*.spec.js"
globals:
  globalThis: "writable"
#  devprint: "readonly"
rules:
  "@typescript-eslint/camelcase": off
  "@typescript-eslint/class-name-casing":
    - off
  "@typescript-eslint/explicit-function-return-type":
    - error
    - allowExpressions: true
  "@typescript-eslint/interface-name-prefix":
    - warn
  "@typescript-eslint/no-empty-interface":
    - error
    - "allowSingleExtends": true
  "@typescript-eslint/no-use-before-define": off
  arrow-body-style: "off"
  arrow-parens: warn
  camelcase: off
  comma-dangle: "off"
  complexity:
    - warn
    - 3
  func-style: warn
  functional/functional-parameters:
    - error
    - enforceParameterCount: false
  functional/immutable-data:
    - error
    - ignorePattern:
        - globalThis
  functional/prefer-readonly-type: off
  functional/no-conditional-statement: off
  functional/no-expression-statement:
    - off
    - ignorePattern:
        - console
        - devprint
        - globalThis
        - render

  functional/no-return-void: off
  id-length:
    - warn
    - {min: 3, exceptions: [_, fn, id, is, BG]}
  id-match:
    - error
    - "^[a-zA-Z_0-9]*$"
    - properties: true
  indent: off
  lines-around-comment:
    - warn
    - ignorePattern: /prettier-ignore/
  max-len:
    - warn
    - code: 90
      comments: 120
      tabWidth: 2
      #      "^\\} from" part is for multi-line imports
      ignorePattern: "Spec|import|^\\} from"
      ignoreUrls: true
  max-lines:
    - warn
    - max: 50
      skipBlankLines: true
  max-params:
    - warn
    - 6
  max-statements:
    - warn
    - 10
  multiline-ternary: "off"
  new-cap: off
  no-console: warn
  no-inline-comments: "off"
  no-magic-numbers:
    - error
    - ignore:
        - 0
        - 1
        - -1
  no-multi-spaces: "off"
  no-param-reassign:
    - error
    - props: true
  no-nested-ternary: "off"
  no-return-await: warn
  no-shadow:
    - error
    - builtinGlobals: true
      allow:
        - "name"
        - "status"
  no-warning-comments: off
  no-undef: error
  no-underscore-dangle:
    - error
    - "allow": ["_id", "_text"]
  no-unneeded-ternary: "off"
  no-unused-vars: warn
  no-use-before-define: "off"
  no-useless-catch: warn
  object-curly-newline: "off"
  one-var: off
  operator-linebreak:
    - warn
    - before
    - overrides:
        +: ignore
        =: ignore
  padding-line-between-statements: "off"
  prefer-destructuring: warn
  radix: "off"
  react/no-string-refs:
    - error
    - noTemplateLiterals: true
  react-hooks/rules-of-hooks: error
  react-hooks/exhaustive-deps: warn
  react/display-name: "off"
  react/prop-types: "off"
  semi: warn
settings: {}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

const [widht, height] = [10, 20];

I use Webstorm's built-in eslint reporting service

What did you expect to happen?
No errors reported

What actually happened? Please include the actual, raw output from ESLint.
No magic number: 10. (no-magic-numbers)

Are you willing to submit a pull request to fix this bug?
noooo...

[yeonjuan]

Thanks for reporting!

• reproduced - Online Demo

In my opinion, It makes sense to treat it as a bug because all numbers are named.

[mdjermanovic]

Something like this might be correct:

/* eslint no-magic-numbers: ["error", { "enforceConst": false }] */

const [a, b] = [1, 2]; // ok

let [a, b] = [1, 2]; // ok

var [a, b] = [1, 2]; // ok

[a, b] = [1, 2]; // error

/* eslint no-magic-numbers: ["error", { "enforceConst": true }] */

const [a, b] = [1, 2]; // ok

let [a, b] = [1, 2]; // error

var [a, b] = [1, 2]; // error

[a, b] = [1, 2]; // error

/* eslint no-magic-numbers: ["error", { "detectObjects": false }] */

[obj.a, obj.b] = [1, 2]; // ok

/* eslint no-magic-numbers: ["error", { "detectObjects": true }] */

[obj.a, obj.b] = [1, 2]; // error

We could find the assignment target for the number, and if it's a MemberExpression then allow/disallow based on detectObjects.

If it's an Identifier, then allow only if the assignment is initializing variable (VariableDeclarator, which must be in a const declaration if "enforceConst": true).

Perhaps we could do this for object patterns as well, maybe even support nested destructuring.

There is also proposal #12611 to optionally allow default assignments.

[yeonjuan]

Is someone working on it? if not, I'll try it. :)

[kaicataldo]

@yeonjuan Go for it!

Edit: Looks like we still need to finalize the design. I think this should be put behind an option (maybe enforceDestructuredValues)?

[yeonjuan]

@kaicataldo

Thanks for replying :) yep I agree, It needs to be designed first. 👍
And I'm not fully sure it really needs an option or not, because I was thought that it can be allowed by default without any option.

I was thought it should just work as @mdjermanovic mentioned. :) - but has no strong preference both (option or by default)

[mdjermanovic]

I also think there is no need for an option in this case.

Maybe there could be another rule to control whether the [] = [] pattern to set/initialize multiple variables in the same assignment is allowed or not (not just for numbers).

[kaicataldo]

I don't have a strong opinion either, and do support this change 👍.

[avalanche1]

I also think there is no need for an option in this case.

I second this.

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[avalanche1]

hmm.. I think there was enough activity here to not auto-close

[mdjermanovic]

Reopening since this is an accepted issue, we just need to figure out the design.

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.