fix(biome_analyze): restore support for multiple consecutive suppression comments with rule instances. (#6844)

Co-authored-by: Arend van Beelen jr. <[email protected]>

Author: sterliakov

.changeset/silent-suits-judge.md

@@ -0,0 +1,5 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#6837](https://github.com/biomejs/biome/issues/6837): Fixed regression with multiple consecutive line suppression comments using instances (like `// biome-ignore lint/correctness/useExhaustiveDependencies(depName): reason`).

crates/biome_analyze/src/lib.rs

@@ -2,12 +2,12 @@
 
 use biome_console::markup;
 use biome_parser::AnyParse;
+use rustc_hash::FxHashSet;
 use std::collections::{BTreeMap, BinaryHeap};
 use std::fmt::{Debug, Display, Formatter};
 use std::ops;
 use std::str::FromStr;
 use std::sync::Arc;
-use suppressions::LineSuppression;
 
 mod analyzer_plugin;
 mod categories;
@@ -489,33 +489,49 @@ where
             // if it matches the current line index, otherwise perform a binary
             // search over all the previously seen suppressions to find one
             // with a matching range
-            let matches_entry = |suppression: &LineSuppression| {
-                if suppression.suppressed_categories.contains(entry.category) {
-                    return true;
-                }
-                if suppression.suppressed_instances.is_empty() {
-                    suppression.matches_rule(&entry.category, &entry.rule)
-                } else {
-                    entry.instances.iter().all(|value| {
-                        suppression
-                            .suppressed_instances
-                            .iter()
-                            .any(|(v, filter)| *filter == entry.rule && v == value.as_ref())
-                    })
-                }
-            };
-
-            let suppression = self
+            let mut is_fully_suppressed = false;
+            // Check that instance-based comments do indeed suppress all instances
+            // Every match is discarded from this set. Use `Option` for lazy init,
+            // because most of the rules do not use instances.
+            let mut instances: Option<FxHashSet<&Box<str>>> = None;
+            for suppression in self
                 .suppressions
                 .overlapping_line_suppressions(&entry.text_range)
                 .iter_mut()
-                .find(|s| s.text_range.contains(start) && matches_entry(s));
+            {
+                if !suppression.text_range.contains(start) {
+                    continue;
+                }
+                let (is_match, is_exhaustive) =
+                    if suppression.suppressed_categories.contains(entry.category) {
+                        (true, true)
+                    } else if !suppression.matches_rule(&entry.category, &entry.rule) {
+                        (false, false)
+                    } else {
+                        match suppression.suppressed_instance.as_ref() {
+                            None => (true, true),
+                            Some(v) => {
+                                let matches_instance = instances
+                                    .get_or_insert_with(|| entry.instances.iter().collect())
+                                    .remove(v);
+                                (matches_instance, false)
+                            }
+                        }
+                    };
+                if is_match {
+                    suppression.did_suppress_signal = true;
+                    is_fully_suppressed =
+                        is_exhaustive || instances.as_ref().is_some_and(|v| v.is_empty());
+                    if is_fully_suppressed {
+                        break;
+                    }
+                }
+            }
 
             // 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 if range_match(self.range, entry.text_range) {
+            if !is_fully_suppressed && range_match(self.range, entry.text_range) {
+                // TODO: would be nice to remove suppressed instances, if any, before emitting
                 (self.emit_signal)(&*entry.signal)?;
             }
 

crates/biome_analyze/src/suppressions.rs

@@ -130,19 +130,21 @@ pub(crate) struct LineSuppression {
     pub(crate) text_range: TextRange,
     /// All rules from groups included here are ignored.
     pub(crate) suppressed_categories: RuleCategories,
-    /// List of all the rules this comment has started suppressing (must be
-    /// removed from the suppressed set on expiration)
-    pub(crate) suppressed_rules: FxHashMap<RuleCategory, FxHashSet<RuleFilter<'static>>>,
-    /// List of all the rule instances this comment has started suppressing.
-    pub(crate) suppressed_instances: FxHashMap<String, RuleFilter<'static>>,
+    /// The rule this comment should be suppressing.
+    pub(crate) suppressed_rule: Option<(RuleCategory, RuleFilter<'static>)>,
+    /// An instance this comment should be suppressing.
+    ///
+    /// For example, this is `foo` in `// biome-ignore lint/correctness/xxx(foo): ...`
+    pub(crate) suppressed_instance: Option<Box<str>>,
     /// 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,
-    /// Set to `true` when this line suppresses a signal that was already suppressed by another entity e.g. top-level suppression
+    /// Points to the previous suppression if this line suppresses a signal
+    /// that was already suppressed by another entity (e.g. top-level suppression)
     pub(crate) already_suppressed: Option<TextRange>,
 }
 
@@ -153,8 +155,8 @@ impl Default for LineSuppression {
             comment_span: Default::default(),
             text_range: Default::default(),
             suppressed_categories: RuleCategories::empty(),
-            suppressed_rules: Default::default(),
-            suppressed_instances: Default::default(),
+            suppressed_rule: Default::default(),
+            suppressed_instance: Default::default(),
             suppressed_plugins: Default::default(),
             suppress_all_plugins: false,
             did_suppress_signal: false,
@@ -165,9 +167,9 @@ impl Default for LineSuppression {
 
 impl LineSuppression {
     pub(crate) fn matches_rule(&self, rule_category: &RuleCategory, filter: &RuleKey) -> bool {
-        self.suppressed_rules
-            .get(rule_category)
-            .is_some_and(|filters| filters.iter().any(|f| f == filter))
+        self.suppressed_rule
+            .as_ref()
+            .is_some_and(|(c, f)| c == rule_category && f == filter)
     }
 }
 
@@ -431,14 +433,8 @@ impl<'analyzer> Suppressions<'analyzer> {
                 }
             }
             Some(filter) => {
-                let filters = suppression
-                    .suppressed_rules
-                    .entry(rule_category)
-                    .or_default();
-                filters.insert(filter);
-                if let Some(instance) = instance {
-                    suppression.suppressed_instances.insert(instance, filter);
-                }
+                suppression.suppressed_rule = Some((rule_category, filter));
+                suppression.suppressed_instance = instance.map(String::into_boxed_str);
             }
         }
         self.line_suppressions.push(suppression);

crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs

@@ -165,7 +165,21 @@ declare_lint_rule! {
     /// ```
     ///
     /// If you wish to ignore multiple dependencies, you can add multiple
-    /// comments and add a reason for each.
+    /// comments and add a reason for each:
+    ///
+    /// ```js
+    /// import { useEffect } from "react";
+    ///
+    /// function component() {
+    ///     let a = 1;
+    ///     let b = 1;
+    ///     // biome-ignore lint/correctness/useExhaustiveDependencies(a): suppress dependency a
+    ///     // biome-ignore lint/correctness/useExhaustiveDependencies(b): suppress dependency b
+    ///     useEffect(() => {
+    ///         console.log(a, b);
+    ///     }, []);
+    /// }
+    /// ```
     ///
     /// ## Options
     ///

crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/invalidSuppressions.jsx

@@ -0,0 +1,34 @@
+import { useEffect } from "react";
+
+function SingleInstanceSuppressionWrong({a, b}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(b): test
+	useEffect(() => {}, [a]);
+}
+
+function SingleInstanceSuppressionDuplicate({a, b}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(b): test
+	// biome-ignore lint/correctness/useExhaustiveDependencies(b): test
+	useEffect(() => {}, [b]);
+}
+
+function SingleInstanceSuppressionNotEnough({a, b}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(a): test
+	useEffect(() => {}, [a, b]);
+}
+
+function SingleInstanceSuppressionNotEnough2({a, b}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(b): test
+	useEffect(() => {}, [a, b]);
+}
+
+function MultiInstanceSuppressionSomeWrong({a, b, c}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(a): test
+	// biome-ignore lint/correctness/useExhaustiveDependencies(c): test
+	useEffect(() => {}, [a, b]);
+}
+
+function MultiInstanceSuppressionAllWrong({a, b, c, d}) {
+	// biome-ignore lint/correctness/useExhaustiveDependencies(c): test
+	// biome-ignore lint/correctness/useExhaustiveDependencies(d): test
+	useEffect(() => {}, [a, b]);
+}