feat: use tokio::Command to capture suggestions

cpp-linter/Cargo.toml

@@ -28,7 +28,7 @@ reqwest = "0.12.24"
 semver = "1.0.27"
 serde = { version = "1.0.228", features = ["derive"] }
 serde_json = "1.0.145"
-tokio = { version = "1.48.0", features = ["macros", "rt-multi-thread"] }
+tokio = { version = "1.48.0", features = ["macros", "rt-multi-thread", "process"] }
 tokio-macros = "2.5.0"
 tokio-stream = "0.1.17"
 which = "8.0.0"

cpp-linter/src/clang_tools/clang_format.rs

@@ -3,13 +3,13 @@
 
 use std::{
     fs,
-    process::Command,
-    sync::{Arc, Mutex, MutexGuard},
+    sync::{Arc, Mutex},
 };
 
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
 use log::Level;
 use serde::Deserialize;
+use tokio::process::Command;
 
 // project-specific crates/modules
 use super::MakeSuggestions;
@@ -18,10 +18,10 @@ use crate::{
     common_fs::{get_line_count_from_offset, FileObj},
 };
 
-#[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>>,
@@ -81,58 +81,61 @@ pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
 }
 
 /// Run clang-tidy for a specific `file`, then parse and return it's XML output.
-pub fn run_clang_format(
-    file: &mut MutexGuard<FileObj>,
+pub async fn run_clang_format(
+    file: &Arc<Mutex<FileObj>>,
     clang_params: &ClangParams,
 ) -> Result<Vec<(log::Level, String)>> {
-    let mut cmd = Command::new(clang_params.clang_format_command.as_ref().unwrap());
     let mut logs = vec![];
-    cmd.args(["--style", &clang_params.style]);
-    let ranges = file.get_ranges(&clang_params.lines_changed_only);
-    for range in &ranges {
-        cmd.arg(format!("--lines={}:{}", range.start(), range.end()));
-    }
-    let file_name = file.name.to_string_lossy().to_string();
-    cmd.arg(file.name.to_path_buf().as_os_str());
+    let program = clang_params.clang_format_command.as_ref().unwrap();
+    let (file_name, mut args, ranges) = {
+        let mut args = vec![];
+        let file = file
+            .lock()
+            .map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
+        args.extend(["--style".to_string(), clang_params.style.clone()]);
+        let ranges = file.get_ranges(&clang_params.lines_changed_only);
+        for range in &ranges {
+            args.push(format!("--lines={}:{}", range.start(), range.end()));
+        }
+        let file_name = file.name.to_string_lossy().to_string();
+        (file_name, args, ranges)
+    };
+    let mut cmd = Command::new(program);
+    cmd.args(&args);
     let patched = if !clang_params.format_review {
         None
     } else {
         logs.push((
             Level::Info,
             format!(
-                "Getting format fixes with \"{} {}\"",
-                clang_params
-                    .clang_format_command
-                    .as_ref()
-                    .unwrap()
-                    .to_str()
-                    .unwrap_or_default(),
-                cmd.get_args()
-                    .map(|a| a.to_string_lossy())
-                    .collect::<Vec<_>>()
-                    .join(" ")
+                "Getting format fixes with \"{} {} {}\"",
+                program.to_string_lossy(),
+                args.join(" "),
+                &file_name
             ),
         ));
+        cmd.arg(&file_name);
         Some(
             cmd.output()
+                .await
                 .with_context(|| format!("Failed to get fixes from clang-format: {file_name}"))?
                 .stdout,
         )
     };
-    cmd.arg("--output-replacements-xml");
+    args.extend(["--output-replacements-xml".to_string(), file_name.clone()]);
+    let mut cmd = Command::new(program);
+    cmd.args(&args);
     logs.push((
         log::Level::Info,
         format!(
             "Running \"{} {}\"",
-            cmd.get_program().to_string_lossy(),
-            cmd.get_args()
-                .map(|x| x.to_string_lossy())
-                .collect::<Vec<_>>()
-                .join(" ")
+            program.to_string_lossy(),
+            args.join(" ")
         ),
     ));
     let output = cmd
         .output()
+        .await
         .with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?;
     if !output.stderr.is_empty() || !output.status.success() {
         logs.push((
@@ -151,14 +154,11 @@ 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() {
-        let original_contents = fs::read(&file.name).with_context(|| {
+        let original_contents = fs::read(&file_name).with_context(|| {
             format!(
                 "Failed to read file's original content before translating byte offsets: {file_name}",
             )
@@ -181,7 +181,12 @@ pub fn run_clang_format(
         }
         format_advice.replacements = filtered_replacements;
     }
-    file.format_advice = Some(format_advice);
+    {
+        let mut file = file
+            .lock()
+            .map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
+        file.format_advice = Some(format_advice);
+    }
     Ok(logs)
 }
 
@@ -196,6 +201,22 @@ 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 {
+            replacements: vec![],
+            patched: None,
+        };
+        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/clang_tidy.rs

@@ -5,19 +5,19 @@ use std::{
     env::{consts::OS, current_dir},
     fs,
     path::PathBuf,
-    process::Command,
-    sync::{Arc, Mutex, MutexGuard},
+    sync::{Arc, Mutex},
 };
 
 // non-std crates
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
 use regex::Regex;
 use serde::Deserialize;
+use tokio::process::Command;
 
 // project-specific modules/crates
 use super::MakeSuggestions;
 use crate::{
-    cli::{ClangParams, LinesChangedOnly},
+    cli::ClangParams,
     common_fs::{normalize_path, FileObj},
 };
 
@@ -249,23 +249,26 @@ pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
 }
 
 /// Run clang-tidy, then parse and return it's output.
-pub fn run_clang_tidy(
-    file: &mut MutexGuard<FileObj>,
+pub async fn run_clang_tidy(
+    file: &Arc<Mutex<FileObj>>,
     clang_params: &ClangParams,
 ) -> Result<Vec<(log::Level, std::string::String)>> {
-    let mut cmd = Command::new(clang_params.clang_tidy_command.as_ref().unwrap());
     let mut logs = vec![];
-    if !clang_params.tidy_checks.is_empty() {
-        cmd.args(["-checks", &clang_params.tidy_checks]);
-    }
-    if let Some(db) = &clang_params.database {
-        cmd.args(["-p", &db.to_string_lossy()]);
-    }
-    for arg in &clang_params.extra_args {
-        cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
-    }
-    let file_name = file.name.to_string_lossy().to_string();
-    if clang_params.lines_changed_only != LinesChangedOnly::Off {
+    let (file_name, mut args) = {
+        let mut args = vec![];
+        let file = file
+            .lock()
+            .map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
+        let file_name = file.name.to_string_lossy().to_string();
+        if !clang_params.tidy_checks.is_empty() {
+            args.extend(["-checks".to_string(), clang_params.tidy_checks.to_owned()]);
+        }
+        if let Some(db) = &clang_params.database {
+            args.extend(["-p".to_string(), db.to_string_lossy().to_string()]);
+        }
+        for arg in &clang_params.extra_args {
+            args.extend(["--extra-arg".to_string(), format!("\"{}\"", arg)]);
+        }
         let ranges = file.get_ranges(&clang_params.lines_changed_only);
         if !ranges.is_empty() {
             let filter = format!(
@@ -276,36 +279,41 @@ pub fn run_clang_tidy(
                     .map(|r| [r.start(), r.end()])
                     .collect::<Vec<_>>()
             );
-            cmd.args(["--line-filter", filter.as_str()]);
+            args.extend(["--line-filter".to_string(), filter]);
         }
-    }
+        (file_name, args)
+    };
     let original_content = if !clang_params.tidy_review {
         None
     } else {
-        cmd.arg("--fix-errors");
-        Some(fs::read_to_string(&file.name).with_context(|| {
+        args.push("--fix-errors".to_string());
+        Some(fs::read_to_string(&file_name).with_context(|| {
             format!(
                 "Failed to cache file's original content before applying clang-tidy changes: {}",
                 file_name.clone()
             )
         })?)
     };
     if !clang_params.style.is_empty() {
-        cmd.args(["--format-style", clang_params.style.as_str()]);
+        args.extend(["--format-style".to_string(), clang_params.style.to_owned()]);
     }
-    cmd.arg(file.name.to_string_lossy().as_ref());
+    args.push(file_name.clone());
+    let program = clang_params.clang_tidy_command.as_ref().unwrap();
+    let mut cmd = Command::new(program);
+    cmd.args(&args);
     logs.push((
         log::Level::Info,
         format!(
             "Running \"{} {}\"",
-            cmd.get_program().to_string_lossy(),
-            cmd.get_args()
-                .map(|x| x.to_string_lossy())
-                .collect::<Vec<_>>()
-                .join(" ")
+            program.to_string_lossy(),
+            args.join(" ")
         ),
     ));
-    let output = cmd.output().unwrap();
+    // ok to unwrap()
+    let output = cmd
+        .output()
+        .await
+        .with_context(|| format!("Failed to run clang-tidy on file: {}", file_name.clone()))?;
     logs.push((
         log::Level::Debug,
         format!(
@@ -322,22 +330,23 @@ pub fn run_clang_tidy(
             ),
         ));
     }
-    file.tidy_advice = Some(parse_tidy_output(
-        &output.stdout,
-        &clang_params.database_json,
-    )?);
+    let mut tidy_advice = parse_tidy_output(&output.stdout, &clang_params.database_json)?;
     if clang_params.tidy_review {
-        if let Some(tidy_advice) = &mut file.tidy_advice {
-            // cache file changes in a buffer and restore the original contents for further analysis
-            tidy_advice.patched =
-                Some(fs::read(&file_name).with_context(|| {
-                    format!("Failed to read changes from clang-tidy: {file_name}")
-                })?);
-        }
+        // cache file changes in a buffer and restore the original contents for further analysis
+        tidy_advice.patched = Some(
+            fs::read(&file_name)
+                .with_context(|| format!("Failed to read changes from clang-tidy: {file_name}"))?,
+        );
         // original_content is guaranteed to be Some() value at this point
         fs::write(&file_name, original_content.unwrap())
             .with_context(|| format!("Failed to restore file's original content: {file_name}"))?;
     }
+    {
+        let mut file = file
+            .lock()
+            .map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
+        file.tidy_advice = Some(tidy_advice);
+    }
     Ok(logs)
 }
 
@@ -420,8 +429,8 @@ mod test {
         )
     }
 
-    #[test]
-    fn use_extra_args() {
+    #[tokio::test]
+    async fn use_extra_args() {
         let exe_path = ClangTool::ClangTidy
             .get_exe_path(
                 &RequestedVersion::from_str(
@@ -431,7 +440,7 @@ mod test {
             )
             .unwrap();
         let file = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
-        let arc_ref = Arc::new(Mutex::new(file));
+        let arc_file = Arc::new(Mutex::new(file));
         let extra_args = vec!["-std=c++17".to_string(), "-Wall".to_string()];
         let clang_params = ClangParams {
             style: "".to_string(),
@@ -447,8 +456,8 @@ mod test {
             clang_tidy_command: Some(exe_path),
             clang_format_command: None,
         };
-        let mut file_lock = arc_ref.lock().unwrap();
-        let logs = run_clang_tidy(&mut file_lock, &clang_params)
+        let logs = run_clang_tidy(&arc_file, &clang_params)
+            .await
             .unwrap()
             .into_iter()
             .filter_map(|(_lvl, msg)| {