fix: properly parse xml with no replacements

cpp-linter/src/clang_tools/clang_format.rs

@@ -18,10 +18,10 @@ use crate::{
     common_fs::{FileObj, get_line_count_from_offset},
 };
 
-#[derive(Debug, Clone, Deserialize, PartialEq, Eq)]
+#[derive(Debug, Clone, Deserialize, PartialEq, Eq, Default)]
 pub struct FormatAdvice {
     /// A list of [`Replacement`]s that clang-tidy wants to make.
-    #[serde(rename(deserialize = "replacement"))]
+    #[serde(rename(deserialize = "replacement"), default)]
     pub replacements: Vec<Replacement>,
 
     pub patched: Option<Vec<u8>>,
@@ -151,10 +151,7 @@ pub fn run_clang_format(
             format!("Failed to parse XML output from clang-format for {file_name}")
         })?
     } else {
-        FormatAdvice {
-            replacements: vec![],
-            patched: None,
-        }
+        FormatAdvice::default()
     };
     format_advice.patched = patched;
     if !format_advice.replacements.is_empty() {
@@ -196,6 +193,19 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn parse_xml_no_replacements() {
+        let xml_raw = r#"<?xml version='1.0'?>
+<replacements xml:space='preserve' incomplete_format='false'>
+</replacements>"#
+            .as_bytes()
+            .to_vec();
+        let expected = FormatAdvice::default();
+        let xml = String::from_utf8(xml_raw).unwrap();
+        let document = quick_xml::de::from_str::<FormatAdvice>(&xml).unwrap();
+        assert_eq!(expected, document);
+    }
+
     #[test]
     fn parse_xml() {
         let xml_raw = r#"<?xml version='1.0'?>

cpp-linter/src/clang_tools/mod.rs

@@ -248,14 +248,15 @@ pub async fn capture_clang_tools_output(
     }
 
     while let Some(output) = executors.join_next().await {
-        if let Ok(out) = output? {
-            let (file_name, logs) = out;
-            rest_api_client.start_log_group(format!("Analyzing {}", file_name.to_string_lossy()));
-            for (level, msg) in logs {
-                log::log!(level, "{}", msg);
-            }
-            rest_api_client.end_log_group();
+        // output?? acts as a fast-fail for any error encountered.
+        // This includes any `spawn()` error and any `analyze_single_file()` error.
+        // Any unresolved tasks are aborted and dropped when an error is returned here.
+        let (file_name, logs) = output??;
+        rest_api_client.start_log_group(format!("Analyzing {}", file_name.to_string_lossy()));
+        for (level, msg) in logs {
+            log::log!(level, "{}", msg);
         }
+        rest_api_client.end_log_group();
     }
     Ok(clang_versions)
 }

cpp-linter/src/run.rs

@@ -165,56 +165,57 @@ mod test {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "-l".to_string(),
             "false".to_string(),
             "--repo-root".to_string(),
             "tests".to_string(),
             "demo/demo.cpp".to_string(),
         ])
-        .await;
-        assert!(result.is_ok());
+        .await
+        .unwrap();
     }
 
     #[tokio::test]
     async fn version_command() {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec!["cpp-linter".to_string(), "version".to_string()]).await;
-        assert!(result.is_ok());
+        run_main(vec!["cpp-linter".to_string(), "version".to_string()])
+            .await
+            .unwrap();
     }
 
     #[tokio::test]
     async fn force_debug_output() {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "-l".to_string(),
             "false".to_string(),
             "-v".to_string(),
             "-i=target|benches/libgit2".to_string(),
         ])
-        .await;
-        assert!(result.is_ok());
+        .await
+        .unwrap();
     }
 
     #[tokio::test]
     async fn no_version_input() {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "-l".to_string(),
             "false".to_string(),
             "-V".to_string(),
         ])
-        .await;
-        assert!(result.is_ok());
+        .await
+        .unwrap();
     }
 
     #[tokio::test]
@@ -223,14 +224,14 @@ mod test {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
             env::set_var("PRE_COMMIT", "1");
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "--lines-changed-only".to_string(),
             "false".to_string(),
             "--ignore=target|benches/libgit2".to_string(),
         ])
-        .await;
-        assert!(result.is_err());
+        .await
+        .unwrap_err();
     }
 
     // Verifies that the system gracefully handles cases where all analysis is disabled.
@@ -240,29 +241,29 @@ mod test {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "-l".to_string(),
             "false".to_string(),
             "--style".to_string(),
             String::new(),
             "--tidy-checks=-*".to_string(),
         ])
-        .await;
-        assert!(result.is_ok());
+        .await
+        .unwrap();
     }
 
     #[tokio::test]
     async fn bad_repo_root() {
         unsafe {
             env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         }
-        let result = run_main(vec![
+        run_main(vec![
             "cpp-linter".to_string(),
             "--repo-root".to_string(),
             "some-non-existent-dir".to_string(),
         ])
-        .await;
-        assert!(result.is_err());
+        .await
+        .unwrap_err();
     }
 }

cpp-linter/tests/reviews.rs

@@ -211,17 +211,19 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
         );
     }
 
-    let mut tool_ignore = "**/*.c".to_string();
+    let mut format_ignore = "**/*.c".to_string();
+    let mut tidy_ignore = format_ignore.clone();
     if test_params.force_lgtm {
-        tool_ignore.push_str("|**/*.cpp|**/*.h");
+        format_ignore.push_str("|**/*.cpp|**/*.h");
+        tidy_ignore.push_str("|**/*.hpp");
     }
     let mut args = vec![
         "cpp-linter".to_string(),
         "-v=debug".to_string(),
         format!("-V={}", clang_version),
         format!("-l={}", test_params.lines_changed_only),
-        format!("--ignore-tidy={}", tool_ignore),
-        format!("--ignore-format={}", tool_ignore),
+        format!("--ignore-tidy={}", tidy_ignore),
+        format!("--ignore-format={}", format_ignore),
         format!("--tidy-review={}", test_params.tidy_review),
         format!("--format-review={}", test_params.format_review),
         format!("--passive-reviews={}", test_params.passive_reviews),