Merge pull request #484 from liamwhite/broken-link-trait

Unwrap Mutex from broken_link_callback

Author: kivikakk

fuzz/fuzz_targets/all_options.rs

@@ -35,13 +35,13 @@ fuzz_target!(|s: &str| {
     parse.default_info_string = Some("rust".to_string());
     parse.relaxed_tasklist_matching = true;
     parse.relaxed_autolinks = true;
-    let mut cb = |link_ref: BrokenLinkReference| {
+    let cb = |link_ref: BrokenLinkReference| {
         Some(ResolvedReference {
             url: link_ref.normalized.to_string(),
             title: link_ref.original.to_string(),
         })
     };
-    parse.broken_link_callback = Some(Arc::new(Mutex::new(&mut cb)));
+    parse.broken_link_callback = Some(Arc::new(cb));
 
     let mut render = RenderOptions::default();
     render.hardbreaks = true;

src/cm.rs

@@ -47,9 +47,9 @@ pub fn format_document_with_plugins<'a>(
     Ok(())
 }
 
-struct CommonMarkFormatter<'a, 'o, 'c> {
+struct CommonMarkFormatter<'a, 'o> {
     node: &'a AstNode<'a>,
-    options: &'o Options<'c>,
+    options: &'o Options,
     v: Vec<u8>,
     prefix: Vec<u8>,
     column: usize,
@@ -72,7 +72,7 @@ enum Escaping {
     Title,
 }
 
-impl<'a, 'o, 'c> Write for CommonMarkFormatter<'a, 'o, 'c> {
+impl<'a, 'o> Write for CommonMarkFormatter<'a, 'o> {
     fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
         self.output(buf, false, Escaping::Literal);
         Ok(buf.len())
@@ -83,8 +83,8 @@ impl<'a, 'o, 'c> Write for CommonMarkFormatter<'a, 'o, 'c> {
     }
 }
 
-impl<'a, 'o, 'c> CommonMarkFormatter<'a, 'o, 'c> {
-    fn new(node: &'a AstNode<'a>, options: &'o Options<'c>) -> Self {
+impl<'a, 'o> CommonMarkFormatter<'a, 'o> {
+    fn new(node: &'a AstNode<'a>, options: &'o Options) -> Self {
         CommonMarkFormatter {
             node,
             options,

src/html.rs

@@ -126,9 +126,9 @@ impl Anchorizer {
     }
 }
 
-struct HtmlFormatter<'o, 'c> {
+struct HtmlFormatter<'o> {
     output: &'o mut WriteWithLast<'o>,
-    options: &'o Options<'c>,
+    options: &'o Options,
     anchorizer: Anchorizer,
     footnote_ix: u32,
     written_footnote_ix: u32,
@@ -361,12 +361,8 @@ where
     Ok(())
 }
 
-impl<'o, 'c: 'o> HtmlFormatter<'o, 'c> {
-    fn new(
-        options: &'o Options<'c>,
-        output: &'o mut WriteWithLast<'o>,
-        plugins: &'o Plugins,
-    ) -> Self {
+impl<'o> HtmlFormatter<'o> {
+    fn new(options: &'o Options, output: &'o mut WriteWithLast<'o>, plugins: &'o Plugins) -> Self {
         HtmlFormatter {
             options,
             output,

src/lib.rs

@@ -101,9 +101,9 @@ pub use xml::format_document_with_plugins as format_xml_with_plugins;
 /// Legacy naming of [`ExtensionOptions`]
 pub type ComrakExtensionOptions = ExtensionOptions;
 /// Legacy naming of [`Options`]
-pub type ComrakOptions<'c> = Options<'c>;
+pub type ComrakOptions = Options;
 /// Legacy naming of [`ParseOptions`]
-pub type ComrakParseOptions<'c> = ParseOptions<'c>;
+pub type ComrakParseOptions = ParseOptions;
 /// Legacy naming of [`Plugins`]
 pub type ComrakPlugins<'a> = Plugins<'a>;
 /// Legacy naming of [`RenderOptions`]

src/parser/inlines.rs

@@ -25,9 +25,9 @@ const MAXBACKTICKS: usize = 80;
 const MAX_LINK_LABEL_LENGTH: usize = 1000;
 const MAX_MATH_DOLLARS: usize = 2;
 
-pub struct Subject<'a: 'd, 'r, 'o, 'c, 'd, 'i> {
+pub struct Subject<'a: 'd, 'r, 'o, 'd, 'i> {
     pub arena: &'a Arena<AstNode<'a>>,
-    options: &'o Options<'c>,
+    options: &'o Options,
     pub input: &'i [u8],
     line: usize,
     pub pos: usize,
@@ -110,10 +110,10 @@ struct WikilinkComponents<'i> {
     link_label: Option<(&'i [u8], usize, usize)>,
 }
 
-impl<'a, 'r, 'o, 'c, 'd, 'i> Subject<'a, 'r, 'o, 'c, 'd, 'i> {
+impl<'a, 'r, 'o, 'd, 'i> Subject<'a, 'r, 'o, 'd, 'i> {
     pub fn new(
         arena: &'a Arena<AstNode<'a>>,
-        options: &'o Options<'c>,
+        options: &'o Options,
         input: &'i [u8],
         line: usize,
         refmap: &'r mut RefMap,
@@ -1548,7 +1548,7 @@ impl<'a, 'r, 'o, 'c, 'd, 'i> Subject<'a, 'r, 'o, 'c, 'd, 'i> {
         // Attempt to use the provided broken link callback if a reference cannot be resolved
         if reff.is_none() {
             if let Some(callback) = &self.options.parse.broken_link_callback {
-                reff = callback.lock().unwrap()(BrokenLinkReference {
+                reff = callback.resolve(BrokenLinkReference {
                     normalized: &lab,
                     original: &unfolded_lab,
                 });

src/parser/mod.rs

@@ -26,7 +26,7 @@ use std::fmt::{self, Debug, Formatter};
 use std::mem;
 use std::panic::RefUnwindSafe;
 use std::str;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 use typed_arena::Arena;
 
 use crate::adapters::HeadingAdapter;
@@ -80,16 +80,16 @@ pub fn parse_document<'a>(
 /// [`ParseOptions::broken_link_callback`].
 #[deprecated(
     since = "0.25.0",
-    note = "The broken link callback has been moved into ParseOptions<'c>."
+    note = "The broken link callback has been moved into ParseOptions."
 )]
-pub fn parse_document_with_broken_link_callback<'a, 'c>(
+pub fn parse_document_with_broken_link_callback<'a>(
     arena: &'a Arena<AstNode<'a>>,
     buffer: &str,
-    options: &Options<'c>,
-    callback: Option<BrokenLinkCallback<'c>>,
+    options: &Options,
+    callback: Arc<dyn BrokenLinkCallback>,
 ) -> &'a AstNode<'a> {
     let mut options_with_callback = options.clone();
-    options_with_callback.parse.broken_link_callback = callback.map(|cb| Arc::new(Mutex::new(cb)));
+    options_with_callback.parse.broken_link_callback = Some(callback);
     parse_document(arena, buffer, &options_with_callback)
 }
 
@@ -100,8 +100,26 @@ pub fn parse_document_with_broken_link_callback<'a, 'c>(
 /// [`BrokenLinkReference`] argument. If a [`ResolvedReference`] is returned, it
 /// is used as the link; otherwise, no link is made and the reference text is
 /// preserved in its entirety.
-pub type BrokenLinkCallback<'c> =
-    &'c mut dyn FnMut(BrokenLinkReference) -> Option<ResolvedReference>;
+pub trait BrokenLinkCallback: RefUnwindSafe + Send + Sync {
+    /// Potentially resolve a single broken link reference.
+    fn resolve(&self, broken_link_reference: BrokenLinkReference) -> Option<ResolvedReference>;
+}
+
+impl Debug for dyn BrokenLinkCallback {
+    fn fmt(&self, formatter: &mut Formatter<'_>) -> Result<(), fmt::Error> {
+        formatter.write_str("<dyn BrokenLinkCallback>")
+    }
+}
+
+impl<F> BrokenLinkCallback for F
+where
+    F: Fn(BrokenLinkReference) -> Option<ResolvedReference>,
+    F: RefUnwindSafe + Send + Sync,
+{
+    fn resolve(&self, broken_link_reference: BrokenLinkReference) -> Option<ResolvedReference> {
+        self(broken_link_reference)
+    }
+}
 
 /// Struct to the broken link callback, containing details on the link reference
 /// which failed to find a match.
@@ -116,7 +134,7 @@ pub struct BrokenLinkReference<'l> {
     pub original: &'l str,
 }
 
-pub struct Parser<'a, 'o, 'c> {
+pub struct Parser<'a, 'o> {
     arena: &'a Arena<AstNode<'a>>,
     refmap: RefMap,
     root: &'a AstNode<'a>,
@@ -135,19 +153,18 @@ pub struct Parser<'a, 'o, 'c> {
     last_line_length: usize,
     last_buffer_ended_with_cr: bool,
     total_size: usize,
-    options: &'o Options<'c>,
+    options: &'o Options,
 }
 
 #[derive(Default, Debug, Clone)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
-/// Umbrella options struct. `'c` represents the lifetime of any callback
-/// closure options may take.
-pub struct Options<'c> {
+/// Umbrella options struct.
+pub struct Options {
     /// Enable CommonMark extensions.
     pub extension: ExtensionOptions,
 
     /// Configure parse-time options.
-    pub parse: ParseOptions<'c>,
+    pub parse: ParseOptions,
 
     /// Configure render-time options.
     pub render: RenderOptions,
@@ -581,10 +598,10 @@ pub struct ExtensionOptions {
 }
 
 #[non_exhaustive]
-#[derive(Default, Clone, Builder)]
+#[derive(Default, Clone, Debug, Builder)]
 #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))]
 /// Options for parser functions.
-pub struct ParseOptions<'c> {
+pub struct ParseOptions {
     /// Punctuation (quotes, full-stops and hyphens) are converted into 'smart' punctuation.
     ///
     /// ```
@@ -643,57 +660,30 @@ pub struct ParseOptions<'c> {
     /// used as the link destination and title if not [`None`].
     ///
     /// ```
-    /// # use std::{str, sync::{Arc, Mutex}};
-    /// # use comrak::{Arena, ResolvedReference, parse_document, format_html, Options, BrokenLinkReference, ParseOptions};
-    /// # use comrak::nodes::{AstNode, NodeValue};
-    /// #
-    /// # fn main() -> std::io::Result<()> {
-    /// let arena = Arena::new();
-    /// let mut cb = |link_ref: BrokenLinkReference| match link_ref.normalized {
+    /// # use std::{str, sync::Arc};
+    /// # use comrak::{markdown_to_html, BrokenLinkReference, Options, ResolvedReference};
+    /// let cb = |link_ref: BrokenLinkReference| match link_ref.normalized {
     ///     "foo" => Some(ResolvedReference {
     ///         url: "https://www.rust-lang.org/".to_string(),
     ///         title: "The Rust Language".to_string(),
     ///     }),
     ///     _ => None,
     /// };
-    /// let options = Options {
-    ///     parse: ParseOptions::builder()
-    ///         .broken_link_callback(Arc::new(Mutex::new(&mut cb)))
-    ///         .build(),
-    ///     ..Default::default()
-    /// };
     ///
-    /// let root = parse_document(
-    ///     &arena,
+    /// let mut options = Options::default();
+    /// options.parse.broken_link_callback = Some(Arc::new(cb));
+    ///
+    /// let output = markdown_to_html(
     ///     "# Cool input!\nWow look at this cool [link][foo]. A [broken link] renders as text.",
     ///     &options,
     /// );
     ///
-    /// let mut output = Vec::new();
-    /// format_html(root, &Options::default(), &mut output)?;
-    /// assert_eq!(str::from_utf8(&output).unwrap(),
+    /// assert_eq!(output,
     ///            "<h1>Cool input!</h1>\n<p>Wow look at this cool \
     ///            <a href=\"https://www.rust-lang.org/\" title=\"The Rust Language\">link</a>. \
     ///            A [broken link] renders as text.</p>\n");
-    /// # Ok(())
-    /// # }
     #[cfg_attr(feature = "arbitrary", arbitrary(default))]
-    pub broken_link_callback: Option<Arc<Mutex<BrokenLinkCallback<'c>>>>,
-}
-
-impl<'c> fmt::Debug for ParseOptions<'c> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        let mut struct_fmt = f.debug_struct("ParseOptions");
-        struct_fmt.field("smart", &self.smart);
-        struct_fmt.field("default_info_string", &self.default_info_string);
-        struct_fmt.field("relaxed_tasklist_matching", &self.relaxed_tasklist_matching);
-        struct_fmt.field("relaxed_autolinks", &self.relaxed_autolinks);
-        struct_fmt.field(
-            "broken_link_callback.is_some()",
-            &self.broken_link_callback.is_some(),
-        );
-        struct_fmt.finish()
-    }
+    pub broken_link_callback: Option<Arc<dyn BrokenLinkCallback>>,
 }
 
 #[non_exhaustive]
@@ -1100,8 +1090,8 @@ struct FootnoteDefinition<'a> {
     total_references: u32,
 }
 
-impl<'a, 'o, 'c: 'o> Parser<'a, 'o, 'c> {
-    fn new(arena: &'a Arena<AstNode<'a>>, root: &'a AstNode<'a>, options: &'o Options<'c>) -> Self {
+impl<'a, 'o> Parser<'a, 'o> {
+    fn new(arena: &'a Arena<AstNode<'a>>, root: &'a AstNode<'a>, options: &'o Options) -> Self {
         Parser {
             arena,
             refmap: RefMap::new(),

src/parser/table.rs

@@ -13,7 +13,7 @@ use super::inlines::count_newlines;
 const MAX_AUTOCOMPLETED_CELLS: usize = 500_000;
 
 pub fn try_opening_block<'a>(
-    parser: &mut Parser<'a, '_, '_>,
+    parser: &mut Parser<'a, '_>,
     container: &'a AstNode<'a>,
     line: &[u8],
 ) -> Option<(&'a AstNode<'a>, bool, bool)> {
@@ -30,7 +30,7 @@ pub fn try_opening_block<'a>(
 }
 
 fn try_opening_header<'a>(
-    parser: &mut Parser<'a, '_, '_>,
+    parser: &mut Parser<'a, '_>,
     container: &'a AstNode<'a>,
     line: &[u8],
 ) -> Option<(&'a AstNode<'a>, bool, bool)> {
@@ -133,7 +133,7 @@ fn try_opening_header<'a>(
 }
 
 fn try_opening_row<'a>(
-    parser: &mut Parser<'a, '_, '_>,
+    parser: &mut Parser<'a, '_>,
     container: &'a AstNode<'a>,
     alignments: &[TableAlignment],
     line: &[u8],
@@ -280,7 +280,7 @@ fn row(string: &[u8], spoiler: bool) -> Option<Row> {
 }
 
 fn try_inserting_table_header_paragraph<'a>(
-    parser: &mut Parser<'a, '_, '_>,
+    parser: &mut Parser<'a, '_>,
     container: &'a AstNode<'a>,
     paragraph_offset: usize,
 ) {

src/tests/api.rs

@@ -32,14 +32,15 @@ fn exercise_full_api() {
     let _: &AstNode = parse_document(&arena, "document", &default_options);
 
     // Ensure the closure can modify its context.
-    let mut blr_ctx_0 = 0;
+    let blr_ctx_0 = Arc::new(Mutex::new(0));
+    let blr_ctx_1 = blr_ctx_0.clone();
     #[allow(deprecated)]
     let _: &AstNode = parse_document_with_broken_link_callback(
         &arena,
         "document",
         &Options::default(),
-        Some(&mut |blr: BrokenLinkReference| {
-            blr_ctx_0 += 1;
+        Arc::new(move |blr: BrokenLinkReference| {
+            *blr_ctx_1.lock().unwrap() += 1;
             let _: &str = blr.normalized;
             let _: &str = blr.original;
             Some(ResolvedReference {
@@ -80,17 +81,16 @@ fn exercise_full_api() {
         .relaxed_tasklist_matching(false)
         .relaxed_autolinks(false);
 
-    let mut blr_ctx_1 = 0;
-    let _parse =
-        parse.broken_link_callback(Arc::new(Mutex::new(&mut |blr: BrokenLinkReference| {
-            blr_ctx_1 += 1;
-            let _: &str = blr.normalized;
-            let _: &str = blr.original;
-            Some(ResolvedReference {
-                url: String::new(),
-                title: String::new(),
-            })
-        })));
+    let blr_ctx_1 = blr_ctx_0.clone();
+    let _parse = parse.broken_link_callback(Arc::new(move |blr: BrokenLinkReference| {
+        *blr_ctx_1.lock().unwrap() += 1;
+        let _: &str = blr.normalized;
+        let _: &str = blr.original;
+        Some(ResolvedReference {
+            url: String::new(),
+            title: String::new(),
+        })
+    }));
 
     let _render = RenderOptions::builder()
         .hardbreaks(false)