Repository: yshavit/mdq
Pull request #31 (2024-06-16 19:24:09)

unify VariantsChecker and NodesChecker
`NodesChecker` was specific to `mdast::Node`. PR #30 added `VariantsChecker` as a more general form of the same thing; this change completes it by replacing `NodesChecker` with `VariantsChecker.


Patch:
diff --git a/src/fmt_md.rs b/src/fmt_md.rs
index 0f72d4d..6e6a80a 100644
--- a/src/fmt_md.rs
+++ b/src/fmt_md.rs
@@ -569,7 +569,7 @@ pub mod tests {
     use super::write_md;
 
     lazy_static! {
-        static ref VARIANTS_CHECKER: VariantsChecker<MdqNode> = crate::new_variants_checker! {MdqNode:
+        static ref VARIANTS_CHECKER: VariantsChecker<MdqNode> = crate::new_variants_checker! (MdqNode {
             Root(_),
             Header(_),
             Paragraph(_),
@@ -613,7 +613,7 @@ pub mod tests {
             Inline(crate::tree::Inline::Image{link: Link{title: Some(_), reference: LinkReference::Shortcut, ..}, ..}),
 
             Inline(crate::tree::Inline::Footnote{..}),
-        };
+        });
     }
 
     #[test]
diff --git a/src/tree.rs b/src/tree.rs
index 1c2a98e..1e864fd 100644
--- a/src/tree.rs
+++ b/src/tree.rs
@@ -699,15 +699,11 @@ mod tests {
     ///
     /// For example, footnote are `[^a]` in markdown; does that identifier get parsed as `"^a"` or `"a"`?
     mod all_nodes {
-        use std::collections::HashSet;
-        use std::sync::{Arc, Mutex};
-        use std::{thread, time};
-
+        use crate::utils_for_test::VariantsChecker;
         use indoc::indoc;
         use lazy_static::lazy_static;
         use markdown::mdast::Node;
         use markdown::{mdast, ParseOptions};
-        use regex::Regex;
 
         use super::*;
 
@@ -746,30 +742,6 @@ mod tests {
             }};
         }
 
-        /// Creates a matcher against [Node] with the given variants, and returns the variant names as a collection.
-        ///
-        /// If you see a compilation failure here, it means the call site is missing variants (or has an unknown
-        /// variant).
-        ///
-        /// This macro assumes that each variant can match a pattern `Node::TheVariant(_)`.
-        macro_rules! nodes_matcher {
-            [$($variant:ident),* $(,)?] => {
-                {
-                    None.map(|n: Node| match n {
-                        $(
-                        Node::$variant(_) => {}
-                        )*
-                        mdx_nodes!{} => {
-                            // If you implement mdx nodes, you should also remove the mdx_nodes macro. That will
-                            // (correctly) break this macro. You should add those MDX arms to the get_mdast_node_names
-                            // function, to ensure that we have tests for them.
-                        }
-                    });
-                    vec![$(stringify!($variant).to_string(),)*].into_iter().collect()
-                }
-            };
-        }
-
         #[test]
         fn root() {
             let (root, _) = parse("hello");
@@ -1660,23 +1632,7 @@ mod tests {
 
         #[test]
         fn all_variants_tested() {
-            let timeout = time::Duration::from_millis(500);
-            let retry_delay = time::Duration::from_millis(50);
-            let start = time::Instant::now();
-            loop {
-                if NODES_CHECKER.all_were_seen() {
-                    break;
-                }
-                if start.elapsed() >= timeout {
-                    let remaining = NODES_CHECKER.remaining_as_copy();
-                    panic!(
-                        "Timed out, and missing {} variants:\n- {}",
-                        remaining.len(),
-                        remaining.join("\n- ")
-                    )
-                }
-                thread::sleep(retry_delay);
-            }
+            NODES_CHECKER.wait_for_all();
         }
 
         fn parse(md: &str) -> (mdast::Root, Lookups) {
@@ -1690,85 +1646,40 @@ mod tests {
             (root, lookups)
         }
 
-        struct NodesChecker {
-            require: Arc<Mutex<HashSet<String>>>,
-        }
-
         lazy_static! {
-            static ref NODES_CHECKER: NodesChecker = NodesChecker::new();
-        }
-
-        impl NodesChecker {
-            fn new() -> Self {
-                Self {
-                    require: Self::get_mdast_node_names(),
-                }
-            }
-
-            fn see(&self, node: &Node) {
-                let node_debug = format!("{:?}", node);
-                let re = Regex::new(r"^\w+").unwrap();
-                let node_name = re.find(&node_debug).unwrap().as_str();
-                self.require.lock().map(|mut set| set.remove(node_name)).unwrap();
-            }
-
-            fn all_were_seen(&self) -> bool {
-                self.require.lock().map(|set| set.is_empty()).unwrap()
-            }
-
-            fn remaining_as_copy(&self) -> Vec<String> {
-                let mut result: Vec<String> = self
-                    .require
-                    .lock()
-                    .map(|set| set.iter().map(|s| s.to_owned()).collect())
-                    .unwrap();
-                result.sort();
-                result
-            }
-
-            /// Returns how many variants of [Node] there are.
-            ///
-            /// We can't use strum to do this, because we don't own the Node code. Instead, we rely on a bit of
-            /// trickery. First, we create a `match` over all the variants, making sure each one is on its own line.
-            /// Then, we use [line!] to get the line counts right before and after that `match`, and do some basic
-            /// arithmetic to figure out how many variants there are.
-            ///
-            /// This isn't 100% fool-proof (it requires manually ensuring that each variant is on its own line, though
-            /// `cargo fmt` helps with that), but it should be good enough in practice.
-            fn get_mdast_node_names() -> Arc<Mutex<HashSet<String>>> {
-                let all_node_names = nodes_matcher![
-                    Root,
-                    BlockQuote,
-                    FootnoteDefinition,
-                    List,
-                    Toml,
-                    Yaml,
-                    Break,
-                    InlineCode,
-                    InlineMath,
-                    Delete,
-                    Emphasis,
-                    FootnoteReference,
-                    Html,
-                    Image,
-                    ImageReference,
-                    Link,
-                    LinkReference,
-                    Strong,
-                    Text,
-                    Code,
-                    Math,
-                    Heading,
-                    Table,
-                    ThematicBreak,
-                    TableRow,
-                    TableCell,
-                    ListItem,
-                    Definition,
-                    Paragraph,
-                ];
-                Arc::new(Mutex::new(all_node_names))
-            }
+            static ref NODES_CHECKER: VariantsChecker<Node> = crate::new_variants_checker!(Node {
+                BlockQuote(_),
+                Break(_),
+                Code(_),
+                Definition(_),
+                Delete(_),
+                Emphasis(_),
+                FootnoteDefinition(_),
+                FootnoteReference(_),
+                Heading(_),
+                Html(_),
+                Image(_),
+                ImageReference(_),
+                InlineCode(_),
+                InlineMath(_),
+                Link(_),
+                LinkReference(_),
+                List(_),
+                ListItem(_),
+                Math(_),
+                Paragraph(_),
+                Root(_),
+                Strong(_),
+                Table(_),
+                TableCell(_),
+                TableRow(_),
+                Text(_),
+                ThematicBreak(_),
+                Toml(_),
+                Yaml(_),
+            } ignore {
+                mdx_nodes!(),
+            });
         }
     }
 


Test patch:
diff --git a/src/utils_for_test.rs b/src/utils_for_test.rs
index b49f2b9..8f64c3e 100644
--- a/src/utils_for_test.rs
+++ b/src/utils_for_test.rs
@@ -7,7 +7,6 @@ pub use test_utils::*;
 mod test_utils {
     use std::{thread, time};
 
-    // TODO unify this with the one in tree.rs.
     pub struct VariantsChecker<E> {
         require: std::sync::Arc<std::sync::Mutex<std::collections::HashSet<String>>>,
         resolver: fn(&E) -> &str,
@@ -55,9 +54,31 @@ mod test_utils {
         }
     }
 
+    /// Creates a new `VariantsChecker` that looks for all the variants of enum `E`.
+    ///
+    /// ```
+    /// new_variants_checker(MyEnum: { Variant1, Variant2(_), ... })
+    /// ```
+    ///
+    /// You can also mark some variants as ignored; these will be added to the pattern match, but not be required to
+    /// be seen:
+    ///
+    /// ```
+    /// new_variants_checker(MyEnum: { Variant1, ... } ignore { Variant2, ... } )
+    /// ```
+    ///
+    /// If you see a compilation failure here, it means the call site is missing variants (or has an unknown
+    /// variant).
+    ///
+    /// We can't use strum to do this for mdast::Node, because we don't own the Node code. Instead, we rely on a bit of
+    /// trickery: we pass in a bunch of arms, and each gets [stringify!]'d and added to a set. Whenever we [see] an
+    /// item, we remove the corresponding string from the set.
+    ///
+    /// This requires that each pattern matches exactly one shape of item; in other words, that there aren't any
+    /// dead-code branches.
     #[macro_export]
     macro_rules! new_variants_checker {
-        {$enum_type:ty : $($variant:pat),* $(,)?} => {
+        ($enum_type:ty { $($variant:pat),* $(,)? } $(ignore { $($ignore_variant:pat),* $(,)? })?) => {
             {
                 use $enum_type::*;
 
@@ -65,6 +86,7 @@ mod test_utils {
                     vec![$(stringify!($variant).to_string(),)*],
                     {|elem| match elem {
                         $($variant => stringify!($variant),)*
+                        $($($ignore_variant => {""},)*)?
                     }}
                 )
             }