Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang-tools-manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
4 changes: 2 additions & 2 deletions cpp-linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down Expand Up @@ -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
Expand Down
41 changes: 26 additions & 15 deletions cpp-linter/src/clang_tools/clang_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{
sync::{Arc, Mutex, MutexGuard},
};

use anyhow::{Context, Result, anyhow};
use log::Level;
use serde::Deserialize;

Expand All @@ -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)]
Expand Down Expand Up @@ -85,11 +85,11 @@ pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String>
pub fn run_clang_format(
file: &mut MutexGuard<FileObj>,
clang_params: &ClangParams,
) -> Result<Vec<(log::Level, String)>> {
) -> Result<Vec<(log::Level, String)>, 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]);
Expand All @@ -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,
)
};
Expand All @@ -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,
Expand All @@ -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::<FormatAdvice>(&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::<FormatAdvice>(&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 {
Expand Down
63 changes: 35 additions & 28 deletions cpp-linter/src/clang_tools/clang_tidy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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<Vec<CompilationUnit>>,
) -> Result<TidyAdvice> {
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<TidyAdvice, ClangCaptureError> {
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) {
Expand Down Expand Up @@ -267,11 +267,11 @@ pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String> {
pub fn run_clang_tidy(
file: &mut MutexGuard<FileObj>,
clang_params: &ClangParams,
) -> Result<Vec<(log::Level, std::string::String)>> {
) -> Result<Vec<(log::Level, String)>, 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() {
Expand Down Expand Up @@ -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,
})?,
)
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if !clang_params.style.is_empty() {
cmd.args(["--format-style", clang_params.style.as_str()]);
Expand All @@ -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!(
Expand All @@ -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)
}
Expand Down
28 changes: 13 additions & 15 deletions cpp-linter/src/clang_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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},
Expand All @@ -39,10 +38,8 @@ use clang_tidy::{CompilationUnit, run_clang_tidy};
fn analyze_single_file(
file: Arc<Mutex<FileObj>>,
clang_params: Arc<ClangParams>,
) -> 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
Expand Down Expand Up @@ -104,15 +101,16 @@ pub async fn capture_clang_tools_output(
version: &RequestedVersion,
mut clang_params: ClangParams,
rest_api_client: &RestClient,
) -> Result<ClangVersions> {
) -> Result<ClangVersions, ClangTaskError> {
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,
Expand All @@ -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,
Expand All @@ -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::<Vec<CompilationUnit>>(&String::from_utf8_lossy(&db_str))
.with_context(|| "Failed to parse compile_commands.json")?,
serde_json::from_str::<Vec<CompilationUnit>>(&String::from_utf8_lossy(&db_str))?,
)
};

Expand Down
59 changes: 59 additions & 0 deletions cpp-linter/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use clang_tools_manager::GetToolError;
use git_bot_feedback::RestClientError;

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -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),
}
Loading