refactor: make suppression action generic via trait (#2955)

crates/biome_analyze/src/lib.rs

@@ -16,6 +16,7 @@ mod registry;
 mod rule;
 mod services;
 mod signals;
+mod suppression_action;
 mod syntax;
 mod visitor;
 
@@ -44,6 +45,7 @@ pub use crate::signals::{
 };
 pub use crate::syntax::{Ast, SyntaxVisitor};
 pub use crate::visitor::{NodeVisitor, Visitor, VisitorContext, VisitorFinishContext};
+pub use suppression_action::{ApplySuppression, SuppressionAction};
 
 use biome_console::markup;
 use biome_diagnostics::{
@@ -70,7 +72,7 @@ pub struct Analyzer<'analyzer, L: Language, Matcher, Break, Diag> {
     /// Language-specific suppression comment parsing function
     parse_suppression_comment: SuppressionParser<Diag>,
     /// Language-specific suppression comment emitter
-    apply_suppression_comment: SuppressionCommentEmitter<L>,
+    suppression_action: Box<dyn SuppressionAction<Language = L>>,
     /// Handles analyzer signals emitted by individual rules
     emit_signal: SignalHandler<'analyzer, L, Break>,
 }
@@ -94,15 +96,15 @@ where
         metadata: &'analyzer MetadataRegistry,
         query_matcher: Matcher,
         parse_suppression_comment: SuppressionParser<Diag>,
-        apply_suppression_comment: SuppressionCommentEmitter<L>,
+        suppression_action: Box<dyn SuppressionAction<Language = L>>,
         emit_signal: SignalHandler<'analyzer, L, Break>,
     ) -> Self {
         Self {
             phases: BTreeMap::new(),
             metadata,
             query_matcher,
             parse_suppression_comment,
-            apply_suppression_comment,
+            suppression_action,
             emit_signal,
         }
     }
@@ -123,7 +125,7 @@ where
             mut query_matcher,
             parse_suppression_comment,
             mut emit_signal,
-            apply_suppression_comment,
+            suppression_action,
         } = self;
 
         let mut line_index = 0;
@@ -143,7 +145,7 @@ where
                 root: &ctx.root,
                 services: &ctx.services,
                 range: ctx.range,
-                apply_suppression_comment,
+                suppression_action: suppression_action.as_ref(),
                 options: ctx.options,
             };
 
@@ -208,7 +210,7 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break, Diag> {
     /// Language-specific suppression comment parsing function
     parse_suppression_comment: SuppressionParser<Diag>,
     /// Language-specific suppression comment emitter
-    apply_suppression_comment: SuppressionCommentEmitter<L>,
+    suppression_action: &'phase dyn SuppressionAction<Language = L>,
     /// Line index at the current position of the traversal
     line_index: &'phase mut usize,
     /// Track active suppression comments per-line, ordered by line index
@@ -280,7 +282,7 @@ where
                     range: self.range,
                     query_matcher: self.query_matcher,
                     signal_queue: &mut self.signal_queue,
-                    apply_suppression_comment: self.apply_suppression_comment,
+                    suppression_action: self.suppression_action,
                     options: self.options,
                 };
 
@@ -305,7 +307,7 @@ where
                     range: self.range,
                     query_matcher: self.query_matcher,
                     signal_queue: &mut self.signal_queue,
-                    apply_suppression_comment: self.apply_suppression_comment,
+                    suppression_action: self.suppression_action,
                     options: self.options,
                 };
 
@@ -755,9 +757,6 @@ pub struct SuppressionCommentEmitterPayload<'a, L: Language> {
     pub diagnostic_text_range: &'a TextRange,
 }
 
-/// Convenient type that to mark a function that is responsible to create a mutation to add a suppression comment.
-type SuppressionCommentEmitter<L> = fn(SuppressionCommentEmitterPayload<L>);
-
 type SignalHandler<'a, L, Break> = &'a mut dyn FnMut(&dyn AnalyzerSignal<L>) -> ControlFlow<Break>;
 
 /// Allow filtering a single rule or group of rules by their names

crates/biome_analyze/src/matcher.rs

@@ -1,6 +1,6 @@
 use crate::{
     AnalyzerOptions, AnalyzerSignal, Phases, QueryMatch, Rule, RuleFilter, RuleGroup, ServiceBag,
-    SuppressionCommentEmitter,
+    SuppressionAction,
 };
 use biome_rowan::{Language, TextRange};
 use std::{
@@ -25,7 +25,7 @@ pub struct MatchQueryParams<'phase, 'query, L: Language> {
     pub query: Query,
     pub services: &'phase ServiceBag,
     pub signal_queue: &'query mut BinaryHeap<SignalEntry<'phase, L>>,
-    pub apply_suppression_comment: SuppressionCommentEmitter<L>,
+    pub suppression_action: &'phase dyn SuppressionAction<Language = L>,
     pub options: &'phase AnalyzerOptions,
 }
 
@@ -198,16 +198,16 @@ where
 mod tests {
     use super::MatchQueryParams;
     use crate::{
-        signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerSignal, ControlFlow,
-        MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag, SignalEntry,
-        SyntaxVisitor,
+        signals::DiagnosticSignal, Analyzer, AnalyzerContext, AnalyzerSignal, ApplySuppression,
+        ControlFlow, MetadataRegistry, Never, Phases, QueryMatcher, RuleKey, ServiceBag,
+        SignalEntry, SuppressionAction, SyntaxVisitor,
     };
     use crate::{AnalyzerOptions, SuppressionKind};
     use biome_diagnostics::{category, DiagnosticExt};
     use biome_diagnostics::{Diagnostic, Severity};
     use biome_rowan::{
         raw_language::{RawLanguage, RawLanguageKind, RawLanguageRoot, RawSyntaxTreeBuilder},
-        AstNode, SyntaxNode, TextRange, TextSize, TriviaPiece,
+        AstNode, BatchMutation, SyntaxNode, SyntaxToken, TextRange, TextSize, TriviaPiece,
     };
     use std::convert::Infallible;
 
@@ -360,11 +360,33 @@ mod tests {
         let mut metadata = MetadataRegistry::default();
         metadata.insert_rule("group", "rule");
 
+        struct TestAction;
+
+        impl SuppressionAction for TestAction {
+            type Language = RawLanguage;
+
+            fn find_token_to_apply_suppression(
+                &self,
+                _: SyntaxToken<Self::Language>,
+            ) -> Option<ApplySuppression<Self::Language>> {
+                None
+            }
+
+            fn apply_suppression(
+                &self,
+                _: &mut BatchMutation<Self::Language>,
+                _: ApplySuppression<Self::Language>,
+                _: &str,
+            ) {
+                unreachable!("")
+            }
+        }
+
         let mut analyzer = Analyzer::new(
             &metadata,
             SuppressionMatcher,
             parse_suppression_comment,
-            |_| unreachable!(),
+            Box::new(TestAction),
             &mut emit_signal,
         );
 

crates/biome_analyze/src/registry.rs

@@ -439,7 +439,7 @@ impl<L: Language + Default> RegistryRule<L> {
                     query_result.clone(),
                     result,
                     params.services,
-                    params.apply_suppression_comment,
+                    params.suppression_action,
                     params.options,
                 ));
 

crates/biome_analyze/src/rule.rs

@@ -1,9 +1,7 @@
 use crate::categories::{ActionCategory, RuleCategory};
 use crate::context::RuleContext;
 use crate::registry::{RegistryVisitor, RuleLanguage, RuleSuppressions};
-use crate::{
-    Phase, Phases, Queryable, SuppressionCommentEmitter, SuppressionCommentEmitterPayload,
-};
+use crate::{Phase, Phases, Queryable, SuppressionAction, SuppressionCommentEmitterPayload};
 use biome_console::fmt::Display;
 use biome_console::{markup, MarkupBuf};
 use biome_diagnostics::advice::CodeSuggestionAdvice;
@@ -621,7 +619,7 @@ pub trait Rule: RuleMeta + Sized {
     fn suppress(
         ctx: &RuleContext<Self>,
         text_range: &TextRange,
-        apply_suppression_comment: SuppressionCommentEmitter<RuleLanguage<Self>>,
+        suppression_action: &dyn SuppressionAction<Language = RuleLanguage<Self>>,
     ) -> Option<SuppressAction<RuleLanguage<Self>>>
     where
         Self: 'static,
@@ -637,7 +635,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();
-            apply_suppression_comment(SuppressionCommentEmitterPayload {
+            suppression_action.apply_suppression_comment(SuppressionCommentEmitterPayload {
                 suppression_text: suppression_text.as_str(),
                 mutation: &mut mutation,
                 token_offset: token,

crates/biome_analyze/src/signals.rs

@@ -4,8 +4,7 @@ use crate::{
     context::RuleContext,
     registry::{RuleLanguage, RuleRoot},
     rule::Rule,
-    AnalyzerDiagnostic, AnalyzerOptions, Queryable, RuleGroup, ServiceBag,
-    SuppressionCommentEmitter,
+    AnalyzerDiagnostic, AnalyzerOptions, Queryable, RuleGroup, ServiceBag, SuppressionAction,
 };
 use biome_console::MarkupBuf;
 use biome_diagnostics::{advice::CodeSuggestionAdvice, Applicability, CodeSuggestion, Error};
@@ -310,7 +309,7 @@ pub(crate) struct RuleSignal<'phase, R: Rule> {
     state: R::State,
     services: &'phase ServiceBag,
     /// An optional action to suppress the rule.
-    apply_suppression_comment: SuppressionCommentEmitter<RuleLanguage<R>>,
+    suppression_action: &'phase dyn SuppressionAction<Language = RuleLanguage<R>>,
     /// A list of strings that are considered "globals" inside the analyzer
     options: &'phase AnalyzerOptions,
 }
@@ -324,17 +323,18 @@ where
         query_result: <<R as Rule>::Query as Queryable>::Output,
         state: R::State,
         services: &'phase ServiceBag,
-        apply_suppression_comment: SuppressionCommentEmitter<
-            <<R as Rule>::Query as Queryable>::Language,
+        suppression_action: &'phase dyn SuppressionAction<
+            Language = <<R as Rule>::Query as Queryable>::Language,
         >,
+
         options: &'phase AnalyzerOptions,
     ) -> Self {
         Self {
             root,
             query_result,
             state,
             services,
-            apply_suppression_comment,
+            suppression_action,
             options,
         }
     }
@@ -404,7 +404,7 @@ where
             };
             if let Some(text_range) = R::text_range(&ctx, &self.state) {
                 if let Some(suppression_action) =
-                    R::suppress(&ctx, &text_range, self.apply_suppression_comment)
+                    R::suppress(&ctx, &text_range, self.suppression_action)
                 {
                     let action = AnalyzerAction {
                         rule_name: Some((<R::Group as RuleGroup>::NAME, R::METADATA.name)),

crates/biome_analyze/src/suppression_action.rs

@@ -0,0 +1,86 @@
+use crate::SuppressionCommentEmitterPayload;
+use biome_rowan::{BatchMutation, Language, SyntaxToken, TextRange, TokenAtOffset};
+
+pub trait SuppressionAction {
+    type Language: Language;
+
+    fn apply_suppression_comment(&self, payload: SuppressionCommentEmitterPayload<Self::Language>) {
+        let SuppressionCommentEmitterPayload {
+            token_offset,
+            mutation,
+            suppression_text,
+            diagnostic_text_range,
+        } = payload;
+
+        // retrieve the most suited, most left token where the diagnostics was emitted
+        let original_token = self.get_token_from_offset(token_offset, diagnostic_text_range);
+
+        // 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())
+        });
+
+        if let Some(apply_suppression) = apply_suppression {
+            self.apply_suppression(mutation, apply_suppression, suppression_text);
+        }
+    }
+
+    /// Finds the first token, starting with the current token and traversing backwards,
+    /// until it find one that has a leading newline trivia.
+    ///
+    /// Sometimes, the offset is between tokens, we need to decide which one to take.
+    ///
+    /// For example:
+    /// ```jsx
+    /// function f() {
+    ///     return <div
+    ///     ><img /> {/* <--- diagnostic emitted in this line */}
+    ///     </div>
+    /// }
+    /// ```
+    ///
+    /// In these case it's best to peek the right token, because it belongs to the node where error actually occurred,
+    /// and becomes easier to add the suppression comment.
+    fn get_token_from_offset(
+        &self,
+        token_at_offset: TokenAtOffset<SyntaxToken<Self::Language>>,
+        diagnostic_text_range: &TextRange,
+    ) -> Option<SyntaxToken<Self::Language>> {
+        match token_at_offset {
+            TokenAtOffset::None => None,
+            TokenAtOffset::Single(token) => Some(token),
+            TokenAtOffset::Between(left_token, right_token) => {
+                let chosen_token =
+                    if right_token.text_range().start() == diagnostic_text_range.start() {
+                        right_token
+                    } else {
+                        left_token
+                    };
+                Some(chosen_token)
+            }
+        }
+    }
+
+    fn find_token_to_apply_suppression(
+        &self,
+        original_token: SyntaxToken<Self::Language>,
+    ) -> Option<ApplySuppression<Self::Language>>;
+
+    fn apply_suppression(
+        &self,
+        mutation: &mut BatchMutation<Self::Language>,
+        apply_suppression: ApplySuppression<Self::Language>,
+        suppression_text: &str,
+    );
+}
+
+/// Convenient type to store useful information
+pub struct ApplySuppression<L: Language> {
+    /// If the token is following by trailing comments
+    pub token_has_trailing_comments: bool,
+    /// The token to attach the suppression
+    pub token_to_apply_suppression: SyntaxToken<L>,
+    /// If the suppression should have a leading newline
+    pub should_insert_leading_newline: bool,
+}