add strictness to tests

Author: 2bndy5

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

@@ -77,6 +77,20 @@ impl Clone for Replacement {
     }
 }
 
+/// Get a string that summarizes the given `--style`
+pub fn summarize_style(style: &str) -> String {
+    if ["google", "chromium", "microsoft", "mozilla", "webkit"].contains(&style) {
+        // capitalize the first letter
+        let mut char_iter = style.chars();
+        let first_char = char_iter.next().unwrap();
+        first_char.to_uppercase().collect::<String>() + char_iter.as_str()
+    } else if style == "llvm" || style == "gnu" {
+        style.to_ascii_uppercase()
+    } else {
+        String::from("Custom")
+    }
+}
+
 /// Get a total count of clang-format advice from the given list of [FileObj]s.
 pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
     let mut total = 0;
@@ -199,7 +213,7 @@ pub fn run_clang_format(
 
 #[cfg(test)]
 mod tests {
-    use super::{FormatAdvice, Replacement};
+    use super::{summarize_style, FormatAdvice, Replacement};
     use serde::Deserialize;
 
     #[test]
@@ -257,4 +271,23 @@ mod tests {
                 .unwrap();
         assert_eq!(expected, document);
     }
+
+    fn formalize_style(style: &str, expected: &str) {
+        assert_eq!(summarize_style(style), expected);
+    }
+
+    #[test]
+    fn formalize_llvm_style() {
+        formalize_style("llvm", "LLVM");
+    }
+
+    #[test]
+    fn formalize_google_style() {
+        formalize_style("google", "Google");
+    }
+
+    #[test]
+    fn formalize_custom_style() {
+        formalize_style("file", "Custom");
+    }
 }

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

@@ -258,7 +258,7 @@ impl ReviewComments {
                     .as_str(),
                 );
             }
-            if total > 0 {
+            if !self.full_patch[t as usize].is_empty() {
                 body.push_str(
                     format!(
                         "\n<details><summary>Click here for the full {tool_name} patch</summary>\n\n```diff\n{}```\n\n</details>\n",
@@ -267,7 +267,11 @@ impl ReviewComments {
                 );
             } else {
                 body.push_str(
-                    format!("\nNo concerns reported by {}. Great job! :tada:", tool_name).as_str(),
+                    format!(
+                        "\nNo concerns reported by {}. Great job! :tada:\n",
+                        tool_name
+                    )
+                    .as_str(),
                 )
             }
         }
@@ -350,6 +354,9 @@ pub trait MakeSuggestions {
                 .expect("Failed to convert patch buffer to string")
                 .as_str(),
         );
+        if summary_only {
+            return;
+        }
         for hunk_id in 0..hunks_total {
             let (hunk, line_count) = patch.hunk(hunk_id).expect("Failed to get hunk from patch");
             hunks_in_patch += 1;
@@ -359,51 +366,43 @@ pub trait MakeSuggestions {
             }
             let (start_line, end_line) = hunk_range.unwrap();
             let mut suggestion = String::new();
-            let suggestion_help = if !summary_only {
-                Some(self.get_suggestion_help(start_line, end_line))
-            } else {
-                None
-            };
+            let suggestion_help = self.get_suggestion_help(start_line, end_line);
             let mut removed = vec![];
             for line_index in 0..line_count {
                 let diff_line = patch
                     .line_in_hunk(hunk_id, line_index)
                     .expect("Failed to get line in a hunk");
                 let line = String::from_utf8(diff_line.content().to_owned())
                     .expect("Failed to convert line buffer to string");
-                if !summary_only {
-                    if ['+', ' '].contains(&diff_line.origin()) {
-                        suggestion.push_str(line.as_str());
-                    } else {
-                        removed.push(
-                            diff_line
-                                .old_lineno()
-                                .expect("Removed line has no line number?!"),
-                        );
-                    }
+                if ['+', ' '].contains(&diff_line.origin()) {
+                    suggestion.push_str(line.as_str());
+                } else {
+                    removed.push(
+                        diff_line
+                            .old_lineno()
+                            .expect("Removed line has no line number?!"),
+                    );
                 }
             }
-            if !summary_only {
-                if suggestion.is_empty() && !removed.is_empty() {
-                    suggestion.push_str(
-                        format!(
-                            "Please remove the line(s)\n- {}",
-                            removed
-                                .iter()
-                                .map(|l| l.to_string())
-                                .collect::<Vec<String>>()
-                                .join("\n- ")
-                        )
-                        .as_str(),
+            if suggestion.is_empty() && !removed.is_empty() {
+                suggestion.push_str(
+                    format!(
+                        "Please remove the line(s)\n- {}",
+                        removed
+                            .iter()
+                            .map(|l| l.to_string())
+                            .collect::<Vec<String>>()
+                            .join("\n- ")
                     )
-                } else {
-                    suggestion = format!("```suggestion\n{suggestion}```",);
-                }
+                    .as_str(),
+                )
+            } else {
+                suggestion = format!("```suggestion\n{suggestion}```",);
             }
             let comment = Suggestion {
                 line_start: start_line,
                 line_end: end_line,
-                suggestion: format!("{}\n{suggestion}", suggestion_help.unwrap_or_default()),
+                suggestion: format!("{suggestion_help}\n{suggestion}"),
                 path: file_name.clone(),
             };
             if !review_comments.is_comment_in_suggestions(&comment) {
@@ -463,4 +462,17 @@ mod tests {
             .to_string()
             .contains(TOOL_NAME)));
     }
+
+    #[test]
+    fn get_exe_by_invalid_path() {
+        let tool_exe = get_clang_tool_exe(TOOL_NAME, "non-existent-path");
+        assert!(tool_exe.is_err());
+    }
+
+    #[test]
+    fn get_exe_by_invalid_name() {
+        let clang_version = env::var("CLANG_VERSION").unwrap_or("16".to_string());
+        let tool_exe = get_clang_tool_exe("not-a-clang-tool", &clang_version);
+        assert!(tool_exe.is_err());
+    }
 }

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

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::{fmt::Display, path::PathBuf};
 
 use clap::ArgMatches;
 
@@ -27,6 +27,16 @@ impl LinesChangedOnly {
     }
 }
 
+impl Display for LinesChangedOnly {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            LinesChangedOnly::Off => write!(f, "false"),
+            LinesChangedOnly::Diff => write!(f, "diff"),
+            LinesChangedOnly::On => write!(f, "true"),
+        }
+    }
+}
+
 /// A structure to contain parsed CLI options.
 pub struct Cli {
     pub version: String,
@@ -134,6 +144,16 @@ impl ThreadComments {
     }
 }
 
+impl Display for ThreadComments {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ThreadComments::On => write!(f, "true"),
+            ThreadComments::Off => write!(f, "false"),
+            ThreadComments::Update => write!(f, "update"),
+        }
+    }
+}
+
 /// A data structure to contain CLI options that relate to
 /// clang-tidy or clang-format arguments.
 #[derive(Debug, Clone, Default)]
@@ -227,7 +247,7 @@ impl Default for FeedbackInput {
 mod test {
     use crate::cli::get_arg_parser;
 
-    use super::Cli;
+    use super::{Cli, LinesChangedOnly, ThreadComments};
 
     #[test]
     fn parse_positional() {
@@ -239,4 +259,21 @@ mod test {
         assert!(not_ignored.contains(&String::from("file1.c")));
         assert!(not_ignored.contains(&String::from("file2.h")));
     }
+
+    #[test]
+    fn display_lines_changed_only_enum() {
+        let input = "diff".to_string();
+        assert_eq!(
+            LinesChangedOnly::from_string(&input),
+            LinesChangedOnly::Diff
+        );
+        assert_eq!(format!("{}", LinesChangedOnly::Diff), input);
+    }
+
+    #[test]
+    fn display_thread_comments_enum() {
+        let input = "false".to_string();
+        assert_eq!(ThreadComments::from_string(&input), ThreadComments::Off);
+        assert_eq!(format!("{}", ThreadComments::Off), input);
+    }
 }

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

@@ -130,26 +130,30 @@ impl FileObj {
     ) {
         let original_content =
             fs::read(&self.name).expect("Failed to read original contents of file");
-
+        let file_name = self
+            .name
+            .to_str()
+            .expect("Failed to convert file extension to string")
+            .replace("\\", "/");
+        let file_path = Path::new(&file_name);
         if let Some(advice) = &self.format_advice {
             if let Some(patched) = &advice.patched {
-                let mut patch = make_patch(&self.name, patched, &original_content);
+                let mut patch = make_patch(file_path, patched, &original_content);
                 advice.get_suggestions(review_comments, self, &mut patch, summary_only);
             }
         }
 
         if let Some(advice) = &self.tidy_advice {
             if let Some(patched) = &advice.patched {
-                let mut patch = make_patch(&self.name, patched, &original_content);
+                let mut patch = make_patch(file_path, patched, &original_content);
                 advice.get_suggestions(review_comments, self, &mut patch, summary_only);
             }
 
+            if summary_only {
+                return;
+            }
+
             // now check for clang-tidy warnings with no fixes applied
-            let file_name = self
-                .name
-                .to_str()
-                .expect("Failed to convert file extension to string")
-                .replace("\\", "/");
             let file_ext = self
                 .name
                 .extension()