Skip to content

Some vital updates from py codebase #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 13, 2024
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
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v4.6.0
hooks:
- id: trailing-whitespace
exclude: cpp-linter-lib/tests/capture_tools_output/cpp-linter/cpp-linter/test_git_lib.patch
Expand All @@ -14,7 +14,7 @@ repos:
- id: mixed-line-ending
args: ["--fix=lf"]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.8
rev: v0.5.2
hooks:
# Run the python linter.
- id: ruff
Expand Down
7 changes: 5 additions & 2 deletions cpp-linter-lib/src/clang_tools/clang_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
use serde_xml_rs::de::Deserializer;

// project-specific crates/modules
use crate::common_fs::{get_line_cols_from_offset, FileObj};
use crate::{
cli::LinesChangedOnly,
common_fs::{get_line_cols_from_offset, FileObj},
};

/// A Structure used to deserialize clang-format's XML output.
#[derive(Debug, Deserialize, PartialEq)]
Expand Down Expand Up @@ -62,7 +65,7 @@
cmd: &mut Command,
file: &FileObj,
style: &str,
lines_changed_only: u8,
lines_changed_only: &LinesChangedOnly,

Check warning on line 68 in cpp-linter-lib/src/clang_tools/clang_format.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_format.rs#L68

Added line #L68 was not covered by tests
) -> FormatAdvice {
cmd.args(["--style", style, "--output-replacements-xml"]);
let ranges = file.get_ranges(lines_changed_only);
Expand Down
35 changes: 29 additions & 6 deletions cpp-linter-lib/src/clang_tools/clang_tidy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
use serde::Deserialize;

// project-specific modules/crates
use crate::common_fs::{normalize_path, FileObj};
use crate::{
cli::LinesChangedOnly,
common_fs::{normalize_path, FileObj},
};

/// Used to deserialize a JSON compilation database
#[derive(Deserialize, Debug)]
Expand Down Expand Up @@ -68,14 +71,34 @@
pub suggestion: Vec<String>,
}

impl TidyNotification {
pub fn diagnostic_link(&self) -> String {
let ret_val = if let Some((category, name)) = self.diagnostic.split_once('-') {
format!(
"[{}](https://clang.llvm.org/extra/clang-tidy/checks/{category}/{name}).html",
self.diagnostic
)

Check warning on line 80 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L75-L80

Added lines #L75 - L80 were not covered by tests
} else {
self.diagnostic.clone()

Check warning on line 82 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L82

Added line #L82 was not covered by tests
};
ret_val
}

Check warning on line 85 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L84-L85

Added lines #L84 - L85 were not covered by tests
}

/// A struct to hold notification from clang-tidy about a single file
pub struct TidyAdvice {
/// A list of notifications parsed from clang-tidy stdout.
pub notes: Vec<TidyNotification>,
}

/// Parses clang-tidy stdout.
///
/// Here it helps to have the JSON database deserialized for normalizing paths present
/// in the notifications.
fn parse_tidy_output(
tidy_stdout: &[u8],
database_json: &Option<CompilationDatabase>,
) -> Vec<TidyNotification> {
) -> TidyAdvice {

Check warning on line 101 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L101

Added line #L101 was not covered by tests
let note_header = Regex::new(r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+)\]$").unwrap();
let mut notification = None;
let mut result = Vec::new();
Expand Down Expand Up @@ -141,19 +164,19 @@
if let Some(note) = notification {
result.push(note);
}
result
TidyAdvice { notes: result }

Check warning on line 167 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L167

Added line #L167 was not covered by tests
}

/// Run clang-tidy, then parse and return it's output.
pub fn run_clang_tidy(
cmd: &mut Command,
file: &FileObj,
checks: &str,
lines_changed_only: u8,
lines_changed_only: &LinesChangedOnly,

Check warning on line 175 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L175

Added line #L175 was not covered by tests
database: &Option<PathBuf>,
extra_args: &Option<Vec<&str>>,
database_json: &Option<CompilationDatabase>,
) -> Vec<TidyNotification> {
) -> TidyAdvice {

Check warning on line 179 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L179

Added line #L179 was not covered by tests
if !checks.is_empty() {
cmd.args(["-checks", checks]);
}
Expand All @@ -165,7 +188,7 @@
cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
}
}
if lines_changed_only > 0 {
if *lines_changed_only != LinesChangedOnly::Off {

Check warning on line 191 in cpp-linter-lib/src/clang_tools/clang_tidy.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/clang_tidy.rs#L191

Added line #L191 was not covered by tests
let ranges = file.get_ranges(lines_changed_only);
let filter = format!(
"[{{\"name\":{:?},\"lines\":{:?}}}]",
Expand Down
18 changes: 10 additions & 8 deletions cpp-linter-lib/src/clang_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@

// project-specific modules/crates
use super::common_fs::FileObj;
use crate::logger::{end_log_group, start_log_group};
use crate::{
cli::LinesChangedOnly,
logger::{end_log_group, start_log_group},
};
pub mod clang_format;
use clang_format::{run_clang_format, FormatAdvice};
pub mod clang_tidy;
use clang_tidy::{run_clang_tidy, CompilationDatabase, TidyNotification};
use clang_tidy::{run_clang_tidy, CompilationDatabase, TidyAdvice};

/// Fetch the path to a clang tool by `name` (ie `"clang-tidy"` or `"clang-format"`) and
/// `version`.
Expand Down Expand Up @@ -69,7 +72,7 @@
/// Runs clang-tidy and/or clang-format and returns the parsed output from each.
///
/// The returned list of [`FormatAdvice`] is parallel to the `files` list passed in
/// here. The returned 2D list of [`TidyNotification`] is also parallel on the first
/// here. The returned 2D list of [`TidyAdvice`] is also parallel on the first
/// dimension. The second dimension is a list of notes specific to a translation unit
/// (each element of `files`).
///
Expand All @@ -80,10 +83,10 @@
version: &str,
tidy_checks: &str,
style: &str,
lines_changed_only: u8,
lines_changed_only: &LinesChangedOnly,

Check warning on line 86 in cpp-linter-lib/src/clang_tools/mod.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/mod.rs#L86

Added line #L86 was not covered by tests
database: Option<PathBuf>,
extra_args: Option<Vec<&str>>,
) -> (Vec<FormatAdvice>, Vec<Vec<TidyNotification>>) {
) -> (Vec<FormatAdvice>, Vec<TidyAdvice>) {

Check warning on line 89 in cpp-linter-lib/src/clang_tools/mod.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/mod.rs#L89

Added line #L89 was not covered by tests
// find the executable paths for clang-tidy and/or clang-format and show version
// info as debugging output.
let clang_tidy_command = if tidy_checks != "-*" {
Expand Down Expand Up @@ -126,9 +129,8 @@
};

// iterate over the discovered files and run the clang tools
let mut all_format_advice: Vec<clang_format::FormatAdvice> = Vec::with_capacity(files.len());
let mut all_tidy_advice: Vec<Vec<clang_tidy::TidyNotification>> =
Vec::with_capacity(files.len());
let mut all_format_advice: Vec<FormatAdvice> = Vec::with_capacity(files.len());
let mut all_tidy_advice: Vec<TidyAdvice> = Vec::with_capacity(files.len());

Check warning on line 133 in cpp-linter-lib/src/clang_tools/mod.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter-lib/src/clang_tools/mod.rs#L132-L133

Added lines #L132 - L133 were not covered by tests
for file in files {
start_log_group(format!("Analyzing {}", file.name.to_string_lossy()));
if let Some(tidy_cmd) = &clang_tidy_command {
Expand Down
11 changes: 11 additions & 0 deletions cpp-linter-lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ use std::fs;
use clap::builder::FalseyValueParser;
use clap::{Arg, ArgAction, ArgMatches, Command};

/// An enum to describe `--lines-changed-only` CLI option's behavior.
#[derive(PartialEq)]
pub enum LinesChangedOnly {
/// All lines are scanned
Off,
/// Only lines in the diff are scanned
Diff,
/// Only lines in the diff with additions are scanned.
On,
}

/// Builds and returns the Command Line Interface's argument parsing object.
pub fn get_arg_parser() -> Command {
Command::new("cpp-linter")
Expand Down
23 changes: 12 additions & 11 deletions cpp-linter-lib/src/common_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::path::{Component, Path};
use std::{fs, io};
use std::{ops::RangeInclusive, path::PathBuf};

use crate::cli::LinesChangedOnly;

/// A structure to represent a file's path and line changes.
#[derive(Debug)]
pub struct FileObj {
Expand Down Expand Up @@ -52,7 +54,7 @@ impl FileObj {
/// A helper function to consolidate a [Vec<u32>] of line numbers into a
/// [Vec<RangeInclusive<u32>>] in which each range describes the beginning and
/// ending of a group of consecutive line numbers.
fn consolidate_numbers_to_ranges(lines: &Vec<u32>) -> Vec<RangeInclusive<u32>> {
fn consolidate_numbers_to_ranges(lines: &[u32]) -> Vec<RangeInclusive<u32>> {
let mut range_start = None;
let mut ranges: Vec<RangeInclusive<u32>> = Vec::new();
for (index, number) in lines.iter().enumerate() {
Expand All @@ -69,13 +71,11 @@ impl FileObj {
ranges
}

pub fn get_ranges(&self, lines_changed_only: u8) -> Vec<RangeInclusive<u32>> {
if lines_changed_only == 2 {
self.diff_chunks.to_vec()
} else if lines_changed_only == 1 {
self.added_ranges.to_vec()
} else {
Vec::new()
pub fn get_ranges(&self, lines_changed_only: &LinesChangedOnly) -> Vec<RangeInclusive<u32>> {
match lines_changed_only {
LinesChangedOnly::Diff => self.diff_chunks.to_vec(),
LinesChangedOnly::On => self.added_ranges.to_vec(),
_ => Vec::new(),
}
}
}
Expand Down Expand Up @@ -249,6 +249,7 @@ mod test {
use std::path::PathBuf;

use super::{get_line_cols_from_offset, list_source_files, normalize_path, FileObj};
use crate::cli::LinesChangedOnly;
use crate::cli::{get_arg_parser, parse_ignore};
use crate::common_fs::is_file_in_list;

Expand Down Expand Up @@ -406,7 +407,7 @@ mod test {
#[test]
fn get_ranges_0() {
let file_obj = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
let ranges = file_obj.get_ranges(0);
let ranges = file_obj.get_ranges(&LinesChangedOnly::Off);
assert!(ranges.is_empty());
}

Expand All @@ -419,7 +420,7 @@ mod test {
added_lines,
diff_chunks.clone(),
);
let ranges = file_obj.get_ranges(2);
let ranges = file_obj.get_ranges(&LinesChangedOnly::Diff);
assert_eq!(ranges, diff_chunks);
}

Expand All @@ -432,7 +433,7 @@ mod test {
added_lines,
diff_chunks,
);
let ranges = file_obj.get_ranges(1);
let ranges = file_obj.get_ranges(&LinesChangedOnly::On);
assert_eq!(ranges, vec![4..=5, 9..=9]);
}
}
Loading
Loading