Skip to content

Commit f4d0189

Browse files
committed
feat: use tokio::process::Command
addresses #192 This coverts functions that run and capture clang tools' suggestions from blocking to async API. update uv.lock file * remove duplicated condition; ranges is empty if lines_changed_only is Off
1 parent 6e6e43d commit f4d0189

File tree

6 files changed

+180
-137
lines changed

6 files changed

+180
-137
lines changed

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cpp-linter/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ reqwest = "0.12.24"
2828
semver = "1.0.27"
2929
serde = { version = "1.0.228", features = ["derive"] }
3030
serde_json = "1.0.145"
31-
tokio = { version = "1.48.0", features = ["macros", "rt-multi-thread"] }
31+
tokio = { version = "1.48.0", features = ["macros", "rt-multi-thread", "process"] }
3232
tokio-macros = "2.5.0"
3333
tokio-stream = "0.1.17"
3434
which = "8.0.0"

cpp-linter/src/clang_tools/clang_format.rs

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
44
use std::{
55
fs,
6-
process::Command,
7-
sync::{Arc, Mutex, MutexGuard},
6+
sync::{Arc, Mutex},
87
};
98

10-
use anyhow::{Context, Result};
9+
use anyhow::{Context, Result, anyhow};
1110
use log::Level;
1211
use serde::Deserialize;
12+
use tokio::process::Command;
1313

1414
// project-specific crates/modules
1515
use super::MakeSuggestions;
@@ -81,58 +81,61 @@ pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
8181
}
8282

8383
/// Run clang-tidy for a specific `file`, then parse and return it's XML output.
84-
pub fn run_clang_format(
85-
file: &mut MutexGuard<FileObj>,
84+
pub async fn run_clang_format(
85+
file: &Arc<Mutex<FileObj>>,
8686
clang_params: &ClangParams,
8787
) -> Result<Vec<(log::Level, String)>> {
88-
let mut cmd = Command::new(clang_params.clang_format_command.as_ref().unwrap());
8988
let mut logs = vec![];
90-
cmd.args(["--style", &clang_params.style]);
91-
let ranges = file.get_ranges(&clang_params.lines_changed_only);
92-
for range in &ranges {
93-
cmd.arg(format!("--lines={}:{}", range.start(), range.end()));
94-
}
95-
let file_name = file.name.to_string_lossy().to_string();
96-
cmd.arg(file.name.to_path_buf().as_os_str());
89+
let program = clang_params.clang_format_command.as_ref().unwrap();
90+
let (file_name, mut args, ranges) = {
91+
let mut args = vec![];
92+
let file = file
93+
.lock()
94+
.map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
95+
args.extend(["--style".to_string(), clang_params.style.clone()]);
96+
let ranges = file.get_ranges(&clang_params.lines_changed_only);
97+
for range in &ranges {
98+
args.push(format!("--lines={}:{}", range.start(), range.end()));
99+
}
100+
let file_name = file.name.to_string_lossy().to_string();
101+
(file_name, args, ranges)
102+
};
103+
let mut cmd = Command::new(program);
104+
cmd.args(&args);
97105
let patched = if !clang_params.format_review {
98106
None
99107
} else {
100108
logs.push((
101109
Level::Info,
102110
format!(
103-
"Getting format fixes with \"{} {}\"",
104-
clang_params
105-
.clang_format_command
106-
.as_ref()
107-
.unwrap()
108-
.to_str()
109-
.unwrap_or_default(),
110-
cmd.get_args()
111-
.map(|a| a.to_string_lossy())
112-
.collect::<Vec<_>>()
113-
.join(" ")
111+
"Getting format fixes with \"{} {} {}\"",
112+
program.to_string_lossy(),
113+
args.join(" "),
114+
&file_name
114115
),
115116
));
117+
cmd.arg(&file_name);
116118
Some(
117119
cmd.output()
120+
.await
118121
.with_context(|| format!("Failed to get fixes from clang-format: {file_name}"))?
119122
.stdout,
120123
)
121124
};
122-
cmd.arg("--output-replacements-xml");
125+
args.extend(["--output-replacements-xml".to_string(), file_name.clone()]);
126+
let mut cmd = Command::new(program);
127+
cmd.args(&args);
123128
logs.push((
124129
log::Level::Info,
125130
format!(
126131
"Running \"{} {}\"",
127-
cmd.get_program().to_string_lossy(),
128-
cmd.get_args()
129-
.map(|x| x.to_string_lossy())
130-
.collect::<Vec<_>>()
131-
.join(" ")
132+
program.to_string_lossy(),
133+
args.join(" ")
132134
),
133135
));
134136
let output = cmd
135137
.output()
138+
.await
136139
.with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?;
137140
if !output.stderr.is_empty() || !output.status.success() {
138141
logs.push((
@@ -155,7 +158,7 @@ pub fn run_clang_format(
155158
};
156159
format_advice.patched = patched;
157160
if !format_advice.replacements.is_empty() {
158-
let original_contents = fs::read(&file.name).with_context(|| {
161+
let original_contents = fs::read(&file_name).with_context(|| {
159162
format!(
160163
"Failed to read file's original content before translating byte offsets: {file_name}",
161164
)
@@ -178,7 +181,12 @@ pub fn run_clang_format(
178181
}
179182
format_advice.replacements = filtered_replacements;
180183
}
181-
file.format_advice = Some(format_advice);
184+
{
185+
let mut file = file
186+
.lock()
187+
.map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
188+
file.format_advice = Some(format_advice);
189+
}
182190
Ok(logs)
183191
}
184192

cpp-linter/src/clang_tools/clang_tidy.rs

Lines changed: 63 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ use std::{
55
env::{consts::OS, current_dir},
66
fs,
77
path::PathBuf,
8-
process::Command,
9-
sync::{Arc, Mutex, MutexGuard},
8+
sync::{Arc, Mutex},
109
};
1110

1211
// non-std crates
13-
use anyhow::{Context, Result};
12+
use anyhow::{Context, Result, anyhow};
1413
use regex::Regex;
1514
use serde::Deserialize;
15+
use tokio::process::Command;
1616

1717
// project-specific modules/crates
1818
use super::MakeSuggestions;
@@ -247,61 +247,70 @@ pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
247247
}
248248

249249
/// Run clang-tidy, then parse and return it's output.
250-
pub fn run_clang_tidy(
251-
file: &mut MutexGuard<FileObj>,
250+
pub async fn run_clang_tidy(
251+
file: &Arc<Mutex<FileObj>>,
252252
clang_params: &ClangParams,
253253
) -> Result<Vec<(log::Level, std::string::String)>> {
254-
let mut cmd = Command::new(clang_params.clang_tidy_command.as_ref().unwrap());
255254
let mut logs = vec![];
256-
if !clang_params.tidy_checks.is_empty() {
257-
cmd.args(["-checks", &clang_params.tidy_checks]);
258-
}
259-
if let Some(db) = &clang_params.database {
260-
cmd.args(["-p", &db.to_string_lossy()]);
261-
}
262-
for arg in &clang_params.extra_args {
263-
cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
264-
}
265-
let file_name = file.name.to_string_lossy().to_string();
266-
let ranges = file.get_ranges(&clang_params.lines_changed_only);
267-
if !ranges.is_empty() {
268-
let filter = format!(
269-
"[{{\"name\":{:?},\"lines\":{:?}}}]",
270-
&file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
271-
ranges
272-
.iter()
273-
.map(|r| [r.start(), r.end()])
274-
.collect::<Vec<_>>()
275-
);
276-
cmd.args(["--line-filter", filter.as_str()]);
277-
}
255+
let (file_name, mut args) = {
256+
let mut args = vec![];
257+
let file = file
258+
.lock()
259+
.map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
260+
let file_name = file.name.to_string_lossy().to_string();
261+
if !clang_params.tidy_checks.is_empty() {
262+
args.extend(["-checks".to_string(), clang_params.tidy_checks.to_owned()]);
263+
}
264+
if let Some(db) = &clang_params.database {
265+
args.extend(["-p".to_string(), db.to_string_lossy().to_string()]);
266+
}
267+
for arg in &clang_params.extra_args {
268+
args.extend(["--extra-arg".to_string(), format!("\"{}\"", arg)]);
269+
}
270+
let ranges = file.get_ranges(&clang_params.lines_changed_only);
271+
if !ranges.is_empty() {
272+
let filter = format!(
273+
"[{{\"name\":{:?},\"lines\":{:?}}}]",
274+
&file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
275+
ranges
276+
.iter()
277+
.map(|r| [r.start(), r.end()])
278+
.collect::<Vec<_>>()
279+
);
280+
args.extend(["--line-filter".to_string(), filter]);
281+
}
282+
(file_name, args)
283+
};
278284
let original_content = if !clang_params.tidy_review {
279285
None
280286
} else {
281-
cmd.arg("--fix-errors");
282-
Some(fs::read_to_string(&file.name).with_context(|| {
287+
args.push("--fix-errors".to_string());
288+
Some(fs::read_to_string(&file_name).with_context(|| {
283289
format!(
284290
"Failed to cache file's original content before applying clang-tidy changes: {}",
285291
file_name.clone()
286292
)
287293
})?)
288294
};
289295
if !clang_params.style.is_empty() {
290-
cmd.args(["--format-style", clang_params.style.as_str()]);
296+
args.extend(["--format-style".to_string(), clang_params.style.to_owned()]);
291297
}
292-
cmd.arg(file.name.to_string_lossy().as_ref());
298+
args.push(file_name.clone());
299+
let program = clang_params.clang_tidy_command.as_ref().unwrap();
300+
let mut cmd = Command::new(program);
301+
cmd.args(&args);
293302
logs.push((
294303
log::Level::Info,
295304
format!(
296305
"Running \"{} {}\"",
297-
cmd.get_program().to_string_lossy(),
298-
cmd.get_args()
299-
.map(|x| x.to_string_lossy())
300-
.collect::<Vec<_>>()
301-
.join(" ")
306+
program.to_string_lossy(),
307+
args.join(" ")
302308
),
303309
));
304-
let output = cmd.output().unwrap();
310+
let output = cmd
311+
.output()
312+
.await
313+
.with_context(|| format!("Failed to run clang-tidy on file: {}", file_name.clone()))?;
305314
logs.push((
306315
log::Level::Debug,
307316
format!(
@@ -318,22 +327,23 @@ pub fn run_clang_tidy(
318327
),
319328
));
320329
}
321-
file.tidy_advice = Some(parse_tidy_output(
322-
&output.stdout,
323-
&clang_params.database_json,
324-
)?);
330+
let mut tidy_advice = parse_tidy_output(&output.stdout, &clang_params.database_json)?;
325331
if clang_params.tidy_review {
326-
if let Some(tidy_advice) = &mut file.tidy_advice {
327-
// cache file changes in a buffer and restore the original contents for further analysis
328-
tidy_advice.patched =
329-
Some(fs::read(&file_name).with_context(|| {
330-
format!("Failed to read changes from clang-tidy: {file_name}")
331-
})?);
332-
}
332+
// cache file changes in a buffer and restore the original contents for further analysis
333+
tidy_advice.patched = Some(
334+
fs::read(&file_name)
335+
.with_context(|| format!("Failed to read changes from clang-tidy: {file_name}"))?,
336+
);
333337
// original_content is guaranteed to be Some() value at this point
334338
fs::write(&file_name, original_content.unwrap())
335339
.with_context(|| format!("Failed to restore file's original content: {file_name}"))?;
336340
}
341+
{
342+
let mut file = file
343+
.lock()
344+
.map_err(|e| anyhow!("Failed to lock mutex: {e:?}"))?;
345+
file.tidy_advice = Some(tidy_advice);
346+
}
337347
Ok(logs)
338348
}
339349

@@ -416,8 +426,8 @@ mod test {
416426
)
417427
}
418428

419-
#[test]
420-
fn use_extra_args() {
429+
#[tokio::test]
430+
async fn use_extra_args() {
421431
let exe_path = ClangTool::ClangTidy
422432
.get_exe_path(
423433
&RequestedVersion::from_str(
@@ -443,8 +453,8 @@ mod test {
443453
clang_tidy_command: Some(exe_path),
444454
clang_format_command: None,
445455
};
446-
let mut file_lock = arc_file.lock().unwrap();
447-
let logs = run_clang_tidy(&mut file_lock, &clang_params)
456+
let logs = run_clang_tidy(&arc_file, &clang_params)
457+
.await
448458
.unwrap()
449459
.into_iter()
450460
.filter_map(|(_lvl, msg)| {

0 commit comments

Comments
 (0)