diff --git a/.changeset/new_top_level_suppression_for_the_analyzer.md b/.changeset/new_top_level_suppression_for_the_analyzer.md new file mode 100644 index 000000000000..e9b5036c2574 --- /dev/null +++ b/.changeset/new_top_level_suppression_for_the_analyzer.md @@ -0,0 +1,34 @@ +--- +cli: minor +--- + +# New top-level suppression for the analyzer + +The Biome analyzer now supports a new top-level suppression. These suppression have to be placed at the top of the file, and they must be followed by two newlines (`\n\n\`). + +The analyzer rules specified inside the block comment will be suppressed for the whole file. + +In the example, we suppress the rules `lint/style/useConst` and `lint/suspicious/noDebugger` for the whole file: + +```js +// main.js +/** + * biome-ignore lint/style/useConst: i like let + * biome-ignore lint/suspicious/noDebugger: needed now + */ + +let path = "/path"; +let _tmp = undefined; +debugger +``` + +In this other example, we suppress `lint/suspicious/noEmptyBlock` for a whole CSS file: + +```css +/** +/* biome-ignore lint/suspicious/noEmptyBlock: it's fine to have empty blocks +*/ + +a {} +span {} +``` diff --git a/.changeset/remove_support_for_legacy_suppressions.md b/.changeset/remove_support_for_legacy_suppressions.md new file mode 100644 index 000000000000..4d84d68a8228 --- /dev/null +++ b/.changeset/remove_support_for_legacy_suppressions.md @@ -0,0 +1,13 @@ +--- +cli: major +--- + +# Remove support for legacy suppressions + +Biome used to support "legacy suppressions" that looked like this: + +```js +// biome-ignore lint(style/useWhile): reason +``` + +This format is no longer supported. diff --git a/.changeset/the_action_quickfixsuppressrule_is_removed.md b/.changeset/the_action_quickfixsuppressrule_is_removed.md new file mode 100644 index 000000000000..524039b91857 --- /dev/null +++ b/.changeset/the_action_quickfixsuppressrule_is_removed.md @@ -0,0 +1,35 @@ +--- +cli: major +--- + +# Remove the code action `quickfix.suppressRule` + +The code action `quickfix.suppressRule` was removed in favour of two new code actions: + +- `quickfix.suppressRule.inline.biome`: a code action that adds a suppression comment for each violation. +- `quickfix.suppressRule.topLevel.biome`: a code action that adds a suppression comment at the top of the file which suppresses a rule for the whole file. + + +Given the following code +```js +let foo = "one"; +debugger +``` + +The code action `quickfix.suppressRule.inline.biome` will result in the following code: +```js +// biome-ignore lint/style/useConst: +let foo = "one"; +// biome-ignore lint/suspicious/noDebugger: +debugger +``` + +The code action `quickfix.suppressRule.topLevel.biome`, instead, will result in the following code: +```js +/** biome-ignore lint/suspicious/noDebugger: */ +/** biome-ignore lint/style/useConst: */ + +let foo = "one"; +debugger; +``` + diff --git a/Cargo.lock b/Cargo.lock index b4787bfaf284..1d74cc3c6e4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -880,6 +880,7 @@ dependencies = [ "biome_json_parser", "biome_json_syntax", "biome_rowan", + "biome_suppression", "biome_test_utils", "insta", "natord", @@ -1229,7 +1230,7 @@ version = "0.5.7" [[package]] name = "biome_wasm" -version = "1.9.3" +version = "1.9.4" dependencies = [ "biome_console", "biome_diagnostics", diff --git a/crates/biome_analyze/src/categories.rs b/crates/biome_analyze/src/categories.rs index 308dfe2dbf58..749a04203151 100644 --- a/crates/biome_analyze/src/categories.rs +++ b/crates/biome_analyze/src/categories.rs @@ -22,7 +22,8 @@ pub enum RuleCategory { } /// Actions that suppress rules should start with this string -pub const SUPPRESSION_ACTION_CATEGORY: &str = "quickfix.suppressRule"; +pub const SUPPRESSION_INLINE_ACTION_CATEGORY: &str = "quickfix.suppressRule.inline"; +pub const SUPPRESSION_TOP_LEVEL_ACTION_CATEGORY: &str = "quickfix.suppressRule.topLevel"; /// The category of a code action, this type maps directly to the /// [CodeActionKind] type in the Language Server Protocol specification @@ -48,7 +49,21 @@ pub enum ActionCategory { Source(SourceActionKind), /// This action is using a base kind not covered by any of the previous /// variants - Other(Cow<'static, str>), + Other(OtherActionCategory), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema) +)] +pub enum OtherActionCategory { + /// Base kind for inline suppressions actions: `quickfix.suppressRule.inline.biome` + InlineSuppression, + /// Base kind for inline suppressions actions: `quickfix.suppressRule.topLevel.biome` + ToplevelSuppression, + /// Generic action that can't be mapped + Generic(Cow<'static, str>), } impl ActionCategory { @@ -57,7 +72,7 @@ impl ActionCategory { /// ## Examples /// /// ``` - /// use biome_analyze::{ActionCategory, RefactorKind}; + /// use biome_analyze::{ActionCategory, RefactorKind, OtherActionCategory}; /// /// assert!(ActionCategory::QuickFix.matches("quickfix")); /// assert!(!ActionCategory::QuickFix.matches("refactor")); @@ -67,6 +82,9 @@ impl ActionCategory { /// /// assert!(ActionCategory::Refactor(RefactorKind::Extract).matches("refactor")); /// assert!(ActionCategory::Refactor(RefactorKind::Extract).matches("refactor.extract")); + /// + /// assert!(ActionCategory::Other(OtherActionCategory::InlineSuppression).matches("quickfix.suppressRule.inline.biome")); + /// assert!(ActionCategory::Other(OtherActionCategory::ToplevelSuppression).matches("quickfix.suppressRule.topLevel.biome")); /// ``` pub fn matches(&self, filter: &str) -> bool { self.to_str().starts_with(filter) @@ -102,7 +120,15 @@ impl ActionCategory { Cow::Owned(format!("source.{tag}.biome")) } - ActionCategory::Other(tag) => Cow::Owned(format!("{tag}.biome")), + ActionCategory::Other(other_action) => match other_action { + OtherActionCategory::InlineSuppression => { + Cow::Borrowed("quickfix.suppressRule.inline.biome") + } + OtherActionCategory::ToplevelSuppression => { + Cow::Borrowed("quickfix.suppressRule.topLevel.biome") + } + OtherActionCategory::Generic(tag) => Cow::Owned(format!("{tag}.biome")), + }, } } } diff --git a/crates/biome_analyze/src/diagnostics.rs b/crates/biome_analyze/src/diagnostics.rs index 44cf2631c876..71929bf4234a 100644 --- a/crates/biome_analyze/src/diagnostics.rs +++ b/crates/biome_analyze/src/diagnostics.rs @@ -1,7 +1,7 @@ -use biome_console::MarkupBuf; +use biome_console::{markup, MarkupBuf}; use biome_diagnostics::{ advice::CodeSuggestionAdvice, category, Advices, Category, Diagnostic, DiagnosticExt, - DiagnosticTags, Error, Location, Severity, Visit, + DiagnosticTags, Error, Location, LogCategory, Severity, Visit, }; use biome_rowan::TextRange; use std::borrow::Cow; @@ -141,7 +141,7 @@ impl AnalyzerDiagnostic { #[derive(Debug, Diagnostic, Clone)] #[diagnostic(severity = Warning)] -pub struct SuppressionDiagnostic { +pub struct AnalyzerSuppressionDiagnostic { #[category] category: &'static Category, #[location(span)] @@ -151,9 +151,12 @@ pub struct SuppressionDiagnostic { message: String, #[tags] tags: DiagnosticTags, + + #[advice] + advice: SuppressionAdvice, } -impl SuppressionDiagnostic { +impl AnalyzerSuppressionDiagnostic { pub(crate) fn new( category: &'static Category, range: TextRange, @@ -164,15 +167,33 @@ impl SuppressionDiagnostic { range, message: message.to_string(), tags: DiagnosticTags::empty(), + advice: SuppressionAdvice::default(), } } - pub(crate) fn with_tags(mut self, tags: DiagnosticTags) -> Self { - self.tags |= tags; + pub(crate) fn note(mut self, message: impl Into, range: impl Into) -> Self { + self.advice.messages.push((message.into(), range.into())); self } } +#[derive(Debug, Default, Clone)] +struct SuppressionAdvice { + messages: Vec<(String, TextRange)>, +} + +impl Advices for SuppressionAdvice { + fn record(&self, visitor: &mut dyn Visit) -> std::io::Result<()> { + for (message, range) in &self.messages { + visitor.record_log(LogCategory::Info, &markup! {{message}})?; + let location = Location::builder().span(range); + + visitor.record_frame(location.build())? + } + Ok(()) + } +} + /// Series of errors encountered when running rules on a file #[derive(Debug, PartialEq, Eq, Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] diff --git a/crates/biome_analyze/src/lib.rs b/crates/biome_analyze/src/lib.rs index dbcd7168fb8d..792b043c6f49 100644 --- a/crates/biome_analyze/src/lib.rs +++ b/crates/biome_analyze/src/lib.rs @@ -1,5 +1,6 @@ #![deny(rustdoc::broken_intra_doc_links)] +use rustc_hash::{FxHashMap, FxHashSet}; use std::cmp::Ordering; use std::collections::{BTreeMap, BinaryHeap}; use std::fmt::{Debug, Display, Formatter}; @@ -24,10 +25,11 @@ mod visitor; pub use biome_diagnostics::category_concat; pub use crate::categories::{ - ActionCategory, RefactorKind, RuleCategories, RuleCategoriesBuilder, RuleCategory, - SourceActionKind, SUPPRESSION_ACTION_CATEGORY, + ActionCategory, OtherActionCategory, RefactorKind, RuleCategories, RuleCategoriesBuilder, + RuleCategory, SourceActionKind, SUPPRESSION_INLINE_ACTION_CATEGORY, + SUPPRESSION_TOP_LEVEL_ACTION_CATEGORY, }; -pub use crate::diagnostics::{AnalyzerDiagnostic, RuleError, SuppressionDiagnostic}; +pub use crate::diagnostics::{AnalyzerDiagnostic, AnalyzerSuppressionDiagnostic, RuleError}; pub use crate::matcher::{InspectMatcher, MatchQueryParams, QueryMatcher, RuleKey, SignalEntry}; pub use crate::options::{AnalyzerConfiguration, AnalyzerOptions, AnalyzerRules}; pub use crate::query::{AddVisitor, QueryKey, QueryMatch, Queryable}; @@ -45,8 +47,7 @@ pub use crate::signals::{ }; pub use crate::syntax::{Ast, SyntaxVisitor}; pub use crate::visitor::{NodeVisitor, Visitor, VisitorContext, VisitorFinishContext}; -use biome_console::markup; -use biome_diagnostics::{category, Applicability, Diagnostic, DiagnosticExt, DiagnosticTags}; +use biome_diagnostics::{category, Diagnostic, DiagnosticExt}; use biome_rowan::{ AstNode, BatchMutation, Direction, Language, SyntaxElement, SyntaxToken, TextRange, TextSize, TokenAtOffset, TriviaPieceKind, WalkEvent, @@ -67,7 +68,7 @@ pub struct Analyzer<'analyzer, L: Language, Matcher, Break, Diag> { /// Executor for the query matches emitted by the visitors query_matcher: Matcher, /// Language-specific suppression comment parsing function - parse_suppression_comment: SuppressionParser, + parse_suppression_comment: SuppressionParser, /// Language-specific suppression comment emitter suppression_action: Box>, /// Handles analyzer signals emitted by individual rules @@ -92,7 +93,7 @@ where pub fn new( metadata: &'analyzer MetadataRegistry, query_matcher: Matcher, - parse_suppression_comment: SuppressionParser, + parse_suppression_comment: SuppressionParser, suppression_action: Box>, emit_signal: SignalHandler<'analyzer, L, Break>, ) -> Self { @@ -127,6 +128,7 @@ where let mut line_index = 0; let mut line_suppressions = Vec::new(); + let mut top_level_suppressions = TopLevelSuppressions::default(); for (index, (phase, mut visitors)) in phases.into_iter().enumerate() { let runner = PhaseRunner { @@ -144,6 +146,7 @@ where range: ctx.range, suppression_action: suppression_action.as_ref(), options: ctx.options, + top_level_suppressions: &mut top_level_suppressions, }; // The first phase being run will inspect the tokens and parse the @@ -176,11 +179,22 @@ where } let signal = DiagnosticSignal::new(|| { - SuppressionDiagnostic::new( + if suppression.already_suppressed { + AnalyzerSuppressionDiagnostic::new( + category!("suppressions/unused"), + suppression.comment_span, + "Suppression comment has no effect because another suppression comment suppresses the same rule.", + ).note( + "This is the suppression comment that was used.", + top_level_suppressions.range.unwrap_or_default() + ) + } else { + AnalyzerSuppressionDiagnostic::new( category!("suppressions/unused"), suppression.comment_span, "Suppression comment has no effect. Remove the suppression or make sure you are suppressing the correct rule.", ) + } }); if let ControlFlow::Break(br) = (emit_signal)(&signal) { @@ -205,7 +219,7 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break, Diag> { /// Queue for pending analyzer signals signal_queue: BinaryHeap>, /// Language-specific suppression comment parsing function - parse_suppression_comment: SuppressionParser, + parse_suppression_comment: SuppressionParser, /// Language-specific suppression comment emitter suppression_action: &'phase dyn SuppressionAction, /// Line index at the current position of the traversal @@ -222,6 +236,36 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break, Diag> { range: Option, /// Analyzer options options: &'phase AnalyzerOptions, + + top_level_suppressions: &'phase mut TopLevelSuppressions, +} + +#[derive(Debug, Default)] +struct TopLevelSuppressions { + filters: FxHashSet>, + range: Option, +} + +impl TopLevelSuppressions { + pub(crate) fn insert(&mut self, filter: RuleFilter<'static>) { + self.filters.insert(filter); + } + + pub(crate) fn suppressed_rule(&self, filter: &RuleKey) -> bool { + self.filters.iter().any(|f| f == filter) + } + + pub(crate) fn expand_range(&mut self, range: TextRange) { + if let Some(current_range) = self.range.as_mut() { + current_range.cover(range); + } else { + self.range = Some(range); + } + } + + pub(crate) fn has_filter(&self, filter: &RuleFilter) -> bool { + self.filters.contains(filter) + } } /// Single entry for a suppression comment in the `line_suppressions` buffer @@ -238,12 +282,14 @@ struct LineSuppression { suppress_all: bool, /// List of all the rules this comment has started suppressing (must be /// removed from the suppressed set on expiration) - suppressed_rules: Vec>, + suppressed_rules: FxHashSet>, /// List of all the rule instances this comment has started suppressing. - suppressed_instances: Vec<(RuleFilter<'static>, String)>, + suppressed_instances: FxHashMap>, /// Set to `true` when a signal matching this suppression was emitted and /// suppressed 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 + already_suppressed: bool, } impl<'a, 'phase, L, Matcher, Break, Diag> PhaseRunner<'a, 'phase, L, Matcher, Break, Diag> @@ -373,6 +419,11 @@ where } } + if self.top_level_suppressions.suppressed_rule(&entry.rule) { + self.signal_queue.pop(); + break; + } + // Search for an active suppression comment covering the range of // this signal: first try to load the last line suppression and see // if it matches the current line index, otherwise perform a binary @@ -421,11 +472,11 @@ where suppression .suppressed_instances .iter() - .any(|(filter, v)| *filter == entry.rule && v == value.as_ref()) + .any(|(v, filter)| *filter == entry.rule && v == value.as_ref()) }) }); - // If the signal is being suppressed mark the line suppression as + // If the signal is being suppressed, mark the line suppression as // hit, otherwise emit the signal if let Some(suppression) = suppression { suppression.did_suppress_signal = true; @@ -445,17 +496,18 @@ where fn handle_comment( &mut self, token: &SyntaxToken, - is_leading: bool, - index: usize, + _is_leading: bool, + _index: usize, text: &str, range: TextRange, ) -> ControlFlow { let mut suppress_all = false; - let mut suppressed_rules = Vec::new(); - let mut suppressed_instances = Vec::new(); - let mut has_legacy = false; + let mut suppressed_rules = FxHashSet::default(); + let mut suppressed_instances = FxHashMap::default(); + let mut is_top_level_suppression = false; + let mut already_suppressed = false; - for result in (self.parse_suppression_comment)(text) { + for result in (self.parse_suppression_comment)(text, token) { let kind = match result { Ok(kind) => kind, Err(diag) => { @@ -475,9 +527,11 @@ where SuppressionKind::Everything => (None, None), SuppressionKind::Rule(rule) => (Some(rule), None), SuppressionKind::RuleInstance(rule, instance) => (Some(rule), Some(instance)), - SuppressionKind::MaybeLegacy(rule) => (Some(rule), None), + SuppressionKind::TopLevel(rule) => (Some(rule), None), }; + is_top_level_suppression |= kind.is_top_level(); + if let Some(rule) = rule { let group_rule = rule.split_once('/'); @@ -489,15 +543,28 @@ where }; match (key, instance) { - (Some(key), Some(value)) => suppressed_instances.push((key, value.to_owned())), + (Some(key), Some(value)) => { + suppressed_instances.insert(value.to_owned(), key); + if kind.is_top_level() { + self.top_level_suppressions.insert(key); + } + if self.top_level_suppressions.has_filter(&key) { + already_suppressed = true + } + } (Some(key), None) => { - suppressed_rules.push(key); - has_legacy |= matches!(kind, SuppressionKind::MaybeLegacy(_)); + if kind.is_top_level() { + self.top_level_suppressions.insert(key); + } + if self.top_level_suppressions.has_filter(&key) { + already_suppressed = true + } + suppressed_rules.insert(key); } _ if range_match(self.range, range) => { // Emit a warning for the unknown rule let signal = DiagnosticSignal::new(move || match group_rule { - Some((group, rule)) => SuppressionDiagnostic::new( + Some((group, rule)) => AnalyzerSuppressionDiagnostic::new( category!("suppressions/unknownRule"), range, format_args!( @@ -505,7 +572,7 @@ where ), ), - None => SuppressionDiagnostic::new( + None => AnalyzerSuppressionDiagnostic::new( category!("suppressions/unknownGroup"), range, format_args!( @@ -527,21 +594,9 @@ where } } - // Emit a warning for legacy suppression syntax - if has_legacy && range_match(self.range, range) { - let signal = DiagnosticSignal::new(move || { - SuppressionDiagnostic::new( - category!("suppressions/deprecatedSuppressionComment"), - range, - "Suppression is using a deprecated syntax", - ) - .with_tags(DiagnosticTags::DEPRECATED_CODE) - }); - - let signal = signal - .with_action(|| update_suppression(self.root, token, is_leading, index, text)); - - (self.emit_signal)(&signal)?; + if is_top_level_suppression { + self.top_level_suppressions.expand_range(range); + return ControlFlow::Continue(()); } if !suppress_all && suppressed_rules.is_empty() && suppressed_instances.is_empty() { @@ -581,6 +636,7 @@ where suppressed_rules, suppressed_instances, did_suppress_signal: false, + already_suppressed, }; self.line_suppressions.push(entry); @@ -624,8 +680,12 @@ fn range_match(filter: Option, range: TextRange) -> bool { /// Signature for a suppression comment parser function /// -/// This function receives the text content of a comment and returns a list of -/// lint suppressions as an optional lint rule (if the lint rule is `None` the +/// This function receives two parameters: +/// 1. The text content of a comment. +/// 2. The range of the token the comment belongs too. The range is calculated from [SyntaxToken::text_range], so the range +/// includes all trivia. +/// +/// It returns the lint suppressions as an optional lint rule (if the lint rule is `None` the /// comment is interpreted as suppressing all lints) /// /// # Examples @@ -635,75 +695,27 @@ fn range_match(filter: Option, range: TextRange) -> bool { /// - `// biome-ignore lint/style/useWhile` -> `vec![Rule("style/useWhile")]` /// - `// biome-ignore lint/style/useWhile(foo)` -> `vec![RuleWithValue("style/useWhile", "foo")]` /// - `// biome-ignore lint/style/useWhile lint/nursery/noUnreachable` -> `vec![Rule("style/useWhile"), Rule("nursery/noUnreachable")]` -/// - `// biome-ignore lint(style/useWhile)` -> `vec![MaybeLegacy("style/useWhile")]` -/// - `// biome-ignore lint(style/useWhile) lint(nursery/noUnreachable)` -> `vec![MaybeLegacy("style/useWhile"), MaybeLegacy("nursery/noUnreachable")]` -type SuppressionParser = fn(&str) -> Vec>; +/// - `/** biome-ignore lint/style/useWhile */` if the comment is top-level -> `vec![TopLevel("style/useWhile")]` +type SuppressionParser = + for<'a> fn(&'a str, &'a SyntaxToken) -> Vec, D>>; +#[derive(Debug, Clone)] /// This enum is used to categorize what is disabled by a suppression comment and with what syntax pub enum SuppressionKind<'a> { + // TODO: docs + TopLevel(&'a str), /// A suppression disabling all lints eg. `// biome-ignore lint` Everything, /// A suppression disabling a specific rule eg. `// biome-ignore lint/style/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 using the legacy syntax to disable a specific rule eg. `// biome-ignore lint(style/useWhile)` - MaybeLegacy(&'a str), } -fn update_suppression( - root: &L::Root, - token: &SyntaxToken, - is_leading: bool, - index: usize, - text: &str, -) -> Option> { - let old_token = token.clone(); - let new_token = token.clone().detach(); - - let old_trivia = if is_leading { - old_token.leading_trivia() - } else { - old_token.trailing_trivia() - }; - - let old_trivia: Vec<_> = old_trivia.pieces().collect(); - - let mut text = text.to_string(); - - while let Some(range_start) = text.find("lint(") { - let range_end = range_start + text[range_start..].find(')')?; - text.replace_range(range_end..range_end + 1, ""); - text.replace_range(range_start + 4..range_start + 5, "/"); +impl<'a> SuppressionKind<'a> { + pub const fn is_top_level(&self) -> bool { + matches!(self, SuppressionKind::TopLevel(_)) } - - let new_trivia = old_trivia.iter().enumerate().map(|(piece_index, piece)| { - if piece_index == index { - (piece.kind(), text.as_str()) - } else { - (piece.kind(), piece.text()) - } - }); - - let new_token = if is_leading { - new_token.with_leading_trivia(new_trivia) - } else { - new_token.with_trailing_trivia(new_trivia) - }; - - let mut mutation = BatchMutation::new(root.syntax().clone()); - mutation.replace_token_discard_trivia(old_token, new_token); - - Some(AnalyzerAction { - rule_name: None, - category: ActionCategory::QuickFix, - applicability: Applicability::Always, - message: markup! { - "Rewrite suppression to use the newer syntax" - } - .to_owned(), - mutation, - }) } /// Payload received by the function responsible to mark a suppression comment diff --git a/crates/biome_analyze/src/matcher.rs b/crates/biome_analyze/src/matcher.rs index a88858bcb53d..64f98f277a22 100644 --- a/crates/biome_analyze/src/matcher.rs +++ b/crates/biome_analyze/src/matcher.rs @@ -349,9 +349,10 @@ mod tests { ControlFlow::Continue(()) }; - fn parse_suppression_comment( - comment: &'_ str, - ) -> Vec, Infallible>> { + fn parse_suppression_comment<'a>( + comment: &'a str, + _token: &'_ SyntaxToken, + ) -> Vec, Infallible>> { comment .trim_start_matches("//") .split(' ') @@ -368,14 +369,14 @@ mod tests { impl SuppressionAction for TestAction { type Language = RawLanguage; - fn find_token_to_apply_suppression( + fn find_token_for_inline_suppression( &self, _: SyntaxToken, ) -> Option> { None } - fn apply_suppression( + fn apply_inline_suppression( &self, _: &mut BatchMutation, _: ApplySuppression, @@ -383,6 +384,15 @@ mod tests { ) { unreachable!("") } + + fn apply_top_level_suppression( + &self, + _: &mut BatchMutation, + _: SyntaxToken, + _: &str, + ) { + unreachable!("") + } } let mut analyzer = Analyzer::new( diff --git a/crates/biome_analyze/src/rule.rs b/crates/biome_analyze/src/rule.rs index e69561e1effb..0d7c9a9a73ab 100644 --- a/crates/biome_analyze/src/rule.rs +++ b/crates/biome_analyze/src/rule.rs @@ -875,9 +875,42 @@ pub trait Rule: RuleMeta + Sized { None } + fn top_level_suppression( + ctx: &RuleContext, + suppression_action: &dyn SuppressionAction>, + ) -> Option>> + where + Self: 'static, + { + if ::Category::CATEGORY == RuleCategory::Lint { + let rule_category = format!( + "lint/{}/{}", + ::NAME, + Self::METADATA.name + ); + let suppression_text = format!("biome-ignore {rule_category}"); + let root = ctx.root(); + + if let Some(first_token) = root.syntax().first_token() { + let mut mutation = root.begin(); + suppression_action.apply_top_level_suppression( + &mut mutation, + first_token, + suppression_text.as_str(), + ); + return Some(SuppressAction { + mutation, + message: markup! { "Suppress rule " {rule_category} " for the whole file."} + .to_owned(), + }); + } + } + None + } + /// Create a code action that allows to suppress the rule. The function /// returns the node to which the suppression comment is applied. - fn suppress( + fn inline_suppression( ctx: &RuleContext, text_range: &TextRange, suppression_action: &dyn SuppressionAction>, @@ -896,7 +929,7 @@ pub trait Rule: RuleMeta + Sized { let root = ctx.root(); let token = root.syntax().token_at_offset(text_range.start()); let mut mutation = root.begin(); - suppression_action.apply_suppression_comment(SuppressionCommentEmitterPayload { + suppression_action.inline_suppression(SuppressionCommentEmitterPayload { suppression_text: suppression_text.as_str(), mutation: &mut mutation, token_offset: token, diff --git a/crates/biome_analyze/src/signals.rs b/crates/biome_analyze/src/signals.rs index d69e97b77f8b..1490cbe6aae0 100644 --- a/crates/biome_analyze/src/signals.rs +++ b/crates/biome_analyze/src/signals.rs @@ -1,15 +1,17 @@ -use crate::categories::SUPPRESSION_ACTION_CATEGORY; +use crate::categories::{ + SUPPRESSION_INLINE_ACTION_CATEGORY, SUPPRESSION_TOP_LEVEL_ACTION_CATEGORY, +}; use crate::{ categories::ActionCategory, context::RuleContext, registry::{RuleLanguage, RuleRoot}, rule::Rule, - AnalyzerDiagnostic, AnalyzerOptions, Queryable, RuleGroup, ServiceBag, SuppressionAction, + AnalyzerDiagnostic, AnalyzerOptions, OtherActionCategory, Queryable, RuleGroup, ServiceBag, + SuppressionAction, }; use biome_console::MarkupBuf; use biome_diagnostics::{advice::CodeSuggestionAdvice, Applicability, CodeSuggestion, Error}; use biome_rowan::{BatchMutation, Language}; -use std::borrow::Cow; use std::iter::FusedIterator; use std::marker::PhantomData; use std::vec::IntoIter; @@ -115,7 +117,15 @@ pub struct AnalyzerAction { impl AnalyzerAction { pub fn is_suppression(&self) -> bool { - self.category.matches(SUPPRESSION_ACTION_CATEGORY) + self.is_inline_suppression() || self.is_top_level_suppression() + } + + pub fn is_inline_suppression(&self) -> bool { + self.category.matches(SUPPRESSION_INLINE_ACTION_CATEGORY) + } + + pub fn is_top_level_suppression(&self) -> bool { + self.category.matches(SUPPRESSION_TOP_LEVEL_ACTION_CATEGORY) } } @@ -390,8 +400,8 @@ where self.options.jsx_runtime(), ) .ok(); + let mut actions = Vec::new(); if let Some(ctx) = ctx { - let mut actions = Vec::new(); if let Some(action) = R::action(&ctx, &self.state) { actions.push(AnalyzerAction { rule_name: Some((::NAME, R::METADATA.name)), @@ -403,11 +413,11 @@ where }; if let Some(text_range) = R::text_range(&ctx, &self.state) { if let Some(suppression_action) = - R::suppress(&ctx, &text_range, self.suppression_action) + R::inline_suppression(&ctx, &text_range, self.suppression_action) { let action = AnalyzerAction { rule_name: Some((::NAME, R::METADATA.name)), - category: ActionCategory::Other(Cow::Borrowed(SUPPRESSION_ACTION_CATEGORY)), + category: ActionCategory::Other(OtherActionCategory::InlineSuppression), applicability: Applicability::Always, mutation: suppression_action.mutation, message: suppression_action.message, @@ -416,6 +426,19 @@ where } } + if let Some(suppression_action) = + R::top_level_suppression(&ctx, self.suppression_action) + { + let action = AnalyzerAction { + rule_name: Some((::NAME, R::METADATA.name)), + category: ActionCategory::Other(OtherActionCategory::ToplevelSuppression), + applicability: Applicability::Always, + mutation: suppression_action.mutation, + message: suppression_action.message, + }; + actions.push(action); + } + AnalyzerActionIter::new(actions) } else { AnalyzerActionIter::new(vec![]) diff --git a/crates/biome_analyze/src/suppression_action.rs b/crates/biome_analyze/src/suppression_action.rs index 8ad8d022bd8f..7d01f889530b 100644 --- a/crates/biome_analyze/src/suppression_action.rs +++ b/crates/biome_analyze/src/suppression_action.rs @@ -4,7 +4,7 @@ use biome_rowan::{BatchMutation, Language, SyntaxToken, TextRange, TokenAtOffset pub trait SuppressionAction { type Language: Language; - fn apply_suppression_comment(&self, payload: SuppressionCommentEmitterPayload) { + fn inline_suppression(&self, payload: SuppressionCommentEmitterPayload) { let SuppressionCommentEmitterPayload { token_offset, mutation, @@ -18,11 +18,11 @@ pub trait SuppressionAction { // considering that our suppression system works via lines, we need to look for the first newline, // so we can place the comment there let apply_suppression = original_token.as_ref().and_then(|original_token| { - self.find_token_to_apply_suppression(original_token.clone()) + self.find_token_for_inline_suppression(original_token.clone()) }); if let Some(apply_suppression) = apply_suppression { - self.apply_suppression(mutation, apply_suppression, suppression_text); + self.apply_inline_suppression(mutation, apply_suppression, suppression_text); } } @@ -62,17 +62,24 @@ pub trait SuppressionAction { } } - fn find_token_to_apply_suppression( + fn find_token_for_inline_suppression( &self, original_token: SyntaxToken, ) -> Option>; - fn apply_suppression( + fn apply_inline_suppression( &self, mutation: &mut BatchMutation, apply_suppression: ApplySuppression, suppression_text: &str, ); + + fn apply_top_level_suppression( + &self, + mutation: &mut BatchMutation, + token: SyntaxToken, + suppression_text: &str, + ); } /// Convenient type to store useful information diff --git a/crates/biome_analyze/src/syntax.rs b/crates/biome_analyze/src/syntax.rs index 7852c8957b34..929c89482d8d 100644 --- a/crates/biome_analyze/src/syntax.rs +++ b/crates/biome_analyze/src/syntax.rs @@ -154,14 +154,14 @@ mod tests { impl SuppressionAction for TestAction { type Language = RawLanguage; - fn find_token_to_apply_suppression( + fn find_token_for_inline_suppression( &self, _: SyntaxToken, ) -> Option> { None } - fn apply_suppression( + fn apply_inline_suppression( &self, _: &mut BatchMutation, _: ApplySuppression, @@ -169,12 +169,21 @@ mod tests { ) { unreachable!("") } + + fn apply_top_level_suppression( + &self, + _: &mut BatchMutation, + _: SyntaxToken, + _: &str, + ) { + unreachable!("") + } } let mut analyzer = Analyzer::new( &metadata, &mut matcher, - |_| -> Vec> { unreachable!() }, + |_, _| -> Vec> { unreachable!() }, Box::new(TestAction), &mut emit_signal, ); diff --git a/crates/biome_cli/tests/cases/suppressions.rs b/crates/biome_cli/tests/cases/suppressions.rs index 1c861b0468b0..4ec7df3d98a0 100644 --- a/crates/biome_cli/tests/cases/suppressions.rs +++ b/crates/biome_cli/tests/cases/suppressions.rs @@ -1,11 +1,10 @@ -use bpaf::Args; -use std::path::Path; - use crate::snap_test::SnapshotPayload; use crate::{assert_cli_snapshot, run_cli, FORMATTED}; use biome_console::BufferConsole; use biome_fs::{FileSystemExt, MemoryFileSystem}; use biome_service::DynRef; +use bpaf::Args; +use std::path::Path; const SUPPRESS_BEFORE: &str = "(1 >= -0)"; const SUPPRESS_AFTER: &str = @@ -231,3 +230,41 @@ fn suppress_skip_ok() { result, )); } + + +#[test] +fn unused_suppression_after_top_level() { + let mut fs = MemoryFileSystem::default(); + let mut console = BufferConsole::default(); + + let file_path = Path::new("file.js"); + fs.insert( + file_path.into(), + *b"/** +* biome-ignore lint/style/useConst: reason +*/ + + +let foo = 2; +/** +* biome-ignore lint/style/useConst: reason +*/ +let bar = 33;", + ); + + let result = run_cli( + DynRef::Borrowed(&mut fs), + &mut console, + Args::from([("lint"), file_path.as_os_str().to_str().unwrap()].as_slice()), + ); + + assert!(result.is_ok(), "run_cli returned {result:?}"); + + assert_cli_snapshot(SnapshotPayload::new( + module_path!(), + "unused_suppression_after_top_level", + fs, + console, + result, + )); +} diff --git a/crates/biome_cli/tests/commands/check.rs b/crates/biome_cli/tests/commands/check.rs index d33d4a10360e..d9c42e4be2f6 100644 --- a/crates/biome_cli/tests/commands/check.rs +++ b/crates/biome_cli/tests/commands/check.rs @@ -1377,36 +1377,6 @@ fn no_supported_file_found() { result, )); } - -#[test] -fn deprecated_suppression_comment() { - let mut fs = MemoryFileSystem::default(); - let mut console = BufferConsole::default(); - - let file_path = Path::new("file.js"); - fs.insert( - file_path.into(), - *b"// biome-ignore lint(suspicious/noDoubleEquals): test -a == b;", - ); - - let result = run_cli( - DynRef::Borrowed(&mut fs), - &mut console, - Args::from([("check"), file_path.as_os_str().to_str().unwrap()].as_slice()), - ); - - assert!(result.is_err(), "run_cli returned {result:?}"); - - assert_cli_snapshot(SnapshotPayload::new( - module_path!(), - "deprecated_suppression_comment", - fs, - console, - result, - )); -} - #[test] fn print_verbose() { let mut fs = MemoryFileSystem::default(); diff --git a/crates/biome_cli/tests/commands/lint.rs b/crates/biome_cli/tests/commands/lint.rs index f43ab9014172..155a414a75f8 100644 --- a/crates/biome_cli/tests/commands/lint.rs +++ b/crates/biome_cli/tests/commands/lint.rs @@ -1559,35 +1559,6 @@ fn no_supported_file_found() { )); } -#[test] -fn deprecated_suppression_comment() { - let mut fs = MemoryFileSystem::default(); - let mut console = BufferConsole::default(); - - let file_path = Path::new("file.js"); - fs.insert( - file_path.into(), - *b"// biome-ignore lint(suspicious/noDoubleEquals): test -a == b;", - ); - - let result = run_cli( - DynRef::Borrowed(&mut fs), - &mut console, - Args::from([("lint"), file_path.as_os_str().to_str().unwrap()].as_slice()), - ); - - assert!(result.is_ok(), "run_cli returned {result:?}"); - - assert_cli_snapshot(SnapshotPayload::new( - module_path!(), - "deprecated_suppression_comment", - fs, - console, - result, - )); -} - #[test] fn print_verbose() { let mut fs = MemoryFileSystem::default(); diff --git a/crates/biome_cli/tests/snapshots/main_cases_suppressions/unused_suppression_after_top_level.snap b/crates/biome_cli/tests/snapshots/main_cases_suppressions/unused_suppression_after_top_level.snap new file mode 100644 index 000000000000..c4e339836c93 --- /dev/null +++ b/crates/biome_cli/tests/snapshots/main_cases_suppressions/unused_suppression_after_top_level.snap @@ -0,0 +1,50 @@ +--- +source: crates/biome_cli/tests/snap_test.rs +expression: content +--- +## `file.js` + +```js +/** +* biome-ignore lint/style/useConst: reason +*/ + + +let foo = 2; +/** +* biome-ignore lint/style/useConst: reason +*/ +let bar = 33; +``` + +# Emitted Messages + +```block +file.js:7:1 suppressions/unused ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Suppression comment has no effect because another suppression comment suppresses the same rule. + + 6 │ let foo = 2; + > 7 │ /** + │ ^^^ + > 8 │ * biome-ignore lint/style/useConst: reason + > 9 │ */ + │ ^^ + 10 │ let bar = 33; + + i This is the suppression comment that was used. + + > 1 │ /** + │ ^^^ + > 2 │ * biome-ignore lint/style/useConst: reason + > 3 │ */ + │ ^^ + 4 │ + + +``` + +```block +Checked 1 file in