Handle errors returned by cmark_resume_with_options() (#244)

Handle errors returned by `cmark_resume_with_options()`

PR Byron/pulldown-cmark-to-cmark#93 in the
cmark_pulldown_to_cmark crate added structure to the error type returned by
`cmark_resume_with_options()`. It also  made it return an error when the
input is an inconsistent stream of `Event`s.

Now, instead of panicking, we can propagate the error to the caller and
report a more specific error.

Co-authored-by: Martin Geisler <martin@geisler.net>

fuzz/fuzz_targets/gettext.rs

@@ -8,5 +8,5 @@ fuzz_target!(|inputs: (Vec<(&str, &str)>, Vec<BookItem>)| {
     let (translations, book_items) = inputs;
     let catalog = create_catalog(translations);
     let mut book = create_book(book_items);
-    translate_book(&catalog, &mut book)
+    let _ = translate_book(&catalog, &mut book); // Err(_) can happen and it's fine.
 });

fuzz/fuzz_targets/group_events.rs

@@ -7,6 +7,7 @@ use pretty_assertions::assert_eq;
 fuzz_target!(|text: String| {
     let events = extract_events(&text, None);
     let flattened_groups = group_events(&events)
+        .expect("Grouping should succeed")
         .into_iter()
         .flat_map(|group| match group {
             Group::Translate { events, .. } | Group::Skip(events) => events,
@@ -16,8 +17,10 @@ fuzz_target!(|text: String| {
     // Comparison through markdown text to detect missing text.
     // Events can't be compared directly because `group_events`
     // may split a event into some events.
-    let text_from_events = reconstruct_markdown(&events, None);
-    let text_from_groups = reconstruct_markdown(&flattened_groups, None);
+    let text_from_events =
+        reconstruct_markdown(&events, None).expect("Failed to reconstruct Markdown from events");
+    let text_from_groups = reconstruct_markdown(&flattened_groups, None)
+        .expect("Failed to reconstruct Markdown from groups");
 
     assert_eq!(text_from_events, text_from_groups);
 });

i18n-helpers/src/gettext.rs

@@ -15,11 +15,13 @@
 //! This file contains main logic used by the binary `mdbook-gettext`.
 
 use super::{extract_events, reconstruct_markdown, translate_events};
+use anyhow::Context;
 use mdbook::book::Book;
 use mdbook::BookItem;
 use polib::catalog::Catalog;
 use polib::message::Message;
 use pulldown_cmark::Event;
+use pulldown_cmark_to_cmark::Error as CmarkError;
 
 /// Strip formatting from a Markdown string.
 ///
@@ -38,11 +40,20 @@ fn strip_formatting(text: &str) -> String {
         .collect()
 }
 
-fn translate(text: &str, catalog: &Catalog) -> String {
+fn translate(text: &str, catalog: &Catalog) -> anyhow::Result<String> {
     let events = extract_events(text, None);
-    let translated_events = translate_events(&events, catalog);
-    let (translated, _) = reconstruct_markdown(&translated_events, None);
-    translated
+    // Translation should always succeed.
+    let translated_events = translate_events(&events, catalog).expect("Failed to translate events");
+    let (translated, _) = reconstruct_markdown(&translated_events, None)
+        .map_err(|e| match e {
+            CmarkError::FormatFailed(_) => e.into(),
+            CmarkError::UnexpectedEvent => anyhow::Error::from(e).context(
+                "Markdown in translated messages (.po) may not be consistent with the original",
+            ),
+        })
+        .context("Failed to reconstruct Markdown after translation")?;
+    // "Failed to reconstruct Markdown after translation"
+    Ok(translated)
 }
 
 /// Update `catalog` with stripped messages from `SUMMARY.md`.
@@ -75,18 +86,29 @@ pub fn add_stripped_summary_translations(catalog: &mut Catalog) {
     }
 }
 
-/// Translate an entire book.
-pub fn translate_book(catalog: &Catalog, book: &mut Book) {
-    book.for_each_mut(|item| match item {
+fn mutate_item(item: &mut BookItem, catalog: &Catalog) -> anyhow::Result<()> {
+    match item {
         BookItem::Chapter(ch) => {
-            ch.content = translate(&ch.content, catalog);
-            ch.name = translate(&ch.name, catalog);
+            ch.content = translate(&ch.content, catalog)?;
+            ch.name = translate(&ch.name, catalog)?;
         }
         BookItem::Separator => {}
         BookItem::PartTitle(title) => {
-            *title = translate(title, catalog);
+            *title = translate(title, catalog)?;
         }
-    });
+    };
+    Ok(())
+}
+
+/// Translate an entire book.
+pub fn translate_book(catalog: &Catalog, book: &mut Book) -> anyhow::Result<()> {
+    // Unfortunately the `Book` API only allows to mutate *all* the items
+    // through a clousure, with no early bail-out in case of error.
+    // Therefore, let's just collect all the results and return the first error
+    // or `Ok(())`.
+    let mut results = Vec::<anyhow::Result<()>>::new();
+    book.for_each_mut(|item| results.push(mutate_item(item, catalog)));
+    results.into_iter().find(|r| r.is_err()).unwrap_or(Ok(()))
 }
 
 #[cfg(test)]
@@ -108,6 +130,14 @@ mod tests {
         catalog
     }
 
+    fn create_book(items: Vec<BookItem>) -> Book {
+        let mut book = Book::new();
+        for item in items {
+            book.push_item(item);
+        }
+        book
+    }
+
     #[test]
     fn test_add_stripped_summary_translations() {
         // Add two messages which map to the same stripped message.
@@ -142,28 +172,28 @@ mod tests {
     #[test]
     fn test_translate_single_line() {
         let catalog = create_catalog(&[("foo bar", "FOO BAR")]);
-        assert_eq!(translate("foo bar", &catalog), "FOO BAR");
+        assert_eq!(translate("foo bar", &catalog).unwrap(), "FOO BAR");
     }
 
     #[test]
     fn test_translate_single_paragraph() {
         let catalog = create_catalog(&[("foo bar", "FOO BAR")]);
         // The output is normalized so the newline disappears.
-        assert_eq!(translate("foo bar\n", &catalog), "FOO BAR");
+        assert_eq!(translate("foo bar\n", &catalog).unwrap(), "FOO BAR");
     }
 
     #[test]
     fn test_translate_paragraph_with_leading_newlines() {
         let catalog = create_catalog(&[("foo bar", "FOO BAR")]);
         // The output is normalized so the newlines disappear.
-        assert_eq!(translate("\n\n\nfoo bar\n", &catalog), "FOO BAR");
+        assert_eq!(translate("\n\n\nfoo bar\n", &catalog).unwrap(), "FOO BAR");
     }
 
     #[test]
     fn test_translate_paragraph_with_trailing_newlines() {
         let catalog = create_catalog(&[("foo bar", "FOO BAR")]);
         // The output is normalized so the newlines disappear.
-        assert_eq!(translate("foo bar\n\n\n", &catalog), "FOO BAR");
+        assert_eq!(translate("foo bar\n\n\n", &catalog).unwrap(), "FOO BAR");
     }
 
     #[test]
@@ -177,7 +207,8 @@ mod tests {
                  \n\
                  last paragraph\n",
                 &catalog
-            ),
+            )
+            .unwrap(),
             "first paragraph\n\
              \n\
              FOO BAR\n\
@@ -203,7 +234,8 @@ mod tests {
                  last\n\
                  paragraph\n",
                 &catalog
-            ),
+            )
+            .unwrap(),
             "FIRST TRANSLATED PARAGRAPH\n\
              \n\
              LAST TRANSLATED PARAGRAPH"
@@ -231,7 +263,7 @@ mod tests {
                  \n\
                  Text after.\n",
                 &catalog
-            ),
+            ).unwrap(),
             "Text before.\n\
              \n\
              ```rust,editable\n\
@@ -248,7 +280,7 @@ mod tests {
     fn test_translate_inline_html() {
         let catalog = create_catalog(&[("foo <b>bar</b> baz", "FOO <b>BAR</b> BAZ")]);
         assert_eq!(
-            translate("foo <b>bar</b> baz", &catalog),
+            translate("foo <b>bar</b> baz", &catalog).unwrap(),
             "FOO <b>BAR</b> BAZ"
         );
     }
@@ -257,7 +289,7 @@ mod tests {
     fn test_translate_block_html() {
         let catalog = create_catalog(&[("foo", "FOO"), ("bar", "BAR")]);
         assert_eq!(
-            translate("<div>\n\nfoo\n\n</div><div>\n\nbar\n\n</div>", &catalog),
+            translate("<div>\n\nfoo\n\n</div><div>\n\nbar\n\n</div>", &catalog).unwrap(),
             "<div>\n\nFOO\n\n</div><div>\n\nBAR\n\n</div>"
         );
     }
@@ -279,7 +311,8 @@ mod tests {
                 | Arrays | `[T; N]`    | `[20, 30, 40]`  |\n\
                 | Tuples | `()`, ...   | `()`, `('x',)`  |",
                 &catalog
-            ),
+            )
+            .unwrap(),
             "\
             ||TYPES|LITERALS|\n\
             |-|-----|--------|\n\
@@ -295,15 +328,15 @@ mod tests {
             ("More details.", "MORE DETAILS."),
         ]);
         assert_eq!(
-            translate("A footnote[^note].\n\n[^note]: More details.", &catalog),
+            translate("A footnote[^note].\n\n[^note]: More details.", &catalog).unwrap(),
             "A FOOTNOTE[^note].\n\n[^note]: MORE DETAILS."
         );
     }
 
     #[test]
     fn test_strikethrough() {
         let catalog = create_catalog(&[("~~foo~~", "~~FOO~~")]);
-        assert_eq!(translate("~~foo~~", &catalog), "~~FOO~~");
+        assert_eq!(translate("~~foo~~", &catalog).unwrap(), "~~FOO~~");
     }
 
     #[test]
@@ -316,7 +349,8 @@ mod tests {
                 - [ ] Bar\n\
                 ",
                 &catalog
-            ),
+            )
+            .unwrap(),
             "\
             - [x] FOO\n\
             - [ ] BAR",
@@ -327,7 +361,7 @@ mod tests {
     fn test_heading_attributes() {
         let catalog = create_catalog(&[("Foo", "FOO"), ("Bar", "BAR")]);
         assert_eq!(
-            translate("# Foo { #id .foo }", &catalog),
+            translate("# Foo { #id .foo }", &catalog).unwrap(),
             "# FOO { #id .foo }"
         );
     }
@@ -343,11 +377,20 @@ mod tests {
                 ````\n\
                 ",
                 &catalog
-            ),
+            )
+            .unwrap(),
             "\
             ````d\n\
             ```\n\
             ````",
         );
     }
+
+    // https://github.com/Byron/pulldown-cmark-to-cmark/issues/91.
+    #[test]
+    fn test_translate_book_invalid() {
+        let catalog = create_catalog(&[("Hello", "Ciao\n---")]);
+        let mut book = create_book(vec![BookItem::PartTitle(String::from("Hello\n---"))]);
+        assert!(translate_book(&catalog, &mut book).is_err());
+    }
 }