Skip to content

Commit cdc871a

Browse files
authored
feat: use concrete error types (#343)
when capturing clang tools' output. I also exported some error types from clang-tools-manager crate.
1 parent 1529d72 commit cdc871a

6 files changed

Lines changed: 136 additions & 61 deletions

File tree

clang-tools-manager/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub use tool::ClangTool;
1919
pub mod utils;
2020

2121
mod version;
22-
pub use version::RequestedVersion;
22+
pub use version::{ClangVersion, GetToolError, RequestedVersion, RequestedVersionParsingError};
2323

2424
mod progress_bar;
2525
pub use progress_bar::ProgressBar;

cpp-linter/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ license.workspace = true
1414
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
1515

1616
[dependencies]
17-
anyhow = { workspace = true }
17+
anyhow = { workspace = true, optional = true }
1818
async-trait = "0.1.89"
1919
chrono = "0.4.44"
2020
clang-tools-manager = { path = "../clang-tools-manager", version = "0.2.0" }
@@ -46,7 +46,7 @@ tempfile = { workspace = true }
4646
reqwest = { workspace = true, features = ["default-tls"] }
4747

4848
[features]
49-
bin = ["reqwest/default-tls", "dep:clap", "dep:colored"]
49+
bin = ["reqwest/default-tls", "dep:clap", "dep:colored", "dep:anyhow"]
5050

5151
[lib]
5252
bench = false

cpp-linter/src/clang_tools/clang_format.rs

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use std::{
77
sync::{Arc, Mutex, MutexGuard},
88
};
99

10-
use anyhow::{Context, Result, anyhow};
1110
use log::Level;
1211
use serde::Deserialize;
1312

@@ -16,6 +15,7 @@ use super::MakeSuggestions;
1615
use crate::{
1716
cli::ClangParams,
1817
common_fs::{FileObj, get_line_count_from_offset},
18+
error::ClangCaptureError,
1919
};
2020

2121
#[derive(Debug, Clone, Deserialize, PartialEq, Eq, Default)]
@@ -85,11 +85,11 @@ pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String>
8585
pub fn run_clang_format(
8686
file: &mut MutexGuard<FileObj>,
8787
clang_params: &ClangParams,
88-
) -> Result<Vec<(log::Level, String)>> {
88+
) -> Result<Vec<(log::Level, String)>, ClangCaptureError> {
8989
let cmd_path = clang_params
9090
.clang_format_command
9191
.as_ref()
92-
.ok_or(anyhow!("clang-format path unknown"))?;
92+
.ok_or(ClangCaptureError::ToolPathUnknown("clang-format"))?;
9393
let mut cmd = Command::new(cmd_path);
9494
let mut logs = vec![];
9595
cmd.args(["--style", &clang_params.style]);
@@ -115,7 +115,10 @@ pub fn run_clang_format(
115115
));
116116
Some(
117117
cmd.output()
118-
.with_context(|| format!("Failed to get fixes from clang-format: {file_name}"))?
118+
.map_err(|e| ClangCaptureError::FailedToRunCommand {
119+
task: format!("get fixes from clang-format {file_name}"),
120+
source: e,
121+
})?
119122
.stdout,
120123
)
121124
};
@@ -133,7 +136,10 @@ pub fn run_clang_format(
133136
));
134137
let output = cmd
135138
.output()
136-
.with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?;
139+
.map_err(|e| ClangCaptureError::FailedToRunCommand {
140+
task: format!("Failed to get replacements from clang-format: {file_name}"),
141+
source: e,
142+
})?;
137143
if !output.stderr.is_empty() || !output.status.success() {
138144
logs.push((
139145
log::Level::Debug,
@@ -144,22 +150,27 @@ pub fn run_clang_format(
144150
));
145151
}
146152
let mut format_advice = if !output.stdout.is_empty() {
147-
let xml = String::from_utf8(output.stdout).with_context(|| {
148-
format!("XML output from clang-format was not UTF-8 encoded: {file_name}")
149-
})?;
150-
quick_xml::de::from_str::<FormatAdvice>(&xml).with_context(|| {
151-
format!("Failed to parse XML output from clang-format for {file_name}")
153+
let xml =
154+
String::from_utf8(output.stdout).map_err(|e| ClangCaptureError::NonUtf8Output {
155+
task: format!("XML output from clang-format (for {file_name})"),
156+
source: e,
157+
})?;
158+
quick_xml::de::from_str::<FormatAdvice>(&xml).map_err(|e| {
159+
ClangCaptureError::XmlParsingFailed {
160+
file_name: file_name.clone(),
161+
source: e,
162+
}
152163
})?
153164
} else {
154165
FormatAdvice::default()
155166
};
156167
format_advice.patched = patched;
157168
if !format_advice.replacements.is_empty() {
158-
let original_contents = fs::read(&file.name).with_context(|| {
159-
format!(
160-
"Failed to read file's original content before translating byte offsets: {file_name}",
161-
)
162-
})?;
169+
let original_contents =
170+
fs::read(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed {
171+
file_name: file_name.clone(),
172+
source: e,
173+
})?;
163174
// get line and column numbers from format_advice.offset
164175
let mut filtered_replacements = Vec::new();
165176
for replacement in &mut format_advice.replacements {

cpp-linter/src/clang_tools/clang_tidy.rs

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ use std::{
1010
};
1111

1212
// non-std crates
13-
use anyhow::{Context, Result, anyhow};
1413
use clang_tools_manager::utils::normalize_path;
1514
use regex::Regex;
1615
use serde::Deserialize;
1716

1817
// project-specific modules/crates
1918
use super::MakeSuggestions;
20-
use crate::{cli::ClangParams, common_fs::FileObj};
19+
use crate::{cli::ClangParams, common_fs::FileObj, error::ClangCaptureError};
2120

2221
/// Used to deserialize a json compilation database's translation unit.
2322
///
@@ -145,17 +144,18 @@ const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[
145144
fn parse_tidy_output(
146145
tidy_stdout: &[u8],
147146
database_json: &Option<Vec<CompilationUnit>>,
148-
) -> Result<TidyAdvice> {
149-
let note_header = Regex::new(NOTE_HEADER)
150-
.with_context(|| "Failed to compile RegExp pattern for note header")?;
151-
let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")
152-
.with_context(|| "Failed to compile RegExp pattern for fixed note")?;
147+
) -> Result<TidyAdvice, ClangCaptureError> {
148+
let note_header = Regex::new(NOTE_HEADER)?;
149+
let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")?;
153150
let mut found_fix = false;
154151
let mut notification = None;
155152
let mut result = Vec::new();
156-
let cur_dir = current_dir().with_context(|| "Failed to access current working directory")?;
153+
let cur_dir = current_dir().map_err(ClangCaptureError::UnknownWorkingDirectory)?;
157154
for line in String::from_utf8(tidy_stdout.to_vec())
158-
.with_context(|| "Failed to convert clang-tidy stdout to UTF-8 string")?
155+
.map_err(|e| ClangCaptureError::NonUtf8Output {
156+
task: "convert clang-tidy stdout".to_string(),
157+
source: e,
158+
})?
159159
.lines()
160160
{
161161
if let Some(captured) = note_header.captures(line) {
@@ -267,11 +267,11 @@ pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String> {
267267
pub fn run_clang_tidy(
268268
file: &mut MutexGuard<FileObj>,
269269
clang_params: &ClangParams,
270-
) -> Result<Vec<(log::Level, std::string::String)>> {
270+
) -> Result<Vec<(log::Level, String)>, ClangCaptureError> {
271271
let cmd_path = clang_params
272272
.clang_tidy_command
273273
.as_ref()
274-
.ok_or(anyhow!("clang-tidy command not located"))?;
274+
.ok_or(ClangCaptureError::ToolPathUnknown("clang-tidy"))?;
275275
let mut cmd = Command::new(cmd_path);
276276
let mut logs = vec![];
277277
if !clang_params.tidy_checks.is_empty() {
@@ -300,12 +300,12 @@ pub fn run_clang_tidy(
300300
None
301301
} else {
302302
cmd.arg("--fix-errors");
303-
Some(fs::read_to_string(&file.name).with_context(|| {
304-
format!(
305-
"Failed to cache file's original content before applying clang-tidy changes: {}",
306-
file_name.clone()
307-
)
308-
})?)
303+
Some(
304+
fs::read_to_string(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed {
305+
file_name: file_name.clone(),
306+
source: e,
307+
})?,
308+
)
309309
};
310310
if !clang_params.style.is_empty() {
311311
cmd.args(["--format-style", clang_params.style.as_str()]);
@@ -322,12 +322,12 @@ pub fn run_clang_tidy(
322322
.join(" ")
323323
),
324324
));
325-
let output = cmd.output().with_context(|| {
326-
format!(
327-
"Failed to execute clang-tidy on file: {}",
328-
file_name.clone()
329-
)
330-
})?;
325+
let output = cmd
326+
.output()
327+
.map_err(|e| ClangCaptureError::FailedToRunCommand {
328+
task: format!("execute clang-tidy on file {file_name}"),
329+
source: e,
330+
})?;
331331
logs.push((
332332
log::Level::Debug,
333333
format!(
@@ -354,13 +354,20 @@ pub fn run_clang_tidy(
354354
if let Some(tidy_advice) = &mut file.tidy_advice {
355355
// cache file changes in a buffer and restore the original contents for further analysis
356356
tidy_advice.patched =
357-
Some(fs::read(&file_name).with_context(|| {
358-
format!("Failed to read changes from clang-tidy: {file_name}")
359-
})?);
357+
Some(
358+
fs::read(&file_name).map_err(|e| ClangCaptureError::ReadFileFailed {
359+
file_name: file_name.clone(),
360+
source: e,
361+
})?,
362+
);
360363
}
361364
// original_content is guaranteed to be Some() value at this point
362-
fs::write(&file_name, original_content)
363-
.with_context(|| format!("Failed to restore file's original content: {file_name}"))?;
365+
fs::write(&file_name, original_content).map_err(|e| {
366+
ClangCaptureError::WriteFileFailed {
367+
file_name,
368+
source: e,
369+
}
370+
})?;
364371
}
365372
Ok(logs)
366373
}

cpp-linter/src/clang_tools/mod.rs

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use std::{
99
};
1010

1111
// non-std crates
12-
use anyhow::{Context, Result, anyhow};
1312
use clang_tools_manager::{ClangTool, RequestedVersion};
1413
use git_bot_feedback::ReviewComment;
1514
use git2::{DiffOptions, Patch};
@@ -18,7 +17,7 @@ use tokio::task::JoinSet;
1817

1918
// project-specific modules/crates
2019
use super::common_fs::FileObj;
21-
use crate::error::SuggestionError;
20+
use crate::error::{ClangCaptureError, ClangTaskError, SuggestionError};
2221
use crate::{
2322
cli::ClangParams,
2423
rest_client::{RestClient, USER_OUTREACH},
@@ -39,10 +38,8 @@ use clang_tidy::{CompilationUnit, run_clang_tidy};
3938
fn analyze_single_file(
4039
file: Arc<Mutex<FileObj>>,
4140
clang_params: Arc<ClangParams>,
42-
) -> Result<(PathBuf, Vec<(log::Level, String)>)> {
43-
let mut file = file
44-
.lock()
45-
.map_err(|_| anyhow!("Failed to lock file mutex"))?;
41+
) -> Result<(PathBuf, Vec<(log::Level, String)>), ClangCaptureError> {
42+
let mut file = file.lock().map_err(|_| ClangCaptureError::MutexPoisoned)?;
4643
let mut logs = vec![];
4744
if clang_params.clang_format_command.is_some() {
4845
if clang_params
@@ -104,15 +101,16 @@ pub async fn capture_clang_tools_output(
104101
version: &RequestedVersion,
105102
mut clang_params: ClangParams,
106103
rest_api_client: &RestClient,
107-
) -> Result<ClangVersions> {
104+
) -> Result<ClangVersions, ClangTaskError> {
108105
let mut clang_versions = ClangVersions::default();
109106
// find the executable paths for clang-tidy and/or clang-format and show version
110107
// info as debugging output.
111108
if clang_params.tidy_checks != "-*" {
112109
let tool = ClangTool::ClangTidy;
113-
let tool_info = version.eval_tool(&tool, false, None).await?.ok_or(anyhow!(
114-
"Failed to find {tool} or install a suitable version"
115-
))?;
110+
let tool_info = version
111+
.eval_tool(&tool, false, None)
112+
.await?
113+
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
116114
log::info!(
117115
"Using {tool} version {}.{}.{}",
118116
tool_info.version.major,
@@ -124,9 +122,10 @@ pub async fn capture_clang_tools_output(
124122
}
125123
if !clang_params.style.is_empty() {
126124
let tool = ClangTool::ClangFormat;
127-
let tool_info = version.eval_tool(&tool, false, None).await?.ok_or(anyhow!(
128-
"Failed to find {tool} or install a suitable version"
129-
))?;
125+
let tool_info = version
126+
.eval_tool(&tool, false, None)
127+
.await?
128+
.ok_or(ClangTaskError::FindToolError(tool.as_str()))?;
130129
log::info!(
131130
"Using {tool} version {}.{}.{}",
132131
tool_info.version.major,
@@ -143,8 +142,7 @@ pub async fn capture_clang_tools_output(
143142
{
144143
clang_params.database_json = Some(
145144
// A compilation database should be UTF-8 encoded, but file paths are not; use lossy conversion.
146-
serde_json::from_str::<Vec<CompilationUnit>>(&String::from_utf8_lossy(&db_str))
147-
.with_context(|| "Failed to parse compile_commands.json")?,
145+
serde_json::from_str::<Vec<CompilationUnit>>(&String::from_utf8_lossy(&db_str))?,
148146
)
149147
};
150148

cpp-linter/src/error.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use clang_tools_manager::GetToolError;
12
use git_bot_feedback::RestClientError;
23

34
#[derive(Debug, thiserror::Error)]
@@ -64,3 +65,61 @@ pub enum ClientError {
6465
#[error(transparent)]
6566
FileObjError(#[from] FileObjError),
6667
}
68+
69+
#[derive(Debug, thiserror::Error)]
70+
pub enum ClangCaptureError {
71+
#[error("Failed to acquire a lock on a file's mutex")]
72+
MutexPoisoned,
73+
#[error("Unknown path to {0} tool; required to invoke it.")]
74+
ToolPathUnknown(&'static str),
75+
#[error("Failed to {task}: {source}")]
76+
FailedToRunCommand {
77+
task: String,
78+
#[source]
79+
source: std::io::Error,
80+
},
81+
#[error("{task} output was not valid UTF-8: {source}")]
82+
NonUtf8Output {
83+
task: String,
84+
#[source]
85+
source: std::string::FromUtf8Error,
86+
},
87+
#[error("Failed to parse XML output from clang-format for {file_name}: {source}")]
88+
XmlParsingFailed {
89+
file_name: String,
90+
#[source]
91+
source: quick_xml::DeError,
92+
},
93+
#[error("Failed to read contents of file '{file_name}': {source}")]
94+
ReadFileFailed {
95+
file_name: String,
96+
#[source]
97+
source: std::io::Error,
98+
},
99+
#[error("Failed to write file '{file_name}': {source}")]
100+
WriteFileFailed {
101+
file_name: String,
102+
#[source]
103+
source: std::io::Error,
104+
},
105+
#[error("Failed to compile regular expression: {0}")]
106+
RegexError(#[from] regex::Error),
107+
#[error("Failed to determine the current working directory: {0}")]
108+
UnknownWorkingDirectory(#[source] std::io::Error),
109+
#[error("Failed to parse integer from string: {0}")]
110+
ParseIntError(#[from] std::num::ParseIntError),
111+
}
112+
113+
#[derive(Debug, thiserror::Error)]
114+
pub enum ClangTaskError {
115+
#[error(transparent)]
116+
GetToolError(#[from] GetToolError),
117+
#[error("Failed to find tool {0} or install a suitable version")]
118+
FindToolError(&'static str),
119+
#[error("Failed to parse compilation database: {0}")]
120+
ParseJsonError(#[from] serde_json::Error),
121+
#[error("Failed to execute task in parallel: {0}")]
122+
JoinError(#[from] tokio::task::JoinError),
123+
#[error(transparent)]
124+
ClangCaptureError(#[from] ClangCaptureError),
125+
}

0 commit comments

Comments
 (0)