feat: add plugin suppression

[wanghaoPolar]

task: #4667

Plugin Suppression

changes are split into 2 parts:

1. during the existing workflow for lint suppression, we need to add new fields to record suppression for plugins
2. during plugin evaluation, we need to handle each diagnosis, suppress it if matched

Part 1: Changes to data structs in each step of existing lint suppression workflow

plugin suppression reuse most of the existing lint suppression workflow, but new fields are added to handle dynamic plugin names that users input.

Step 1: parse comment string to Suppression<'a>

file: crates/biome_suppression/src/lib.rs

each suppression comment is parse a single Suppression<'a>

here, a new 'subcategory' is added to the parse result

In the case for plugin, category will be 'plugin', and subcategory will be the custom plugin name that users input

#[derive(Debug, PartialEq, Eq)]
pub struct Suppression<'a> {
    // ...
-   pub categories: Vec<(&'a Category, Option<&'a str>)>, // Vec<(category, value)>
+   pub categories: Vec<(&'a Category, Option<&'a str>, Option<&'a str>)>, // Vec<(category, subcategory, value)>
}

Step 2: Suppression<'a> processed into AnalyzerSuppression

file: crates/biome_analyze/src/lib.rs

Suppression<'a> is then processed into AnalyzerSuppression

previously AnalyzerSuppression.kind has 3 variants: Everything, Rule(&'a str), RuleInstance(&'a str, &'a str)
a new variant Plugin(&'a str) is added for plugin

pub struct AnalyzerSuppression<'a> {
    /// The kind of suppression
    pub(crate) kind: AnalyzerSuppressionKind<'a>,

    /// The range where the `biome-ignore` comment is placed inside the whole text
    pub(crate) ignore_range: Option<TextRange>,

    /// The kind of `biome-ignore` comment used for this suppression
    pub(crate) variant: AnalyzerSuppressionVariant,
}

pub enum AnalyzerSuppressionKind<'a> {
    /// A suppression disabling all lints eg. `// biome-ignore lint`
    Everything,
    /// A suppression disabling a specific rule eg. `// biome-ignore lint/complexity/useWhile`
    Rule(&'a str),
    /// A suppression to be evaluated by a specific rule eg. `// biome-ignore lint/correctness/useExhaustiveDependencies(foo)`
    RuleInstance(&'a str, &'a str),
+    /// A suppression disabling a plugin eg. `// biome-ignore plugin/my-plugin`
+    Plugin(&'a str),
}

Step 3: AnalyzerSuppression pushed to PhaseRunner.suppression

see crates/biome_analyze/src/suppressions.rs

top level suppression, new field plugins is added

pub struct TopLevelSuppression {
    /// Whether this suppression suppresses all filters
    pub(crate) suppress_all: bool,
    /// Filters for the current suppression
    pub(crate) filters: FxHashSet<RuleFilter<'static>>,
+   /// Current suppressed plugins
+   pub(crate) plugins: FxHashSet<String>,
    /// The range of the comment
    pub(crate) comment_range: TextRange,

    /// The range covered by the current suppression.
    /// Eventually, it should hit the entire document
    pub(crate) range: TextRange,
}

for line suppression, new field suppressed_plugins is added

pub(crate) struct LineSuppression {
    /// Line index this comment is suppressing lint rules for
    pub(crate) line_index: usize,
    /// Range of source text covered by the suppression comment
    pub(crate) comment_span: TextRange,
    /// Range of source text this comment is suppressing lint rules for
    pub(crate) text_range: TextRange,
    /// Set to true if this comment has set the `suppress_all` flag to true
    /// (must be restored to false on expiration)
    pub(crate) suppress_all: bool,
    /// List of all the rules this comment has started suppressing (must be
    /// removed from the suppressed set on expiration)
    pub(crate) suppressed_rules: FxHashSet<RuleFilter<'static>>,
    /// List of all the rule instances this comment has started suppressing.
    pub(crate) suppressed_instances: FxHashMap<String, RuleFilter<'static>>,
+   /// List of plugins this comment has started suppressing
+   pub(crate) suppressed_plugins: FxHashSet<String>,
    /// Set to `true` when a signal matching this suppression was emitted and
    /// suppressed
    pub(crate) did_suppress_signal: bool,
    /// Set to `true` when this line suppresses a signal that was already suppressed by another entity e.g. top-level suppression
    pub(crate) already_suppressed: Option<TextRange>,
}

for range suppression

range suppression for plugin is not supported, and an error is reported if there is a range suppression for plugin

the reason it's not supported is because plugin is run after lint phases.
During lint phase, range suppressions are dynamiclly updated while parsing // biome-range-start and // biome-range-end.

after lint phases are finished, all range suppressions have empty filters.

Why not reuse current Category/RuleFilter to avoid adding new fileds

in optimal cases, we can treat plugin/xxx same as lint/xxx to avoid adding new fields and making changes to exising workflow.

however, currently it's not implemented as such because step 1 and 3 of above section both require pre-register static string and codegen, thus doesn't allow for dynamic values.
but for plugin, it must allow user to use dynamic plugin names.

step 1:
Suppression<'a> stores &'a Category instead of arbitrary string

#[derive(Debug)]
pub struct Category {
    name: &'static str,
    link: Option<&'static str>,
}

but we cannot init a new Category outside of the biome_diagnostics_categories crate, which serves as a registry for all known diagnostic categories (currently this registry is fully static and generated at compile time).

step 3:

filter is required to be a RuleFilter<'static>

pub enum RuleFilter<'a> {
    Group(&'a str),
    Rule(&'a str, &'a str),
}

see map_to_rule_filter in crates/biome_analyze/src/suppressions.rs, this requires the group/rule to be registered in MetadataRegistry, which is currently done during codegen using declare_lint_group!

due to these 2 reasons, it will be troublesome to fit in the new plugin suppression into existing flow without adding new fields.

Part 2: Handle plugin diagnosis

Plugins are run after exiting lint runners are finished.

During plugin evaluation, everytime there is a diagnosis, we will check to see if it is suppressed

see crates/biome_analyze/src/lib.rs, most logic is copied from flush_matches, except for:

1. top level suppression is matched using plugins
2. line suppression is matched using suppressed_plugins
3. range suppression is not supported and thus skipped

[arendjr]

Great work! Just left a few comments.

[wanghaoPolar]

Hi @arendjr , thanks for help reviewing!

I pushed 2 new commit:

In a7e06b6 I fixed the above issues you raised:

1. created 📎 Implement range suppression of plugin diagnostics #5175 and added to the message
2. for parse_category method, I moved the error report logic into this function, so I kept the return type as Result
3. after I add a new field subcategory to RuleDiagnostic, clippy report large size difference between variants on enum DiagnosticKind { Rule(RuleDiagnostic), Raw(Error) }, so I fixed it according to clippy's suggestion (change to Rule(Box<RuleDiagnostic>))

In ba70dee I handled the case of // biome-ignore plugin by treat it as ignore all plugin reported diagnosis.
I added related unit test and snap tests for this.

[codspeed-hq]

CodSpeed Performance Report

Merging #5139 will not alter performance

_{Comparing wanghaoPolar:feat/ignore-plugin-diagnostic (24992b3) with next (79a7d5c)}

Summary

✅ 94 untouched benchmarks

[wanghaoPolar]

24992b3 : change syntax to // biome-ignore lint/plugin/myPlugin

[wanghaoPolar]

Perhaps I should disallow the usage of // biome-ignore lint/plugin (to suppress diagnosis of all plugins)? cannot think of a use case of it.

[ematipico]

Our linter allows suppressing all rules that belong to a group e.g. lint/style, so why not?

Plus, with the arrival of new suppressions like the top-level suppression, it's something that users might need

[arendjr]

Thanks so much!

[ematipico]

@arendjr any chance to address my comment? #5139 (comment)

[arendjr]

Of course!

Our linter allows suppressing all rules that belong to a group e.g. lint/style, so why not?

The way I understood it, the PR allows this in its current form, and @wanghaoPolar was contemplating whether to disallow it because they saw no use case for it. Based on your comment I also thought we might want to keep it as is, so I merged as is. Let me know if I misunderstood!

[wanghaoPolar]

Yes, in current implementation using 'lint/plugin' disables diagnosis raised by all plugins.

In my previous comment I was asking if I should disallow it instead.