feat! treat warnings as errors by default

Add a --merciful option to subcommands for which this matters. Adjust
tests to invoke subplot with that option as necessary.

Sponsored-by: author

7 files changed, 103 insertions, 42 deletions

diff --git a/check b/check
index 9dac5d8..f9b331c 100755
--- a/check
+++ b/check
@@ -178,6 +178,7 @@ class Runcmd:
                 "metadata",
                 "-o",
                 "json",
+                "--merciful",
                 filename,
             ],
             stdout=PIPE,
diff --git a/src/bin/cli/mod.rs b/src/bin/cli/mod.rs
index a16df87..2c43c39 100644
--- a/src/bin/cli/mod.rs
+++ b/src/bin/cli/mod.rs
@@ -35,7 +35,7 @@ impl TryFrom<&mut Document> for Metadata {
     type Error = subplot::SubplotError;
     fn try_from(doc: &mut Document) -> std::result::Result<Self, Self::Error> {
         let mut sources: Vec<_> = doc
-            .sources(None)
+            .sources(&None)
             .into_iter()
             .map(|p| filename(Some(&p)))
             .collect();
diff --git a/src/bin/subplot.rs b/src/bin/subplot.rs
index 2ad2df1..6d098f3 100644
--- a/src/bin/subplot.rs
+++ b/src/bin/subplot.rs
@@ -6,8 +6,7 @@ use anyhow::Result;
 use log::{debug, trace};
 use structopt::StructOpt;
 use subplot::{
-    codegen, load_document, load_document_with_pullmark, resource, DataFile, Document, MarkupOpts,
-    Style,
+    codegen, load_document, resource, DataFile, Document, MarkupOpts, Style, SubplotError,
 };
 use time::{format_description::FormatItem, macros::format_description, OffsetDateTime};
 
@@ -134,6 +133,10 @@ impl Resources {
 /// extract all embedded files.  if the output directory
 /// is not specified then this will extract to the current directory.
 struct Extract {
+    /// Allow warnings in document?
+    #[structopt(long)]
+    merciful: bool,
+
     /// Directory to write extracted files to
     #[structopt(
         name = "DIR",
@@ -162,7 +165,7 @@ impl Extract {
     }
 
     fn run(&self) -> Result<()> {
-        let doc = load_document(&self.filename, Style::default(), None)?;
+        let doc = load_linted_doc(&self.filename, Style::default(), &None, self.merciful)?;
 
         let files: Vec<&DataFile> = if self.embedded.is_empty() {
             doc.files()
@@ -249,6 +252,10 @@ impl Filter {
 /// document.  This can then render that either as a plain text report for humans,
 /// or as a JSON object for further scripted processing.
 struct Metadata {
+    /// Allow warnings in document?
+    #[structopt(long)]
+    merciful: bool,
+
     /// Form that you want the output to take
     #[structopt(short = "o", default_value = "plain", possible_values=&["plain", "json"])]
     output_format: cli::OutputFormat,
@@ -264,7 +271,7 @@ impl Metadata {
     }
 
     fn run(&self) -> Result<()> {
-        let mut doc = load_document_with_pullmark(&self.filename, Style::default(), None)?;
+        let mut doc = load_linted_doc(&self.filename, Style::default(), &None, self.merciful)?;
         let meta = cli::Metadata::try_from(&mut doc)?;
         match self.output_format {
             cli::OutputFormat::Plain => meta.write_out(),
@@ -279,6 +286,10 @@ impl Metadata {
 ///
 /// Process a subplot document and typeset it using Pandoc.
 struct Docgen {
+    /// Allow warnings in document?
+    #[structopt(long)]
+    merciful: bool,
+
     /// The template to use from the document.
     ///
     /// If not specified, subplot will try and find a unique template name from the document
@@ -309,26 +320,7 @@ impl Docgen {
             trace!("PDF output chosen");
             style.typeset_links_as_notes();
         }
-        let mut doc = load_document(&self.input, style, None)?;
-        trace!("Got doc, now linting it");
-        doc.lint()?;
-        trace!("Doc linted ok");
-        let meta = doc.meta();
-        trace!("Looking for template, meta={:#?}", meta);
-        let template = self
-            .template
-            .as_deref()
-            .map(Ok)
-            .unwrap_or_else(|| doc.template())
-            .unwrap_or("");
-        let template = template.to_string();
-        trace!("Template: {:#?}", template);
-        if !doc.check_named_files_exist(&template)?
-            || !doc.check_matched_steps_have_impl(&template)
-            || !doc.check_embedded_files_are_used(&template)?
-        {
-            eprintln!("Continuing despite warnings");
-        }
+        let mut doc = load_linted_doc(&self.input, style, &self.template, self.merciful)?;
 
         let mut pandoc = pandoc::new();
         // Metadata date from command line or file mtime. However, we
@@ -348,7 +340,7 @@ impl Docgen {
         pandoc.add_option(pandoc::PandocOption::Standalone);
         pandoc.add_option(pandoc::PandocOption::NumberSections);
 
-        if Self::need_output(&mut doc, &template, &self.output) {
+        if Self::need_output(&mut doc, &self.template, &self.output) {
             doc.typeset();
             pandoc.set_input_format(pandoc::InputFormat::Json, vec![]);
             pandoc.set_input(pandoc::InputKind::Pipe(doc.ast()?));
@@ -374,13 +366,13 @@ impl Docgen {
         time.format(DATE_FORMAT).unwrap()
     }
 
-    fn need_output(doc: &mut subplot::Document, template: &str, output: &Path) -> bool {
+    fn need_output(doc: &mut subplot::Document, template: &Option<String>, output: &Path) -> bool {
         let output = match Self::mtime(output) {
             Err(_) => return true,
             Ok(ts) => ts,
         };
 
-        for filename in doc.sources(Some(template)) {
+        for filename in doc.sources(template) {
             let source = match Self::mtime(&filename) {
                 Err(_) => return true,
                 Ok(ts) => ts,
@@ -458,6 +450,41 @@ impl Codegen {
     }
 }
 
+fn load_linted_doc(
+    filename: &Path,
+    style: Style,
+    template: &Option<String>,
+    merciful: bool,
+) -> Result<Document, SubplotError> {
+    let mut doc = load_document(filename, style, None)?;
+    trace!("Got doc, now linting it");
+    doc.lint()?;
+    trace!("Doc linted ok");
+
+    let meta = doc.meta();
+    trace!("Looking for template, meta={:#?}", meta);
+    let template = template
+        .as_deref()
+        .map(Ok)
+        .unwrap_or_else(|| doc.template())
+        .unwrap_or("");
+    let template = template.to_string();
+    trace!("Template: {:#?}", template);
+    doc.check_named_files_exist(&template)?;
+    doc.check_matched_steps_have_impl(&template);
+    doc.check_embedded_files_are_used(&template)?;
+
+    for w in doc.warnings() {
+        eprintln!("WARNING: {}", w);
+    }
+
+    if !doc.warnings().is_empty() && !merciful {
+        return Err(SubplotError::Warnings(doc.warnings().len()));
+    }
+
+    Ok(doc)
+}
+
 fn real_main() {
     trace!("Starting Subplot");
     let argparser = Toplevel::clap();
diff --git a/src/doc.rs b/src/doc.rs
index d6ab697..db37389 100644
--- a/src/doc.rs
+++ b/src/doc.rs
@@ -161,6 +161,11 @@ impl<'a> Document {
         style: Style,
         template: Option<&str>,
     ) -> Result<Document> {
+        trace!(
+            "Document::from_file: basedir={} filename={}",
+            basedir.display(),
+            filename.display()
+        );
         let markdowns = vec![filename.to_path_buf()];
 
         let mut pandoc = pandoc::new();
@@ -180,6 +185,7 @@ impl<'a> Document {
             pandoc::PandocOutput::ToBuffer(o) => o,
             _ => return Err(SubplotError::NotJson),
         };
+        trace!("Pandoc was happy");
         let doc = Document::from_json(basedir, markdowns, &output, style, template)?;
         trace!("Loaded document OK");
         Ok(doc)
@@ -223,7 +229,7 @@ impl<'a> Document {
     ///
     /// The sources are any files that affect the output so that if
     /// the source file changes, the output needs to be re-generated.
-    pub fn sources(&mut self, template: Option<&str>) -> Vec<PathBuf> {
+    pub fn sources(&mut self, template: &Option<String>) -> Vec<PathBuf> {
         let mut names = vec![];
 
         for x in self.meta().bindings_filenames() {
@@ -352,7 +358,10 @@ impl<'a> Document {
                         if matches!(step.types().get(name.as_str()), Some(CaptureType::File))
                             && !filenames.contains(&text.to_lowercase())
                         {
-                            self.warnings.push(Warning::UnknownEmbeddedFile(text.to_string()));
+                            self.warnings.push(Warning::UnknownEmbeddedFile(
+                                scenario.title().to_string(),
+                                text.to_string(),
+                            ));
                             okay = false;
                         }
                     }
@@ -386,7 +395,8 @@ impl<'a> Document {
             }
         }
         for filename in filenames.iter() {
-            self.warnings.push(Warning::UnusedEmbeddedFile(filename.to_string()));
+            self.warnings
+                .push(Warning::UnusedEmbeddedFile(filename.to_string()));
         }
 
         // We always succeed. Subplot's own subplot had valid cases of
@@ -410,7 +420,10 @@ impl<'a> Document {
             for step in scenario.steps() {
                 if step.function().is_none() {
                     trace!("Missing step implementation: {:?}", step.text());
-                    self.warnings.push(Warning::MissingStepImplementation(step.text().to_string()));
+                    self.warnings.push(Warning::MissingStepImplementation(
+                        scenario.title().to_string(),
+                        step.text().to_string(),
+                    ));
                     okay = false;
                 }
             }
diff --git a/src/error.rs b/src/error.rs
index 31fca60..6e7eddd 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -8,6 +8,10 @@ use thiserror::Error;
 /// Define all the kinds of errors any part of this crate can return.
 #[derive(Debug, Error)]
 pub enum SubplotError {
+    /// Document has non-fatal errors.
+    #[error("Document has {0} warnings.")]
+    Warnings(usize),
+
     /// Subplot could not find a file named as a bindings file.
     #[error("binding file could not be found: {0}: {1}")]
     BindingsFileNotFound(PathBuf, std::io::Error),
@@ -338,16 +342,21 @@ pub type Result<T> = std::result::Result<T, SubplotError>;
 /// A warning, or non-fatal error.
 ///
 /// Errors prevent Subplot from producing output. Warnings don't do that.
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum Warning {
     /// Document refers to an embedded file that doesn't exist.
-    UnknownEmbeddedFile(String),
+    #[error(
+        "Document refers to an embedded file that doesn't exist: \"{1}\"\n  in scenario \"{0}\""
+    )]
+    UnknownEmbeddedFile(String, String),
 
     /// Embedded file is not used by any scenario.
+    #[error("Embedded file is not used by any scenario: \"{0}\"")]
     UnusedEmbeddedFile(String),
 
     /// Missing step implementation.
-    MissingStepImplementation(String),
+    #[error("Missing step implementation: \"{1}\"\n  in scenario \"{0}\"")]
+    MissingStepImplementation(String, String),
 }
 
 /// A list of warnings.
diff --git a/subplot.md b/subplot.md
index 5098d74..18fe8d5 100644
--- a/subplot.md
+++ b/subplot.md
@@ -509,7 +509,7 @@ could be of the form `then some output is present` or `then it exits successfull
 
 With all that in mind, a good scenario looks like
 
-```scenario
+```
 given the necessary starting conditions
 when I do the required actions
 then the desired outcome is achieved
@@ -628,6 +628,16 @@ This is still line number three, but would it be obvious?
 [roadmap]: https://crates.io/search?q=roadmap
 
 
+### Use embedded file
+
+This scenario makes sure the sample files are used in a scenario so
+that they don't cause warnings.
+
+~~~scenario
+given file numbered-lines.txt
+given file not-numbered-lines.txt
+~~~
+
 ## Document metadata
 
 Pandoc supports, and Subplot makes use of, a [YAML metadata block][] in a
@@ -809,7 +819,7 @@ given file badfilename.md
 and file b.yaml
 and file f.py
 and an installed subplot
-when I run subplot docgen badfilename.md -o foo.pdf
+when I run subplot docgen --merciful badfilename.md -o foo.pdf
 then file foo.pdf exists
 when I try to run subplot codegen --run badfilename.md -o test.py
 then command fails
@@ -1182,7 +1192,7 @@ given file aliases.md
 given file b.yaml
 given file f.py
 given an installed subplot
-when I run subplot docgen aliases.md -o aliases.html
+when I run subplot docgen --merciful aliases.md -o aliases.html
 then command is successful
 then file aliases.html matches regex /given<[^>]*> precondition foo/
 then file aliases.html matches regex /when<[^>]*> I do bar/
@@ -2572,7 +2582,7 @@ in a subplot.
 ~~~scenario
 given file embedded.md
 and an installed subplot
-when I run subplot docgen embedded.md -o foo.html
+when I run subplot docgen --merciful embedded.md -o foo.html
 then file foo.html exists
 and file foo.html matches regex /embedded\.txt/
 ~~~
@@ -2753,7 +2763,7 @@ embedded files that aren't used.
 ~~~scenario
 given file unusedfile.md
 and an installed subplot
-when I try to run subplot docgen unusedfile.md -o unusedfile.html
+when I try to run subplot docgen --merciful unusedfile.md -o unusedfile.html
 then command is successful
 and file unusedfile.html exists
 and stderr contains "thisisnotused.txt"
@@ -2894,7 +2904,7 @@ The `subplot metadata` command lists embedded files in its output.
 ~~~scenario
 given file two-embedded.md
 and an installed subplot
-when I run subplot metadata two-embedded.md
+when I run subplot metadata --merciful two-embedded.md
 then stdout contains "foo.txt"
 and stdout contains "bar.yaml"
 ~~~
@@ -3247,7 +3257,7 @@ It does not have a YAML metadata block.
 given file embedded-file.md
 and file expected.txt
 and an installed subplot
-when I run subplot extract embedded-file.md foo.txt -d .
+when I run subplot extract --merciful embedded-file.md foo.txt -d .
 then files foo.txt and expected.txt match
 ~~~
 
diff --git a/subplotlib/build.rs b/subplotlib/build.rs
index d1eeefd..5b02a27 100644
--- a/subplotlib/build.rs
+++ b/subplotlib/build.rs
@@ -28,6 +28,7 @@ fn main() {
             panic!("missing include file: {}", inc.display());
         }
         println!("cargo:rerun-if-changed={}", inc.display());
+        println!("cargo:rerun-if-changed={}", entry.display());
         subplot_build::codegen(Path::new(&entry)).expect("failed to generate code with Subplot");
     }
 }