diff --git a/clang-tools-manager/src/lib.rs b/clang-tools-manager/src/lib.rs index 1fbecaab..5fc2e4eb 100644 --- a/clang-tools-manager/src/lib.rs +++ b/clang-tools-manager/src/lib.rs @@ -19,7 +19,7 @@ pub use tool::ClangTool; pub mod utils; mod version; -pub use version::RequestedVersion; +pub use version::{ClangVersion, GetToolError, RequestedVersion, RequestedVersionParsingError}; mod progress_bar; pub use progress_bar::ProgressBar; diff --git a/cpp-linter/Cargo.toml b/cpp-linter/Cargo.toml index 6f138ec6..fff11c39 100644 --- a/cpp-linter/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -14,7 +14,7 @@ license.workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -anyhow = { workspace = true } +anyhow = { workspace = true, optional = true } async-trait = "0.1.89" chrono = "0.4.44" clang-tools-manager = { path = "../clang-tools-manager", version = "0.2.0" } @@ -46,7 +46,7 @@ tempfile = { workspace = true } reqwest = { workspace = true, features = ["default-tls"] } [features] -bin = ["reqwest/default-tls", "dep:clap", "dep:colored"] +bin = ["reqwest/default-tls", "dep:clap", "dep:colored", "dep:anyhow"] [lib] bench = false diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index a9e0e7a5..f67cf3a4 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -7,7 +7,6 @@ use std::{ sync::{Arc, Mutex, MutexGuard}, }; -use anyhow::{Context, Result, anyhow}; use log::Level; use serde::Deserialize; @@ -16,6 +15,7 @@ use super::MakeSuggestions; use crate::{ cli::ClangParams, common_fs::{FileObj, get_line_count_from_offset}, + error::ClangCaptureError, }; #[derive(Debug, Clone, Deserialize, PartialEq, Eq, Default)] @@ -85,11 +85,11 @@ pub fn tally_format_advice(files: &[Arc>]) -> Result pub fn run_clang_format( file: &mut MutexGuard, clang_params: &ClangParams, -) -> Result> { +) -> Result, ClangCaptureError> { let cmd_path = clang_params .clang_format_command .as_ref() - .ok_or(anyhow!("clang-format path unknown"))?; + .ok_or(ClangCaptureError::ToolPathUnknown("clang-format"))?; let mut cmd = Command::new(cmd_path); let mut logs = vec![]; cmd.args(["--style", &clang_params.style]); @@ -115,7 +115,10 @@ pub fn run_clang_format( )); Some( cmd.output() - .with_context(|| format!("Failed to get fixes from clang-format: {file_name}"))? + .map_err(|e| ClangCaptureError::FailedToRunCommand { + task: format!("get fixes from clang-format {file_name}"), + source: e, + })? .stdout, ) }; @@ -133,7 +136,10 @@ pub fn run_clang_format( )); let output = cmd .output() - .with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?; + .map_err(|e| ClangCaptureError::FailedToRunCommand { + task: format!("Failed to get replacements from clang-format: {file_name}"), + source: e, + })?; if !output.stderr.is_empty() || !output.status.success() { logs.push(( log::Level::Debug, @@ -144,22 +150,27 @@ pub fn run_clang_format( )); } let mut format_advice = if !output.stdout.is_empty() { - let xml = String::from_utf8(output.stdout).with_context(|| { - format!("XML output from clang-format was not UTF-8 encoded: {file_name}") - })?; - quick_xml::de::from_str::(&xml).with_context(|| { - format!("Failed to parse XML output from clang-format for {file_name}") + let xml = + String::from_utf8(output.stdout).map_err(|e| ClangCaptureError::NonUtf8Output { + task: format!("XML output from clang-format (for {file_name})"), + source: e, + })?; + quick_xml::de::from_str::(&xml).map_err(|e| { + ClangCaptureError::XmlParsingFailed { + file_name: file_name.clone(), + source: e, + } })? } else { FormatAdvice::default() }; format_advice.patched = patched; if !format_advice.replacements.is_empty() { - let original_contents = fs::read(&file.name).with_context(|| { - format!( - "Failed to read file's original content before translating byte offsets: {file_name}", - ) - })?; + let original_contents = + fs::read(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed { + file_name: file_name.clone(), + source: e, + })?; // get line and column numbers from format_advice.offset let mut filtered_replacements = Vec::new(); for replacement in &mut format_advice.replacements { diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index b9da583d..74be74ed 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -10,14 +10,13 @@ use std::{ }; // non-std crates -use anyhow::{Context, Result, anyhow}; use clang_tools_manager::utils::normalize_path; use regex::Regex; use serde::Deserialize; // project-specific modules/crates use super::MakeSuggestions; -use crate::{cli::ClangParams, common_fs::FileObj}; +use crate::{cli::ClangParams, common_fs::FileObj, error::ClangCaptureError}; /// Used to deserialize a json compilation database's translation unit. /// @@ -145,17 +144,18 @@ const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[ fn parse_tidy_output( tidy_stdout: &[u8], database_json: &Option>, -) -> Result { - let note_header = Regex::new(NOTE_HEADER) - .with_context(|| "Failed to compile RegExp pattern for note header")?; - let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$") - .with_context(|| "Failed to compile RegExp pattern for fixed note")?; +) -> Result { + let note_header = Regex::new(NOTE_HEADER)?; + let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")?; let mut found_fix = false; let mut notification = None; let mut result = Vec::new(); - let cur_dir = current_dir().with_context(|| "Failed to access current working directory")?; + let cur_dir = current_dir().map_err(ClangCaptureError::UnknownWorkingDirectory)?; for line in String::from_utf8(tidy_stdout.to_vec()) - .with_context(|| "Failed to convert clang-tidy stdout to UTF-8 string")? + .map_err(|e| ClangCaptureError::NonUtf8Output { + task: "convert clang-tidy stdout".to_string(), + source: e, + })? .lines() { if let Some(captured) = note_header.captures(line) { @@ -267,11 +267,11 @@ pub fn tally_tidy_advice(files: &[Arc>]) -> Result { pub fn run_clang_tidy( file: &mut MutexGuard, clang_params: &ClangParams, -) -> Result> { +) -> Result, ClangCaptureError> { let cmd_path = clang_params .clang_tidy_command .as_ref() - .ok_or(anyhow!("clang-tidy command not located"))?; + .ok_or(ClangCaptureError::ToolPathUnknown("clang-tidy"))?; let mut cmd = Command::new(cmd_path); let mut logs = vec![]; if !clang_params.tidy_checks.is_empty() { @@ -300,12 +300,12 @@ pub fn run_clang_tidy( None } else { cmd.arg("--fix-errors"); - 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() - ) - })?) + Some( + fs::read_to_string(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed { + file_name: file_name.clone(), + source: e, + })?, + ) }; if !clang_params.style.is_empty() { cmd.args(["--format-style", clang_params.style.as_str()]); @@ -322,12 +322,12 @@ pub fn run_clang_tidy( .join(" ") ), )); - let output = cmd.output().with_context(|| { - format!( - "Failed to execute clang-tidy on file: {}", - file_name.clone() - ) - })?; + let output = cmd + .output() + .map_err(|e| ClangCaptureError::FailedToRunCommand { + task: format!("execute clang-tidy on file {file_name}"), + source: e, + })?; logs.push(( log::Level::Debug, format!( @@ -354,13 +354,20 @@ pub fn run_clang_tidy( 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}") - })?); + Some( + fs::read(&file_name).map_err(|e| ClangCaptureError::ReadFileFailed { + file_name: file_name.clone(), + source: e, + })?, + ); } // original_content is guaranteed to be Some() value at this point - fs::write(&file_name, original_content) - .with_context(|| format!("Failed to restore file's original content: {file_name}"))?; + fs::write(&file_name, original_content).map_err(|e| { + ClangCaptureError::WriteFileFailed { + file_name, + source: e, + } + })?; } Ok(logs) } diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 180c31db..d57e3720 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -9,7 +9,6 @@ use std::{ }; // non-std crates -use anyhow::{Context, Result, anyhow}; use clang_tools_manager::{ClangTool, RequestedVersion}; use git_bot_feedback::ReviewComment; use git2::{DiffOptions, Patch}; @@ -18,7 +17,7 @@ use tokio::task::JoinSet; // project-specific modules/crates use super::common_fs::FileObj; -use crate::error::SuggestionError; +use crate::error::{ClangCaptureError, ClangTaskError, SuggestionError}; use crate::{ cli::ClangParams, rest_client::{RestClient, USER_OUTREACH}, @@ -39,10 +38,8 @@ use clang_tidy::{CompilationUnit, run_clang_tidy}; fn analyze_single_file( file: Arc>, clang_params: Arc, -) -> Result<(PathBuf, Vec<(log::Level, String)>)> { - let mut file = file - .lock() - .map_err(|_| anyhow!("Failed to lock file mutex"))?; +) -> Result<(PathBuf, Vec<(log::Level, String)>), ClangCaptureError> { + let mut file = file.lock().map_err(|_| ClangCaptureError::MutexPoisoned)?; let mut logs = vec![]; if clang_params.clang_format_command.is_some() { if clang_params @@ -104,15 +101,16 @@ pub async fn capture_clang_tools_output( version: &RequestedVersion, mut clang_params: ClangParams, rest_api_client: &RestClient, -) -> Result { +) -> Result { let mut clang_versions = ClangVersions::default(); // find the executable paths for clang-tidy and/or clang-format and show version // info as debugging output. if clang_params.tidy_checks != "-*" { let tool = ClangTool::ClangTidy; - let tool_info = version.eval_tool(&tool, false, None).await?.ok_or(anyhow!( - "Failed to find {tool} or install a suitable version" - ))?; + let tool_info = version + .eval_tool(&tool, false, None) + .await? + .ok_or(ClangTaskError::FindToolError(tool.as_str()))?; log::info!( "Using {tool} version {}.{}.{}", tool_info.version.major, @@ -124,9 +122,10 @@ pub async fn capture_clang_tools_output( } if !clang_params.style.is_empty() { let tool = ClangTool::ClangFormat; - let tool_info = version.eval_tool(&tool, false, None).await?.ok_or(anyhow!( - "Failed to find {tool} or install a suitable version" - ))?; + let tool_info = version + .eval_tool(&tool, false, None) + .await? + .ok_or(ClangTaskError::FindToolError(tool.as_str()))?; log::info!( "Using {tool} version {}.{}.{}", tool_info.version.major, @@ -143,8 +142,7 @@ pub async fn capture_clang_tools_output( { clang_params.database_json = Some( // A compilation database should be UTF-8 encoded, but file paths are not; use lossy conversion. - serde_json::from_str::>(&String::from_utf8_lossy(&db_str)) - .with_context(|| "Failed to parse compile_commands.json")?, + serde_json::from_str::>(&String::from_utf8_lossy(&db_str))?, ) }; diff --git a/cpp-linter/src/error.rs b/cpp-linter/src/error.rs index 7c20d6aa..26f2b859 100644 --- a/cpp-linter/src/error.rs +++ b/cpp-linter/src/error.rs @@ -1,3 +1,4 @@ +use clang_tools_manager::GetToolError; use git_bot_feedback::RestClientError; #[derive(Debug, thiserror::Error)] @@ -64,3 +65,61 @@ pub enum ClientError { #[error(transparent)] FileObjError(#[from] FileObjError), } + +#[derive(Debug, thiserror::Error)] +pub enum ClangCaptureError { + #[error("Failed to acquire a lock on a file's mutex")] + MutexPoisoned, + #[error("Unknown path to {0} tool; required to invoke it.")] + ToolPathUnknown(&'static str), + #[error("Failed to {task}: {source}")] + FailedToRunCommand { + task: String, + #[source] + source: std::io::Error, + }, + #[error("{task} output was not valid UTF-8: {source}")] + NonUtf8Output { + task: String, + #[source] + source: std::string::FromUtf8Error, + }, + #[error("Failed to parse XML output from clang-format for {file_name}: {source}")] + XmlParsingFailed { + file_name: String, + #[source] + source: quick_xml::DeError, + }, + #[error("Failed to read contents of file '{file_name}': {source}")] + ReadFileFailed { + file_name: String, + #[source] + source: std::io::Error, + }, + #[error("Failed to write file '{file_name}': {source}")] + WriteFileFailed { + file_name: String, + #[source] + source: std::io::Error, + }, + #[error("Failed to compile regular expression: {0}")] + RegexError(#[from] regex::Error), + #[error("Failed to determine the current working directory: {0}")] + UnknownWorkingDirectory(#[source] std::io::Error), + #[error("Failed to parse integer from string: {0}")] + ParseIntError(#[from] std::num::ParseIntError), +} + +#[derive(Debug, thiserror::Error)] +pub enum ClangTaskError { + #[error(transparent)] + GetToolError(#[from] GetToolError), + #[error("Failed to find tool {0} or install a suitable version")] + FindToolError(&'static str), + #[error("Failed to parse compilation database: {0}")] + ParseJsonError(#[from] serde_json::Error), + #[error("Failed to execute task in parallel: {0}")] + JoinError(#[from] tokio::task::JoinError), + #[error(transparent)] + ClangCaptureError(#[from] ClangCaptureError), +}