Skip to content

Commit ff91fd8

Browse files
committed
use an enum to statically type --lines-changed-only
1 parent acc0d5d commit ff91fd8

File tree

6 files changed

+45
-25
lines changed

6 files changed

+45
-25
lines changed

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ use serde::Deserialize;
88
use serde_xml_rs::de::Deserializer;
99

1010
// project-specific crates/modules
11-
use crate::common_fs::{get_line_cols_from_offset, FileObj};
11+
use crate::{
12+
cli::LinesChangedOnly,
13+
common_fs::{get_line_cols_from_offset, FileObj},
14+
};
1215

1316
/// A Structure used to deserialize clang-format's XML output.
1417
#[derive(Debug, Deserialize, PartialEq)]
@@ -62,7 +65,7 @@ pub fn run_clang_format(
6265
cmd: &mut Command,
6366
file: &FileObj,
6467
style: &str,
65-
lines_changed_only: u8,
68+
lines_changed_only: &LinesChangedOnly,
6669
) -> FormatAdvice {
6770
cmd.args(["--style", style, "--output-replacements-xml"]);
6871
let ranges = file.get_ranges(lines_changed_only);

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ use regex::Regex;
1212
use serde::Deserialize;
1313

1414
// project-specific modules/crates
15-
use crate::common_fs::{normalize_path, FileObj};
15+
use crate::{
16+
cli::LinesChangedOnly,
17+
common_fs::{normalize_path, FileObj},
18+
};
1619

1720
/// Used to deserialize a JSON compilation database
1821
#[derive(Deserialize, Debug)]
@@ -169,7 +172,7 @@ pub fn run_clang_tidy(
169172
cmd: &mut Command,
170173
file: &FileObj,
171174
checks: &str,
172-
lines_changed_only: u8,
175+
lines_changed_only: &LinesChangedOnly,
173176
database: &Option<PathBuf>,
174177
extra_args: &Option<Vec<&str>>,
175178
database_json: &Option<CompilationDatabase>,
@@ -185,7 +188,7 @@ pub fn run_clang_tidy(
185188
cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
186189
}
187190
}
188-
if lines_changed_only > 0 {
191+
if *lines_changed_only != LinesChangedOnly::Off {
189192
let ranges = file.get_ranges(lines_changed_only);
190193
let filter = format!(
191194
"[{{\"name\":{:?},\"lines\":{:?}}}]",

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ use which::{which, which_in};
1010

1111
// project-specific modules/crates
1212
use super::common_fs::FileObj;
13-
use crate::logger::{end_log_group, start_log_group};
13+
use crate::{
14+
cli::LinesChangedOnly,
15+
logger::{end_log_group, start_log_group},
16+
};
1417
pub mod clang_format;
1518
use clang_format::{run_clang_format, FormatAdvice};
1619
pub mod clang_tidy;
@@ -80,7 +83,7 @@ pub fn capture_clang_tools_output(
8083
version: &str,
8184
tidy_checks: &str,
8285
style: &str,
83-
lines_changed_only: u8,
86+
lines_changed_only: &LinesChangedOnly,
8487
database: Option<PathBuf>,
8588
extra_args: Option<Vec<&str>>,
8689
) -> (Vec<FormatAdvice>, Vec<TidyAdvice>) {

cpp-linter-lib/src/cli.rs

+11
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ use std::fs;
66
use clap::builder::FalseyValueParser;
77
use clap::{Arg, ArgAction, ArgMatches, Command};
88

9+
/// An enum to describe `--lines-changed-only` CLI option's behavior.
10+
#[derive(PartialEq)]
11+
pub enum LinesChangedOnly {
12+
/// All lines are scanned
13+
Off,
14+
/// Only lines in the diff are scanned
15+
Diff,
16+
/// Only lines in the diff with additions are scanned.
17+
On,
18+
}
19+
920
/// Builds and returns the Command Line Interface's argument parsing object.
1021
pub fn get_arg_parser() -> Command {
1122
Command::new("cpp-linter")

cpp-linter-lib/src/common_fs.rs

+12-11
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use std::path::{Component, Path};
55
use std::{fs, io};
66
use std::{ops::RangeInclusive, path::PathBuf};
77

8+
use crate::cli::LinesChangedOnly;
9+
810
/// A structure to represent a file's path and line changes.
911
#[derive(Debug)]
1012
pub struct FileObj {
@@ -52,7 +54,7 @@ impl FileObj {
5254
/// A helper function to consolidate a [Vec<u32>] of line numbers into a
5355
/// [Vec<RangeInclusive<u32>>] in which each range describes the beginning and
5456
/// ending of a group of consecutive line numbers.
55-
fn consolidate_numbers_to_ranges(lines: &Vec<u32>) -> Vec<RangeInclusive<u32>> {
57+
fn consolidate_numbers_to_ranges(lines: &[u32]) -> Vec<RangeInclusive<u32>> {
5658
let mut range_start = None;
5759
let mut ranges: Vec<RangeInclusive<u32>> = Vec::new();
5860
for (index, number) in lines.iter().enumerate() {
@@ -69,13 +71,11 @@ impl FileObj {
6971
ranges
7072
}
7173

72-
pub fn get_ranges(&self, lines_changed_only: u8) -> Vec<RangeInclusive<u32>> {
73-
if lines_changed_only == 2 {
74-
self.diff_chunks.to_vec()
75-
} else if lines_changed_only == 1 {
76-
self.added_ranges.to_vec()
77-
} else {
78-
Vec::new()
74+
pub fn get_ranges(&self, lines_changed_only: &LinesChangedOnly) -> Vec<RangeInclusive<u32>> {
75+
match lines_changed_only {
76+
LinesChangedOnly::Diff => self.diff_chunks.to_vec(),
77+
LinesChangedOnly::On => self.added_ranges.to_vec(),
78+
_ => Vec::new(),
7979
}
8080
}
8181
}
@@ -249,6 +249,7 @@ mod test {
249249
use std::path::PathBuf;
250250

251251
use super::{get_line_cols_from_offset, list_source_files, normalize_path, FileObj};
252+
use crate::cli::LinesChangedOnly;
252253
use crate::cli::{get_arg_parser, parse_ignore};
253254
use crate::common_fs::is_file_in_list;
254255

@@ -406,7 +407,7 @@ mod test {
406407
#[test]
407408
fn get_ranges_0() {
408409
let file_obj = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
409-
let ranges = file_obj.get_ranges(0);
410+
let ranges = file_obj.get_ranges(&LinesChangedOnly::Off);
410411
assert!(ranges.is_empty());
411412
}
412413

@@ -419,7 +420,7 @@ mod test {
419420
added_lines,
420421
diff_chunks.clone(),
421422
);
422-
let ranges = file_obj.get_ranges(2);
423+
let ranges = file_obj.get_ranges(&LinesChangedOnly::Diff);
423424
assert_eq!(ranges, diff_chunks);
424425
}
425426

@@ -432,7 +433,7 @@ mod test {
432433
added_lines,
433434
diff_chunks,
434435
);
435-
let ranges = file_obj.get_ranges(1);
436+
let ranges = file_obj.get_ranges(&LinesChangedOnly::On);
436437
assert_eq!(ranges, vec![4..=5, 9..=9]);
437438
}
438439
}

cpp-linter-lib/src/run.rs

+6-7
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use openssl_probe;
1313

1414
// project specific modules/crates
1515
use crate::clang_tools::capture_clang_tools_output;
16-
use crate::cli::{convert_extra_arg_val, get_arg_parser, parse_ignore};
16+
use crate::cli::{convert_extra_arg_val, get_arg_parser, parse_ignore, LinesChangedOnly};
1717
use crate::common_fs::{list_source_files, FileObj};
1818
use crate::github_api::GithubApiClient;
1919
use crate::logger::{self, end_log_group, start_log_group};
@@ -90,15 +90,14 @@ pub fn run_main(args: Vec<String>) -> i32 {
9090
.unwrap()
9191
.as_str()
9292
{
93-
"false" => 0,
94-
"true" => 1,
95-
"diff" => 2,
96-
_ => unreachable!(),
93+
"true" => LinesChangedOnly::On,
94+
"diff" => LinesChangedOnly::Diff,
95+
_ => LinesChangedOnly::Off,
9796
};
9897
let files_changed_only = args.get_flag("files-changed-only");
9998

10099
start_log_group(String::from("Get list of specified source files"));
101-
let files: Vec<FileObj> = if lines_changed_only != 0 || files_changed_only {
100+
let files: Vec<FileObj> = if lines_changed_only != LinesChangedOnly::Off || files_changed_only {
102101
// parse_diff(github_rest_api_payload)
103102
rest_api_client.get_list_of_changed_files(&extensions, &ignored, &not_ignored)
104103
} else {
@@ -128,7 +127,7 @@ pub fn run_main(args: Vec<String>) -> i32 {
128127
args.get_one::<String>("version").unwrap(),
129128
args.get_one::<String>("tidy-checks").unwrap(),
130129
user_inputs.style.as_str(),
131-
lines_changed_only,
130+
&lines_changed_only,
132131
database_path,
133132
extra_args,
134133
);

0 commit comments

Comments
 (0)