[no-magic-numbers] - Add an option to omit numbers check in array declaration

[avalanche1]

What rule do you want to change?
no-magic-numbers
Does this change cause the rule to produce more or fewer warnings?
less
How will the change be implemented? (New option, new default behavior, etc.)?
new option to skip showing error
Please provide some example code that this change will affect:

const arr = [0, 1, 2];

also here - #11383
What does the rule currently do for this code?
shows error
What will the rule do after it's changed?
not show error
Are you willing to submit a pull request to implement this change?
nope

[kaicataldo]

I still feel the same way I did in this comment. This seems like the exact scenario the rule is designed to warn on, and in cases where the code author doesn’t think it’s necessary, they can add a disable comment.

Do you mind expanding on what problem you’re trying to solve and why this should be added to the core rule?

[avalanche1]

Use cases:

• I want to hard-code a data array. It is comprised of certain numbers. Why should I explain each number to eslint?
• I want to iterate [1, 2, 3] in my React component tree. It's easier to write [1, 2, 3].map(...) then Array.from({ length: 3 }, (v, i) => i+1) and way more understandable

I would rather much like to know why it isn't there by default. And who might want to do this???

const foo = 1
const bar = 2
const baz = 3
const arr = [foo, bar, baz]

[kaicataldo]

I want to hard-code a data array. It is comprised of certain numbers. Why should I explain each number to eslint?

This is the purpose of the no-magic-numbers rule. If you don't want to have to do this, it might be best to turn the rule off. I don't use this rule in my personal projects because I also don't like having to name every number.

I would rather much like to know why it isn't there by default. And who might want to do this???

I don't think it can be the default, because this is quite literally the purpose of the rule. The case you have outlined makes it a clear sequence that doesn't need explanation, but consider an array of numbers like: const arr = [1529, 20000, 23145];. It's not clear what these numbers mean - they're "magic numbers". ESLint can't know if it's a sequence that is easily identifiable by humans and therefore doesn't need an explanation, so we have to warn on all cases.

[avalanche1]

I don't use this rule in my personal projects because I also don't like having to name every number.

We do use it in production project and it helps a lot with stuff like const result = calculate_area(10, 20); reports errors and we fix it with

const widht = 10;
const height = 20;
const result = calculate_area(widht, height);

(5 min later) Wait a second! I've just consulted with guys and it looks like this rule also reports errors with code like this (which is currently our code style of choice):

const [widht, height] = [10, 20];
const result = calculate_area(widht, height);

This must definitely be fixed!!

I don't think it can be the default, because this is quite literally the purpose of the rule. The case you have outlined makes it a clear sequence that doesn't need explanation, but consider an array of numbers like: const arr = [1529, 20000, 23145];. It's

Okay, let's not make it default. I'm rooting here for an option to disable this check for the arrays only. And what you have put as an example is a perfect use case. I agree, const arr = [1529, 20000, 23145]; looks puzzling; but a small change - const studyResultsDistribution = [1529, 20000, 23145]; makes it totally clear what the numbers are. (I repeat, there are cases when we want to hard-code numbers).
And it is also cumbersome for us to put // eslint-disable-next-line no-magic-numbers comment, because according to our Style Guide, if we insert eslint-disable comments - we have to put another line above, describing why it's been disabled. Like this:

// Explain: Disabled because there's no option to omit checking of arrays.
// eslint-disable-next-line no-magic-numbers
const [widht, height] = [10, 20]

This yields two unnecessary code lines and this weighs heavily on our other rule - max-lines, which imposes a restriction on total lines in a module and includes comments in its count (we don't appreciate verbose many-lines-spanning comments).

I will repeat: We do not wish to disable this rule altogether, because it helps A LOT.

[kaicataldo]

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

This case being reported does feel like a bug to me, because these are named. I agree it would be nice to update this rule to check for hard-coded numbers that are assigned using destructuring (both array and object).

[mdjermanovic]

And it is also cumbersome for us to put // eslint-disable-next-line no-magic-numbers comment, because according to our Style Guide, if we insert eslint-disable comments - we have to put another line above, describing why it's been disabled. Like this:

// Explain: Disabled because there's no option to omit checking of arrays.
// eslint-disable-next-line no-magic-numbers
const [widht, height] = [10, 20]

A side note, it's in plan for ESLint v7.0.0 to support descriptions in directive comments:

// eslint-disable-next-line no-magic-numbers -- there's no option to omit checking of arrays
const [widht, height] = [10, 20];

[mdjermanovic]

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

This case being reported does feel like a bug to me, because these are named.

Maybe we could discuss this particular pattern separately, as it might make sense to allow this by default without any options?

[avalanche1]

@mdjermanovic #12892

[avalanche1]

Still me and my team would like to have an option for arrays as I initially requested.

[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 issues failing to reach accepted status after 21 days tend to
never be accepted, 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.

[alejandroclaro]

My two cents in this issue.

We have a similar problem. We need to integrate with communication that require sending binary data. Most of the commands are just a set of bytes that are fixed. For example:

const moveCommand = [ 0xff, 0xcf, 0x00, 0xee, 0x12, 0x15 ]

Of course each byte has no meaning. So make no sense trying to give them names. Besides, we have very long commands and many of them.

This happens only in some classes in a big system, but we also have may of these classes. We would like to make sure we detect magic number, but also we don't want to populate the code with comment to silence this rule in these cases.

[mdjermanovic]

@alejandroclaro I think it isn't likely that this option will reach consensus, given that this issue had one 👎 and no 👍 from team members.

As an alternative, you can use eslint-rule-composer to easily make a custom rule that extends the no-magic-numbers rule and filters out array elements:

const ruleComposer = require('eslint-rule-composer');
const eslint = require('eslint');

const noMagicNumbersRule = new eslint.Linter().getRules().get('no-magic-numbers');

module.exports = ruleComposer.filterReports(
  noMagicNumbersRule,
  problem => problem.node.parent.type !== 'ArrayExpression'
);