-
Notifications
You must be signed in to change notification settings - Fork 604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[eslint-patch] Implementation for ESLint Bulk Suppressions v0.1.0 #4366
Merged
octogonz
merged 41 commits into
microsoft:main
from
kevin-y-ang:feat/working-prototype-for-eslint-bulk-suppressions
Nov 22, 2023
Merged
[eslint-patch] Implementation for ESLint Bulk Suppressions v0.1.0 #4366
octogonz
merged 41 commits into
microsoft:main
from
kevin-y-ang:feat/working-prototype-for-eslint-bulk-suppressions
Nov 22, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin-y-ang
requested review from
iclanton,
octogonz,
apostolisms,
D4N14L,
dmichon-msft and
patmill
as code owners
September 28, 2023 18:29
kevin-y-ang
force-pushed
the
feat/working-prototype-for-eslint-bulk-suppressions
branch
2 times, most recently
from
October 26, 2023 02:07
e70e4fe
to
a3940d9
Compare
kevin-y-ang
changed the title
[eslint-patch] working demo for eslint-bulk-suppressions design proposal
[eslint-patch] Implementation for ESLint Bulk Suppressions v1.0
Oct 27, 2023
kevin-y-ang
changed the title
[eslint-patch] Implementation for ESLint Bulk Suppressions v1.0
[eslint-patch] Implementation for ESLint Bulk Suppressions v0.1.0
Oct 27, 2023
octogonz
reviewed
Oct 27, 2023
octogonz
reviewed
Oct 27, 2023
octogonz
reviewed
Oct 27, 2023
...hstack/eslint-bulk/feat-working-prototype-for-eslint-bulk-suppressions_2023-10-26-23-46.json
Outdated
Show resolved
Hide resolved
octogonz
reviewed
Oct 27, 2023
octogonz
reviewed
Oct 28, 2023
octogonz
reviewed
Oct 28, 2023
kevin-y-ang
force-pushed
the
feat/working-prototype-for-eslint-bulk-suppressions
branch
from
October 31, 2023 21:52
f7021e6
to
a0dbfee
Compare
octogonz
reviewed
Nov 21, 2023
...stack/eslint-patch/feat-working-prototype-for-eslint-bulk-suppressions_2023-09-28-19-54.json
Outdated
Show resolved
Hide resolved
kevin-y-ang
force-pushed
the
feat/working-prototype-for-eslint-bulk-suppressions
branch
from
November 21, 2023 02:39
bda8e97
to
b500e33
Compare
…for-eslint-bulk-suppressions_2023-09-28-19-54.json Co-authored-by: Pete Gonzalez <[email protected]>
…, but it is a JavaScript entry point not a shell script; replacing it with Heft's "lib/exports" convention
…ttps://github.com/kevin-y-ang/rushstack into feat/working-prototype-for-eslint-bulk-suppressions
… a minimum version check
octogonz
reviewed
Nov 21, 2023
eslint/eslint-patch/src/eslint-bulk-suppressions/generate-patched-file.ts
Outdated
Show resolved
Hide resolved
octogonz
approved these changes
Nov 21, 2023
octogonz
reviewed
Nov 21, 2023
eslint/eslint-patch/src/eslint-bulk-suppressions/cli/utils/print-help.ts
Outdated
Show resolved
Hide resolved
eslint/eslint-patch/src/eslint-bulk-suppressions/bulk-suppressions-patch.ts
Show resolved
Hide resolved
eslint/eslint-patch/src/eslint-bulk-suppressions/bulk-suppressions-patch.ts
Show resolved
Hide resolved
This was referenced Nov 30, 2023
This was referenced Feb 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Design Proposal: bulk suppressions for ESLint
This is a version 0.1.0 implementation for the above design proposal for "eslint-bulk-suppressions". Please read the rationale there.
What is ESLint Bulk Suppressions?
A tool that allows bulk suppression of ESLint warnings/errors in a large, old codebase when introducing new ESLint rules.
What it does
This tool is designed to address the issue of introducing new ESLint rules to a large, old codebase, which often results in hundreds to tens of thousands of retroactive issues being reported by ESLint. This can clutter the ESLint output, making it difficult to read and potentially causing developers to overlook new ESLint issues. It also makes it impractical to use merge request pipelines that block ESLint warnings/errors.
The tool provides a mechanism for recording all retroactively introduced ESLint warnings/errors in a "bulk suppressions" file, hiding them from the ESLint output. This allows developers to still get most of the benefits of ESLint, as any new code written will be annotated by ESLint and can be fixed in bite-sized portions. It also allows the use of merge request pipelines to block newly written error-prone code without blocking legacy code that has been battle-tested.
Why it's a patch
The bulk suppressions feature is implemented as a monkey-patch, inspired by the modern-module-resolution implementation. We prefer it as a patch because it allows users to opt-in to using the tool at their own discretion. Similar to modern-module-resolution, the use case is much more pronounced in large codebases where ESLint warnings/errors can appear at magnitudes of thousands rather than tens. Besides reducing bundle size, this also allows us to gauge interest in this tool. If there's a lot of interest, we can submit an ESLint RFC.
This approach inevitably results in forwards compatibility issues with versions of ESLint. The patch has some logic to determine which version of ESLint you're using and uses the corresponding patch file.
How to use it
To use the tool, you need to add a
require()
call to the top of the .eslintrc.js file for each project that you want to use the tool with, for example:.eslintrc.js
With the @rushstack/eslint-bulk helper package
The eslint-bulk package is a set of command line tools to use with the ESLint bulk suppressions
patch. The commands are optional as they are just a thin wrapper over eslint shipped with the correct
environment variables to interface with the patch.
eslint-bulk suppress
Use this command to automatically generate bulk suppressions for the given files and given rules.
Supply the files as the main argument. The "files" argument is a glob pattern thatfollows the same
rules as the "eslint" command.
eslint-bulk cleanup
Use this command to automatically delete unused suppression entries for the given files in the
corresponding .eslint-bulk-suppressions.json file(s). Supply the files as the main argument. The
"files" argument is a glob pattern thatfollows the same rules as the "eslint" command.
[NOT RECOMMENDED] With Environment Variables
Generate a new .eslint-bulk-suppressions.json file (or append to an existing file) that tracks the specified rules for your project by prepending your eslint command with
ESLINT_BULK_SUPPRESS_RULES={rule_name1},{rulename2},{rulename3}
You can also use a wildcard character (
*
) to suppress all rules:ESLINT_BULK_SUPPRESS_RULES=* eslint path/to/directory
To delete unused suppression entries for the given file in the corresponding .eslint-bulk-suppressions.json file, prepend your eslint command with
CLEANUP_ESLINT_BULK_SUPPRESSIONS=true
, for example:To temporarily turn off the bulk suppressions functionality, prepend your eslint command with
USE_ESLINT_BULK_SUPPRESSIONS=false
, for example:Implementation Details
This is implemented as a monkey-patch, inspired by modern-module-resolution implementation. By calling the eslint-bulk-suppressions module in an .eslintrc.js file with
require('@rushstack/eslint-config/patch/eslint-bulk-suppressions')
, the patch will hijack ESLint's Linter class (located in lib/linter/linter.js) and override all of the Linter class's methods to point to a patched version of linter.js that supports bulk suppression files.Because we are hijacking ESLint, this implementation uses CLI environment variables to control behavior:
ESLINT_BULK_SUPPRESS_RULES={rule_name1},{rulename2},{rulename3} rushx eslint
USE_ESLINT_BULK_SUPPRESSIONS=false rushx eslint