Skip to content

Commit 1b66ad6

Browse files
committed
deserialize CLI options into structs from parsed results
1 parent 0fbd22d commit 1b66ad6

File tree

13 files changed

+277
-179
lines changed

13 files changed

+277
-179
lines changed

cpp-linter-lib/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ documentation.workspace = true
88
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
99

1010
[dependencies]
11-
clap = { version = "4.5.16" }
11+
clap = "4.5.16"
1212
git2 = "0.19.0"
1313
lenient_semver = "0.4.2"
1414
log = "0.4.22"

cpp-linter-lib/README.md

+11-3
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,19 @@
22
# cpp-linter-lib
33

44
This crate contains the the library used as a backend for the
5-
`cpp-linter` binary executable.
5+
`cpp-linter` binary executable. The main focus of `cpp-linter` is as follows:
6+
7+
- [x] Lint C/C++ sources using clang-format and clang-tidy.
8+
- [x] Respect file changes when run in a CI workflow on Github.
9+
- [x] Provide feedback via Github's REST API in the any of the following forms:
10+
- [x] step summary
11+
- [x] thread comments
12+
- [x] file annotation
13+
- [ ] pull request review suggestions
614

715
Since the [cpp-linter python package][pypi-org] now uses this library
8-
as a binding, the native binary's `main()` method is also present in this
9-
library.
16+
as a binding, the native binary's `main()` behavior is also present in this
17+
library (see `run::run_main()`).
1018

1119
See also the [CLI document hosted on github][gh-pages].
1220

cpp-linter-lib/examples/gh_rest_api.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ pub async fn main() -> Result<(), Box<dyn Error>> {
1414
let client_controller = GithubApiClient::new();
1515

1616
let file_filter = FileFilter::new(
17-
&["target", ".github"],
17+
&["target".to_string(), ".github".to_string()],
1818
vec!["cpp".to_string(), "hpp".to_string()],
1919
);
2020

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

+1-16
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ use which::{which, which_in};
1818
// project-specific modules/crates
1919
use super::common_fs::FileObj;
2020
use crate::{
21-
cli::LinesChangedOnly,
22-
common_fs::FileFilter,
21+
cli::ClangParams,
2322
logger::{end_log_group, start_log_group},
2423
};
2524
pub mod clang_format;
@@ -77,20 +76,6 @@ pub fn get_clang_tool_exe(name: &str, version: &str) -> Result<PathBuf, &'static
7776
}
7877
}
7978

80-
#[derive(Debug, Clone)]
81-
pub struct ClangParams {
82-
pub tidy_checks: String,
83-
pub lines_changed_only: LinesChangedOnly,
84-
pub database: Option<PathBuf>,
85-
pub extra_args: Option<Vec<String>>,
86-
pub database_json: Option<CompilationDatabase>,
87-
pub style: String,
88-
pub clang_tidy_command: Option<PathBuf>,
89-
pub clang_format_command: Option<PathBuf>,
90-
pub tidy_filter: FileFilter,
91-
pub format_filter: FileFilter,
92-
}
93-
9479
/// This creates a task to run clang-tidy and clang-format on a single file.
9580
///
9681
/// Returns a Future that infallibly resolves to a 2-tuple that contains

cpp-linter-lib/src/cli.rs renamed to cpp-linter-lib/src/cli/mod.rs

+17-28
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
//! This module holds the Command Line Interface design.
2+
use std::path::PathBuf;
23

34
// non-std crates
45
use clap::builder::{ArgPredicate, FalseyValueParser};
5-
use clap::{Arg, ArgAction, ArgGroup, ArgMatches, Command};
6+
use clap::{value_parser, Arg, ArgAction, ArgGroup, ArgMatches, Command};
67

7-
/// An enum to describe `--lines-changed-only` CLI option's behavior.
8-
#[derive(PartialEq, Clone, Debug)]
9-
pub enum LinesChangedOnly {
10-
/// All lines are scanned
11-
Off,
12-
/// Only lines in the diff are scanned
13-
Diff,
14-
/// Only lines in the diff with additions are scanned.
15-
On,
16-
}
8+
mod structs;
9+
pub use structs::{ClangParams, Cli, FeedbackInput, LinesChangedOnly, ThreadComments};
1710

1811
/// Builds and returns the Command Line Interface's argument parsing object.
1912
pub fn get_arg_parser() -> Command {
@@ -38,9 +31,10 @@ thread comments or file annotations.\n\n",
3831
Arg::new("database")
3932
.long("database")
4033
.short('p')
34+
.value_parser(value_parser!(PathBuf))
4135
.help_heading("clang-tidy options")
4236
.help(
43-
"The path that is used to read a compile command database.
37+
"The path that is used to read a compile command database.
4438
For example, it can be a CMake build directory in which a file named
4539
compile_commands.json exists (set `CMAKE_EXPORT_COMPILE_COMMANDS` to `ON`).
4640
When no build path is specified, a search for compile_commands.json will be
@@ -155,7 +149,6 @@ the current working directory if not using a CI runner).\n\n",
155149
.short('D')
156150
.long("ignore-tidy")
157151
.value_delimiter('|')
158-
.default_value("")
159152
.help_heading("clang-tidy options")
160153
.help(
161154
"Similar to [`--ignore`](#-i---ignore) but applied
@@ -167,7 +160,6 @@ exclusively to files analyzed by clang-tidy.\n\n",
167160
.short('M')
168161
.long("ignore-format")
169162
.value_delimiter('|')
170-
.default_value("")
171163
.help_heading("clang-format options")
172164
.help(
173165
"Similar to [`--ignore`](#-i---ignore) but applied
@@ -178,7 +170,7 @@ exclusively to files analyzed by clang-format.\n\n",
178170
Arg::new("lines-changed-only")
179171
.short('l')
180172
.long("lines-changed-only")
181-
.value_parser(["true", "false", "diff"])
173+
.value_parser(["true", "on", "1", "false", "off", "0", "diff"])
182174
.default_value("true")
183175
.help_heading("Source options")
184176
.help(
@@ -220,21 +212,21 @@ This is automatically enabled if
220212
.action(ArgAction::Append)
221213
.help_heading("clang-tidy options")
222214
.help(
223-
"A string of extra arguments passed to clang-tidy for use as
215+
r#"A string of extra arguments passed to clang-tidy for use as
224216
compiler arguments. This can be specified more than once for each
225217
additional argument. Recommend using quotes around the value and
226218
avoid using spaces between name and value (use `=` instead):
227219
228220
```shell
229-
cpp-linter --extra-arg=\"-std=c++17\" --extra-arg=\"-Wall\"
230-
```",
221+
cpp-linter --extra-arg="-std=c++17" --extra-arg="-Wall"
222+
```"#,
231223
),
232224
)
233225
.arg(
234226
Arg::new("thread-comments")
235227
.long("thread-comments")
236228
.short('g')
237-
.value_parser(["true", "false", "updated"])
229+
.value_parser(["true", "on", "1", "false", "off", "0", "updated"])
238230
.default_value("false")
239231
.help_heading("feedback options")
240232
.help(
@@ -381,11 +373,9 @@ mod test {
381373
fn extra_arg_1() {
382374
let args = parser_args(vec!["cpp-linter", "--extra-arg='-std=c++17 -Wall'"]);
383375
let extras = convert_extra_arg_val(&args);
384-
assert!(extras.is_some());
385-
if let Some(extra_args) = extras {
386-
assert_eq!(extra_args.len(), 2);
387-
assert_eq!(extra_args, ["-std=c++17", "-Wall"])
388-
}
376+
let extra_args = extras.expect("extra-arg not parsed properly");
377+
assert_eq!(extra_args.len(), 2);
378+
assert_eq!(extra_args, ["-std=c++17", "-Wall"])
389379
}
390380

391381
#[test]
@@ -397,9 +387,8 @@ mod test {
397387
]);
398388
let extras = convert_extra_arg_val(&args);
399389
assert!(extras.is_some());
400-
if let Some(extra_args) = extras {
401-
assert_eq!(extra_args.len(), 2);
402-
assert_eq!(extra_args, ["-std=c++17", "-Wall"])
403-
}
390+
let extra_args = extras.expect("extra-arg not parsed properly");
391+
assert_eq!(extra_args.len(), 2);
392+
assert_eq!(extra_args, ["-std=c++17", "-Wall"])
404393
}
405394
}

cpp-linter-lib/src/cli/structs.rs

+198
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
use std::path::PathBuf;
2+
3+
use clap::ArgMatches;
4+
5+
use super::convert_extra_arg_val;
6+
use crate::{clang_tools::clang_tidy::CompilationDatabase, common_fs::FileFilter};
7+
8+
/// An enum to describe `--lines-changed-only` CLI option's behavior.
9+
#[derive(PartialEq, Clone, Debug)]
10+
pub enum LinesChangedOnly {
11+
/// All lines are scanned
12+
Off,
13+
/// Only lines in the diff are scanned
14+
Diff,
15+
/// Only lines in the diff with additions are scanned.
16+
On,
17+
}
18+
19+
impl LinesChangedOnly {
20+
fn from_string(val: &str) -> LinesChangedOnly {
21+
match val {
22+
"true" | "on" | "1" => LinesChangedOnly::On,
23+
"diff" => LinesChangedOnly::Diff,
24+
_ => LinesChangedOnly::Off,
25+
}
26+
}
27+
}
28+
29+
/// A structure to contain parsed CLI options.
30+
pub struct Cli {
31+
pub version: String,
32+
pub verbosity: bool,
33+
pub extensions: Vec<String>,
34+
pub repo_root: String,
35+
pub lines_changed_only: LinesChangedOnly,
36+
pub files_changed_only: bool,
37+
pub ignore: Vec<String>,
38+
pub style: String,
39+
pub ignore_format: Vec<String>,
40+
pub ignore_tidy: Vec<String>,
41+
pub tidy_checks: String,
42+
pub database: Option<PathBuf>,
43+
pub extra_arg: Option<Vec<String>>,
44+
pub thread_comments: ThreadComments,
45+
pub no_lgtm: bool,
46+
pub step_summary: bool,
47+
pub file_annotations: bool,
48+
}
49+
50+
impl From<&ArgMatches> for Cli {
51+
/// Construct a [`Cli`] instance from a [`ArgMatches`] instance (after options are parsed from CLI).
52+
fn from(args: &ArgMatches) -> Self {
53+
let ignore = args
54+
.get_many::<String>("ignore")
55+
.unwrap()
56+
.map(|s| s.to_owned())
57+
.collect::<Vec<_>>();
58+
let ignore_tidy = if let Some(val) = args.get_many::<String>("ignore-tidy") {
59+
val.map(|s| s.to_owned()).collect::<Vec<_>>()
60+
} else {
61+
vec![]
62+
};
63+
let ignore_format = if let Some(val) = args.get_many::<String>("ignore-format") {
64+
val.map(|s| s.to_owned()).collect::<Vec<_>>()
65+
} else {
66+
vec![]
67+
};
68+
let extra_arg = convert_extra_arg_val(args);
69+
70+
let lines_changed_only = LinesChangedOnly::from_string(
71+
args.get_one::<String>("lines-changed-only")
72+
.unwrap()
73+
.as_str(),
74+
);
75+
76+
let thread_comments = ThreadComments::from_string(
77+
args.get_one::<String>("thread-comments").unwrap().as_str(),
78+
);
79+
80+
let extensions = args
81+
.get_many::<String>("extensions")
82+
.unwrap()
83+
.map(|s| s.to_string())
84+
.collect::<Vec<_>>();
85+
86+
Self {
87+
version: args.get_one::<String>("version").unwrap().to_owned(),
88+
verbosity: args.get_one::<String>("verbosity").unwrap().as_str() == "debug",
89+
extensions,
90+
repo_root: args.get_one::<String>("repo-root").unwrap().to_owned(),
91+
lines_changed_only,
92+
files_changed_only: args.get_flag("files-changed-only"),
93+
ignore,
94+
style: args.get_one::<String>("style").unwrap().to_owned(),
95+
ignore_format,
96+
ignore_tidy,
97+
tidy_checks: args.get_one::<String>("tidy-checks").unwrap().to_owned(),
98+
database: args.get_one::<PathBuf>("database").map(|v| v.to_owned()),
99+
extra_arg,
100+
no_lgtm: args.get_flag("no-lgtm"),
101+
step_summary: args.get_flag("step-summary"),
102+
thread_comments,
103+
file_annotations: args.get_flag("file-annotations"),
104+
}
105+
}
106+
}
107+
108+
/// An enum to describe `--thread-comments` CLI option's behavior.
109+
#[derive(PartialEq, Clone, Debug)]
110+
pub enum ThreadComments {
111+
/// Always post a new comment and delete any outdated ones.
112+
On,
113+
/// Do not post thread comments.
114+
Off,
115+
/// Only update existing thread comments.
116+
/// If none exist, then post a new one.
117+
Update,
118+
}
119+
120+
impl ThreadComments {
121+
fn from_string(val: &str) -> ThreadComments {
122+
match val {
123+
"true" | "on" | "1" => ThreadComments::On,
124+
"update" => ThreadComments::Update,
125+
_ => ThreadComments::Off,
126+
}
127+
}
128+
}
129+
130+
/// A data structure to contain CLI options that relate to
131+
/// clang-tidy or clang-format arguments.
132+
#[derive(Debug, Clone)]
133+
pub struct ClangParams {
134+
pub tidy_checks: String,
135+
pub lines_changed_only: LinesChangedOnly,
136+
pub database: Option<PathBuf>,
137+
pub extra_args: Option<Vec<String>>,
138+
pub database_json: Option<CompilationDatabase>,
139+
pub style: String,
140+
pub clang_tidy_command: Option<PathBuf>,
141+
pub clang_format_command: Option<PathBuf>,
142+
pub tidy_filter: FileFilter,
143+
pub format_filter: FileFilter,
144+
}
145+
146+
impl From<&Cli> for ClangParams {
147+
/// Construct a [`ClangParams`] instance from a [`Cli`] instance.
148+
fn from(args: &Cli) -> Self {
149+
ClangParams {
150+
tidy_checks: args.tidy_checks.clone(),
151+
lines_changed_only: args.lines_changed_only.clone(),
152+
database: args.database.clone(),
153+
extra_args: args.extra_arg.clone(),
154+
database_json: None,
155+
style: args.style.clone(),
156+
clang_tidy_command: None,
157+
clang_format_command: None,
158+
tidy_filter: FileFilter::new(&args.ignore_tidy, args.extensions.clone()),
159+
format_filter: FileFilter::new(&args.ignore_format, args.extensions.clone()),
160+
}
161+
}
162+
}
163+
164+
/// A struct to contain CLI options that relate to
165+
/// [`ResApiClient.post_feedback()`](fn@crate::rest_api::ResApiClient.post_feedback()).
166+
pub struct FeedbackInput {
167+
pub thread_comments: ThreadComments,
168+
pub no_lgtm: bool,
169+
pub step_summary: bool,
170+
pub file_annotations: bool,
171+
pub style: String,
172+
}
173+
174+
impl From<&Cli> for FeedbackInput {
175+
/// Construct a [`FeedbackInput`] instance from a [`Cli`] instance.
176+
fn from(args: &Cli) -> Self {
177+
FeedbackInput {
178+
style: args.style.clone(),
179+
no_lgtm: args.no_lgtm,
180+
step_summary: args.step_summary,
181+
thread_comments: args.thread_comments.clone(),
182+
file_annotations: args.file_annotations,
183+
}
184+
}
185+
}
186+
187+
impl Default for FeedbackInput {
188+
/// Construct a [`FeedbackInput`] instance with default values.
189+
fn default() -> Self {
190+
FeedbackInput {
191+
thread_comments: ThreadComments::Off,
192+
no_lgtm: true,
193+
step_summary: false,
194+
file_annotations: true,
195+
style: "llvm".to_string(),
196+
}
197+
}
198+
}

0 commit comments

Comments
 (0)