From 85212e8ddc54c6ede71912127eaf84aac403f10d Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 2 Apr 2022 09:50:39 +0300 Subject: 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 --- check | 1 + src/bin/cli/mod.rs | 2 +- src/bin/subplot.rs | 81 +++++++++++++++++++++++++++++++++++------------------ src/doc.rs | 21 +++++++++++--- src/error.rs | 15 ++++++++-- subplot.md | 24 +++++++++++----- subplotlib/build.rs | 1 + 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 { 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, 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, + merciful: bool, +) -> Result { + 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 { + 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 { + pub fn sources(&mut self, template: &Option) -> Vec { 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 = std::result::Result; /// 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"); } } -- cgit v1.2.1