refactor(link-forever): fail_on_warnings in early returns

Author: tonywu6

crates/mdbookkit/src/bin/link_forever/diagnostic.rs

@@ -14,6 +14,7 @@ use super::{Environment, LinkSpan, LinkStatus, LinkText, Pages, RelativeLink};
 
 impl Environment {
     pub fn report<'a>(&'a self, content: &'a Pages<'a>) -> Reporter<'a> {
+        // BTreeMap: sort output by paths
         let mut sorted: BTreeMap<&'_ Url, BTreeMap<LinkStatus, Vec<LinkDiagnostic<'_>>>> =
             Default::default();
 

crates/mdbookkit/src/bin/link_forever/git.rs

@@ -14,12 +14,15 @@ use super::{Config, CustomPermalink, Environment, GitHubPermalink, PermalinkForm
 
 impl Environment {
     pub fn try_from_env(book: &PreprocessorContext) -> Result<Result<Self>> {
+        let config = config_from_book::<Config>(&book.config, "link-forever")
+            .context("failed to read preprocessor config from book.toml")?;
+
         let repo = match Repository::open_from_env()
             .context("preprocessor requires a git repository to work")
             .context("failed to find a git repository")
         {
             Ok(repo) => repo,
-            Err(err) => return Ok(Err(err)),
+            Err(err) => return config.fail_on_warnings.adjusted(Ok(Err(err))),
         };
 
         let vcs_root = repo
@@ -44,12 +47,12 @@ impl Environment {
             }
         });
 
-        let config = config_from_book::<Config>(&book.config, "link-forever")?;
-
         let Some(reference) =
             get_head(&repo).context("failed to get a tag or commit id to HEAD")?
         else {
-            return Ok(Err(anyhow!("no commit found in this repo")));
+            return config
+                .fail_on_warnings
+                .adjusted(Ok(Err(anyhow!("no commit found in this repo"))));
         };
 
         let fmt_link: Box<dyn PermalinkFormat> = {
@@ -71,6 +74,7 @@ impl Environment {
                             .context("failed to determine GitHub url to use for permalinks")
                             .pipe(Err)
                             .pipe(Ok)
+                            .pipe(|result| config.fail_on_warnings.adjusted(result))
                     }
                 };
                 let (owner, repo) = remote_as_github(repo.as_ref())

crates/mdbookkit/src/env.rs

@@ -13,7 +13,7 @@ use tap::Pipe;
 #[derive(Deserialize, Debug, Default, Clone, Copy)]
 #[serde(rename_all = "lowercase")]
 pub enum ErrorHandling {
-    /// Fail if the environment variable `CI` is set to a value other than `0`.
+    /// Fail if the environment variable `CI` is set to a value other than `0` or `false`.
     /// Environments like GitHub Actions configure this automatically.
     #[default]
     #[serde(rename = "ci")]
@@ -35,10 +35,9 @@ impl ErrorHandling {
                         .pipe(Err)
                 }
                 Self::Env => {
-                    let ci = std::env::var("CI").unwrap_or("".into());
-                    if matches!(ci.as_str(), "" | "0" | "false") {
+                    let Some(ci) = Self::warning_as_error() else {
                         return Ok(());
-                    }
+                    };
                     anyhow!("treating warnings as errors because fail-on-unresolved is \"ci\" and CI={ci}")
                         .context("preprocessor has errors")
                         .pipe(Err)
@@ -47,6 +46,22 @@ impl ErrorHandling {
             _ => Ok(()),
         }
     }
+
+    pub fn adjusted<T, E>(&self, result: Result<Result<T, E>, E>) -> Result<Result<T, E>, E> {
+        match result {
+            Ok(Err(error)) if Self::warning_as_error().is_some() => Err(error),
+            result => result,
+        }
+    }
+
+    fn warning_as_error() -> Option<String> {
+        let ci = std::env::var("CI").unwrap_or("".into());
+        if matches!(ci.as_str(), "" | "0" | "false") {
+            None
+        } else {
+            Some(ci)
+        }
+    }
 }
 
 pub fn string_from_stdin() -> Result<String> {

crates/mdbookkit/tests/link_forever.rs

@@ -120,12 +120,25 @@ fn test_minimum_env() -> Result<()> {
     log::info!("then: book builds with warnings");
     Command::new("mdbook")
         .arg("build")
+        .env("CI", "false")
         .env("PATH", &path)
         .current_dir(&root)
         .assert()
         .success()
         .stderr(predicate::str::contains("requires a git repository"));
 
+    log::info!("when: CI=true");
+
+    log::info!("then: preprocessor fails");
+    Command::new("mdbook")
+        .arg("build")
+        .env("CI", "true")
+        .env("PATH", &path)
+        .current_dir(&root)
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains("requires a git repository"));
+
     log::info!("when: repo has no commit");
     Command::new("git")
         .arg("init")
@@ -137,6 +150,7 @@ fn test_minimum_env() -> Result<()> {
     log::info!("then: book builds with warnings");
     Command::new("mdbook")
         .arg("build")
+        .env("CI", "false")
         .env("PATH", &path)
         .current_dir(&root)
         .assert()
@@ -159,6 +173,7 @@ fn test_minimum_env() -> Result<()> {
     log::info!("then: book builds with warnings");
     Command::new("mdbook")
         .arg("build")
+        .env("CI", "false")
         .env("PATH", &path)
         .current_dir(&root)
         .assert()

crates/mdbookkit/tests/rustdoc_link.rs

@@ -237,8 +237,9 @@ fn test_minimum_env() -> Result<()> {
             .stderr(
                 predicate::str::contains("failed to spawn rust-analyzer")
                     // https://github.com/rust-lang/rustup/issues/3846
-                    // rustup shims rust-analyzer even when it's not installed
+                    // rustup shims rust-analyzer when it's not installed
                     .or(predicate::str::contains("Unknown binary 'rust-analyzer")),
+                // ^ doesn't have a closing `'` because on windows it says 'rust-analyzer.exe'
             );
 
         log::info!("when: code extension is installed");