Update severity of hints

[molant]

Severity of hints

Current status

Hints have an associated severity (off, warning, error) that can be set
via a .hintrc file when using the CLI. E.g.:

{
    "hints": {
        "hint-name": "error"
    }
}

The browser extension or the online scanner do not have the possibility to
change the severity. What's more, a lot of users rely in the provided
configurations (e.g.: configuration-web-recommended).

The problem with this is that it is difficult for developers to know what is
really important and should be fixed right away, and what is not as relevant:

If everything is important, then nothing is.

On top of that:

• there are hints that report several message and not all of them have the
  same importance
• some hints do not respect the user's severity for some of the messages

This RFC proposes significant changes to the way the severity is treated in
webhint so we provide more actionable feedback and help developers be more
succesfull.

Proposal

TL;DR;

Have hints report with the same severity accross all the different mediums
(cli, vs code, browser extension, and online scanner) by:

1. Changing the available severity levels for hints.
2. Making hints responsible of setting the severity for each report.
3. Letting users change only if a hint is enable or disable (plus some extra
   configuration in the applicable hints).

Hint severity level

The idea is to get closer to the severity options available in
VS Code:

Severity	Description	VS Code	Browser
off	Disabled, no reporting will come from it
error	Something that the user should get fixed right away		TBD
warning	Something that the developer should probably look into		TBD
hint*	Something minor to look into if you have time		TBD
information*	FYI for the user		TBD

The new values are hint and information.

Notes:

• We don't have to add 2 new severity levels, we could decide after reviewing
  the reports of the hints if it makes sense.
• In VS Code, Hint is less intrussive than Information. We will have to
  decide if we swap them or if we are ok as that (in the description about it
  doesn't quite match the expectations I think).

Hint severity examples

As mentioned previously, there are a few hints that report multiple things
were the relevance is not the same. An example would be http-compression
where we report about:

• Not using compression
• Using compression on binaries
• Not using brotli
• Not respecting Accept-Encoding: identity

Probably the first 2 should be error, brotli a warning or a hint and
the one about identity a hint or information.

Having all the reports with the same severity does not help the user focus on
what's important (have compression enabled for only text based resources
(see GitHub comment)

Another example will be with the compat-api hint.
Any reports coming from this hint are marked as a "warning" in the default
configurations. This will be the default with the new schema with the caveat
that if we feel highly confident a feature may cause a compatibility issue,
it could be upgraded to be an "error".

On the other side, the hint has a list of "ignored feautes". These are
features that are considered safe for graceful degradation (e.g. integrity
attribute) and do not trigger any report. When one of this features is found,
instead of ignoring it a report with the severity "hint" could be created.

There is also an on going conversation to
annotate features which can be used safely without support.
Having this information will help automate and make more accurate the
scenarios described above for compat-api.

User configuration

With the proposed changes, a user will only be able to decide which hints are
enabled or not. The user configuration will then look like this:

{
    "connector": {
        "name": "puppeteer",
        "options": {
            "waitUntil": "networkidle2"
        }
    },
    "hintsTimeout": 120000,
    "hints": {
        "amp-validator": "off",
        "apple-touch-icons": "on",
        "button-type": "off",
        "content-type": ["on", {
            ".*\\.js": "application/javascript; charset=utf-8"
        }],
        ...
        "no-vulnerable-javascript-libraries": "on"
    }
}

If the value of a hint key is an array, then the first member should be
"on|off" and the second and object with the specific configuration for
that hint.

Also, if the value is an object, webhint will assume the status is "on"
and that object will be the configuration for that hint.

Note: Please note that not all hints accept further configurations.

In the example above we are using content-type but other examples of hints
that accept configurations are: axe, http-compression, compat-api, etc.

As a shortcut, the property hints could accept an array of hint names that
will be turned on:

{
    "connector": {
        "name": "puppeteer",
        "options": {
            "waitUntil": "networkidle2"
        }
    },
    "hintsTimeout": 120000,
    "hints": [
        "apple-touch-icons",
        "content-type",
        ...
        "no-vulnerable-javascript-libraries"
    ]
}

Exit code and verbosity level

Because now hints will be responsible of setting the severity of each report,
the user will need a way to filter out the messages they are not concerned
about. To achieve that a new property minimumSeverity will be added to the
.hintrc:

{
    "connector": {
        "name": "puppeteer",
        "options": {
            "waitUntil": "networkidle2"
        }
    },
    "hintsTimeout": 120000,
    "hints": [
        "apple-touch-icons",
        "content-type",
        ...
        "no-vulnerable-javascript-libraries"
    ],
    "minimumSeverity": "error|warning|hint|information"
}

If the user selects error, all reports with a different severity will not be
outputed.
If they select warning, error and warning errors will be reported.
If a user selects warning and there are only warning reports, the
exit code should be different than 0.

How to decide the severity of a report

The guideline we are using to decide the severity is:

• Error - Will definitely not work as intended in at least one of the target browsers
• Warning - Minor impact or may not cause problems in practice (even if technically "wrong")
• Hint - Fine for now, but could cause problems in the future if something changes
• Information - Additional context or FYI, possibly to help with other parts of a feature

---

Initial discussion in #2968

[molant]

Keeping in mind that earlier this year we decided to have smaller PRs and that updating everything could take us quite a bit this is what I'm proposing. It might not be the most effective in terms of code (will have to add code that will be removed later) but I think it will allow us to keep working on other areas of the project while not breaking our users until it is really necessary.

1. Add support for the new schema while still supporting the old one. This will be a minor increment in the version. For a period of time that schema will not really work as hints will not support it.
2. Update hints to support the user given severity or the one we end up deciding. We can update hints individually. This should also be a minor patch because it's a new functionality. Not sure if we should update the documentation at this point 🤔
3. Work on the HTML formatter and VS Code and browser extensions and the new severity visualizations
4. Once we have all the hints updated will be time to break things:
   • Remove support for old schema
   • Remove path in hints for user defined severity (maybe we could have a util that we update first and then we work on removing the code as patch versions?) Actually this is not needed as the context will use the severity provided by the hint or the one configured by the user.
   • Update configurations to use new schema

I mentioned earlier hint documentation. We should probably update the docs to tell what's the severity of the hint so the user can easily know. Potentially we could have the severity (or the maximum severity?) as a meta property and use that for sorting the results in the different formatters (or maybe just browser extension and HTML formatter).

What do you think? Am I missing something?

@sarvaje @antross @digitarald