Skip to content

Commit a99c456

Browse files
softiussoftius
committed
Add RFC for baseline support
1 parent b67bec5 commit a99c456

File tree

1 file changed

+201
-0
lines changed

1 file changed

+201
-0
lines changed

‎designs/2024-baseline-support/README.md

+201
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
- Repo: eslint/eslint
2+
- Start Date: 2024-04-20
3+
- RFC PR: (leave this empty, to be filled in later)
4+
- Authors: [Iacovos Constantinou](https://github.com/softius)
5+
6+
# Introduce baseline system to suppress existing errors
7+
8+
## Summary
9+
10+
<!-- One-paragraph explanation of the feature. -->
11+
12+
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.
13+
14+
## Motivation
15+
16+
<!-- Why are we doing this? What use cases does it support? What is the expected
17+
outcome? -->
18+
19+
Enabling a new lint rule as `error` can be painful when the codebase has many violations and the rule isn't auto-fixable. A good example, is `no-explicit-any`. Unless the rule is enabled at the early stages of the project, is becoming harder and harder to enable it as the codebase grows. Existing violations must be resolved before enabling the rule, but while doing that other violations might creep in.
20+
21+
This can be counterintuitive for enabling new rules as `error`, since the developers need to address the violations before-hand in one way or another. The suggested solution suppress existing violations, allowing the developers to address these at their own pace. It also reports any new violations making it easier to identify and address future violations.
22+
23+
## Detailed Design
24+
25+
<!--
26+
This is the bulk of the RFC.
27+
28+
Explain the design with enough detail that someone familiar with ESLint
29+
can implement it by reading this document. Please get into specifics
30+
of your approach, corner cases, and examples of how the change will be
31+
used. Be sure to define any new terms in this section.
32+
-->
33+
34+
The suggested solution introduces the concept of a baseline file; A JSON file containing the number of errors that must be ignored for each rule in each file. By design, the baseline is disabled by default and it doesn't affect existing or new projects, unless the baseline file is generated.
35+
36+
Here is how the baseline file looks like. This indicates that the file `"src/app/components/foobar/foobar.component.ts"` has one error for the rule `@typescript-eslint/no-explicit-any` that is acceptable to be ignored.
37+
38+
```
39+
{
40+
"src/app/components/foobar/foobar.component.ts": {
41+
"@typescript-eslint/no-explicit-any": 1
42+
}
43+
}
44+
```
45+
46+
### Generating the baseline
47+
48+
A new option `--generate-baseline` can be added to ESLint CLI. When provided, the baseline is generated and saved in `.eslint-baseline.json`. If the file already exists, it gets over-written. Note that this is a boolean flag option (no values are accepted). For example:
49+
50+
``` bash
51+
eslint --generate-baseline ./src
52+
```
53+
54+
The above will go through each result item and rules, and count the number of errors ( `severity == 2` ). If one or more such errors are found, the necessary details must be stored in the baseline file. The process should take place right after the fixes are applied to avoid counting errors that are about to be fixed.
55+
56+
### Matching against the baseline
57+
58+
The suggested solution always compares against the baseline, given that the baseline file `.eslint-baseline.json` exists. This makes it easier for existing and new projects to adopt the baseline without the need to adjust scripts in `package.json` and CI/CD workflows.
59+
60+
This will go through each result item and rules, and check each rule against the baseline. If the file and rule are part of the baseline, we decrease the number of errors to be ignored (in memory), and remove the result item. This needs to take place after the fixes are applied so that we compare the non-fixable issues only. It also needs to take place before the error counting, so that the remaining errors are counted correctly.
61+
62+
### Maintaining a lean baseline
63+
64+
When using the baseline, there is a chance that an error is resolved but the baseline file is not updated. This might allow new errors to creep in without noticing. To ensure that the baseline is always up to date, eslint can exit with an error code when there are ignored errors that do not occur anymore. To fix this, the developer can regenerate the baseline file.
65+
66+
### Caching
67+
68+
Caching must contain the full list of detected errors, even those matched against the baseline. This approach has the following benefits:
69+
70+
- Generating the baseline can be based on the cache file and should be faster when the cache file is used.
71+
- Allows developers to re-generate the baseline or even adjust it manually and re-lint still taking the same cache into consideration.
72+
- It even allows developers to delete completely the baseline and still take advantage of the cached file in subsequent runs.
73+
74+
## Documentation
75+
76+
<!--
77+
How will this RFC be documented? Does it need a formal announcement
78+
on the ESLint blog to explain the motivation?
79+
-->
80+
81+
We should update [Command Line Interface Reference](https://eslint.org/docs/latest/use/command-line-interface) to document the newly introduced option. A dedicated section should be added in Documentation to explain how baseline works.
82+
83+
## Drawbacks
84+
85+
<!--
86+
Why should we *not* do this? Consider why adding this into ESLint
87+
might not benefit the project or the community. Attempt to think
88+
about any opposing viewpoints that reviewers might bring up.
89+
90+
Any change has potential downsides, including increased maintenance
91+
burden, incompatibility with other tools, breaking existing user
92+
experience, etc. Try to identify as many potential problems with
93+
implementing this RFC as possible.
94+
-->
95+
96+
The baseline can be generated and used only when linting files. It can not be leveraged when using `stdin` since it relies on file paths.
97+
98+
## Backwards Compatibility Analysis
99+
100+
<!--
101+
How does this change affect existing ESLint users? Will any behavior
102+
change for them? If so, how are you going to minimize the disruption
103+
to existing users?
104+
-->
105+
106+
If the baseline file is not generated, ESLint CLI behavior will not change. This change is therefore backwards compatible to start with.
107+
108+
If the baseline file is generated, ESLint CLI will compare the errors against the errors included in the baseline. Hence it might report less errors than before and someone might argue that this is not backwards compatible since the behavior changes for them. However, as discussed earlier this should facilitate the adoption of the baseline without worrying about adjusting scripts in `package.json` and CI/CD workflow. Plus, the baseline can be easily deleted and cancel the new behavior.
109+
110+
Furthermore, we are adding one more reason to exit with an error code (see "Maintaining a lean baseline"). This might be have some negative side-effects to wrapper scripts that assume that error messages are available when that happens. We could introduce a different exit code, to differentiate between exiting due to unresolved errors or ignored errors that do not occur anymore.
111+
112+
## Alternatives
113+
114+
<!--
115+
What other designs did you consider? Why did you decide against those?
116+
117+
This section should also include prior art, such as whether similar
118+
projects have already implemented a similar feature.
119+
-->
120+
121+
Unfortunately existing approaches do not address the issue at its core and come with their own set of drawbacks. It is worth mentioning that the suggested solution is based on [how baseline works in PHPStan](https://phpstan.org/user-guide/baseline).
122+
123+
The following sections are extracted from [Change Request: Introduce a system to suppress existing errors](https://github.com/eslint/eslint/issues/16755) where [@jfmengels](https://github.com/jfmengels) did a detailed analysis about existing approaches and their drawbacks.
124+
125+
### Using warnings
126+
127+
This use-case is apparently what the "warn" severity level is for.
128+
129+
A large problem with warnings is that as soon as there are more than a few warnings, you don't notice new ones showing up. A common practice I've seen quite often is to avoid warnings altogether, and to only use errors to avoid new problems creeping in. But that doesn't solve the problem of all the existing errors.
130+
131+
Also, users can too easily ignore the new errors, so in a way, the rule is enabled without being enforced when IMO the point of a linter is to enforce rules.
132+
133+
### Using disable comments
134+
135+
One can use disable comments to temporarily suppress errors, by adding a comment like `// eslint-disable rule-name -- FIX THIS LATER`
136+
137+
Disable comments can be used to enable a rule as an error early, by adding them everywhere where an error is currently reported (and that is actually something that can be automated by some linters).
138+
139+
But disable comments have the tendency to be hard to distinguish from other disable comments created for reasons such as false positives or disagreements on the rule, especially when there is no enforced need to add a message on the comment. Meaning that once you decide to tackle the existing errors, they can be hard to detect (or to distinguish from ones that are disabled for good reasons).
140+
141+
They also "pollute" the codebase in a way that is quite visible, and makes users numb to the fact of using disable comments.
142+
143+
### Ignoring parts of the project
144+
145+
It is also possible to simply disable the rule in each file that is currently reporting errors, either through manually configuring the rule in the ESLint config, or by adding a disable comment at the top of the file that disables the rule for the entire file.
146+
147+
This has multiple downsides:
148+
149+
* While new errors are enforced in the other files, new errors can creep in the ignored files
150+
* If/when the errors in the ignored files get removed, the user has to remember to re-enable the rule on this file. Otherwise new errors can creep in also.
151+
152+
153+
## Open Questions
154+
155+
<!--
156+
This section is optional, but is suggested for a first draft.
157+
158+
What parts of this proposal are you unclear about? What do you
159+
need to know before you can finalize this RFC?
160+
161+
List the questions that you'd like reviewers to focus on. When
162+
you've received the answers and updated the design to reflect them,
163+
you can remove this section.
164+
-->
165+
166+
None so far.
167+
168+
## Help Needed
169+
170+
<!--
171+
This section is optional.
172+
173+
Are you able to implement this RFC on your own? If not, what kind
174+
of help would you need from the team?
175+
-->
176+
177+
I expect to implement this change.
178+
179+
## Frequently Asked Questions
180+
181+
<!--
182+
This section is optional but suggested.
183+
184+
Try to anticipate points of clarification that might be needed by
185+
the people reviewing this RFC. Include those questions and answers
186+
in this section.
187+
-->
188+
189+
No questions so far.
190+
191+
## Related Discussions
192+
193+
* [Change Request: Introduce a system to suppress existing errors](https://github.com/eslint/eslint/issues/16755)
194+
* [PHPStan - The Baseline](https://phpstan.org/user-guide/baseline)
195+
196+
<!--
197+
This section is optional but suggested.
198+
199+
If there is an issue, pull request, or other URL that provides useful
200+
context for this proposal, please include those links here.
201+
-->

0 commit comments

Comments
 (0)