Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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::WriteFileFailed {
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