feat(rome_analyze): add a warning for unused suppression comments

crates/rome_analyze/src/lib.rs

@@ -162,6 +162,27 @@ where
             }
         }
 
+        for suppression in line_suppressions {
+            if suppression.did_suppress_signal {
+                continue;
+            }
+
+            let signal = DiagnosticSignal::new(|| {
+                let diag = SuppressionDiagnostic::new(
+                    ctx.file_id,
+                    category!("suppressions/unused"),
+                    suppression.comment_span,
+                    "Suppression comment is not being used",
+                );
+
+                AnalyzerDiagnostic::from_error(diag.into())
+            });
+
+            if let ControlFlow::Break(br) = (emit_signal)(&signal) {
+                return Some(br);
+            }
+        }
+
         None
     }
 }
@@ -203,6 +224,8 @@ struct PhaseRunner<'analyzer, 'phase, L: Language, Matcher, Break> {
 struct LineSuppression {
     /// Line index this comment is suppressing lint rules for
     line_index: usize,
+    /// Range of source text covered by the suppression comment
+    comment_span: TextRange,
     /// Range of source text this comment is suppressing lint rules for
     text_range: TextRange,
     /// Set to true if this comment has set the `suppress_all` flag to true
@@ -211,6 +234,9 @@ struct LineSuppression {
     /// List of all the rules this comment has started suppressing (must be
     /// removed from the suppressed set on expiration)
     suppressed_rules: Vec<RuleFilter<'static>>,
+    /// Set to `true` when a signal matching this suppression was emitted and
+    /// suppressed
+    did_suppress_signal: bool,
 }
 
 impl<'a, 'phase, L, Matcher, Break> PhaseRunner<'a, 'phase, L, Matcher, Break>
@@ -344,14 +370,14 @@ where
             // if it matchs the current line index, otherwise perform a binary
             // search over all the previously seen suppressions to find one
             // with a matching range
-            let suppression = self
-                .line_suppressions
-                .last()
-                .filter(|suppression| {
-                    suppression.line_index == *self.line_index
-                        && suppression.text_range.start() <= start
-                })
-                .or_else(|| {
+            let suppression = self.line_suppressions.last_mut().filter(|suppression| {
+                suppression.line_index == *self.line_index
+                    && suppression.text_range.start() <= start
+            });
+
+            let suppression = match suppression {
+                Some(suppression) => Some(suppression),
+                None => {
                     let index = self.line_suppressions.binary_search_by(|suppression| {
                         if suppression.text_range.end() < entry.text_range.start() {
                             Ordering::Less
@@ -362,21 +388,26 @@ where
                         }
                     });
 
-                    Some(&self.line_suppressions[index.ok()?])
-                });
+                    index.ok().map(|index| &mut self.line_suppressions[index])
+                }
+            };
 
-            let is_suppressed = suppression.map_or(false, |suppression| {
+            let suppression = suppression.filter(|suppression| {
                 if suppression.suppress_all {
                     return true;
                 }
+
                 suppression
                     .suppressed_rules
                     .iter()
                     .any(|filter| *filter == entry.rule)
             });
 
-            // Emit the signal if the rule that created it is not currently being suppressed
-            if !is_suppressed {
+            // 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;
+            } else {
                 (self.emit_signal)(&*entry.signal)?;
             }
 
@@ -479,9 +510,11 @@ where
 
         let entry = LineSuppression {
             line_index,
+            comment_span: range,
             text_range: range,
             suppress_all,
             suppressed_rules: suppressions,
+            did_suppress_signal: false,
         };
 
         self.line_suppressions.push(entry);

crates/rome_analyze/src/matcher.rs

@@ -211,7 +211,7 @@ mod tests {
             let mut builder = RawSyntaxTreeBuilder::new();
 
             builder.start_node(RawLanguageKind::ROOT);
-            builder.start_node(RawLanguageKind::EXPRESSION_LIST);
+            builder.start_node(RawLanguageKind::SEPARATED_EXPRESSION_LIST);
 
             builder.start_node(RawLanguageKind::LITERAL_EXPRESSION);
             builder.token_with_trivia(
@@ -289,6 +289,16 @@ mod tests {
                 &[TriviaPiece::new(TriviaPieceKind::Newline, 1)],
             );
 
+            builder.token_with_trivia(
+                RawLanguageKind::SEMICOLON_TOKEN,
+                "//group/rule\n;\n",
+                &[
+                    TriviaPiece::new(TriviaPieceKind::SingleLineComment, 12),
+                    TriviaPiece::new(TriviaPieceKind::Newline, 1),
+                ],
+                &[TriviaPiece::new(TriviaPieceKind::Newline, 1)],
+            );
+
             builder.finish_node();
             builder.finish_node();
 
@@ -356,6 +366,10 @@ mod tests {
                     category!("args/fileNotFound"),
                     TextRange::new(TextSize::from(97), TextSize::from(108))
                 ),
+                (
+                    category!("suppressions/unused"),
+                    TextRange::new(TextSize::from(110), TextSize::from(122))
+                ),
             ]
         );
     }

crates/rome_diagnostics_categories/src/categories.rs

@@ -99,6 +99,7 @@ define_dategories! {
 
     "suppressions/unknownGroup",
     "suppressions/unknownRule",
+    "suppressions/unused",
     // Used in tests and examples
     "args/fileNotFound",
     "flags/invalid",

crates/rome_js_analyze/src/lib.rs

@@ -251,10 +251,11 @@ mod tests {
 
         let options = AnalyzerOptions::default();
         analyze(FileId::zero(), &parsed.tree(), filter, &options, |signal| {
-            if let Some(mut diag) = signal.diagnostic() {
-                diag.set_severity(Severity::Warning);
+            if let Some(diag) = signal.diagnostic() {
                 let code = diag.category().unwrap();
-                panic!("unexpected diagnostic {code:?}");
+                if code != category!("suppressions/unused") {
+                    panic!("unexpected diagnostic {code:?}");
+                }
             }
 
             ControlFlow::<Never>::Continue(())

crates/rome_js_analyze/tests/specs/correctness/noUnreachable/SuppressionComments.js.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 99
 expression: SuppressionComments.js
 ---
 # Input
@@ -45,4 +46,17 @@ SuppressionComments.js:5:5 lint/correctness/noUnreachable ━━━━━━━
 
 ```
 
+```
+SuppressionComments.js:1:1 suppressions/unused ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Suppression comment is not being used
+
+  > 1 │ // rome-ignore lint(correctness/noUnreachable): this comment does nothing
+      │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    2 │ function SuppressionComments1() {
+    3 │     beforeReturn();
+
+
+```
+
 

crates/rome_service/src/file_handlers/javascript.rs

@@ -10,7 +10,7 @@ use rome_analyze::{
     AnalysisFilter, AnalyzerOptions, ControlFlow, GroupCategory, Never, QueryMatch,
     RegistryVisitor, RuleCategories, RuleCategory, RuleFilter, RuleGroup,
 };
-use rome_diagnostics::{Applicability, CodeSuggestion};
+use rome_diagnostics::{v2::category, Applicability, CodeSuggestion};
 use rome_formatter::{FormatError, Printed};
 use rome_fs::RomePath;
 use rome_js_analyze::{analyze, analyze_with_inspect_matcher, visit_registry, RuleError};
@@ -221,8 +221,15 @@ fn lint(params: LintParams) -> LintResults {
         .iter()
         .any(|diag| diag.severity() <= v2::Severity::Error);
 
+    let has_lint = params.filter.categories.contains(RuleCategories::LINT);
+
     analyze(file_id, &tree, params.filter, &analyzer_options, |signal| {
         if let Some(mut diagnostic) = signal.diagnostic() {
+            // Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass
+            if !has_lint && diagnostic.category() == Some(category!("suppressions/unused")) {
+                return ControlFlow::<Never>::Continue(());
+            }
+
             diagnostic_count += 1;
 
             // We do now check if the severity of the diagnostics should be changed.