feat(lsp): show bulk-suppressed violations as faded diagnostics#19791
feat(lsp): show bulk-suppressed violations as faded diagnostics#19791wagenet wants to merge 21 commits into
Conversation
- Reading once the file located in `cwd` in Runtime::run - Read only the file content - Diff subset of this file content in each worker. - Basic test integration
Found a case not correctly handle, when the project doesn’t have a file. This case will cover when we are handling the cli args. The diff operation is done outside of rayon, but it can be done in each worker. Until we have a complete integration suite, it won’t be refactored. I wanted a working version, not a perfect design. Next commit will be showing errors when any of the issues are found. Currently is being printed as simple logs in the stdout.
Now we can test correctly the following situations: - No file but not cli provided: No suppression required. - No file but cli suppress_all provided: Suppression init and creation - File but not provided any args: filtering working as expected
Pending to not show this error if an cli arguments is passed.
The new hash map is only saved in the following situations: - If file doesn’t exist , `supress-all` is provided, and a diff is detected, then the file is created. - If file exist, `supress-all` or `prune-all` is provided, and a diff is detected, then the file is updated. The rest of situations, the linter doesn’t make the effort to save the new data. Saving the new suppression data hide the reporting of all diff detected.
Integration test are spplited on two kind: - Snapshot testing that only check the output is the expect in some situations - Serialization and desearilazation tests As the second one required handle files, thoses doesn’t assert the cli output only the file content
…for reading Removed the need of read twice the file due moving value in Rayon, readings are made in a papaya hash map for concurrent readings. Suppression file tracking has his own struct instead of create the data through SupressionManager. This avoid move the value and the need to read again the file. On top of that, this data only make create in the context of a worker. The path is clean to only emit the diff to update the current file.
All the heavy work has been moved to the workers. Instead of building from zero the same Hashmap we are doing the following: - A Read-Only concurrent hash map. From here the workers will read the data to calculate the diff required to update the OG. - Only in update mode this diff are send through a channel. Doing this as once the file exist we don’t expect a lot of changes between runs. - Only update the file if the args are present AND diffs are found. Otherwise, we do nothing.
- All JSPlugins errors are being recorded in the file. Left Typescript as it required more changes. - Add JSPlugin test - Manual tested the binary in Posthog and oxlint-react-compiler-rules - Improved errors messages, and added a label to suggest the user what command run to update the file. - Some basic message in the cli output if the file is created or updated.
- Remove SuppressionId, this information already exist. Probably the reason of the performance regression. - SuppressionManager inited `apps/oxlint`. This replace the logic if the file has been created or updated. Pending the state if a diff haven’t been found, this will avoid render the file has been updated when it isn’t. - Integration test for malformed json. The error don’t panic, the files are linted but a message is rendered with the error. - `OxcDiagnositc` is transformed to `RuleName`. Pending to use TryFrom instead of From, as it can fail. - Removed misleading TODO comment
- `—fix —suppress-all` test without file not generate a `oxlint-suppressions.json` file and fixes are apply. - `—fix —-suppress-all`test having a initial suppression file, is not behaving correctly as the fix aren’t apply. Are filtered due the suppresion file. Pending to be fixed. - Using Option instead on initiating a default SuppressionFile struct. This solve an edge case of emitting errors when the file was malformed. Pending add more test with a combination of fix commands, a test that prune if only `fix` is provided.
…ify file update if the file has been really updated - Only mentioine the file has been update once the file is write instead of detecting if it exists. - Filtering the messages with the suppression only after of this fix has been apply, not before. - Pending more integration tests handling different of fix
Adds LSP support to show `oxlint-suppressions.json`-suppressed violations as faded/de-emphasised diagnostics (`DiagnosticTag::UNNECESSARY`) rather than hiding them entirely, with a toggle and configurable severity. New LSP options (sent via `initializationOptions`): - `showSuppressedViolations: bool` (default: `false`) - `suppressedViolationSeverity: "original" | "hint" | "warning" | "error"` Infrastructure changes: - `Message.suppressed: bool` field added (default `false`) - `SuppressionManager::mark_suppressed()` marks violations in place instead of dropping them (LSP counterpart to `suppress_lint_diagnostics`) - `LintRunner::run_source_marking_suppressed()` for the LSP mark path - `SuppressionTracking` uses `Arc<HashMap>` instead of `papaya` for simpler sharing semantics CLI behaviour is unchanged (suppressed violations are still dropped). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds support for displaying oxlint-suppressions.json bulk-suppressed violations in the LSP as de-emphasized (faded) diagnostics, while extending the CLI bulk-suppression infrastructure (create/prune/update and reporting).
Changes:
- Introduces suppression tracking/management in
oxc_linterand threads it through the CLI runtime. - Adds LSP options and a “mark suppressed” execution path that keeps suppressed violations and tags them as
UNNECESSARY. - Updates CLI help/snapshots and adds multiple suppression-related fixtures/snapshots/tests.
Reviewed changes
Copilot reviewed 100 out of 100 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tasks/website_linter/src/snapshots/cli_terminal.snap | Documents new CLI suppression flags in website linter snapshot. |
| tasks/website_linter/src/snapshots/cli.snap | Documents new CLI suppression flags in website linter snapshot. |
| crates/oxc_linter/src/suppression/tracking.rs | Implements suppression-file model, parsing, diffs, and serialization. |
| crates/oxc_linter/src/suppression/mod.rs | Adds suppression manager API including “mark suppressed” mode. |
| crates/oxc_linter/src/service/runtime.rs | Integrates suppression filtering/diffing into runtime lint execution. |
| crates/oxc_linter/src/service/mod.rs | Threads suppression manager through lint service execution. |
| crates/oxc_linter/src/options/mod.rs | Adds suppression-related options to linter options. |
| crates/oxc_linter/src/lint_runner.rs | Adds LSP-focused run mode that marks suppressed messages. |
| crates/oxc_linter/src/lib.rs | Exposes suppression APIs and updates JS-plugin path handling. |
| crates/oxc_linter/src/fixer/mod.rs | Adds Message.suppressed flag to propagate suppression state. |
| crates/oxc_linter/src/context/mod.rs | Minor formatting change in diagnostic emission flow. |
| apps/oxlint/test/fixtures/suppression_js_plugin/plugin.js | Test fixture: JS plugin emitting a diagnostic for suppression tests. |
| apps/oxlint/test/fixtures/suppression_js_plugin/oxlint-suppressions.json | Test fixture: suppression file for JS plugin diagnostic. |
| apps/oxlint/test/fixtures/suppression_js_plugin/output.snap.md | Test fixture: expected output snapshot for JS plugin suppression. |
| apps/oxlint/test/fixtures/suppression_js_plugin/options.json | Test fixture: CLI args for JS plugin suppression scenario. |
| apps/oxlint/test/fixtures/suppression_js_plugin/files/test.js | Test fixture: source file triggering JS plugin diagnostic. |
| apps/oxlint/test/fixtures/suppression_js_plugin/.oxlintrc.json | Test fixture: config enabling JS plugin suppression scenario. |
| apps/oxlint/src/snapshots/fixtures__suppression_without_file_@oxlint.snap | CLI snapshot for suppression behavior when file missing. |
| apps/oxlint/src/snapshots/fixtures__suppression_without_file_--suppress-all@oxlint.snap | CLI snapshot for --suppress-all without existing suppression file. |
| apps/oxlint/src/snapshots/fixtures__suppression_report_only_from_one_file_@oxlint.snap | CLI snapshot for per-file suppression reporting behavior. |
| apps/oxlint/src/snapshots/fixtures__suppression_report_one_of_the_errors_from_one_file_@oxlint.snap | CLI snapshot for selective reporting when counts exceed suppression. |
| apps/oxlint/src/snapshots/fixtures__suppression_report_one_new_error_but_filter_the_rest_@oxlint.snap | CLI snapshot for “new violation appears” behavior. |
| apps/oxlint/src/snapshots/fixtures__suppression_prune_errors_warning_@oxlint.snap | CLI snapshot for pruning suppression entries. |
| apps/oxlint/src/snapshots/fixtures__suppression_not_reporting_new_errors_@oxlint.snap | CLI snapshot for “no new errors reported” behavior. |
| apps/oxlint/src/snapshots/fixtures__suppression_not_file_reporting_errors_@oxlint.snap | CLI snapshot for default behavior without suppression file. |
| apps/oxlint/src/snapshots/fixtures__suppression_less_rules_violations_warning_@oxlint.snap | CLI snapshot for decreased violation counts behavior. |
| apps/oxlint/src/snapshots/fixtures__suppression_file_malformed_@oxlint.snap | CLI snapshot for malformed suppression file handling. |
| apps/oxlint/src/snapshots/fixtures__suppression_eslint_file_format_@oxlint.snap | CLI snapshot for eslint-style suppression file format behavior. |
| apps/oxlint/src/output_formatter/mod.rs | Adds suppression-file action reporting to formatter info struct. |
| apps/oxlint/src/output_formatter/json.rs | Updates JSON formatter tests for new command info field. |
| apps/oxlint/src/output_formatter/default.rs | Emits created/updated/malformed suppression-file notices in default output. |
| apps/oxlint/src/lsp/server_linter.rs | Loads suppressions for LSP and routes to mark-suppressed run path. |
| apps/oxlint/src/lsp/options.rs | Adds LSP initialization options for showing/overriding suppressed violations. |
| apps/oxlint/src/lsp/error_with_position.rs | Tags suppressed diagnostics as UNNECESSARY and removes code actions. |
| apps/oxlint/src/lint.rs | Wires suppression manager into CLI run and adds extensive suppression tests. |
| apps/oxlint/src/command/lint.rs | Adds CLI flags for suppression generation/pruning and parser tests. |
| apps/oxlint/fixtures/suppression_without_file/folder_b/test.js | Fixture data for suppression tests. |
| apps/oxlint/fixtures/suppression_without_file/folder_a/test.js | Fixture data for suppression tests. |
| apps/oxlint/fixtures/suppression_without_file/.oxlintrc.json | Fixture config for suppression tests. |
| apps/oxlint/fixtures/suppression_with_suppress_all_arg_and_no_file/oxlint-suppressions-expected.json | Fixture expected suppression-file output. |
| apps/oxlint/fixtures/suppression_with_suppress_all_arg_and_no_file/folder_a/test.js | Fixture data for suppression tests. |
| apps/oxlint/fixtures/suppression_with_suppress_all_arg_and_no_file/.oxlintrc.json | Fixture config for suppression tests. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_no_file/files/test.js | Fixture data for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_no_file/files/test-backup.js | Fixture backup data for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_no_file/.oxlintrc.json | Fixture config for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/oxlint-suppressions.json | Fixture suppression file for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/oxlint-suppressions-expected.json | Fixture expected suppression-file output for fix scenario. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/oxlint-suppressions-backup.json | Fixture backup suppression file for fix scenario. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/files/test.js | Fixture data for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/files/test-backup.js | Fixture backup data for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_suppress_all_and_fix_arg_and_file/.oxlintrc.json | Fixture config for suppression + fix behavior. |
| apps/oxlint/fixtures/suppression_with_prune_all_arg_and_no_file/folder_b/test.js | Fixture data for prune behavior when file missing. |
| apps/oxlint/fixtures/suppression_with_prune_all_arg_and_no_file/folder_a/test.js | Fixture data for prune behavior when file missing. |
| apps/oxlint/fixtures/suppression_with_prune_all_arg_and_no_file/.oxlintrc.json | Fixture config for prune behavior when file missing. |
| apps/oxlint/fixtures/suppression_with_arg_and_pruned_errors/oxlint-suppressions.json | Fixture suppression file for prune behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_pruned_errors/oxlint-suppressions-expected.json | Fixture expected suppression-file output for prune behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_pruned_errors/oxlint-suppressions-backup.json | Fixture backup suppression file for prune behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_pruned_errors/folder_a/test.js | Fixture data for prune behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_pruned_errors/.oxlintrc.json | Fixture config for prune behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_increased_errors/oxlint-suppressions.json | Fixture suppression file for increased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_increased_errors/oxlint-suppressions-expected.json | Fixture expected suppression-file output for increased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_increased_errors/oxlint-suppressions-backup.json | Fixture backup suppression file for increased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_increased_errors/folder_a/test.js | Fixture data for increased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_increased_errors/.oxlintrc.json | Fixture config for increased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_decreased_errors/oxlint-suppressions.json | Fixture suppression file for decreased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_decreased_errors/oxlint-suppressions-expected.json | Fixture expected suppression-file output for decreased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_decreased_errors/oxlint-suppressions-backup.json | Fixture backup suppression file for decreased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_decreased_errors/folder_a/test.js | Fixture data for decreased-count behavior. |
| apps/oxlint/fixtures/suppression_with_arg_and_decreased_errors/.oxlintrc.json | Fixture config for decreased-count behavior. |
| apps/oxlint/fixtures/suppression_report_only_from_one_file/oxlint-suppressions.json | Fixture suppression file for per-file reporting behavior. |
| apps/oxlint/fixtures/suppression_report_only_from_one_file/folder_b/test.js | Fixture data for per-file reporting behavior. |
| apps/oxlint/fixtures/suppression_report_only_from_one_file/folder_a/test.js | Fixture data for per-file reporting behavior. |
| apps/oxlint/fixtures/suppression_report_only_from_one_file/.oxlintrc.json | Fixture config for per-file reporting behavior. |
| apps/oxlint/fixtures/suppression_report_one_of_the_errors_from_one_file/oxlint-suppressions.json | Fixture suppression file for partial reporting behavior. |
| apps/oxlint/fixtures/suppression_report_one_of_the_errors_from_one_file/folder_b/test.js | Fixture data for partial reporting behavior. |
| apps/oxlint/fixtures/suppression_report_one_of_the_errors_from_one_file/.oxlintrc.json | Fixture config for partial reporting behavior. |
| apps/oxlint/fixtures/suppression_report_one_new_error_but_filter_the_rest/oxlint-suppressions.json | Fixture suppression file for “new error” behavior. |
| apps/oxlint/fixtures/suppression_report_one_new_error_but_filter_the_rest/folder_b/test.js | Fixture data for “new error” behavior. |
| apps/oxlint/fixtures/suppression_report_one_new_error_but_filter_the_rest/folder_a/test.js | Fixture data for “new error” behavior. |
| apps/oxlint/fixtures/suppression_report_one_new_error_but_filter_the_rest/.oxlintrc.json | Fixture config for “new error” behavior. |
| apps/oxlint/fixtures/suppression_prune_errors_warning/oxlint-suppressions.json | Fixture suppression file for prune-warning behavior. |
| apps/oxlint/fixtures/suppression_prune_errors_warning/folder_b/test.js | Fixture data for prune-warning behavior. |
| apps/oxlint/fixtures/suppression_prune_errors_warning/folder_a/test.js | Fixture data for prune-warning behavior. |
| apps/oxlint/fixtures/suppression_prune_errors_warning/.oxlintrc.json | Fixture config for prune-warning behavior. |
| apps/oxlint/fixtures/suppression_not_reporting_new_errors/oxlint-suppressions.json | Fixture suppression file for “no new errors” behavior. |
| apps/oxlint/fixtures/suppression_not_reporting_new_errors/folder_b/test.js | Fixture data for “no new errors” behavior. |
| apps/oxlint/fixtures/suppression_not_reporting_new_errors/folder_a/test.js | Fixture data for “no new errors” behavior. |
| apps/oxlint/fixtures/suppression_not_reporting_new_errors/.oxlintrc.json | Fixture config for “no new errors” behavior. |
| apps/oxlint/fixtures/suppression_not_file_reporting_errors/folder_b/test.js | Fixture data for default/no-suppression behavior. |
| apps/oxlint/fixtures/suppression_not_file_reporting_errors/folder_a/test.js | Fixture data for default/no-suppression behavior. |
| apps/oxlint/fixtures/suppression_not_file_reporting_errors/.oxlintrc.json | Fixture config for default/no-suppression behavior. |
| apps/oxlint/fixtures/suppression_less_rules_violations_warning/oxlint-suppressions.json | Fixture suppression file for decreased-count warning behavior. |
| apps/oxlint/fixtures/suppression_less_rules_violations_warning/folder_b/test.js | Fixture data for decreased-count warning behavior. |
| apps/oxlint/fixtures/suppression_less_rules_violations_warning/.oxlintrc.json | Fixture config for decreased-count warning behavior. |
| apps/oxlint/fixtures/suppression_file_malformed/test.js | Fixture data for malformed suppression file behavior. |
| apps/oxlint/fixtures/suppression_file_malformed/oxlint-suppressions.json | Fixture malformed suppression file content. |
| apps/oxlint/fixtures/suppression_file_malformed/.oxlintrc.json | Fixture config for malformed suppression file behavior. |
| apps/oxlint/fixtures/suppression_eslint_file_format/test.js | Fixture data for eslint-format suppression file behavior. |
| apps/oxlint/fixtures/suppression_eslint_file_format/oxlint-suppressions.json | Fixture eslint-format suppression file content. |
| apps/oxlint/fixtures/suppression_eslint_file_format/.oxlintrc.json | Fixture config for eslint-format suppression file behavior. |
Comments suppressed due to low confidence (7)
crates/oxc_linter/src/suppression/tracking.rs:194
- The help text suggests running
oxlint --suppress--all(double--), but the CLI flag is--suppress-all. This makes the suggested remediation command incorrect; update the string (and regenerate any affected snapshots).
crates/oxc_linter/src/suppression/tracking.rs:124 - This error message says "Failed to parse oxlint config" but the code is parsing the suppression rules file (
oxlint-suppressions.json). The wording is misleading for users diagnosing suppression-file issues; update it to refer to the suppression file (and keep the filename consistent across related errors).
apps/oxlint/src/lint.rs:385 - This
println!appears to be temporary debugging output and will pollute CLI stdout. Remove it, or convert it to atracing::debug!/trace!log behind appropriate filtering so JSON/default output remains stable.
apps/oxlint/src/output_formatter/mod.rs:72 - Doc comment typo/inaccuracy: it mentions "bulk-suppressions.json" and "created or update". The file is
oxlint-suppressions.json, and the phrase should be "created or updated" (also consider reflowing to keep line lengths reasonable).
crates/oxc_linter/src/lint_runner.rs:304 run_source_marking_suppressediterates overfilesbut passes the samemessagesvector (containing diagnostics for all files) intomark_suppressed, which counts/marks without filtering by file. This works today because the LSP calls it with a single file, but the function signature implies multi-file support and the current implementation will mis-mark suppressions iffiles.len() > 1. Consider enforcing single-file usage (assert/early return) or restructuring so marking is applied per-file message set.
crates/oxc_linter/src/service/runtime.rs:705- This
println!looks like leftover debugging output and will spam stdout during--fixruns. Please remove it (and also remove the now-unuseda = messages.len()binding introduced only for this print) or gate it behind the existing tracing/logging facilities.
crates/oxc_linter/src/suppression/tracking.rs:49 RuleName::from(&Message)falls back to the literal "plugin_name/rule_name" when the diagnostic has no code scope/number. This can cause suppression tracking/diffs to include internal/non-rule diagnostics (there areMessage::new(OxcDiagnostic::error(...))call sites withoutwith_error_code, e.g. JS plugin failures). Consider returningOption<RuleName>and skipping messages without a real<plugin>/<rule>code in suppression tracking/marking, rather than using a placeholder key.
| let (suppression_diff_channel, tx_suppression_diff_channel) = | ||
| std::sync::mpsc::channel::<SuppressionDiff>(); |
There was a problem hiding this comment.
std::sync::mpsc::channel() returns (Sender, Receiver), but the variables here are named/inverted as if the first were a receiver. This leads to later .send(...) / .recv() calls being made on the wrong type and will not compile. Swap the bindings to (tx_suppression_diff, rx_suppression_diff) (or rename accordingly) and update call sites to use the sender for send and receiver for recv.
| let mut have_at_least_one_diff = false; | ||
| while let Ok(diff) = tx_suppression_diff_channel.recv() { | ||
| if !have_at_least_one_diff { | ||
| have_at_least_one_diff = true; | ||
| } | ||
|
|
||
| suppression_manager.update(diff); | ||
| } |
There was a problem hiding this comment.
Even after fixing the channel binding, this recv() loop can block forever because the original sender is still alive in this scope. Ensure all Sender handles are dropped before entering the receive loop (e.g. drop(tx_suppression_diff_channel)), or collect diffs without a blocking loop (e.g. try_iter()), so run() cannot deadlock at the end of linting.
| let diff_suppression_sender = suppression_diff_channel.clone(); | ||
|
|
||
| for diff in diffs { | ||
| diff_suppression_sender.send(diff.clone()).unwrap(); | ||
| } |
There was a problem hiding this comment.
This clones suppression_diff_channel and calls .send(...) on it, but after swapping the channel bindings this should be cloning the Sender (tx) rather than the receiver. As written it won’t compile; after fixing the binding, use the sender clone here (and you don’t need to clone() each diff before sending if ownership can be moved).
| let filename = Filename::new(path.strip_prefix(&self.cwd).unwrap()); | ||
|
|
There was a problem hiding this comment.
path.strip_prefix(&self.cwd).unwrap() can panic: earlier you explicitly handle the strip_prefix failure case by using SuppressionFile::default(), but this unwrap ignores that and assumes it always succeeds. Reuse the already-computed relative path (or fall back safely) instead of unwrapping here.
|
Closing as stale, since the implementation has changed significantly. |
Summary
Adds LSP support to show
oxlint-suppressions.json-suppressed violations as faded/de-emphasised diagnostics (DiagnosticTag::UNNECESSARY) instead of hiding them entirely.New LSP options (via
initializationOptions):showSuppressedViolations: bool(default:false) — toggle; off preserves current behavioursuppressedViolationSeverity: "original" | "hint" | "warning" | "error"— severity override for faded diagnosticsBehaviour:
DiagnosticTag::UNNECESSARY→ faded in VS Code / nvim-lspoxlint-suppressions.jsonsuppressions are affected; inlineeslint-disableremains fully silentlint_filesstill drops suppressed violations)Infrastructure changes over #19328
Message.suppressed: bool(defaultfalse) — signal between suppression layer and LSPSuppressionManager::mark_suppressed(path, messages)— marks violations in-place instead of dropping, using binary per-rule logic matchingsuppress_lint_diagnosticsLintRunner::run_source_marking_suppressed()— LSP path that applies mark modeSuppressionTrackingmaps changed frompapaya::HashMaptoArc<HashMap>for simpler sharing semanticsTest plan
oxlint-suppressions.jsonwith N suppressed violations for a ruleshowSuppressedViolations: falsehides faded violations entirelysuppressedViolationSeverity: "hint"overrides severity of faded diagnosticscargo test -p oxc_linterpasses unchanged🤖 Generated with Claude Code