Address PR review

crates/rome_analyze/src/query.rs

@@ -6,24 +6,30 @@ use crate::registry::NodeLanguage;
 pub trait Queryable: Sized {
     type Language: Language;
 
+    /// Statically declares which [QueryMatch] variant is matched by this
+    /// [Queryable] type. For instance the [Ast] queryable matches on
+    /// [QueryMatch::Syntax], so its key is defined as [QueryKey::Syntax]
     const KEY: QueryKey<Self::Language>;
 
     /// Unwrap an instance of `Self` from a [QueryMatch].
     ///
     /// ## Panics
     ///
-    /// If the type is mismatched
+    /// If the [QueryMatch] variant of `query` doesn't match `Self::KEY`
     fn unwrap_match(query: &QueryMatch<Self::Language>) -> Self;
 }
 
-pub enum QueryKey<L: Language> {
-    Syntax(SyntaxKindSet<L>),
-}
-
+/// Enumerate all the types of [Queryable] analyzer visitors may emit
 pub enum QueryMatch<L: Language> {
     Syntax(SyntaxNode<L>),
 }
 
+/// Mirrors the variants of [QueryMatch] to statically compute which queries a
+/// given [Queryable] type can match
+pub enum QueryKey<L: Language> {
+    Syntax(SyntaxKindSet<L>),
+}
+
 /// Query type usable by lint rules to match on specific [AstNode] types
 #[derive(Clone)]
 pub struct Ast<N>(pub N);

crates/rome_analyze/src/registry.rs

@@ -114,10 +114,7 @@ where
 
         // Run all the rules registered to this QueryMatch
         for rule in rules {
-            let result = (rule.run)(file_id, root, query, &mut self.emit_signal);
-            if let ControlFlow::Break(b) = result {
-                return ControlFlow::Break(b);
-            }
+            (rule.run)(file_id, root, query, &mut self.emit_signal)?;
         }
 
         ControlFlow::Continue(())
@@ -163,10 +160,7 @@ impl<L: Language, B> RegistryRule<L, B> {
 
             for result in R::run(&ctx) {
                 let signal = RuleSignal::<R>::new(file_id, root, &query_result, result);
-
-                if let ControlFlow::Break(b) = callback(&signal) {
-                    return ControlFlow::Break(b);
-                }
+                callback(&signal)?;
             }
 
             ControlFlow::Continue(())

crates/rome_analyze/src/syntax.rs

@@ -7,6 +7,10 @@ use crate::{ControlFlow, QueryMatch, Visitor, VisitorContext};
 /// event for the [SyntaxNode] being entered
 pub struct SyntaxVisitor<L: Language, F> {
     has_suppressions: F,
+    /// If a subtree is currently being skipped by the visitor, for instance
+    /// because it has a suppression comment, this stores the root [SyntaxNode]
+    /// of that subtree. The visitor will then ignore all events until it
+    /// receives a [WalkEvent::Leave] for the `skip_subtree` node
     skip_subtree: Option<SyntaxNode<L>>,
 }
 
@@ -38,7 +42,7 @@ where
             WalkEvent::Leave(node) => {
                 if let Some(skip_subtree) = &self.skip_subtree {
                     if skip_subtree == node {
-                        self.skip_subtree.take();
+                        self.skip_subtree = None;
                     }
                 }
 

crates/rome_analyze/src/visitor.rs

@@ -17,6 +17,7 @@ type MatchQuery<'a, L, B> =
     Box<dyn FnMut(FileId, &LanguageRoot<L>, &QueryMatch<L>) -> ControlFlow<B> + 'a>;
 
 impl<'a, L: Language, B> VisitorContext<'a, L, B> {
+    /// Run all the rules with a `Query` matching `quary_match`
     pub fn match_query(&mut self, query_match: &QueryMatch<L>) -> ControlFlow<B> {
         (self.match_query)(self.file_id, &self.root, query_match)
     }
@@ -60,7 +61,13 @@ pub trait NodeVisitor<V, B>: Sized {
     ) -> ControlFlow<B>;
 }
 
-/// Create a single unified [Visitor] type from a set of [NodeVisitor]s
+/// Creates a single struct implementing [Visitor] over a collection of type
+/// implementing the [NodeVisitor] helper trait. Unlike the global [Visitor],
+/// node visitors are transient: they get instantiated when the traversal
+/// enters the corresponding node and destroyed when the node is exited. They
+/// are intended as a building blocks for creating and managing the state of
+/// complex visitors by allowing the implementation to be split over multiple
+/// smaller components.
 ///
 /// # Example
 ///
@@ -121,20 +128,15 @@ macro_rules! merge_node_visitors {
                         $(
                             if <<$visitor as $crate::NodeVisitor<$name<B>, B>>::Node as ::rome_rowan::AstNode>::can_cast(kind) {
                                 let node = <<$visitor as $crate::NodeVisitor<$name<B>, B>>::Node as ::rome_rowan::AstNode>::unwrap_cast(node.clone());
-                                return match <$visitor as $crate::NodeVisitor<$name<B>, B>>::enter(node, ctx, self) {
-                                    ::std::ops::ControlFlow::Continue(state) => {
-                                        let stack_index = self.stack.len();
-                                        let ty_index = self.$id.len();
-
-                                        self.$id.push((stack_index, state));
-                                        self.stack.push((::std::any::TypeId::of::<$visitor>(), ty_index));
-
-                                        ::std::ops::ControlFlow::Continue(())
-                                    }
-                                    ::std::ops::ControlFlow::Break(value) => {
-                                        ::std::ops::ControlFlow::Break(value)
-                                    }
-                                };
+                                let state = <$visitor as $crate::NodeVisitor<$name<B>, B>>::enter(node, ctx, self)?;
+
+                                let stack_index = self.stack.len();
+                                let ty_index = self.$id.len();
+
+                                self.$id.push((stack_index, state));
+                                self.stack.push((::std::any::TypeId::of::<$visitor>(), ty_index));
+
+                                return ::std::ops::ControlFlow::Continue(());
                             }
                         )*
                     }