feat(lsp): improve the user experience of the analyzer in the editor (#…

…2576)

* feat(lsp): improve the user experience of the analyzer in the editor

* address PR review

crates/rome_analyze/src/analyzers/no_delete.rs

@@ -54,7 +54,8 @@ impl Rule for NoDelete {
         )?;
 
         Some(RuleAction {
-            category: ActionCategories::empty(),
+            category: ActionCategories::SUGGESTION,
+            message: markup! { "Replace with undefined assignment" }.to_owned(),
             root,
         })
     }

crates/rome_analyze/src/analyzers/no_double_equals.rs

@@ -50,7 +50,8 @@ impl Rule for NoDoubleEquals {
         )?;
 
         Some(RuleAction {
-            category: ActionCategories::empty(),
+            category: ActionCategories::SUGGESTION,
+            message: markup! { "Replace with strict equality" }.to_owned(),
             root,
         })
     }

crates/rome_analyze/src/analyzers/use_single_var_declarator.rs

@@ -91,7 +91,8 @@ impl Rule for UseSingleVarDeclarator {
         );
 
         Some(RuleAction {
-            category: ActionCategories::empty(),
+            category: ActionCategories::SUGGESTION,
+            message: markup! { "Break out into multiple declarations" }.to_owned(),
             root: root.replace_node_discard_trivia(prev_parent, next_parent)?,
         })
     }

crates/rome_analyze/src/analyzers/use_while.rs

@@ -80,7 +80,8 @@ impl Rule for UseWhile {
         )?;
 
         Some(RuleAction {
-            category: ActionCategories::empty(),
+            category: ActionCategories::SUGGESTION,
+            message: markup! { "Use a while loop" }.to_owned(),
             root,
         })
     }

crates/rome_analyze/src/assists/flip_bin_exp.rs

@@ -1,3 +1,4 @@
+use rome_console::markup;
 use rome_js_factory::make;
 use rome_js_syntax::{
     JsAnyRoot, JsBinaryExpression, JsBinaryExpressionFields, JsBinaryOperator, JsSyntaxKind, T,
@@ -48,6 +49,7 @@ impl Rule for FlipBinExp {
 
         Some(RuleAction {
             category: ActionCategories::REFACTOR,
+            message: markup! { "Flip Binary Expression" }.to_owned(),
             root: root.replace_node(node.clone(), new_node)?,
         })
     }

crates/rome_analyze/src/lib.rs

@@ -1,4 +1,7 @@
-use rome_js_syntax::{JsAnyRoot, TextRange};
+use rome_js_syntax::{
+    suppression::{has_suppressions_category, SuppressionCategory},
+    JsAnyRoot, TextRange, WalkEvent,
+};
 use rome_rowan::AstNode;
 
 mod analyzers;
@@ -32,13 +35,53 @@ where
 {
     let registry = RuleRegistry::with_filter(&filter);
 
-    for node in root.syntax().descendants() {
+    let mut iter = root.syntax().preorder();
+    while let Some(event) = iter.next() {
+        let node = match event {
+            WalkEvent::Enter(node) => node,
+            WalkEvent::Leave(_) => continue,
+        };
+
         if let Some(range) = filter.range {
             if node.text_range().ordering(range).is_ne() {
+                iter.skip_subtree();
                 continue;
             }
         }
 
+        if has_suppressions_category(SuppressionCategory::Lint, &node) {
+            iter.skip_subtree();
+            continue;
+        }
+
         registry.analyze(root, node, &mut callback);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use rome_js_parser::{parse, SourceType};
+
+    use crate::{analyze, AnalysisFilter};
+
+    #[test]
+    fn suppression() {
+        const SOURCE: &str = "
+            // rome-ignore lint(noDoubleEquals): test
+            function isEqual(a, b) {
+                return a == b;
+            }
+        ";
+
+        let parsed = parse(SOURCE, 0, SourceType::js_module());
+
+        analyze(&parsed.tree(), AnalysisFilter::default(), |signal| {
+            if let Some(diag) = signal.diagnostic() {
+                assert_ne!(
+                    diag.rule_name, "noDoubleEquals",
+                    "unexpected diagnostic signal raised"
+                );
+            }
+        });
+    }
+}

crates/rome_analyze/src/registry.rs

@@ -130,5 +130,6 @@ pub struct RuleAction {
     /// The category this action belongs to, this will influence how clients
     /// may chose to present this action to the user
     pub category: ActionCategories,
+    pub message: MarkupBuf,
     pub root: JsAnyRoot,
 }

crates/rome_analyze/src/signals.rs

@@ -29,6 +29,7 @@ pub struct AnalyzerDiagnostic {
 pub struct AnalyzerAction {
     pub rule_name: &'static str,
     pub category: ActionCategories,
+    pub message: MarkupBuf,
     pub root: JsAnyRoot,
 }
 
@@ -69,6 +70,7 @@ impl<'a, R: Rule> AnalyzerSignal for RuleSignal<'a, R> {
         R::action(self.root.clone(), &self.node, &self.state).map(|action| AnalyzerAction {
             rule_name: R::NAME,
             category: action.category,
+            message: action.message,
             root: action.root,
         })
     }

crates/rome_js_formatter/src/utils/mod.rs

@@ -17,8 +17,9 @@ pub(crate) use binary_like_expression::{format_binary_like_expression, JsAnyBina
 pub(crate) use format_conditional::{format_conditional, Conditional};
 pub(crate) use member_chain::format_call_expression;
 use rome_formatter::{normalize_newlines, FormatResult};
+use rome_js_syntax::suppression::{has_suppressions_category, SuppressionCategory};
 use rome_js_syntax::{
-    JsAnyExpression, JsAnyFunction, JsAnyRoot, JsAnyStatement, JsInitializerClause, JsLanguage,
+    JsAnyExpression, JsAnyFunction, JsAnyStatement, JsInitializerClause, JsLanguage,
     JsTemplateElement, JsTemplateElementFields, Modifiers, TsTemplateElement,
     TsTemplateElementFields, TsType,
 };
@@ -141,239 +142,8 @@ pub(crate) fn format_head_body_statement(
     }
 }
 
-/// Single instance of a suppression comment, with the following syntax:
-///
-/// `// rome-ignore { <category> { (<value>) }? }+: <reason>`
-///
-/// The category broadly describes what feature is being suppressed (formatting,
-/// linting, ...) with the value being and optional, category-specific name of
-/// a specific element to disable (for instance a specific lint name). A single
-/// suppression may specify one or more categories + values, for instance to
-/// disable multiple lints at once
-///
-/// A suppression must specify a reason: this part has no semantic meaning but
-/// is required to document why a particular feature is being disable for this
-/// line (lint false-positive, specific formatting requirements, ...)
-#[derive(Debug, PartialEq, Eq)]
-struct Suppression<'a> {
-    /// List of categories for this suppression
-    ///
-    /// Categories are pair of the category name +
-    /// an optional category value
-    categories: Vec<(&'a str, Option<&'a str>)>,
-    /// Reason for this suppression comment to exist
-    reason: &'a str,
-}
-
-const CATEGORY_FORMAT: &str = "format";
-
-fn parse_suppression_comment(comment: &str) -> impl Iterator<Item = Suppression> {
-    let (head, mut comment) = comment.split_at(2);
-    let is_block_comment = match head {
-        "//" => false,
-        "/*" => {
-            comment = comment
-                .strip_suffix("*/")
-                .expect("block comment with no closing token");
-            true
-        }
-        token => panic!("comment with unknown opening token {token:?}"),
-    };
-
-    comment.lines().filter_map(move |line| {
-        // Eat start of line whitespace
-        let mut line = line.trim_start();
-
-        // If we're in a block comment eat stars, then whitespace again
-        if is_block_comment {
-            line = line.trim_start_matches('*').trim_start()
-        }
-
-        // Check for the rome-ignore token or skip the line entirely
-        line = line.strip_prefix("rome-ignore")?.trim_start();
-
-        let mut categories = Vec::new();
-
-        loop {
-            // Find either a colon opening parenthesis or space
-            let separator = line.find(|c: char| c == ':' || c == '(' || c.is_whitespace())?;
-
-            let (category, rest) = line.split_at(separator);
-            let category = category.trim_end();
-
-            // Skip over and match the separator
-            let (separator, rest) = rest.split_at(1);
-
-            match separator {
-                // Colon token: stop parsing categories
-                ":" => {
-                    if !category.is_empty() {
-                        categories.push((category, None));
-                    }
-
-                    line = rest.trim_start();
-                    break;
-                }
-                // Paren token: parse a category + value
-                "(" => {
-                    let paren = rest.find(')')?;
-
-                    let (value, rest) = rest.split_at(paren);
-                    let value = value.trim();
-
-                    categories.push((category, Some(value)));
-
-                    line = rest.strip_prefix(')').unwrap().trim_start();
-                }
-                // Whitespace: push a category without value
-                _ => {
-                    if !category.is_empty() {
-                        categories.push((category, None));
-                    }
-
-                    line = rest.trim_start();
-                }
-            }
-        }
-
-        let reason = line.trim_end();
-        Some(Suppression { categories, reason })
-    })
-}
-
 pub(crate) fn has_formatter_suppressions(node: &JsSyntaxNode) -> bool {
-    // Lists cannot have a suppression comment attached, it must
-    // belong to either the entire parent node or one of the children
-    let kind = node.kind();
-    if JsAnyRoot::can_cast(kind) || kind.is_list() {
-        return false;
-    }
-
-    let first_token = match node.first_token() {
-        Some(token) => token,
-        None => return false,
-    };
-
-    first_token
-        .leading_trivia()
-        .pieces()
-        .filter_map(|trivia| trivia.as_comments())
-        .any(|comment| {
-            parse_suppression_comment(comment.text())
-                .flat_map(|suppression| suppression.categories)
-                .any(|category| category.0 == CATEGORY_FORMAT)
-        })
-}
-
-#[cfg(test)]
-mod tests {
-    use super::{parse_suppression_comment, Suppression};
-
-    #[test]
-    fn parse_simple_suppression() {
-        assert_eq!(
-            parse_suppression_comment("// rome-ignore parse: explanation1").collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", None)],
-                reason: "explanation1"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment("/** rome-ignore parse: explanation2 */").collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", None)],
-                reason: "explanation2"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment(
-                "/**
-                  * rome-ignore parse: explanation3
-                  */"
-            )
-            .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", None)],
-                reason: "explanation3"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment(
-                "/**
-                  * hello
-                  * rome-ignore parse: explanation4
-                  */"
-            )
-            .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", None)],
-                reason: "explanation4"
-            }],
-        );
-    }
-
-    #[test]
-    fn parse_multiple_suppression() {
-        assert_eq!(
-            parse_suppression_comment("// rome-ignore parse(foo) parse(dog): explanation")
-                .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", Some("foo")), ("parse", Some("dog"))],
-                reason: "explanation"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment("/** rome-ignore parse(bar) parse(cat): explanation */")
-                .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", Some("bar")), ("parse", Some("cat"))],
-                reason: "explanation"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment(
-                "/**
-                  * rome-ignore parse(yes) parse(frog): explanation
-                  */"
-            )
-            .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", Some("yes")), ("parse", Some("frog"))],
-                reason: "explanation"
-            }],
-        );
-
-        assert_eq!(
-            parse_suppression_comment(
-                "/**
-                  * hello
-                  * rome-ignore parse(wow) parse(fish): explanation
-                  */"
-            )
-            .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("parse", Some("wow")), ("parse", Some("fish"))],
-                reason: "explanation"
-            }],
-        );
-    }
-
-    #[test]
-    fn parse_multiple_suppression_categories() {
-        assert_eq!(
-            parse_suppression_comment("// rome-ignore format lint: explanation")
-                .collect::<Vec<_>>(),
-            vec![Suppression {
-                categories: vec![("format", None), ("lint", None)],
-                reason: "explanation"
-            }],
-        );
-    }
+    has_suppressions_category(SuppressionCategory::Format, node)
 }
 
 /// This function consumes a list of modifiers and applies a predictable sorting.

crates/rome_js_syntax/src/lib.rs

@@ -8,6 +8,7 @@ pub mod expr_ext;
 pub mod modifier_ext;
 pub mod numbers;
 pub mod stmt_ext;
+pub mod suppression;
 mod syntax_node;
 mod union_ext;