various improvements to src

2bndy5 · 2bndy5 · commit 25bee9269415 · 2025-01-02T00:54:56.000-08:00
ignore logger init errors
diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs
@@ -135,8 +135,8 @@ pub fn run_clang_format(
                     .to_str()
                     .unwrap_or_default(),
                 cmd.get_args()
-                    .map(|a| a.to_str().unwrap())
-                    .collect::<Vec<&str>>()
+                    .map(|a| a.to_string_lossy())
+                    .collect::<Vec<_>>()
                     .join(" ")
             ),
         ));
@@ -153,8 +153,8 @@ pub fn run_clang_format(
             "Running \"{} {}\"",
             cmd.get_program().to_string_lossy(),
             cmd.get_args()
-                .map(|x| x.to_str().unwrap())
-                .collect::<Vec<&str>>()
+                .map(|x| x.to_string_lossy())
+                .collect::<Vec<_>>()
                 .join(" ")
         ),
     ));
diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs
@@ -264,15 +264,17 @@ pub fn run_clang_tidy(
     let file_name = file.name.to_string_lossy().to_string();
     if clang_params.lines_changed_only != LinesChangedOnly::Off {
         let ranges = file.get_ranges(&clang_params.lines_changed_only);
-        let filter = format!(
-            "[{{\"name\":{:?},\"lines\":{:?}}}]",
-            &file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
-            ranges
-                .iter()
-                .map(|r| [r.start(), r.end()])
-                .collect::<Vec<_>>()
-        );
-        cmd.args(["--line-filter", filter.as_str()]);
+        if !ranges.is_empty() {
+            let filter = format!(
+                "[{{\"name\":{:?},\"lines\":{:?}}}]",
+                &file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
+                ranges
+                    .iter()
+                    .map(|r| [r.start(), r.end()])
+                    .collect::<Vec<_>>()
+            );
+            cmd.args(["--line-filter", filter.as_str()]);
+        }
     }
     let mut original_content = None;
     if clang_params.tidy_review {
@@ -294,8 +296,8 @@ pub fn run_clang_tidy(
             "Running \"{} {}\"",
             cmd.get_program().to_string_lossy(),
             cmd.get_args()
-                .map(|x| x.to_str().unwrap())
-                .collect::<Vec<&str>>()
+                .map(|x| x.to_string_lossy())
+                .collect::<Vec<_>>()
                 .join(" ")
         ),
     ));
diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs
@@ -390,14 +390,13 @@ pub fn convert_extra_arg_val(args: &ArgMatches) -> Vec<String> {
     let mut val = args.get_many::<String>("extra-arg").unwrap_or_default();
     if val.len() == 1 {
         // specified once; split and return result
-        return val
-            .next()
+        val.next()
             .unwrap()
             .trim_matches('\'')
             .trim_matches('"')
             .split(' ')
             .map(|i| i.to_string())
-            .collect();
+            .collect()
     } else {
         // specified multiple times; just return
         val.map(|i| i.to_string()).collect()
diff --git a/cpp-linter/src/common_fs/file_filter.rs b/cpp-linter/src/common_fs/file_filter.rs
@@ -105,10 +105,15 @@ impl FileFilter {
             let glob_matched =
                 glob_match(pattern, file_name.to_string_lossy().to_string().as_str());
             let pat = PathBuf::from(&pattern);
-            if glob_matched
+            if pattern.as_str() == "./"
+                || glob_matched
                 || (pat.is_file() && file_name == pat)
                 || (pat.is_dir() && file_name.starts_with(pat))
             {
+                log::debug!(
+                    "file {file_name:?} is in {}ignored with domain {pattern:?}.",
+                    if is_ignored { "" } else { "not " }
+                );
                 return true;
             }
         }
diff --git a/cpp-linter/src/git.rs b/cpp-linter/src/git.rs
@@ -105,13 +105,10 @@ pub fn parse_diff(diff: &git2::Diff, file_filter: &FileFilter) -> Vec<FileObj> {
     for file_idx in 0..diff.deltas().count() {
         let diff_delta = diff.get_delta(file_idx).unwrap();
         let file_path = diff_delta.new_file().path().unwrap().to_path_buf();
-        if [
-            git2::Delta::Added,
-            git2::Delta::Modified,
-            git2::Delta::Renamed,
-        ]
-        .contains(&diff_delta.status())
-            && file_filter.is_source_or_ignored(&file_path)
+        if matches!(
+            diff_delta.status(),
+            git2::Delta::Added | git2::Delta::Modified | git2::Delta::Renamed,
+        ) && file_filter.is_source_or_ignored(&file_path)
         {
             let (added_lines, diff_chunks) =
                 parse_patch(&Patch::from_diff(diff, file_idx).unwrap().unwrap());
diff --git a/cpp-linter/src/logger.rs b/cpp-linter/src/logger.rs
@@ -2,7 +2,6 @@
 
 use std::env;
 
-use anyhow::{Error, Result};
 use colored::{control::set_override, Colorize};
 use log::{Level, LevelFilter, Metadata, Record};
 
@@ -46,30 +45,35 @@ impl log::Log for SimpleLogger {
 /// A function to initialize the private `LOGGER`.
 ///
 /// The logging level defaults to [`LevelFilter::Info`].
-/// Returns a [`SetLoggerError`](struct@log::SetLoggerError) if the `LOGGER` is already initialized.
-pub fn init() -> Result<()> {
+/// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError)
+/// if the `LOGGER` is already initialized.
+pub fn try_init() {
     let logger: SimpleLogger = SimpleLogger;
     if matches!(
         env::var("CPP_LINTER_COLOR").as_deref(),
         Ok("on" | "1" | "true")
     ) {
         set_override(true);
     }
-    log::set_boxed_logger(Box::new(logger))
-        .map(|()| log::set_max_level(LevelFilter::Info))
-        .map_err(Error::from)
+    if let Err(e) =
+        log::set_boxed_logger(Box::new(logger)).map(|()| log::set_max_level(LevelFilter::Info))
+    {
+        // logger singleton already instantiated.
+        // we'll just use whatever the current config is.
+        log::debug!("{e:?}");
+    }
 }
 
 #[cfg(test)]
 mod test {
     use std::env;
 
-    use super::{init, SimpleLogger};
+    use super::{try_init, SimpleLogger};
 
     #[test]
     fn trace_log() {
         env::set_var("CPP_LINTER_COLOR", "true");
-        init().unwrap_or(());
+        try_init();
         assert!(SimpleLogger::level_color(&log::Level::Trace).contains("TRACE"));
         log::set_max_level(log::LevelFilter::Trace);
         log::trace!("A dummy log statement for code coverage");
diff --git a/cpp-linter/src/rest_api/github/mod.rs b/cpp-linter/src/rest_api/github/mod.rs
@@ -241,7 +241,7 @@ impl RestApiClient for GithubApiClient {
             self.post_annotations(files, feedback_inputs.style.as_str());
         }
         if feedback_inputs.step_summary {
-            comment = Some(self.make_comment(
+            comment = Some(Self::make_comment(
                 files,
                 format_checks_failed,
                 tidy_checks_failed,
@@ -259,7 +259,7 @@ impl RestApiClient for GithubApiClient {
         if feedback_inputs.thread_comments != ThreadComments::Off {
             // post thread comment for PR or push event
             if comment.as_ref().is_some_and(|c| c.len() > 65535) || comment.is_none() {
-                comment = Some(self.make_comment(
+                comment = Some(Self::make_comment(
                     files,
                     format_checks_failed,
                     tidy_checks_failed,
@@ -334,7 +334,7 @@ mod test {
     ) -> (String, String) {
         let tmp_dir = tempdir().unwrap();
         let rest_api_client = GithubApiClient::new().unwrap();
-        logger::init().unwrap();
+        logger::try_init();
         if env::var("ACTIONS_STEP_DEBUG").is_ok_and(|var| var == "true") {
             assert!(rest_api_client.debug_enabled);
             log::set_max_level(log::LevelFilter::Debug);
diff --git a/cpp-linter/src/rest_api/github/serde_structs.rs b/cpp-linter/src/rest_api/github/serde_structs.rs
@@ -49,6 +49,8 @@ pub struct GithubChangedFile {
     pub previous_filename: Option<String>,
     /// The individual patch that describes the file's changes.
     pub patch: Option<String>,
+    /// The number of changes to the file contents.
+    pub changes: i64,
 }
 
 /// A structure for deserializing a Push event's changed files.
diff --git a/cpp-linter/src/rest_api/mod.rs b/cpp-linter/src/rest_api/mod.rs
@@ -230,7 +230,6 @@ pub trait RestApiClient {
     /// Returns the markdown comment as a string as well as the total count of
     /// `format_checks_failed` and `tidy_checks_failed` (in respective order).
     fn make_comment(
-        &self,
         files: &[Arc<Mutex<FileObj>>],
         format_checks_failed: u64,
         tidy_checks_failed: u64,
@@ -513,7 +512,7 @@ mod test {
         assert!(headers
             .insert("link", HeaderValue::from_str("; rel=\"next\"").unwrap())
             .is_none());
-        logger::init().unwrap();
+        logger::try_init();
         log::set_max_level(log::LevelFilter::Debug);
         let result = TestClient::try_next_page(&headers);
         assert!(result.is_none());
@@ -528,7 +527,7 @@ mod test {
                 HeaderValue::from_str("<not a domain>; rel=\"next\"").unwrap()
             )
             .is_none());
-        logger::init().unwrap();
+        logger::try_init();
         log::set_max_level(log::LevelFilter::Debug);
         let result = TestClient::try_next_page(&headers);
         assert!(result.is_none());
@@ -553,7 +552,7 @@ mod test {
             remaining: "remaining".to_string(),
             retry: "retry".to_string(),
         };
-        logger::init().unwrap();
+        logger::try_init();
         log::set_max_level(log::LevelFilter::Debug);
 
         let mut server = Server::new_async().await;
diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs
@@ -58,9 +58,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
         return Ok(());
     }
 
-    if let Err(e) = logger::init() {
-        log::warn!("logger attempt to re-init: {e:?}");
-    }
+    logger::try_init();
 
     if cli.version == "NO-VERSION" {
         log::error!("The `--version` arg is used to specify which version of clang to use.");
@@ -147,12 +145,8 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
         .post_feedback(&arc_files, user_inputs, clang_versions)
         .await?;
     rest_api_client.end_log_group();
-    if env::var("PRE_COMMIT").is_ok_and(|v| v == "1") {
-        if checks_failed > 1 {
-            return Err(anyhow!("Some checks did not pass"));
-        } else {
-            return Ok(());
-        }
+    if env::var("PRE_COMMIT").is_ok_and(|v| v == "1") && checks_failed > 1 {
+        return Err(anyhow!("Some checks did not pass"));
     }
     Ok(())
 }
diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs
@@ -74,7 +74,7 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) {
     let mut server = mock_server().await;
     env::set_var("GITHUB_API_URL", server.url());
     env::set_current_dir(tmp.path()).unwrap();
-    logger::init().unwrap();
+    logger::try_init();
     log::set_max_level(log::LevelFilter::Debug);
     let gh_client = GithubApiClient::new();
     if test_params.fail_serde_event_payload || test_params.no_event_payload {

Original file line number	Diff line number	Diff line change
`@@ -105,10 +105,15 @@ impl FileFilter {`
`105`	`105`	`let glob_matched =`
`106`	`106`	`glob_match(pattern, file_name.to_string_lossy().to_string().as_str());`
`107`	`107`	`let pat = PathBuf::from(&pattern);`
`108`		`- if glob_matched`
	`108`	`+ if pattern.as_str() == "./"`
	`109`	`+ \|\| glob_matched`
`109`	`110`	`\|\| (pat.is_file() && file_name == pat)`
`110`	`111`	`\|\| (pat.is_dir() && file_name.starts_with(pat))`
`111`	`112`	`{`
	`113`	`+ log::debug!(`
	`114`	`+ "file {file_name:?} is in {}ignored with domain {pattern:?}.",`
	`115`	`+ if is_ignored { "" } else { "not " }`
	`116`	`+ );`
`112`	`117`	`return true;`
`113`	`118`	`}`
`114`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,8 @@ pub struct GithubChangedFile {`
`49`	`49`	`pub previous_filename: Option<String>,`
`50`	`50`	`/// The individual patch that describes the file's changes.`
`51`	`51`	`pub patch: Option<String>,`
	`52`	`+ /// The number of changes to the file contents.`
	`53`	`+ pub changes: i64,`
`52`	`54`	`}`
`53`	`55`
`54`	`56`	`/// A structure for deserializing a Push event's changed files.`