final review

Author: 2bndy5

cpp-linter/src/clang_tools/mod.rs

@@ -146,7 +146,7 @@ pub struct ClangVersions {
     pub tidy_version: Option<String>,
 }
 
-/// Run `clang-tool --version`, the extract and return the version number.
+/// Run `clang-tool --version`, then extract and return the version number.
 fn capture_clang_version(clang_tool: &PathBuf) -> Result<String> {
     let output = Command::new(clang_tool).arg("--version").output()?;
     let stdout = String::from_utf8_lossy(&output.stdout);
@@ -266,10 +266,12 @@ impl ReviewComments {
                 ("clang-tidy", clang_versions.tidy_version.as_ref())
             };
 
-            if self.tool_total[t as usize].is_none() {
+            let tool_total = if let Some(total) = self.tool_total[t as usize] {
+                total
+            } else {
                 // review was not requested from this tool or the tool was not used at all
                 continue;
-            }
+            };
 
             // If the tool's version is unknown, then we don't need to output this line.
             // NOTE: If the tool was invoked at all, then the tool's version shall be known.
@@ -285,8 +287,6 @@ impl ReviewComments {
                 }
             }
 
-            // at this point tool_total is Some() value; unwrap() is safe here
-            let tool_total = self.tool_total[t as usize].unwrap();
             if total != tool_total {
                 body.push_str(
                     format!(
@@ -389,9 +389,7 @@ pub trait MakeSuggestions {
                 .with_context(|| format!("Failed to convert patch to string: {file_name}"))?
                 .as_str(),
         );
-        if review_comments.tool_total[is_tidy_tool as usize].is_none() {
-            review_comments.tool_total[is_tidy_tool as usize] = Some(0);
-        }
+        review_comments.tool_total[is_tidy_tool as usize].get_or_insert(0);
         if summary_only {
             return Ok(());
         }

cpp-linter/src/common_fs/mod.rs

@@ -158,6 +158,7 @@ impl FileObj {
                 .unwrap_or_default()
                 .to_str()
                 .unwrap_or_default();
+            // Count of clang-tidy diagnostics that had no fixes applied
             let mut total = 0;
             for note in &advice.notes {
                 if note.fixed_lines.is_empty() {

cpp-linter/tests/reviews.rs

@@ -60,6 +60,17 @@ impl Default for TestParams {
     }
 }
 
+fn generate_tool_summary(review_enabled: bool, force_lgtm: bool, tool_name: &str) -> String {
+    if !review_enabled {
+        return String::new();
+    }
+    if force_lgtm {
+        format!("No concerns reported by {}. Great job! :tada:", tool_name)
+    } else {
+        format!("Click here for the full {} patch", tool_name)
+    }
+}
+
 async fn setup(lib_root: &Path, test_params: &TestParams) {
     env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
     env::set_var("GITHUB_EVENT_NAME", "pull_request");
@@ -161,24 +172,16 @@ async fn setup(lib_root: &Path, test_params: &TestParams) {
         } else {
             "REQUEST_CHANGES"
         };
-        let tidy_summary = if test_params.tidy_review {
-            if test_params.force_lgtm {
-                "No concerns reported by clang-tidy. Great job! :tada:"
-            } else {
-                "Click here for the full clang-tidy patch"
-            }
-        } else {
-            ""
-        };
-        let format_summary = if test_params.format_review {
-            if test_params.force_lgtm {
-                "No concerns reported by clang-format. Great job! :tada:"
-            } else {
-                "Click here for the full clang-format patch"
-            }
-        } else {
-            ""
-        };
+        let tidy_summary = generate_tool_summary(
+            test_params.tidy_review,
+            test_params.force_lgtm,
+            "clang-tidy",
+        );
+        let format_summary = generate_tool_summary(
+            test_params.format_review,
+            test_params.force_lgtm,
+            "clang-format",
+        );
         let review_summary = format!(
             "{}## Cpp-linter Review.*{format_summary}.*{tidy_summary}.*{}",
             regex::escape(format!("{}", COMMENT_MARKER.escape_default()).as_str()),