feat: Introduce baseline system to suppress existing errors

[softius]

Summary

Declare currently reported errors as the "baseline", so that they are not being reported in subsequent runs. It allows developers to enable one or more rules as error and be notified when new ones show up.

Related Issues

• Change Request: Introduce a system to suppress existing errors

[linux-foundation-easycla]

• ✅login: softius / (a99c456)
• ✅login: softius / (a99c456, f9b3976)

The committers listed above are authorized under a signed CLA.

[bradzacher]

the issue with using a count is that it's possible to change the file without changing the count - i.e. you add as many new reports as you remove.

EG in this example the code has meaningfully changed, but the total number of reports is unchanged. So the file would not violate the baseline.

/// before

/* eslint no-console: error */
export function test(condition) {
  if (condition) {
    console.log();
  } else {
    return 1;
  }
}

/// after

/* eslint no-console: error */
export function test(condition) {
  if (condition) {
    return 1;
  } else {
    console.log();
  }
}

personally I've preferred more granular checks - eg the line + col as it's stricter. It's more likely to false-positive, but it's less likely to false-negative (which is more valuable, IMO).

[ondrejmirtes]

Hey, PHPStan creator here (where the baseline is implemented for the past almost 5 years). It's the most popular feature by far (yeah, the most popular feature of a static analyser is ignoring errors).

Turns out that the count is the right amount of granularity. Expecting "having this error in this file this many times is okay" is really useful. If you included the line number in the baseline entry, it'd get obsolete really fast. It'd essentially be an inline ignore, but in an external file. Not really practical.

If someone manages to remove an error but add it in a different place in the same file, good for them. Having a baseline means you're ignoring some errors on purpose, and this situation most likely means it's the same error, it just moved.

PHPStan is a little bit more granular becauses the baseline includes exact error messages being ignored, but this proposal only includes error types. Maybe that's something to consider.

An alternative approach that preserves line numbers would be to also include the git commit hash from the moment the baseline was generated. So "this error on line 12 in commit 34afddac". This would allow shifting expected errors around based on git diff. So three commits later, if there was a new line added on line 5, the baseline would expect the error to occur on line 13.

But that can become computationally expensive as the diff between the original commit and HEAD grows bigger (if the baseline is not regenerated for a long time).

Anyway, I'm really excited other programming tools to adopt the baseline concept, it's really useful!

[softius]

From my point view, the baseline is an "agreement" between the developers and eslint that it is acceptable to have X number of errors, from a particular rule, for a particular file.

You are both right @ondrejmirtes @bradzacher , someone could fix one error and create another one or move it to a different line and this won't be reported. Personally, I don't see that as a problem. If that is necessary, maybe it can be resolved at the git / ci/cd level similarly to the suggestion provided by @bradzacher below.

[bradzacher]

the alternative we've introduced at Canva is that we designate specific rules as "in migration" and we only consider reports from those rules if they exist in changed files (according to git comparison against the main branch).

With this system developers must address lint errors if they touch a file but otherwise they can be ignored.

This does require integration with the relevant source control system - though we've found it works quite well.

[fasttime]

Thanks for the RFC @softius. Can you please sign the CLA as requested in this comment?

[ondrejmirtes]

In case of PHPStan, this flag optionally accepts a filename. In some cases people generate multiple different baselines. It allows you to have a different baseline from the points in time where you enabled different rules.

It'd also mean that these baseline files would have to be referenced from the main eslint config because they couldn't be auto-discovered anymore. That's a potential downside.

[softius]

Yes, exactly. I was planning to include this initially in the RFC but I decided to leave it out eventually, mostly to avoid expanding the scope.

[EliSchleifer]

This baseline approach is also used by rubocop. The problem with these approaches is the baseline files have to be managed by humans and kept up to date.

As we built - our hold-the-line feature built into trunk check. Effectively does this dynamically. So nothing has to be maintained by humans as rules get enabled. Everything get's grandfathered in by any tool that integrates with our runner.

[ondrejmirtes]

• @EliSchleifer It's impressive but I don't like it. Means the user doesn't have insight into pre-existing issues and cannot work to make the issues go away (make the baseline smaller, pay technical debt). Unless I'm missing something you either make the tool to have to look at diffs for the rest of your life, or turn the feature off one day and fix all the pre-existing issues at once (which will never happen).