style: start phasing out .unwrap() calls

cpp-linter/clippy.toml

@@ -0,0 +1 @@
+allow-unwrap-in-tests = true

cpp-linter/src/clang_tools/clang_format.rs

@@ -7,7 +7,7 @@ use std::{
     sync::{Arc, Mutex, MutexGuard},
 };
 
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, anyhow};
 use log::Level;
 use serde::Deserialize;
 
@@ -54,11 +54,12 @@ pub struct Replacement {
 
 /// Get a string that summarizes the given `--style`
 pub fn summarize_style(style: &str) -> String {
-    if ["google", "chromium", "microsoft", "mozilla", "webkit"].contains(&style) {
+    let mut char_iter = style.chars();
+    if ["google", "chromium", "microsoft", "mozilla", "webkit"].contains(&style)
+        && let Some(first_char) = char_iter.next()
+    {
         // 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()
+        first_char.to_ascii_uppercase().to_string() + char_iter.as_str()
     } else if style == "llvm" || style == "gnu" {
         style.to_ascii_uppercase()
     } else {
@@ -67,25 +68,31 @@ pub fn summarize_style(style: &str) -> String {
 }
 
 /// 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 {
+pub fn tally_format_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64> {
     let mut total = 0;
     for file in files {
-        let file = file.lock().unwrap();
+        let file = file
+            .lock()
+            .map_err(|_| anyhow!("Failed to acquire lock on mutex for a source file"))?;
         if let Some(advice) = &file.format_advice
             && !advice.replacements.is_empty()
         {
             total += 1;
         }
     }
-    total
+    Ok(total)
 }
 
 /// Run clang-format for a specific `file`, then parse and return it's XML output.
 pub fn run_clang_format(
     file: &mut MutexGuard<FileObj>,
     clang_params: &ClangParams,
 ) -> Result<Vec<(log::Level, String)>> {
-    let mut cmd = Command::new(clang_params.clang_format_command.as_ref().unwrap());
+    let cmd_path = clang_params
+        .clang_format_command
+        .as_ref()
+        .ok_or(anyhow!("clang-format path unknown"))?;
+    let mut cmd = Command::new(cmd_path);
     let mut logs = vec![];
     cmd.args(["--style", &clang_params.style]);
     let ranges = file.get_ranges(&clang_params.lines_changed_only);
@@ -101,12 +108,7 @@ pub fn run_clang_format(
             Level::Info,
             format!(
                 "Getting format fixes with \"{} {}\"",
-                clang_params
-                    .clang_format_command
-                    .as_ref()
-                    .unwrap()
-                    .to_str()
-                    .unwrap_or_default(),
+                cmd.get_program().to_string_lossy(),
                 cmd.get_args()
                     .map(|a| a.to_string_lossy())
                     .collect::<Vec<_>>()

cpp-linter/src/clang_tools/clang_tidy.rs

@@ -10,7 +10,7 @@ use std::{
 };
 
 // non-std crates
-use anyhow::{Context, Result};
+use anyhow::{Context, Result, anyhow};
 use regex::Regex;
 use serde::Deserialize;
 
@@ -148,14 +148,18 @@ fn parse_tidy_output(
     tidy_stdout: &[u8],
     database_json: &Option<Vec<CompilationUnit>>,
 ) -> Result<TidyAdvice> {
-    let note_header = Regex::new(NOTE_HEADER).unwrap();
-    let fixed_note =
-        Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$").unwrap();
+    let note_header = Regex::new(NOTE_HEADER)
+        .with_context(|| "Failed to compile RegExp pattern for note header")?;
+    let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")
+        .with_context(|| "Failed to compile RegExp pattern for fixed note")?;
     let mut found_fix = false;
     let mut notification = None;
     let mut result = Vec::new();
-    let cur_dir = current_dir().unwrap();
-    for line in String::from_utf8(tidy_stdout.to_vec()).unwrap().lines() {
+    let cur_dir = current_dir().with_context(|| "Failed to access current working directory")?;
+    for line in String::from_utf8(tidy_stdout.to_vec())
+        .with_context(|| "Failed to convert clang-tidy stdout to UTF-8 string")?
+        .lines()
+    {
         if let Some(captured) = note_header.captures(line) {
             // First check that the diagnostic name is a actual diagnostic name.
             // Sometimes clang-tidy uses square brackets to enclose additional context
@@ -197,14 +201,12 @@ fn parse_tidy_output(
                     filename = normalize_path(&PathBuf::from_iter([&cur_dir, &filename]));
                 }
                 debug_assert!(filename.is_absolute());
-                if filename.is_absolute() && filename.starts_with(&cur_dir) {
+                if filename.is_absolute()
+                    && let Ok(file_n) = filename.strip_prefix(&cur_dir)
+                {
                     // if this filename can't be made into a relative path, then it is
                     // likely not a member of the project's sources (ie /usr/include/stdio.h)
-                    filename = filename
-                        .strip_prefix(&cur_dir)
-                        // we already checked above that filename.starts_with(current_directory)
-                        .unwrap()
-                        .to_path_buf();
+                    filename = file_n.to_path_buf();
                 }
 
                 notification = Some(TidyNotification {
@@ -247,10 +249,12 @@ fn parse_tidy_output(
 }
 
 /// Get a total count of clang-tidy advice from the given list of [FileObj]s.
-pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
+pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64> {
     let mut total = 0;
     for file in files {
-        let file = file.lock().unwrap();
+        let file = file
+            .lock()
+            .map_err(|_| anyhow!("Failed to acquire lock on mutex for a source file"))?;
         if let Some(advice) = &file.tidy_advice {
             for tidy_note in &advice.notes {
                 let file_path = PathBuf::from(&tidy_note.filename);
@@ -260,15 +264,19 @@ pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> u64 {
             }
         }
     }
-    total
+    Ok(total)
 }
 
 /// Run clang-tidy, then parse and return it's output.
 pub fn run_clang_tidy(
     file: &mut MutexGuard<FileObj>,
     clang_params: &ClangParams,
 ) -> Result<Vec<(log::Level, std::string::String)>> {
-    let mut cmd = Command::new(clang_params.clang_tidy_command.as_ref().unwrap());
+    let cmd_path = clang_params
+        .clang_tidy_command
+        .as_ref()
+        .ok_or(anyhow!("clang-tidy command not located"))?;
+    let mut cmd = Command::new(cmd_path);
     let mut logs = vec![];
     if !clang_params.tidy_checks.is_empty() {
         cmd.args(["-checks", &clang_params.tidy_checks]);
@@ -318,7 +326,12 @@ pub fn run_clang_tidy(
                 .join(" ")
         ),
     ));
-    let output = cmd.output().unwrap();
+    let output = cmd.output().with_context(|| {
+        format!(
+            "Failed to execute clang-tidy on file: {}",
+            file_name.clone()
+        )
+    })?;
     logs.push((
         log::Level::Debug,
         format!(
@@ -339,7 +352,9 @@ pub fn run_clang_tidy(
         &output.stdout,
         &clang_params.database_json,
     )?);
-    if clang_params.tidy_review {
+    if clang_params.tidy_review
+        && let Some(original_content) = &original_content
+    {
         if let Some(tidy_advice) = &mut file.tidy_advice {
             // cache file changes in a buffer and restore the original contents for further analysis
             tidy_advice.patched =
@@ -348,7 +363,7 @@ pub fn run_clang_tidy(
                 })?);
         }
         // original_content is guaranteed to be Some() value at this point
-        fs::write(&file_name, original_content.unwrap())
+        fs::write(&file_name, original_content)
             .with_context(|| format!("Failed to restore file's original content: {file_name}"))?;
     }
     Ok(logs)

cpp-linter/src/clang_tools/mod.rs

@@ -1,3 +1,4 @@
+#![deny(clippy::unwrap_used)]
 //! This module holds the functionality related to running clang-format and/or
 //! clang-tidy.
 
@@ -60,10 +61,12 @@ impl ClangTool {
     pub fn get_exe_path(&self, version: &RequestedVersion) -> Result<PathBuf> {
         let name = self.as_str();
         match version {
-            RequestedVersion::Path(path_buf) => {
-                which_in(name, Some(path_buf), current_dir().unwrap())
-                    .map_err(|_| anyhow!("Could not find {self} by path"))
-            }
+            RequestedVersion::Path(path_buf) => which_in(
+                name,
+                Some(path_buf),
+                current_dir().with_context(|| "Failed to access current working directory.")?,
+            )
+            .map_err(|_| anyhow!("Could not find {self} by path")),
             // Thus, we should use whatever is installed and added to $PATH.
             RequestedVersion::SystemDefault | RequestedVersion::NoValue => {
                 which(name).map_err(|_| anyhow!("Could not find clang tool by name"))
@@ -118,12 +121,17 @@ impl ClangTool {
     fn capture_version(clang_tool: &PathBuf) -> Result<String> {
         let output = Command::new(clang_tool).arg("--version").output()?;
         let stdout = String::from_utf8_lossy(&output.stdout);
-        let version_pattern = Regex::new(r"(?i)version[^\d]*([\d.]+)").unwrap();
+        let version_pattern = Regex::new(r"(?i)version[^\d]*([\d.]+)")
+            .with_context(|| "Failed to allocate RegExp pattern (for clang version parsing) ")?;
         let captures = version_pattern.captures(&stdout).ok_or(anyhow!(
             "Failed to find version number in `{} --version` output",
             clang_tool.to_string_lossy()
         ))?;
-        Ok(captures.get(1).unwrap().as_str().to_string())
+        Ok(captures
+            .get(1)
+            .ok_or(anyhow!("Failed to get version capture group"))?
+            .as_str()
+            .to_string())
     }
 }
 
@@ -434,53 +442,54 @@ pub trait MakeSuggestions {
             })?;
             hunks_in_patch += 1;
             let hunk_range = file_obj.is_hunk_in_diff(&hunk);
-            if hunk_range.is_none() {
-                continue;
-            }
-            let (start_line, end_line) = hunk_range.unwrap();
-            let mut suggestion = String::new();
-            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)
-                    .with_context(|| format!("Failed to get line {line_index} in a hunk {hunk_id} of patch for {file_name}"))?;
-                let line = String::from_utf8(diff_line.content().to_owned())
-                    .with_context(|| format!("Failed to convert line {line_index} buffer to string in hunk {hunk_id} of patch for {file_name}"))?;
-                if ['+', ' '].contains(&diff_line.origin()) {
-                    suggestion.push_str(line.as_str());
-                } else {
-                    removed.push(
-                        diff_line
-                            .old_lineno()
-                            .expect("Removed line should have a line number"),
-                    );
+            match hunk_range {
+                None => continue,
+                Some((start_line, end_line)) => {
+                    let mut suggestion = String::new();
+                    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)
+                            .with_context(|| format!("Failed to get line {line_index} in a hunk {hunk_id} of patch for {file_name}"))?;
+                        let line = String::from_utf8(diff_line.content().to_owned())
+                            .with_context(|| format!("Failed to convert line {line_index} buffer to string in hunk {hunk_id} of patch for {file_name}"))?;
+                        if ['+', ' '].contains(&diff_line.origin()) {
+                            suggestion.push_str(line.as_str());
+                        } else {
+                            removed.push(
+                                diff_line
+                                    .old_lineno()
+                                    .expect("Removed line should have a line number"),
+                            );
+                        }
+                    }
+                    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(),
+                        )
+                    } else {
+                        suggestion = format!("```suggestion\n{suggestion}```");
+                    }
+                    let comment = Suggestion {
+                        line_start: start_line,
+                        line_end: end_line,
+                        suggestion: format!("{suggestion_help}\n{suggestion}"),
+                        path: file_name.clone(),
+                    };
+                    if !review_comments.is_comment_in_suggestions(&comment) {
+                        review_comments.comments.push(comment);
+                    }
                 }
             }
-            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(),
-                )
-            } else {
-                suggestion = format!("```suggestion\n{suggestion}```");
-            }
-            let comment = Suggestion {
-                line_start: start_line,
-                line_end: end_line,
-                suggestion: format!("{suggestion_help}\n{suggestion}"),
-                path: file_name.clone(),
-            };
-            if !review_comments.is_comment_in_suggestions(&comment) {
-                review_comments.comments.push(comment);
-            }
         }
         review_comments.tool_total[is_tidy_tool] =
             Some(review_comments.tool_total[is_tidy_tool].unwrap_or_default() + hunks_in_patch);

cpp-linter/src/cli/mod.rs

@@ -1,3 +1,5 @@
+#![deny(clippy::unwrap_used)]
+
 //! This module holds the Command Line Interface design.
 use std::{path::PathBuf, str::FromStr};
 
@@ -437,17 +439,17 @@ pub struct FeedbackOptions {
 /// the value will be split at spaces.
 pub fn convert_extra_arg_val(args: &[String]) -> Vec<String> {
     let mut val = args.iter();
-    if val.len() == 1 {
+    if args.len() == 1
+        && let Some(v) = val.next()
+    {
         // specified once; split and return result
-        val.next()
-            .unwrap()
-            .trim_matches('\'')
+        v.trim_matches('\'')
             .trim_matches('"')
             .split(' ')
             .map(|i| i.to_string())
             .collect()
     } else {
-        // specified multiple times; just return
+        // specified multiple times; just return a clone of the values
         val.map(|i| i.to_string()).collect()
     }
 }