diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 97466b67..6cae0780 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -77,6 +77,14 @@ restores the separator row with widths derived from the final table body. use this macro and supply a descriptive expect message that identifies the pattern whose compilation failed. +`src/html.rs`: + +- `collect_matching`: Performs the canonical pre-order, depth-first DOM + traversal and clones each matching handle into the supplied output vector. + `collect_tables` and `collect_rows` delegate their predicates to this helper; + all new DOM collection must reuse it rather than introduce another recursive + walker. + `src/textproc.rs`: - `leading_indent(s: &str) -> &str`: Returns the leading whitespace prefix of @@ -874,6 +882,10 @@ cleared. `flush_raw` exists for the fenced-block escape path: it emits the buffered lines verbatim without conversion, so raw HTML inside a fenced code block is preserved unchanged. +DOM conversion uses `collect_tables` and `collect_rows` as semantic entry +points. Both delegate traversal to the canonical `collect_matching` walker, so +pre-order traversal, handle cloning, and child borrowing remain consistent. + ### 1.4. `DefinitionScanState` (`src/footnotes/renumber/definitions.rs`) `DefinitionScanState` accumulates the footnote-definition rewrite plan during a diff --git a/src/html.rs b/src/html.rs index 608e0049..fbd07f72 100644 --- a/src/html.rs +++ b/src/html.rs @@ -102,24 +102,31 @@ fn is_element(handle: &Handle, tag: &str) -> bool { /// Returns `true` if `handle` represents a `` or `` element. fn is_table_cell(handle: &Handle) -> bool { is_element(handle, "td") || is_element(handle, "th") } -/// Walks the DOM tree collecting `` nodes under `handle`. -fn collect_tables(handle: &Handle, tables: &mut Vec) { - if is_element(handle, "table") { - tables.push(handle.clone()); +/// DOM input is unbounded; bounded model checking is not a good fit for +/// verifying traversal over arbitrary tree depth. Property tests cover +/// invariants via random small-tree generation instead. +/// +/// Walks the DOM tree in pre-order, cloning nodes that satisfy `pred` into `out`. +fn collect_matching(handle: &Handle, pred: F, out: &mut Vec) +where + F: Fn(&Handle) -> bool + Copy, +{ + if pred(handle) { + out.push(handle.clone()); } for child in handle.children.borrow().iter() { - collect_tables(child, tables); + collect_matching(child, pred, out); } } +/// Walks the DOM tree collecting `
` nodes under `handle`. +fn collect_tables(handle: &Handle, tables: &mut Vec) { + collect_matching(handle, |node| is_element(node, "table"), tables); +} + /// Collects all `` nodes beneath `handle`. fn collect_rows(handle: &Handle, rows: &mut Vec) { - if is_element(handle, "tr") { - rows.push(handle.clone()); - } - for child in handle.children.borrow().iter() { - collect_rows(child, rows); - } + collect_matching(handle, |node| is_element(node, "tr"), rows); } fn is_bold_tag(tag: &str) -> bool { diff --git a/src/html_tests.rs b/src/html_tests.rs index 4cdc75e9..f9c6cbd4 100644 --- a/src/html_tests.rs +++ b/src/html_tests.rs @@ -49,9 +49,51 @@ mod proptest_tests { //! These generated cases complement the parent test module by checking //! `HtmlTableState` behaviour across varied open and close sequences. + use std::rc::Rc; + + use html5ever::{driver::ParseOpts, parse_document, tendril::TendrilSink}; + use markup5ever_rcdom::{Handle, RcDom}; use proptest::prelude::*; - use super::HtmlTableState; + use super::{HtmlTableState, collect_matching, is_element}; + + fn html_fragment_strategy() -> impl Strategy { + ( + proptest::collection::vec(proptest::collection::vec(0usize..=4, 0..=6), 0..=4), + 0usize..=4, + ) + .prop_map(|(tables, nested_depth)| { + let nested_tables = (0..nested_depth).fold(String::new(), |nested, level| { + format!("
level-{level}{nested}
") + }); + let mut html = tables.into_iter().fold(String::new(), |mut html, rows| { + html.push_str(""); + for cell_count in rows { + html.push_str(""); + for index in 0..cell_count { + html.push_str(""); + } + html.push_str(""); + } + html.push_str("
cell-"); + html.push_str(&index.to_string()); + html.push_str("
"); + html + }); + html.push_str(&nested_tables); + html + }) + } + + fn parse_html(source: String) -> RcDom { + parse_document(RcDom::default(), ParseOpts::default()).one(source) + } + + fn collect_tag(document: &Handle, tag: &'static str) -> Vec { + let mut matches = Vec::new(); + collect_matching(document, |node| is_element(node, tag), &mut matches); + matches + } proptest! { #[test] @@ -92,5 +134,39 @@ mod proptest_tests { prop_assert!(!state.in_html()); prop_assert_eq!(state.depth, 0); } + + #[test] + fn collect_matching_count_equals_source_tag_count( + source in html_fragment_strategy(), + tag in prop_oneof![Just("table"), Just("tr"), Just("td")], + ) { + let expected_count = source.matches(&format!("<{tag}")).count(); + let dom = parse_html(source); + + prop_assert_eq!(collect_tag(&dom.document, tag).len(), expected_count); + } + + #[test] + fn collect_matching_order_is_deterministic( + source in html_fragment_strategy(), + tag in prop_oneof![Just("table"), Just("tr"), Just("td")], + ) { + let dom = parse_html(source); + let first = collect_tag(&dom.document, tag); + let second = collect_tag(&dom.document, tag); + + prop_assert_eq!(first.len(), second.len()); + prop_assert!(first.iter().zip(&second).all(|(left, right)| Rc::ptr_eq(left, right))); + } + + #[test] + fn collect_matching_returns_empty_for_non_matching_documents( + content in "[a-z ]{0,64}", + tag in prop_oneof![Just("table"), Just("tr"), Just("td")], + ) { + let dom = parse_html(format!("

{content}

")); + + prop_assert!(collect_tag(&dom.document, tag).is_empty()); + } } } diff --git a/tests/html_snapshots.rs b/tests/html_snapshots.rs new file mode 100644 index 00000000..1e9c69f4 --- /dev/null +++ b/tests/html_snapshots.rs @@ -0,0 +1,59 @@ +//! Snapshot coverage for representative HTML-to-Markdown table conversions. + +use mdtablefix::convert_html_tables; +use rstest::rstest; + +fn snapshot_conversion(name: &str, input: &str) { + let lines = input.lines().map(ToString::to_string).collect::>(); + let output = convert_html_tables(&lines).join("\n"); + + insta::with_settings!({ + snapshot_path => "snapshots", + prepend_module_to_snapshot => false, + }, { + insta::assert_snapshot!(name, output); + }); +} + +#[rstest] +#[case::single_row( + "html_single_row_table", + "\n\n
NameValue
" +)] +#[case::multi_row( + "html_multi_row_table", + concat!( + "\n", + "\n", + "\n", + "\n", + "
NameValue
Alpha1
Beta22
", + ) +)] +#[case::nested( + "html_nested_table", + concat!( + "\n", + "\n", + "\n", + "
OuterValue
\n", + "\n", + "\n", + "
Inner
Nested
Tail
", + ) +)] +#[case::sectioned( + "html_sectioned_table", + concat!( + "\n", + "\n", + "\n", + "\n", + "\n", + "\n", + "
NameValue
Alpha1
Beta2
", + ) +)] +fn snapshots_html_table_conversion(#[case] name: &str, #[case] input: &str) { + snapshot_conversion(name, input); +} diff --git a/tests/snapshots/html_multi_row_table.snap b/tests/snapshots/html_multi_row_table.snap new file mode 100644 index 00000000..67d2eb12 --- /dev/null +++ b/tests/snapshots/html_multi_row_table.snap @@ -0,0 +1,8 @@ +--- +source: tests/html_snapshots.rs +expression: output +--- +| Name | Value | +| ----- | ----- | +| Alpha | 1 | +| Beta | 22 | diff --git a/tests/snapshots/html_nested_table.snap b/tests/snapshots/html_nested_table.snap new file mode 100644 index 00000000..154626fe --- /dev/null +++ b/tests/snapshots/html_nested_table.snap @@ -0,0 +1,12 @@ +--- +source: tests/html_snapshots.rs +expression: output +--- +| Outer | Value | +| --- | --- | +| Inner Nested | Tail | +| Inner | +| Nested | +| Inner | +| ------ | +| Nested | diff --git a/tests/snapshots/html_sectioned_table.snap b/tests/snapshots/html_sectioned_table.snap new file mode 100644 index 00000000..3ccbae1a --- /dev/null +++ b/tests/snapshots/html_sectioned_table.snap @@ -0,0 +1,8 @@ +--- +source: tests/html_snapshots.rs +expression: output +--- +| Name | Value | +| ----- | ----- | +| Alpha | 1 | +| Beta | 2 | diff --git a/tests/snapshots/html_single_row_table.snap b/tests/snapshots/html_single_row_table.snap new file mode 100644 index 00000000..744f432f --- /dev/null +++ b/tests/snapshots/html_single_row_table.snap @@ -0,0 +1,6 @@ +--- +source: tests/html_snapshots.rs +expression: output +--- +| Name | Value | +| ---- | ----- |