numeric-separators-style: Add onlyIfContainsSeparator option

[noftaly]

Added an option for each literal type, named checkOnlyIfSeparator.
As explained in the docs, it:
Check if the group sizes are valid only if there are groups, separated with an _.

Example:

const foo = 100000; // Pass, because there are no groups, so no check was performed.
const bar = 1_000_000; // Pass, because the group sizes are correct.
const baz = 10_0_000_0; // Fail, because there are groups, but they are incorrect.

This option is disabled by default.

Closes #868

[fisker]

I'm not sure if we should add this option, but you didn't test 12_34.5678e1234.5678, should we ignore all, or just part?

[noftaly]

Considering it has a separator, I think the entire number should be checked, to be more consistent. Do you want me to add a test for more "complicated" numbers? I did not do any because the option short-circuits the whole check so it will never get to the decimal and exponential part.

I'm not sure if we should add this option

No problem, I did not know if I should wait for a "help-wanted" label, so I just did it anyway 😄 Don't hesitate to close the PR if you don't think it should be added.

[sindresorhus]

While I wouldn't personally use this option, I can see how it would be useful if you want to enforce "correctness", but not style. We could use it in, for example, the unopinionated preset (#896).

[sindresorhus]

Considering it has a separator, I think the entire number should be checked, to be more consistent.

👍

[sindresorhus]

@fisker Are you ok with merging this?

[fisker]

Yes, I'm fine with this, but I think the option can have a better name, like onlyIfSeparatorExists or onlyIfSeparatorPresents?

And do we want a top level one to easier set for all?

Also, need more tests on different options, and seperator in different position of 1234.5678e1234

[sindresorhus]

onlyIfSeparatorExists

👍

[sindresorhus]

Alternatively: onlyIfContainsSeparator

[sindresorhus]

And do we want a top level one to easier set for all?

👍

[noftaly]

And do we want a top level one to easier set for all?

Just to be sure, do you want only one at the top level, or one for each types and a "master" setting at the top level that overrides other settings if set?

[fisker]

Both, if top one is set, all default to it.

[noftaly]

Both, if top one is set, all default to it.

Doesn't it add too many complexity and redundancy? I thought of doing that too but I think it might confuse the user and clutter the options...

[fisker]

{
  onlyIfContainsSeparator: false, // default, can  be  omitted
  binary: {
    onlyIfContainsSeparator: true
  }
}

{
  onlyIfContainsSeparator: true,
  binary: {
    onlyIfContainsSeparator: false
  }
}

Pretty clear to me, think this way you can set onlyIfContainsSeparator at top level, but can be override on each.

[noftaly]

Okay, I'll do that then 👍🏼

[noftaly]

Just curious about the actions, why do the lint errors appear in the build job and not in the lint job? (cf my last failing commit)

[fisker]

lint is not really lint, maybe we need rename it, it's run rules on codebase, 😄

[fisker]

I changed the logic a little bit, ebf5aee, I think this is more reasonable.