refactor: reach block-bodied-closure error messages, and clarify the fix hint

crates/agent/src/agent_platform.rs

@@ -83,7 +83,7 @@ impl ManagedFile {
     pub fn ensure_contents(&mut self, contents: &[u8]) -> eyre::Result<bool> {
         let destination_exists = self.path.try_exists().wrap_err_with(|| {
             format!(
-                "Couldn't check existence of destination file {f}",
+                "couldn't check existence of destination file {f}",
                 f = self.path.display()
             )
         })?;
@@ -93,7 +93,7 @@ impl ManagedFile {
             true => {
                 let current_contents = std::fs::read(self.path.as_path()).wrap_err_with(|| {
                     format!(
-                        "Couldn't read current file contents of {f}",
+                        "couldn't read current file contents of {f}",
                         f = self.path.display()
                     )
                 })?;
@@ -124,26 +124,26 @@ fn safe_copy(destination_path: &Path, source_path: &Path) -> eyre::Result<()> {
     let mut tmp_destination = NamedTempFile::with_suffix_in(".tmp", destination_dirname)
         .with_context(|| {
             format!(
-                "Couldn't create temporary file in destination directory {d}",
+                "couldn't create temporary file in destination directory {d}",
                 d = destination_dirname.display()
             )
         })?;
     std::io::copy(&mut source_file, &mut tmp_destination).with_context(|| {
         format!(
-            "Couldn't copy file contents from {s} to {d}",
+            "couldn't copy file contents from {s} to {d}",
             s = source_path.display(),
             d = tmp_destination.path().display()
         )
     })?;
     let destination_file = tmp_destination.persist(destination_path).with_context(|| {
         format!(
-            "Couldn't persist contents to destination file {d}",
+            "couldn't persist contents to destination file {d}",
             d = destination_path.display()
         )
     })?;
     destination_file.sync_all().with_context(|| {
         format!(
-            "Couldn't sync file data of destination file {d}",
+            "couldn't sync file data of destination file {d}",
             d = destination_path.display()
         )
     })?;
@@ -167,25 +167,25 @@ fn safe_write(destination_path: &Path, contents: &[u8]) -> eyre::Result<()> {
         .tempfile_in(destination_dirname)
         .with_context(|| {
             format!(
-                "Couldn't create temporary file in destination directory {d}",
+                "couldn't create temporary file in destination directory {d}",
                 d = destination_dirname.display()
             )
         })?;
     tmp_destination.write_all(contents).with_context(|| {
         format!(
-            "Couldn't write file contents to {d}",
+            "couldn't write file contents to {d}",
             d = destination_path.display()
         )
     })?;
     let destination_file = tmp_destination.persist(destination_path).with_context(|| {
         format!(
-            "Couldn't persist contents to destination file {d}",
+            "couldn't persist contents to destination file {d}",
             d = destination_path.display()
         )
     })?;
     destination_file.sync_all().with_context(|| {
         format!(
-            "Couldn't sync file data of destination file {d}",
+            "couldn't sync file data of destination file {d}",
             d = destination_path.display()
         )
     })?;

crates/agent/src/extension_services/k8s_pod_handler.rs

@@ -287,7 +287,7 @@ impl KubernetesPodServicesHandler {
             .await
             .wrap_err_with(|| {
                 format!(
-                    "Failed to rename {} to {}",
+                    "failed to rename {} to {}",
                     tmp_path.display(),
                     pod_spec_path.display()
                 )
@@ -326,7 +326,7 @@ impl KubernetesPodServicesHandler {
             }
             Err(e) => {
                 return Err(e).wrap_err_with(|| {
-                    format!("Failed to remove pod spec {}", pod_spec_path.display())
+                    format!("failed to remove pod spec {}", pod_spec_path.display())
                 });
             }
         }
@@ -1015,13 +1015,13 @@ Environment="NO_PROXY=127.0.0.1,localhost,.svc,.svc.cluster.local"
         let kubelet_dir = Path::new(KUBERNETES_POD_DIR);
         std::fs::create_dir_all(kubelet_dir).wrap_err_with(|| {
             format!(
-                "Failed to create kubelet directory at {}",
+                "failed to create kubelet directory at {}",
                 kubelet_dir.display()
             )
         })?;
 
         let dir_iter = std::fs::read_dir(kubelet_dir).wrap_err_with(|| {
-            format!("Failed to read kubelet directory {}", kubelet_dir.display())
+            format!("failed to read kubelet directory {}", kubelet_dir.display())
         })?;
 
         for entry in dir_iter {

crates/agent/src/hbn.rs

@@ -98,7 +98,7 @@ pub async fn run_in_container_shell(cmd: &str) -> Result<(), eyre::Report> {
     run_in_container(&container_id, &["bash", "-c", cmd], check_result)
         .await
         .wrap_err_with(|| {
-            format!("Failed executing '{cmd}' in container. Check logs in /var/log/doca/hbn/")
+            format!("failed executing '{cmd}' in container. check logs in /var/log/doca/hbn/")
         })?;
     Ok(())
 }

crates/machine-controller/src/scout_firmware_scripts.rs

@@ -116,7 +116,7 @@ fn find_scout_script_in(
 
     let script = fs::read(&script_path).wrap_err_with(|| {
         format!(
-            "failed to read Scout firmware script {}",
+            "failed to read scout firmware script {}",
             script_path.display()
         )
     })?;
@@ -249,14 +249,14 @@ fn script_url(pxe_public_base_url: &str, relative_path: &str) -> String {
 fn read_metadata(path: &Path) -> eyre::Result<ScoutFirmwareScriptMetadata> {
     let metadata = fs::read_to_string(path).wrap_err_with(|| {
         format!(
-            "failed to read Scout firmware script metadata {}",
+            "failed to read scout firmware script metadata {}",
             path.display()
         )
     })?;
 
     toml::from_str(&metadata).wrap_err_with(|| {
         format!(
-            "failed to parse Scout firmware script metadata {}",
+            "failed to parse scout firmware script metadata {}",
             path.display()
         )
     })
@@ -461,7 +461,7 @@ mod tests {
         assert!(
             error
                 .to_string()
-                .contains("failed to parse Scout firmware script metadata")
+                .contains("failed to parse scout firmware script metadata")
         );
     }
 }

crates/xtask/src/error_message_case.rs

@@ -69,13 +69,13 @@ const CONV_METHODS: &[&str] = &["into", "to_string", "to_owned", "into_owned"];
 const FORMAT_MACROS: &[&str] = &["format", "format_args"];
 
 // TODO: `format!(...)` is now peeled when it's the message argument of a
-// recognized error slot (see `leading_str_lit`), but a few forms remain out of
-// reach: a `format!` behind a *block*-bodied closure (`.wrap_err_with(|| {
-// format!(...) })`), struct-literal error fields (`CarbideError::Internal {
-// message: "..." }`), other error enums' constructors, a manual `impl Display`
-// (`write!`), and an error site nested inside another macro's body (a `bail!`
-// inside `tokio::select!`, which `syn` keeps as an opaque token stream). Those
-// are left as-is; widening the checker further, and re-sweeping, is a follow-up.
+// recognized error slot, through expression- or block-bodied closures (see
+// `leading_str_lit`). A few forms still remain out of reach: struct-literal
+// error fields (`CarbideError::Internal { message: "..." }`), other error enums'
+// constructors, a manual `impl Display` (`write!`), and an error site nested
+// inside another macro's body (a `bail!` inside `tokio::select!`, which `syn`
+// keeps as an opaque token stream). Those are left as-is; widening the checker
+// further, and re-sweeping what it then catches, is a follow-up.
 
 pub fn check(fix: bool) -> eyre::Result<CheckOutcome> {
     let repo_root = PathBuf::from(REPO_ROOT).canonicalize()?;
@@ -251,10 +251,18 @@ impl CheckOutcome {
                 Problem::Capitalized => (c + 1, p),
                 Problem::TrailingPeriod => (c, p + 1),
             });
+        let total = self.violations.len();
         println!(
-            "{} error-message style violations ({cap} capitalized, {period} trailing period). \
-             Run `cargo xtask lint-error-messages --fix` to fix.",
-            self.violations.len(),
+            "\n{total} error-message style violation(s) ({cap} capitalized, {period} trailing period)."
+        );
+        println!(
+            "Error messages should be a lowercase phrase with no trailing period (C-GOOD-ERR)."
+        );
+        println!("\nTo fix these automatically, run:\n");
+        println!("    cargo xtask lint-error-messages --fix");
+        println!(
+            "\nA deliberate capital (an acronym or a case-sensitive token) can be kept with a \
+             `// xtask:allow-error-case` comment on that line. See STYLE_GUIDE.md."
         );
         std::process::exit(1);
     }
@@ -408,9 +416,10 @@ fn macro_message(mac: &Macro) -> Option<LitStr> {
     leading_str_lit(args.iter().nth(index)?)
 }
 
-/// Peels closures, references, trivial conversions, and a wrapping `format!` to
-/// find a leading string literal — the message inside `|| "...".into()`, `&"..."`,
-/// `"...".to_string()`, or `CarbideError::invalid_argument(format!("...", x))`.
+/// Peels closures (expression- or block-bodied), references, trivial conversions,
+/// and a wrapping `format!` to find a leading string literal — the message inside
+/// `|| "...".into()`, `&"..."`, `"...".to_string()`, `|| { format!("...") }`, or
+/// `CarbideError::invalid_argument(format!("...", x))`.
 /// Returns an owned `LitStr` (it may be parsed out of a `format!` body), but its
 /// span still points at the real source literal, so a fix rewrites the right bytes.
 fn leading_str_lit(expr: &Expr) -> Option<LitStr> {
@@ -439,6 +448,13 @@ fn leading_str_lit(expr: &Expr) -> Option<LitStr> {
             leading_str_lit(args.first()?)
         }
         Expr::Closure(closure) => leading_str_lit(&closure.body),
+        // A block-bodied closure (or a bare block) forwards to its tail
+        // expression -- the block's value -- so `.wrap_err_with(|| { format!(...) })`
+        // is reached just like the expression-bodied `|| format!(...)`.
+        Expr::Block(block) => block.block.stmts.last().and_then(|tail| match tail {
+            syn::Stmt::Expr(e, None) => leading_str_lit(e),
+            _ => None,
+        }),
         Expr::Reference(reference) => leading_str_lit(&reference.expr),
         Expr::Paren(paren) => leading_str_lit(&paren.expr),
         Expr::Group(group) => leading_str_lit(&group.expr),
@@ -782,4 +798,31 @@ mod tests {
         assert!(fixed.contains(r#""Bare log line stays""#), "{fixed}");
         assert!(fixed.contains(r#""Plain format stays""#), "{fixed}");
     }
+
+    /// A `format!` inside a *block*-bodied closure is reached too -- the common
+    /// `.wrap_err_with(|| { format!(...) })` form, not just `|| format!(...)`.
+    #[test]
+    fn end_to_end_rewrite_reaches_block_closure() {
+        let src = concat!(
+            "fn f() -> anyhow::Result<()> {\n",
+            "    thing().wrap_err_with(|| {\n",
+            "        format!(\"Couldn't reach {host}\")\n",
+            "    })?;\n",
+            "    Ok(())\n",
+            "}\n",
+        );
+        let ast = syn::parse_file(src).unwrap();
+        let mut findings = Vec::new();
+        let mut collector = Collector {
+            lines: src.lines().collect(),
+            out: &mut findings,
+        };
+        collector.visit_file(&ast);
+        let (fixed, applied) = apply_fixes(src, &findings);
+        assert_eq!(applied, 1, "{fixed}");
+        assert!(
+            fixed.contains(r#"format!("couldn't reach {host}")"#),
+            "{fixed}"
+        );
+    }
 }

crates/xtask/src/workspace_deps.rs

@@ -185,7 +185,7 @@ impl Workspace {
                     } else if let Some(version) = dep.as_str() {
                         let version = Version::from(version).with_context(|| {
                             format!(
-                                "Error parsing version for dependency `{}` in {}",
+                                "error parsing version for dependency `{}` in {}",
                                 dep_name,
                                 toml_path.display()
                             )