Skip to content
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

feat: add plugin suppression #5139

Merged
merged 5 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 69 additions & 4 deletions crates/biome_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

use biome_console::markup;
use biome_parser::AnyParse;
use suppressions::LineSuppression;
use std::cmp::Ordering;
use std::collections::{BTreeMap, BinaryHeap};
use std::fmt::{Debug, Display, Formatter};
use std::ops;
Expand Down Expand Up @@ -190,10 +192,56 @@ 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.clone().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| {
if suppression.suppress_all {
return true;
}
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 +698,14 @@ impl<'a> AnalyzerSuppression<'a> {
}
}

pub fn plugin(plugin_name: &'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 +726,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. `// biome-ignore plugin/my-plugin`
Plugin(&'a str),
}

/// Takes a [Suppression] and returns a [AnalyzerSuppression]
Expand All @@ -682,9 +740,16 @@ 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!("plugin") {
if let Some(subcategory) = subcategory {
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
47 changes: 45 additions & 2 deletions crates/biome_analyze/src/suppressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct TopLevelSuppression {
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,

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

pub(crate) fn insert_plugin(&mut self, kind: &AnalyzerSuppressionKind) {
if let AnalyzerSuppressionKind::Plugin(plugin_name) = kind {
self.plugins.insert(plugin_name.to_string());
}
}

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.plugins.contains(plugin_name)
}

pub(crate) fn expand_range(&mut self, range: TextRange) {
self.range.cover(range);
}
Expand All @@ -89,6 +102,8 @@ 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` when a signal matching this suppression was emitted and
/// suppressed
pub(crate) did_suppress_signal: bool,
Expand Down Expand Up @@ -139,6 +154,15 @@ impl RangeSuppressions {
text_range: TextRange,
already_suppressed: Option<TextRange>,
) -> Result<(), AnalyzerSuppressionDiagnostic> {
if let Some(RuleFilter::Group("plugin")) = filter {
return Err(AnalyzerSuppressionDiagnostic::new(
category!("suppressions/incorrect"),
text_range,
markup!{"Found a "<Emphasis>"biome-range-"</Emphasis>" suppression on a plugin. This is not supported"}
).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 +294,7 @@ impl<'analyzer> Suppressions<'analyzer> {
fn push_line_suppression(
&mut self,
filter: Option<RuleFilter<'static>>,
plugin: Option<String>,
instance: Option<String>,
current_range: TextRange,
already_suppressed: Option<TextRange>,
Expand All @@ -284,6 +309,12 @@ impl<'analyzer> Suppressions<'analyzer> {
suppression.suppressed_rules.clear();
suppression.suppressed_instances.clear();
}
Some(RuleFilter::Group("plugin")) => { // TODO use predefined rule filter here
if let Some(plugin) = plugin {
suppression.suppressed_plugins.insert(plugin);
}
suppression.suppress_all = false;
}
Some(filter) => {
suppression.suppressed_rules.insert(filter);
if let Some(instance) = instance {
Expand Down Expand Up @@ -312,6 +343,9 @@ impl<'analyzer> Suppressions<'analyzer> {
if let Some(instance) = instance {
suppression.suppressed_instances.insert(instance, filter);
}
if let Some(plugin) = plugin {
suppression.suppressed_plugins.insert(plugin);
}
}
}
self.line_suppressions.push(suppression);
Expand All @@ -329,6 +363,7 @@ impl<'analyzer> Suppressions<'analyzer> {
AnalyzerSuppressionKind::Everything => return Ok(None),
AnalyzerSuppressionKind::Rule(rule) => rule,
AnalyzerSuppressionKind::RuleInstance(rule, _) => rule,
AnalyzerSuppressionKind::Plugin(_) => return Ok(Some(RuleFilter::Group("plugin"))),
};

let group_rule = rule.split_once('/');
Expand Down Expand Up @@ -357,11 +392,18 @@ 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(plugin_name) => Some((*plugin_name).to_string()),
_ => None,
}
}

pub(crate) fn push_suppression(
&mut self,
suppression: &AnalyzerSuppression,
Expand All @@ -370,11 +412,12 @@ 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 = 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)
self.push_line_suppression(filter, plugin, instances, comment_range, already_suppressed)
}
AnalyzerSuppressionVariant::TopLevel => self.top_level_suppression.push_suppression(
suppression,
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/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ struct CategoryVisitor;

#[cfg(feature = "serde")]
fn deserialize_parse<E: serde::de::Error>(code: &str) -> Result<&'static Category, E> {
println!("called deserialize_parse");
code.parse().map_err(|()| {
serde::de::Error::custom(format_args!("failed to deserialize category from {code}"))
})
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()`"
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 366
expression: preferObjectSpreadSuppression.grit
---
# Input
```js
// biome-ignore plugin/preferObjectSpreadSuppression: reason
Object.assign({ foo: 'bar'}, baz);

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// biome-ignore plugin/preferObjectSpreadSuppression: reason
Object.assign({ foo: 'bar'}, baz);
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()`"
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 366
expression: preferObjectSpreadSuppressionAll.grit
---
# Input
```js
// biome-ignore-all plugin/preferObjectSpreadSuppressionAll: reason

console.log("foo");

Object.assign({ foo: 'bar'}, baz);

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// biome-ignore-all plugin/preferObjectSpreadSuppressionAll: reason

console.log("foo");

Object.assign({ foo: 'bar'}, baz);
5 changes: 4 additions & 1 deletion crates/biome_js_analyze/tests/spec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,11 @@ fn run_plugin_test(input: &'static str, _: &str, _: &str, _: &str) {
Err(err) => panic!("Cannot load plugin: {err:?}"),
};

// Enable at least 1 rule so that PhaseRunner will be called
// which is necessary to parse and store supression comments
let rule_filter = RuleFilter::Rule("nursery", "noCommonJs");
let filter = AnalysisFilter {
enabled_rules: Some(&[]),
enabled_rules: Some(slice::from_ref(&rule_filter)),
..AnalysisFilter::default()
};

Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl CommentStyle for JsCommentStyle {
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<JsLanguage>) -> CommentKind {
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_json_formatter/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl CommentStyle for JsonCommentStyle {
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
Loading
Loading