Merge branch 'fix-docs' into 'main'

Add a bunch of docs and doctest fixes for subplotlib

See merge request subplot/subplot!170

8 files changed, 234 insertions, 13 deletions

diff --git a/check b/check
index eb6b509..7168226 100755
--- a/check
+++ b/check
@@ -180,7 +180,7 @@ def check_rust(r, strict=False):
     elif strict:
         sys.exit("Strict Rust checks specified, but clippy was not found")
     r.runcmd(["cargo", "test"])
-    r.runcmd(["cargo", "fmt"])
+    r.runcmd(["cargo", "fmt", "--", "--check"])
 
 
 def check_subplots(r):
@@ -220,6 +220,9 @@ def check_subplotlib(r):
     # Run Rust tests for the subplotlib library.
     r.runcmd(["cargo", "test", "--lib"], cwd="subplotlib")
 
+    # Run Rust doctests for the subplotlib library.
+    r.runcmd(["cargo", "test", "--doc"], cwd="subplotlib")
+
     # Find subplots for subplotlib.
     mds = find_files("subplotlib/*.md", lambda f: True)
 
@@ -231,11 +234,16 @@ def check_subplotlib(r):
         base, _ = os.path.splitext(md)
         test_rs = os.path.join("tests", base + ".rs")
         r.codegen(md, test_rs, cwd=dirname)
-        r.cargo(["fmt"], cwd=dirname)
-        r.cargo(["test", "--test", base], cwd=dirname)
         r.docgen(md, base + ".html", cwd=dirname)
         r.docgen(md, base + ".pdf", cwd=dirname)
 
+    # Format the code once more to keep things clean
+    r.title("Formatting subplotlib")
+    r.cargo(["fmt", "-p", "subplotlib"], cwd=dirname)
+    # Run all of the integration suites (many of which have come from the above)
+    r.title("Running subplotlib integration tests")
+    r.cargo(["test", "-p", "subplotlib", "--tests"])
+
 
 def check_tooling(r):
     """Check build environment for tooling the test suite needs"""
diff --git a/subplotlib-derive/src/lib.rs b/subplotlib-derive/src/lib.rs
index e5b8ab6..160643d 100644
--- a/subplotlib-derive/src/lib.rs
+++ b/subplotlib-derive/src/lib.rs
@@ -1,6 +1,9 @@
 use proc_macro::TokenStream;
 use proc_macro2::Span;
-use syn::{parse_macro_input, parse_quote, Error, FnArg, Ident, ItemFn, Pat, ReturnType, Type};
+use syn::{
+    parse_macro_input, parse_quote, Error, FnArg, Ident, ItemFn, Pat, PathArguments, ReturnType,
+    Type,
+};
 
 use quote::quote;
 
@@ -42,6 +45,27 @@ fn ty_is_scenariocontext(ty: &Type) -> bool {
 }
 
 #[throws(Error)]
+fn ty_as_path(ty: &Type) -> String {
+    if let Type::Path(p) = ty {
+        let mut ret = String::new();
+        let mut colons = p.path.leading_colon.is_some();
+        for seg in &p.path.segments {
+            if !matches!(seg.arguments, PathArguments::None) {
+                throw!(Error::new_spanned(seg, "unexpected path segment arguments"));
+            }
+            if colons {
+                ret.push_str("::");
+            }
+            colons = true;
+            ret.push_str(&seg.ident.to_string());
+        }
+        ret
+    } else {
+        throw!(Error::new_spanned(ty, "expected a type path"));
+    }
+}
+
+#[throws(Error)]
 fn check_step_declaration(step: &ItemFn) {
     // Step functions must be declared very simply as:
     // fn stepfunctionname(context: &mut Context)
@@ -174,6 +198,12 @@ fn process_step(mut input: ItemFn) -> proc_macro2::TokenStream {
 
     input.attrs.retain(|f| !f.path.is_ident("context"));
 
+    let docs: Vec<_> = input
+        .attrs
+        .iter()
+        .filter(|attr| attr.path.is_ident("doc"))
+        .collect();
+
     let fields = input
         .sig
         .inputs
@@ -223,6 +253,7 @@ fn process_step(mut input: ItemFn) -> proc_macro2::TokenStream {
             #[allow(non_camel_case_types)]
             #[allow(unused)]
             #[derive(Default)]
+            #[doc(hidden)]
             pub struct Builder {
                 #(#structfields),*
             }
@@ -335,7 +366,44 @@ fn process_step(mut input: ItemFn) -> proc_macro2::TokenStream {
         }
     };
 
+    let call_docs = {
+        let mut contextattrs = String::new();
+        let outer_ctx = if ty_is_scenariocontext(&contexttype) {
+            None
+        } else {
+            Some(&contexttype)
+        };
+        for context in outer_ctx.into_iter().chain(contexts.iter()) {
+            contextattrs.push_str(&format!("\n    #[context({:?})]", ty_as_path(context)?));
+        }
+        let func_args: Vec<_> = fields
+            .iter()
+            .map(|(ident, _)| format!("{}", ident))
+            .collect();
+        let func_args = func_args.join(", ");
+        format!(
+            r#"
+    Call [this step][self] function from another.
+
+    If you want to call this step function from another, you will
+    need to do something like this:
+
+    ```rust,ignore
+    #[step]{contextattrs}
+    fn defer_to_{stepname}(context: &ScenarioContext) {{
+        //...
+        {stepname}::call(context, {func_args})?;
+        // ...
+    }}
+    ```
+    "#,
+            stepname = stepname,
+            contextattrs = contextattrs,
+            func_args = func_args,
+        )
+    };
     let ret = quote! {
+        #(#docs)*
         #vis mod #stepname {
             use super::*;
             pub(crate) use super::#contexttype;
@@ -345,17 +413,20 @@ fn process_step(mut input: ItemFn) -> proc_macro2::TokenStream {
 
             #[throws(StepError)]
             #[allow(unused)] // It's okay for step functions to not be used
+            #[doc(hidden)]
             #input
 
+            #[doc = #call_docs]
             pub fn call(___context___: &ScenarioContext, #(#inputargs),*) -> StepResult {
                 #call_body
             }
 
             #[allow(unused_variables)]
+            #[doc(hidden)]
             pub fn register_contexts(scenario: &Scenario) {
                 #register_fn_body
             }
-    }
+        }
     };
 
     ret
diff --git a/subplotlib/src/file.rs b/subplotlib/src/file.rs
index e44c0ef..f695a89 100644
--- a/subplotlib/src/file.rs
+++ b/subplotlib/src/file.rs
@@ -18,11 +18,15 @@ use base64::decode;
 ///
 /// ```rust
 /// # use subplotlib::prelude::*;
+/// # #[derive(Default)]
 /// # struct Context {}
+/// # impl ContextElement for Context {}
 /// #[step]
 /// fn step_using_a_file(context: &mut Context, somefile: SubplotDataFile) {
 ///     // context.stash_file(somefile);
 /// }
+///
+/// # fn main() {}
 /// ```
 ///
 /// For the generated test to correctly recognise how to pass a file in, you
diff --git a/subplotlib/src/prelude.rs b/subplotlib/src/prelude.rs
index 68f3ec4..4f5b781 100644
--- a/subplotlib/src/prelude.rs
+++ b/subplotlib/src/prelude.rs
@@ -1,17 +1,135 @@
-//! The subplotlib prelude
+//! The subplotlib prelude.
+//!
+//! This prelude is automatically imported into all generated subplot test suites
+//! using the `rust` template.  Effectively they get
+//!
+//! ```rust
+//! use subplotlib::prelude::*;
+//! ```
+//!
+//! inserted into them at the top of the file.
+//!
+//! You should familiarise yourself with the context-related types if you
+//! are writing your own contexts, or writing step functions which use
+//! the contexts provided by the [step library][crate::steplibrary] itself.
+//!
+//! The primary thing you will need to learn, as a step function author, is
+//! the [`#[step]`][macro@step] attribute macro, and it interacts with contexts.
 
 // Re-export fehler's macros so that step functions can use them
-#[doc(inline)]
 pub use fehler::throw;
-#[doc(inline)]
+/// Indicate what type a function throws
+///
+/// This attribute macro comes from the [`fehler`] crate and is used
+/// to indicate that a function "throws" a particular kind of error.
+///
+/// ```rust
+/// # use std::io;
+/// # use fehler::throws;
+/// #[throws(io::Error)]
+/// fn create_thingy() {
+///     // something which might cause an io::Error
+/// }
+/// # fn main() {}
+/// ```
+///
+/// It transforms a function such that the above function would be compiled
+/// effectively as:
+///
+/// ```rust
+/// # use std::io;
+/// fn create_thingy() -> Result<(), io::Error> {
+///     // something which might cause an io::Error
+///     Ok(())
+/// }
+/// ```
+///
+/// Return statements and the final expression of the function automatically
+/// get wrappered with [`Ok`][std::result::Result::Ok].  You can use the
+/// [`throw`][macro@throw] macro inside such a function to automatically return
+/// an error.
+///
+// When https://github.com/rust-lang/rust/issues/83976 is resolved, the below
+// can be added to this doc string.
+// You can see more documentation on this in the [fehler crate docs][fehler::throws].
+// Alternatively we can get rid of this entirely if and when
+// https://github.com/rust-lang/rust/issues/81893 is fixed.
 pub use fehler::throws;
 
 // Re-export the lazy_static macro
-#[doc(inline)]
+#[doc(hidden)]
 pub use lazy_static::lazy_static;
 
 // Re-export subplotlib-derive's macros so that #[step] works
-#[doc(inline)]
+// We have to document it here because we're referring to things
+// inside subplotlib.
+/// Mark a function as a Subplot step function.
+///
+/// This attribute macro is used to indicate that a given function is
+/// a step which can be bound to Subplot scenario statements.
+///
+/// In its simplest form, you use this thusly:
+///
+/// ```rust
+/// # use subplotlib::prelude::*;
+/// # #[derive(Default)] struct SomeContextType;
+/// # impl ContextElement for SomeContextType {}
+/// #[step]
+/// fn my_step_function(context: &mut SomeContextType, arg1: &str, arg2: &str)
+/// {
+///     // Do something appropriate here.
+/// }
+/// # fn main() {}
+/// ```
+///
+/// Step functions must be pretty basic.  For now at least they cannot be async,
+/// and they cannot be generic, take variadic arguments, be unsafe, etc.
+///
+/// The first argument to the step function is considered the step's context.
+/// Anything which implements
+/// [`ContextElement`] can be used here. You
+/// can take either a `&ContextType` or `&mut ContextType` as your context. It's
+/// likely best to take the shared borrow if you're not altering the context at all.
+///
+/// If you require access to a number of context objects as part of your step
+/// function implementation, then there is the high level container
+/// [`ScenarioContext`].  You should take
+/// this high level container as a shared borrow, and then within your step function
+/// you can access other contexts as follows:
+///
+/// ```rust
+/// # use subplotlib::prelude::*;
+/// # #[derive(Default)] struct ContextA;
+/// # #[derive(Default)] struct ContextB;
+/// # impl ContextElement for ContextA {}
+/// # impl ContextElement for ContextB {}
+/// # impl ContextA { fn get_thingy(&self) -> Result<usize, StepError> { Ok(0) } }
+/// # impl ContextB { fn do_mut_thing(&mut self, _n: usize, _arg: &str) -> Result<(), StepError> { Ok(()) } }
+/// #[step]
+/// #[context(ContextA)]
+/// #[context(ContextB)]
+/// fn my_step(context: &ScenarioContext, arg: &str) {
+///     let thingy = context.with(|ctx: &ContextA| ctx.get_thingy(), false )?;
+///     context.with_mut(|ctx: &mut ContextB| ctx.do_mut_thing(thingy, arg), false)?;
+/// }
+/// # fn main() {}
+/// ```
+///
+/// Importantly here there is the `#[context(SomeContext)]` attribute to tell
+/// the system that you use that context in your step function (without this,
+/// the relevant context may not be initialised for the scenario).  The mechanism
+/// to then access contexts from the step function is the
+/// [`ScenarioContext::with`] and
+/// [`ScenarioContext::with_mut`]
+/// functions which allow shared, or mutable, access to scenario contexts respectively.
+///
+/// These functions take two arguments, the first is a closure which will be run
+/// with access to the given context and whose return value will be ultimately
+/// returned by the call to the function.  The second is whether or not to defuse
+/// the poison on the context mutex.  In normal steps this should be `false` since
+/// you want a step to fail if the context has been poisoned.  However, in cleanup
+/// related step functions you probably want to defuse the poison and be careful in
+/// how you then use the contexts so that you can clean up effectively.
 pub use subplotlib_derive::step;
 
 // Re-export the step result types
diff --git a/subplotlib/src/scenario.rs b/subplotlib/src/scenario.rs
index cd83ac1..cca7721 100644
--- a/subplotlib/src/scenario.rs
+++ b/subplotlib/src/scenario.rs
@@ -81,7 +81,7 @@ pub trait ContextElement: Default + Send + 'static {
 
     /// Exit from a step function
     ///
-    /// See [the `enter_step` function][ContextElement::enter_step] for most
+    /// See [the `step_starts` function][ContextElement::step_starts] for most
     /// details of this.
     ///
     /// Any error returned from this will be masked if the step function itself
diff --git a/subplotlib/src/step.rs b/subplotlib/src/step.rs
index 6ce3b0a..fba0216 100644
--- a/subplotlib/src/step.rs
+++ b/subplotlib/src/step.rs
@@ -68,7 +68,7 @@ impl ScenarioStep {
     }
 
     /// Register any context types needed by this step
-    pub fn register_contexts(&self, scenario: &Scenario) {
+    pub(crate) fn register_contexts(&self, scenario: &Scenario) {
         (*self.reg)(scenario);
     }
 }
diff --git a/subplotlib/src/steplibrary/files.rs b/subplotlib/src/steplibrary/files.rs
index 169111d..35f841a 100644
--- a/subplotlib/src/steplibrary/files.rs
+++ b/subplotlib/src/steplibrary/files.rs
@@ -19,6 +19,13 @@ pub use crate::prelude::*;
 pub use super::datadir::Datadir;
 
 #[derive(Default)]
+/// Context data for the `files` step library
+///
+/// This context contains a mapping from filename to metadata so that
+/// the various steps remember metadata and then query it later can find it.
+///
+/// This context depends on, and will automatically register, the context for
+/// the [`datadir`][crate::steplibrary::datadir] step library.
 pub struct Files {
     metadata: HashMap<String, Metadata>,
 }
@@ -29,6 +36,13 @@ impl ContextElement for Files {
     }
 }
 
+/// Create a file on disk from an embedded file
+///
+/// # `given file {embedded_file}`
+///
+/// Create a file in the data dir from an embedded file.
+///
+/// This defers to [`create_from_embedded_with_other_name`]
 #[step]
 #[context(Datadir)]
 pub fn create_from_embedded(context: &ScenarioContext, embedded_file: SubplotDataFile) {
@@ -36,6 +50,12 @@ pub fn create_from_embedded(context: &ScenarioContext, embedded_file: SubplotDat
     create_from_embedded_with_other_name::call(context, &filename_on_disk, embedded_file)
 }
 
+/// Create a file on disk from an embedded file with a given name
+///
+/// # `given file {filename_on_disk} from {embedded_file}`
+///
+/// Creates a file in the data dir from an embedded file, but giving it a
+/// potentially different name.
 #[step]
 pub fn create_from_embedded_with_other_name(
     context: &Datadir,
diff --git a/subplotlib/subplotlib.md b/subplotlib/subplotlib.md
index 872bd62..c46134a 100644
--- a/subplotlib/subplotlib.md
+++ b/subplotlib/subplotlib.md
@@ -7,7 +7,7 @@ bindings:
 functions:
   - helpers/subplotlib_context.rs
   - helpers/subplotlib_impl.rs
----
+...
 
 # Rust library for assisting with running subplots