Feature Request: Summary of rules which emitted diagnostics

[Conaclos]

While I was testing Rome linter on the TypeScript Compiler code base, I noticed that the UX for adopting the Rome linter is not so good: The maximum number of diagnostics is easily reached, and it is hard to know which rule to keep or to disable.

I have several suggestions to improve the situation:

1. Add a summary of which rule emitted a diagnostic and the number of diagnostic it emitted.
2. Add a CLI filter to display the diagnostics of a given rule.
3. Add a CLI command to ignore a rule (this automatically change the Rome config file).

I will open a comment for each suggestion.

These features could help a project to adopt Rome.
This is also valuable to test Rome against big code base to detect false positives or pedantic rules.

[Conaclos]

1. Add a summary of which rule emitted a diagnostic and the number of diagnostic it emitted.

When I run rome lint <directory>, the command outputs some diagnostics.
On a big codebase, the number of diagnostics easily reach thousands.

A first step is to identify which rule make sense in the repository.
There is currently no way to know exactly which rule reported at least one diagnostics and how many diagnostics was emitted for a rule.

I suggest adding a summary at the end of the output of rome lint (and others commands like rome check?).
For example:

$ rome lint src
...
src/services/types.ts:1121:5 lint/nursery/useNamingConvention ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ This property name should be in camelCase.
  
  > 1121 │     InsertSpaceAfterConstructor?: boolean;
         │     ^^^^^^^^^^^^^^^^^^^^^^^^^^
  
  ℹ The name could be renamed to `insertSpaceAfterConstructor`.


The number of diagnostics exceeds the number allowed by Rome.
Diagnostics not shown: 7564.
Checked 657 file(s) in 3s
Skipped 2 file(s)

Summary:

Diagnostic number    Rule name
2509                 lint/nursery/useNamingConvention
2000                 lint/style/useSingleCaseStatement
...

Another possibility is adding a CLI flag --summary and suggesting it when the number of diagnostic is high:

$ rome lint src
...
src/services/types.ts:1121:5 lint/nursery/useNamingConvention ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ This property name should be in camelCase.
  
  > 1121 │     InsertSpaceAfterConstructor?: boolean;
         │     ^^^^^^^^^^^^^^^^^^^^^^^^^^
  
  ℹ The name could be renamed to `insertSpaceAfterConstructor`.


The number of diagnostics exceeds the number allowed by Rome.
Diagnostics not shown: 7564.
Checked 657 file(s) in 3s
Skipped 2 file(s)

Run `rome lint --summary src` to print a summary of the rules that emitted diagnistics.

$ rome lint --summary src
Summary:

Diagnostic number    Rule name
2509                 lint/nursery/useNamingConvention
2000                 lint/style/useSingleCaseStatement
...

However, this could require running a second time the linter.

[ematipico]

I like it! Although, I would implement it only for the command lint. It would make this command more specialized for running lint rules, while the check command has a broader reach.

However, this could require running a second time the linter.

I think we don't need a second pass. Our diagnostics already have all the information we need process this output.

I would implement this using --summary, because this could become quite lengthy, considering that our number or rules grows with the time.

[Conaclos]

2. Add a CLI filter to display the diagnostics of a given rule.

The summary (see 1.) helps to identify the rules that report diagnostics.
We should provide a way to quickly display some diagnostics of a rule to look at some code.
This could help to identify whither the rule is relevant for the code base and how much refactoring it requires.

I suggest adding a new option --only-lint-rule (or a better name...).
For instance:

$ rome lint --only-lint-rule useSingleCaseStatement src
...
src/services/types.ts:1121:5 lint/nursery/useNamingConvention ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ⚠ This property name should be in camelCase.
  
  > 1121 │     InsertSpaceAfterConstructor?: boolean;
         │     ^^^^^^^^^^^^^^^^^^^^^^^^^^
  
  ℹ The name could be renamed to `insertSpaceAfterConstructor`.


The number of diagnostics exceeds the number allowed by Rome.
Diagnostics not shown: 2509.
Checked 657 file(s) in 3s
Skipped 2 file(s)

[ematipico]

How would this new argument work with the --summary argument that you proposed?

[Conaclos]

Not sure.

I think we should either keep the behavior of summary, i.e. printing the summary of all triggered rules (regardless which rule is filtered), or we should make the two options mutually exclusive.

[Conaclos]

3. Add a CLI command to ignore a rule (this automatically change the Rome config file).

Once I detected a rule that is too pedantic / too noisy in the current state of the code base, I could like to have a way to easily update the config. Something like:

rome config --disable lint/nursery/useNamingConvention

[Conaclos]

I am not 100% sure about this suggestion.
This could be a sign that the current config is maybe too verbose / too complex.

[ematipico]

The rome config command is something that we plan to do, but it requires some other works in order to make it achievable. I would exclude it from this proposal for now.