Skip to content

Commit

Permalink
feat: add plugin suppression (#5139)
Browse files Browse the repository at this point in the history
Co-authored-by: Wang Hao <[email protected]>
Co-authored-by: Arend van Beelen jr. <[email protected]>
  • Loading branch information
3 people authored Feb 24, 2025
1 parent 79a7d5c commit c8c738e
Show file tree
Hide file tree
Showing 20 changed files with 444 additions and 66 deletions.
4 changes: 2 additions & 2 deletions crates/biome_analyze/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct AnalyzerDiagnostic {
impl From<RuleDiagnostic> for AnalyzerDiagnostic {
fn from(rule_diagnostic: RuleDiagnostic) -> Self {
Self {
kind: DiagnosticKind::Rule(rule_diagnostic),
kind: DiagnosticKind::Rule(Box::new(rule_diagnostic)),
code_suggestion_list: vec![],
}
}
Expand All @@ -35,7 +35,7 @@ impl From<RuleDiagnostic> for AnalyzerDiagnostic {
#[derive(Debug)]
enum DiagnosticKind {
/// It holds various info related to diagnostics emitted by the rules
Rule(RuleDiagnostic),
Rule(Box<RuleDiagnostic>),
/// We have raw information to create a basic [Diagnostic]
Raw(Error),
}
Expand Down
66 changes: 62 additions & 4 deletions crates/biome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use biome_console::markup;
use biome_parser::AnyParse;
use std::cmp::Ordering;
use std::collections::{BTreeMap, BinaryHeap};
use std::fmt::{Debug, Display, Formatter};
use std::ops;
Expand Down Expand Up @@ -190,10 +191,52 @@ where
for plugin in plugins {
let root: AnyParse = ctx.root.syntax().as_send().expect("not a root node").into();
for diagnostic in plugin.evaluate(root, ctx.options.file_path.clone()) {
let signal = DiagnosticSignal::new(|| diagnostic.clone());
let name = diagnostic.subcategory.clone().expect("");
let text_range = diagnostic.span.expect("");

if let ControlFlow::Break(br) = (emit_signal)(&signal) {
return Some(br);
// 1. check for top level suppression
if suppressions.top_level_suppression.suppressed_plugin(&name)
|| suppressions.top_level_suppression.suppress_all
{
break;
}

// 2. check for range suppression is not supprted because:
// plugin is handled separately after basic analyze phases
// at this point, we have read to the end of file, all `// biome-ignore-end` is processed, thus all range suppressions is cleared

// 3. check for line suppression
let suppression = {
let index = suppressions
.line_suppressions
.binary_search_by(|suppression| {
if suppression.text_range.end() < text_range.start() {
Ordering::Less
} else if text_range.end() < suppression.text_range.start() {
Ordering::Greater
} else {
Ordering::Equal
}
});

index
.ok()
.map(|index| &mut suppressions.line_suppressions[index])
};

let suppression = suppression.filter(|suppression| {
suppression.suppress_all
|| suppression.suppress_all_plugins
|| suppression.suppressed_plugins.contains(&name)
});

if let Some(suppression) = suppression {
suppression.did_suppress_signal = true;
} else {
let signal = DiagnosticSignal::new(|| diagnostic.clone());
if let ControlFlow::Break(br) = (emit_signal)(&signal) {
return Some(br);
}
}
}
}
Expand Down Expand Up @@ -650,6 +693,14 @@ impl<'a> AnalyzerSuppression<'a> {
}
}

pub fn plugin(plugin_name: Option<&'a str>) -> Self {
Self {
kind: AnalyzerSuppressionKind::Plugin(plugin_name),
ignore_range: None,
variant: AnalyzerSuppressionVariant::Line,
}
}

#[must_use]
pub fn with_ignore_range(mut self, ignore_range: TextRange) -> Self {
self.ignore_range = Some(ignore_range);
Expand All @@ -670,6 +721,8 @@ pub enum AnalyzerSuppressionKind<'a> {
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. `// lint/biome-ignore plugin/my-plugin`
Plugin(Option<&'a str>),
}

/// Takes a [Suppression] and returns a [AnalyzerSuppression]
Expand All @@ -682,9 +735,14 @@ pub fn to_analyzer_suppressions(
piece_range.add_start(suppression.range().start()).start(),
piece_range.add_start(suppression.range().end()).start(),
);
for (key, value) in suppression.categories {
for (key, subcategory, value) in suppression.categories {
if key == category!("lint") {
result.push(AnalyzerSuppression::everything().with_variant(&suppression.kind));
} else if key == category!("lint/plugin") {
let suppression = AnalyzerSuppression::plugin(subcategory)
.with_ignore_range(ignore_range)
.with_variant(&suppression.kind);
result.push(suppression);
} else {
let category = key.name();
if let Some(rule) = category.strip_prefix("lint/") {
Expand Down
7 changes: 7 additions & 0 deletions crates/biome_analyze/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,6 +1231,7 @@ pub trait Rule: RuleMeta + Sized {
pub struct RuleDiagnostic {
#[category]
pub(crate) category: &'static Category,
pub(crate) subcategory: Option<String>,
#[location(span)]
pub(crate) span: Option<TextRange>,
#[message]
Expand Down Expand Up @@ -1309,6 +1310,7 @@ impl RuleDiagnostic {
let message = markup!({ title }).to_owned();
Self {
category,
subcategory: None,
span: span.as_span(),
message: MessageAndDescription::from(message),
tags: DiagnosticTags::empty(),
Expand Down Expand Up @@ -1408,6 +1410,11 @@ impl RuleDiagnostic {
pub fn advices(&self) -> &RuleAdvice {
&self.rule_advice
}

pub fn subcategory(mut self, subcategory: String) -> Self {
self.subcategory = Some(subcategory);
self
}
}

/// Code Action object returned by a single analysis rule
Expand Down
77 changes: 73 additions & 4 deletions crates/biome_analyze/src/suppressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@ use biome_diagnostics::category;
use biome_rowan::{TextRange, TextSize};
use rustc_hash::{FxHashMap, FxHashSet};

const PLUGIN_LINT_RULE_FILTER: RuleFilter<'static> = RuleFilter::Group("lint/plugin");

#[derive(Debug, Default)]
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>>,
/// Whether this suppression suppresses all plugins
pub(crate) suppress_all_plugins: bool,
/// Current suppressed plugins
pub(crate) plugins: FxHashSet<String>,
/// The range of the comment
pub(crate) comment_range: TextRange,

Expand Down Expand Up @@ -48,6 +54,7 @@ impl TopLevelSuppression {
// The absence of a filter means that it's a suppression all
match filter {
None => self.suppress_all = true,
Some(PLUGIN_LINT_RULE_FILTER) => self.insert_plugin(&suppression.kind),
Some(filter) => self.insert(filter),
}
self.comment_range = comment_range;
Expand All @@ -59,10 +66,26 @@ impl TopLevelSuppression {
self.filters.insert(filter);
}

pub(crate) fn insert_plugin(&mut self, kind: &AnalyzerSuppressionKind) {
match kind {
AnalyzerSuppressionKind::Plugin(Some(name)) => {
self.plugins.insert((*name).to_string());
}
AnalyzerSuppressionKind::Plugin(None) => {
self.suppress_all_plugins = true;
}
_ => {}
}
}

pub(crate) fn suppressed_rule(&self, filter: &RuleKey) -> bool {
self.filters.iter().any(|f| f == filter)
}

pub(crate) fn suppressed_plugin(&self, plugin_name: &str) -> bool {
self.suppress_all_plugins || self.plugins.contains(plugin_name)
}

pub(crate) fn expand_range(&mut self, range: TextRange) {
self.range.cover(range);
}
Expand All @@ -89,6 +112,10 @@ pub(crate) struct LineSuppression {
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 if this comment suppress all plugins
pub(crate) suppress_all_plugins: bool,
/// Set to `true` when a signal matching this suppression was emitted and
/// suppressed
pub(crate) did_suppress_signal: bool,
Expand Down Expand Up @@ -139,6 +166,15 @@ impl RangeSuppressions {
text_range: TextRange,
already_suppressed: Option<TextRange>,
) -> Result<(), AnalyzerSuppressionDiagnostic> {
if let Some(PLUGIN_LINT_RULE_FILTER) = filter {
return Err(AnalyzerSuppressionDiagnostic::new(
category!("suppressions/incorrect"),
text_range,
markup!{"Found a "<Emphasis>"biome-ignore-<range>"</Emphasis>" suppression on plugin. This is not supported. See https://github.com/biomejs/biome/issues/5175"}
).hint(markup!{
"Remove this suppression."
}.to_owned()));
}
if suppression.is_range_start() {
if let Some(range_suppression) = self.suppressions.last_mut() {
match filter {
Expand Down Expand Up @@ -270,6 +306,7 @@ impl<'analyzer> Suppressions<'analyzer> {
fn push_line_suppression(
&mut self,
filter: Option<RuleFilter<'static>>,
plugin_name: Option<String>,
instance: Option<String>,
current_range: TextRange,
already_suppressed: Option<TextRange>,
Expand All @@ -283,6 +320,16 @@ impl<'analyzer> Suppressions<'analyzer> {
suppression.suppress_all = true;
suppression.suppressed_rules.clear();
suppression.suppressed_instances.clear();
suppression.suppressed_plugins.clear();
}
Some(PLUGIN_LINT_RULE_FILTER) => {
if let Some(plugin_name) = plugin_name {
suppression.suppressed_plugins.insert(plugin_name);
suppression.suppress_all_plugins = false;
} else {
suppression.suppress_all_plugins = true;
}
suppression.suppress_all = false;
}
Some(filter) => {
suppression.suppressed_rules.insert(filter);
Expand All @@ -307,6 +354,13 @@ impl<'analyzer> Suppressions<'analyzer> {
None => {
suppression.suppress_all = true;
}
Some(PLUGIN_LINT_RULE_FILTER) => {
if let Some(plugin_name) = plugin_name {
suppression.suppressed_plugins.insert(plugin_name);
} else {
suppression.suppress_all_plugins = true;
}
}
Some(filter) => {
suppression.suppressed_rules.insert(filter);
if let Some(instance) = instance {
Expand All @@ -329,6 +383,7 @@ impl<'analyzer> Suppressions<'analyzer> {
AnalyzerSuppressionKind::Everything => return Ok(None),
AnalyzerSuppressionKind::Rule(rule) => rule,
AnalyzerSuppressionKind::RuleInstance(rule, _) => rule,
AnalyzerSuppressionKind::Plugin(_) => return Ok(Some(PLUGIN_LINT_RULE_FILTER)),
};

let group_rule = rule.split_once('/');
Expand Down Expand Up @@ -357,11 +412,20 @@ impl<'analyzer> Suppressions<'analyzer> {

fn map_to_rule_instances(&self, suppression_kind: &AnalyzerSuppressionKind) -> Option<String> {
match suppression_kind {
AnalyzerSuppressionKind::Everything | AnalyzerSuppressionKind::Rule(_) => None,
AnalyzerSuppressionKind::Everything
| AnalyzerSuppressionKind::Rule(_)
| AnalyzerSuppressionKind::Plugin(_) => None,
AnalyzerSuppressionKind::RuleInstance(_, instances) => Some((*instances).to_string()),
}
}

fn map_to_plugin_name(&self, suppression_kind: &AnalyzerSuppressionKind) -> Option<String> {
match suppression_kind {
AnalyzerSuppressionKind::Plugin(Some(plugin_name)) => Some((*plugin_name).to_string()),
_ => None,
}
}

pub(crate) fn push_suppression(
&mut self,
suppression: &AnalyzerSuppression,
Expand All @@ -370,12 +434,17 @@ impl<'analyzer> Suppressions<'analyzer> {
) -> Result<(), AnalyzerSuppressionDiagnostic> {
let filter = self.map_to_rule_filter(&suppression.kind, comment_range)?;
let instances = self.map_to_rule_instances(&suppression.kind);
let plugin_name: Option<String> = self.map_to_plugin_name(&suppression.kind);
self.last_suppression = Some(suppression.variant.clone());
let already_suppressed = self.already_suppressed(filter.as_ref(), &comment_range);
match suppression.variant {
AnalyzerSuppressionVariant::Line => {
self.push_line_suppression(filter, instances, comment_range, already_suppressed)
}
AnalyzerSuppressionVariant::Line => self.push_line_suppression(
filter,
plugin_name,
instances,
comment_range,
already_suppressed,
),
AnalyzerSuppressionVariant::TopLevel => self.top_level_suppression.push_suppression(
suppression,
filter,
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_css_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl CommentStyle for CssCommentStyle {
parse_suppression_comment(text)
.filter_map(Result::ok)
.flat_map(|suppression| suppression.categories)
.any(|(key, _)| key == category!("format"))
.any(|(key, ..)| key == category!("format"))
}

fn get_comment_kind(comment: &SyntaxTriviaPieceComments<Self::Language>) -> CommentKind {
Expand Down
1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ define_categories! {
"lint/security",
"lint/style",
"lint/suspicious",
"lint/plugin",

// Suppression comments
"suppressions/parse",
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_graphql_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl CommentStyle for GraphqlCommentStyle {
parse_suppression_comment(text)
.filter_map(Result::ok)
.flat_map(|suppression| suppression.categories)
.any(|(key, _)| key == category!("format"))
.any(|(key, ..)| key == category!("format"))
}

fn get_comment_kind(_comment: &SyntaxTriviaPieceComments<Self::Language>) -> CommentKind {
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_html_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl CommentStyle for HtmlCommentStyle {
parse_suppression_comment(text)
.filter_map(Result::ok)
.flat_map(|suppression| suppression.categories)
.any(|(key, _)| key == category!("format"))
.any(|(key, ..)| key == category!("format"))
}

fn get_comment_kind(_comment: &SyntaxTriviaPieceComments<HtmlLanguage>) -> CommentKind {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
`Object.assign($args)` where {
register_diagnostic(
span = $args,
message = "Prefer object spread instead of `Object.assign()`"
)
}
Loading

0 comments on commit c8c738e

Please sign in to comment.