From 37cb730139bb51141f6038c26ffd9bedee7e94bc Mon Sep 17 00:00:00 2001 From: Yuval Shavit Date: Sun, 16 Jun 2024 15:25:14 -0400 Subject: [PATCH] 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. --- src/fmt_md.rs | 4 +- src/tree.rs | 159 ++++++++++-------------------------------- src/utils_for_test.rs | 26 ++++++- 3 files changed, 61 insertions(+), 128 deletions(-) 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 = crate::new_variants_checker! {MdqNode: + static ref VARIANTS_CHECKER: VariantsChecker = 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>>, - } - 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 { - let mut result: Vec = 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>> { - 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 = 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!(), + }); } } 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 { require: std::sync::Arc>>, 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 => {""},)*)? }} ) }