Add more disable error configuration

[nex3]

This makes it possible to add exceptions to disable errors, and to
configure their severity, using the same syntax as is used for lint rules.

[jeddy3]

@nex3 Thank you. It's a significant improvement to the reporting feature.

It's fantastic you could do this in a non-breaking way. It's also great that you were able to carry over the except pattern from the rules.

What's the CLI report* flags behaviour, i.e. does the presence of a flag set the primary option to true regardless of what's in the configuration object?

What are your thoughts on parity between the CLI flags and the configuration object, i.e. being able to specify primary and secondary options on the CLI, as the benchmarking tool does? My gut feeling is that we should move towards a configuration-object-first approach, and only add CLI flags where there's a concrete use case to justify the added complexity.

Lastly, two tests are failing:

 ● warns that the rule is deprecated

    expect(received).toHaveLength(expected)

    Expected length: 1
    Received length: 0
    Received array:  []

      16 |              const result = output.results[0];
      17 | 
    > 18 |              expect(result.deprecations).toHaveLength(1);
         |                                          ^
      19 |              expect(result.deprecations[0].text).toEqual(
      20 |                      `'${ruleName}' has been deprecated. Instead use 'property-allowed-list'.`,
      21 |              );

      at lib/rules/property-whitelist/__tests__/index.js:18:31

Test Suites: 1 failed, 1 total
Tests:       1 failed, 27 passed, 28 total

And the same for the property-blacklist rule. It fails locally for me too. Strangely, the failing tests seem unrelated to this pull request's changes. My only thought is that result is now being mutated somewhere and that both rules starting with "property" has something to do with it. On a quick scan through, I couldn't see where a mutation might happen, though.

[nex3]

What's the CLI report* flags behaviour, i.e. does the presence of a flag set the primary option to true regardless of what's in the configuration object?

Yes, exactly that. The CLI totally overrides the config file.

What are your thoughts on parity between the CLI flags and the configuration object, i.e. being able to specify primary and secondary options on the CLI, as the benchmarking tool does? My gut feeling is that we should move towards a configuration-object-first approach, and only add CLI flags where there's a concrete use case to justify the added complexity.

I agree with your gut. I don't think it's worth trying to add more JSON-parsing on the command line, especially since adding arguments to a flag that historically hasn't taken any is tricky from a compatibility standpoint.

Lastly, two tests are failing:

 ● warns that the rule is deprecated

    expect(received).toHaveLength(expected)

    Expected length: 1
    Received length: 0
    Received array:  []

      16 |              const result = output.results[0];
      17 | 
    > 18 |              expect(result.deprecations).toHaveLength(1);
         |                                          ^
      19 |              expect(result.deprecations[0].text).toEqual(
      20 |                      `'${ruleName}' has been deprecated. Instead use 'property-allowed-list'.`,
      21 |              );

      at lib/rules/property-whitelist/__tests__/index.js:18:31

Test Suites: 1 failed, 1 total
Tests:       1 failed, 27 passed, 28 total

And the same for the property-blacklist rule. It fails locally for me too. Strangely, the failing tests seem unrelated to this pull request's changes. My only thought is that result is now being mutated somewhere and that both rules starting with "property" has something to do with it. On a quick scan through, I couldn't see where a mutation might happen, though.

Looks like this was because the rules were being configured in those tests with the value [], which the revamped rule setting normalization now interprets as "unset". I fixed the settings so this should now work.

[jeddy3]

Looks like this was because the rules were being configured in those tests with the value [], which the revamped rule setting normalization now interprets as "unset".

Thank you for digging into this.

[] is a valid configuration for the *list as they set this flag:

rule.primaryOptionArray = true;

For the *-disallowed rules, [] is almost equivalent to unsetting; it won't trigger the deprecating warning, though. However, [] for the *-allowed rules means allow nothing, which is how a user can, for example, disallow all attribute selector operators.

Can the revamped rule setting normalisation take this into account somehow?

We mimicked ESLint's configuration structure, but in hindsight, we should've avoided array-based options as they're problematic.

---

Lint is failing on the TypeScript checks:

 Error: lib/invalidScopeDisables.js(19,23): error TS2532: Object is possibly 'undefined'.

[nex3]

Fixed and fixed!

[jeddy3]

Fixed and fixed!

Thank you. Conceptually everything looks great, and I think the pull request is ready for code reviews.

[jathak]

LGTM

[jathak]

Would it make sense to add a test where this is a regex? Or is this sufficiently covered by existing tests for optionsMatches?

[nex3]

It looks like other rules that allow RegExps don't explicitly test them, so I think it's probably covered.

[jeddy3]

LGTM, thanks!

I should be able to make time to do a release over the weekend. I believe @jathak has access to the stylelint npm (and can add you if you like) if you want to release before then.

[jeddy3]

• Added: exceptions and severity options to report* configuration object properties (#5143).