Merge #2232

2232: Use anyhow::Result in xtask, add contexts r=matklad a=killercup

This builds on #2231 but was actually done before that. You see, the
cause for #2231 was that I got this error message:

    Error: Error { kind: Io(Os { code: 2, kind: NotFound, message: "No such file or directory" }) }

Just switching to `anyhow::Result` got me stack traces (when setting
`RUST_LIB_BACKTRACE=1`) that at least showed

    stack backtrace:
      0: std::backtrace::Backtrace::create
      1: std::backtrace::Backtrace::capture
      2: anyhow::error::<impl core::convert::From<E> for anyhow::Error>::from
      3: xtask::install_server
      4: xtask::install
      5: xtask::main
      6: std::rt::lang_start::{{closure}}
      7: std::panicking::try::do_call
      8: __rust_maybe_catch_panic
      9: std::rt::lang_start_internal
      10: std::rt::lang_start
      11: main

With the added contexts (not at all exhaustive), the error became

    Error: install server

    Caused by:
        0: build AutoCfg with target directory
        1: No such file or directory (os error 2)

Since anyhow is such a small thing (no new transitive dependencies!),
and in general gives you `Result<T, Box<dyn Error>>` on steroids, I
think this a nice small change. The only slightly annoying thing was to
replace all the `Err(format!(…))?` calls (haven't even looked at whether
we can make it support wrapping strings though), but the `bail!` macro
is shorter anyway :)

Co-authored-by: Pascal Hertleif <[email protected]>

7 files changed, 36 insertions, 29 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 97452951c..c9ab3ef10 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -9,6 +9,11 @@ dependencies = [
 ]
 
 [[package]]
+name = "anyhow"
+version = "1.0.19"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
+[[package]]
 name = "arrayvec"
 version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1823,6 +1828,7 @@ dependencies = [
 name = "xtask"
 version = "0.1.0"
 dependencies = [
+ "anyhow 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "pico-args 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "proc-macro2 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1847,6 +1853,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [metadata]
 "checksum aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d"
+"checksum anyhow 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)" = "57114fc2a6cc374bce195d3482057c846e706d252ff3604363449695684d7a0d"
 "checksum arrayvec 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cff77d8686867eceff3105329d4698d96c2391c176d5d03adc90c7389162b5b8"
 "checksum atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "1803c647a3ec87095e7ae7acfca019e98de5ec9a7d01343f611cf3152ed71a90"
 "checksum autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "1d49d90015b3c36167a20fe2810c5cd875ad504b39cff3d4eae7977e6b7c1cb2"
diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml
index 72b39c3b4..bba4be192 100644
--- a/xtask/Cargo.toml
+++ b/xtask/Cargo.toml
@@ -13,3 +13,4 @@ quote = "1.0.2"
 proc-macro2 = "1.0.1"
 ron = "0.5.1"
 serde = { version = "1.0.0", features = ["derive"] }
+anyhow = "1.0.19"
diff --git a/xtask/src/bin/pre-commit.rs b/xtask/src/bin/pre-commit.rs
index cc6ccb25e..44507fb74 100644
--- a/xtask/src/bin/pre-commit.rs
+++ b/xtask/src/bin/pre-commit.rs
@@ -19,10 +19,10 @@ fn update_staged() -> Result<()> {
         .current_dir(&root)
         .output()?;
     if !output.status.success() {
-        Err(format!(
+        anyhow::bail!(
             "`git diff --diff-filter=MAR --name-only --cached` exited with {}",
             output.status
-        ))?;
+        );
     }
     for line in String::from_utf8(output.stdout)?.lines() {
         run(&format!("git update-index --add {}", root.join(line).to_string_lossy()), ".")?;
diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs
index 770b55a9a..53f524f42 100644
--- a/xtask/src/codegen.rs
+++ b/xtask/src/codegen.rs
@@ -52,7 +52,7 @@ fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> {
         _ => (),
     }
     if mode == Mode::Verify {
-        Err(format!("`{}` is not up-to-date", path.display()))?;
+        anyhow::bail!("`{}` is not up-to-date", path.display());
     }
     eprintln!("updating {}", path.display());
     fs::write(path, contents)?;
@@ -101,10 +101,8 @@ fn do_extract_comment_blocks(text: &str, allow_blocks_with_empty_lins: bool) ->
         let is_comment = line.starts_with(prefix);
         if is_comment {
             block.push(line[prefix.len()..].to_string());
-        } else {
-            if !block.is_empty() {
-                res.push(mem::replace(&mut block, Vec::new()))
-            }
+        } else if !block.is_empty() {
+            res.push(mem::replace(&mut block, Vec::new()));
         }
     }
     if !block.is_empty() {
diff --git a/xtask/src/codegen/gen_parser_tests.rs b/xtask/src/codegen/gen_parser_tests.rs
index d0f0f683b..2977da2fa 100644
--- a/xtask/src/codegen/gen_parser_tests.rs
+++ b/xtask/src/codegen/gen_parser_tests.rs
@@ -102,10 +102,10 @@ fn tests_from_dir(dir: &Path) -> Result<Tests> {
         for test in collect_tests(&text) {
             if test.ok {
                 if let Some(old_test) = res.ok.insert(test.name.clone(), test) {
-                    return Err(format!("Duplicate test: {}", old_test.name).into());
+                    anyhow::bail!("Duplicate test: {}", old_test.name);
                 }
             } else if let Some(old_test) = res.err.insert(test.name.clone(), test) {
-                return Err(format!("Duplicate test: {}", old_test.name).into());
+                anyhow::bail!("Duplicate test: {}", old_test.name);
             }
         }
         Ok(())
diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs
index bae4c4650..bfee2f9c8 100644
--- a/xtask/src/lib.rs
+++ b/xtask/src/lib.rs
@@ -2,10 +2,10 @@
 
 pub mod codegen;
 
+use anyhow::Context;
+pub use anyhow::Result;
 use std::{
-    env,
-    error::Error,
-    fs,
+    env, fs,
     io::{Error as IoError, ErrorKind},
     path::{Path, PathBuf},
     process::{Command, Output, Stdio},
@@ -13,8 +13,6 @@ use std::{
 
 use crate::codegen::Mode;
 
-pub type Result<T> = std::result::Result<T, Box<dyn Error>>;
-
 const TOOLCHAIN: &str = "stable";
 
 pub fn project_root() -> PathBuf {
@@ -69,7 +67,7 @@ pub fn run_rustfmt(mode: Mode) -> Result<()> {
         .status()
     {
         Ok(status) if status.success() => (),
-        _ => install_rustfmt()?,
+        _ => install_rustfmt().context("install rustfmt")?,
     };
 
     if mode == Mode::Verify {
@@ -112,7 +110,7 @@ pub fn run_clippy() -> Result<()> {
         .status()
     {
         Ok(status) if status.success() => (),
-        _ => install_clippy()?,
+        _ => install_clippy().context("install clippy")?,
     };
 
     let allowed_lints = [
@@ -162,9 +160,9 @@ where
     let exec = args.next().unwrap();
     let mut cmd = Command::new(exec);
     f(cmd.args(args).current_dir(proj_dir).stderr(Stdio::inherit()));
-    let output = cmd.output()?;
+    let output = cmd.output().with_context(|| format!("running `{}`", cmdline))?;
     if !output.status.success() {
-        Err(format!("`{}` exited with {}", cmdline, output.status))?;
+        anyhow::bail!("`{}` exited with {}", cmdline, output.status);
     }
     Ok(output)
 }
diff --git a/xtask/src/main.rs b/xtask/src/main.rs
index 84842b428..7eab1c949 100644
--- a/xtask/src/main.rs
+++ b/xtask/src/main.rs
@@ -9,6 +9,7 @@
 //! `.cargo/config`.
 mod help;
 
+use anyhow::Context;
 use autocfg;
 use core::fmt::Write;
 use core::str;
@@ -114,21 +115,21 @@ fn handle_extra_flags(e: pico_args::Error) -> Result<()> {
             write!(&mut invalid_flags, "{}, ", flag)?;
         }
         let (invalid_flags, _) = invalid_flags.split_at(invalid_flags.len() - 2);
-        Err(format!("Invalid flags: {}", invalid_flags).into())
+        anyhow::bail!("Invalid flags: {}", invalid_flags)
     } else {
-        Err(e.to_string().into())
+        anyhow::bail!(e.to_string())
     }
 }
 
 fn install(opts: InstallOpt) -> Result<()> {
     if cfg!(target_os = "macos") {
-        fix_path_for_mac()?
+        fix_path_for_mac().context("Fix path for mac")?
     }
     if let Some(server) = opts.server {
-        install_server(server)?;
+        install_server(server).context("install server")?;
     }
     if let Some(client) = opts.client {
-        install_client(client)?;
+        install_client(client).context("install client")?;
     }
     Ok(())
 }
@@ -140,7 +141,7 @@ fn fix_path_for_mac() -> Result<()> {
         const ROOT_DIR: &str = "";
         let home_dir = match env::var("HOME") {
             Ok(home) => home,
-            Err(e) => Err(format!("Failed getting HOME from environment with error: {}.", e))?,
+            Err(e) => anyhow::bail!("Failed getting HOME from environment with error: {}.", e),
         };
 
         [ROOT_DIR, &home_dir]
@@ -154,12 +155,12 @@ fn fix_path_for_mac() -> Result<()> {
     if !vscode_path.is_empty() {
         let vars = match env::var_os("PATH") {
             Some(path) => path,
-            None => Err("Could not get PATH variable from env.")?,
+            None => anyhow::bail!("Could not get PATH variable from env."),
         };
 
         let mut paths = env::split_paths(&vars).collect::<Vec<_>>();
         paths.append(&mut vscode_path);
-        let new_paths = env::join_paths(paths)?;
+        let new_paths = env::join_paths(paths).context("build env PATH")?;
         env::set_var("PATH", &new_paths);
     }
 
@@ -198,7 +199,7 @@ fn install_client(ClientOpt::VsCode: ClientOpt) -> Result<()> {
 
     let code_binary = match code_binary {
         Some(it) => it,
-        None => Err("Can't execute `code --version`. Perhaps it is not in $PATH?")?,
+        None => anyhow::bail!("Can't execute `code --version`. Perhaps it is not in $PATH?"),
     };
 
     Cmd {
@@ -219,8 +220,10 @@ fn install_client(ClientOpt::VsCode: ClientOpt) -> Result<()> {
     .run_with_output()?;
 
     if !str::from_utf8(&output.stdout)?.contains("ra-lsp") {
-        Err("Could not install the Visual Studio Code extension. \
-             Please make sure you have at least NodeJS 10.x installed and try again.")?;
+        anyhow::bail!(
+            "Could not install the Visual Studio Code extension. \
+             Please make sure you have at least NodeJS 10.x installed and try again."
+        );
     }
 
     Ok(())