style: follow up to #204

- Allow boolean options to be treated as flags if their default value is false.
- Some other CLI options can now accept no value (like a flag) including `--verbosity`
- Use `std::str::FromStr` trait to parse the given version:
    * if no value is given, then behave exactly like the `cpp-linter version` subcommand (print version and exit)
    * if a path is given look for clang-tools in that path. This only ensures the given path exists, not if the clang- tool is present; that is done later if the tool is needed.
    * expand support for version specifiers using `semver::VersionReq`. This removes the need for `lenient_semver` dependency and allows the user to specify a range of clang versions. For example, `>=14, <16`, `=12.0.1`, or simply `16` (would be treated as `=16`). See [semver::VersionReq docs][ver-req-docs] for more detail.
- adjust docs to better describe accepted/possible values.

Due to the changes in parsing the user-given version, there is a new enum, `ClangTool` that enforces type-safety about finding the clang tools (when they are needed).

* Instead of passing the tool name (as a str), the `ClangTool` enum is used to avoid typos and convey explicit support for only clang-tidy and clang-format.
* Getting the clang tool's path and version are now instance methods of the `ClangTool` enum.

[ver-req-docs]: https://docs.rs/semver/1.0.27/semver/struct.VersionReq.html

Author: 2bndy5

Cargo.lock

@@ -21,7 +21,6 @@ colored = "3.0.0"
 fast-glob = "1.0.0"
 futures = "0.3.31"
 git2 = "0.20.2"
-lenient_semver = "0.4.2"
 log = { version = "0.4.28", features = ["std"] }
 quick-xml = { version = "0.38.3", features = ["serialize"] }
 regex = "1.12.2"

cpp-linter/Cargo.toml

@@ -9,8 +9,8 @@ use std::{
     sync::{Arc, Mutex, MutexGuard},
 };
 
-use anyhow::{Context, Result};
 // non-std crates
+use anyhow::{Context, Result};
 use regex::Regex;
 use serde::Deserialize;
 
@@ -344,16 +344,14 @@ pub fn run_clang_tidy(
 #[cfg(test)]
 mod test {
     use std::{
-        env,
-        path::PathBuf,
-        sync::{Arc, Mutex},
+        env, path::PathBuf, str::FromStr, sync::{Arc, Mutex}
     };
 
     use regex::Regex;
 
     use crate::{
-        clang_tools::get_clang_tool_exe,
-        cli::{ClangParams, LinesChangedOnly},
+        clang_tools::ClangTool,
+        cli::{ClangParams, LinesChangedOnly, RequestedVersion},
         common_fs::FileObj,
     };
 
@@ -421,11 +419,14 @@ mod test {
 
     #[test]
     fn use_extra_args() {
-        let exe_path = get_clang_tool_exe(
-            "clang-tidy",
-            env::var("CLANG_VERSION").unwrap_or("".to_string()).as_str(),
-        )
-        .unwrap();
+        let exe_path = ClangTool::ClangTidy
+            .get_exe_path(
+                &RequestedVersion::from_str(
+                    env::var("CLANG_VERSION").unwrap_or("".to_string()).as_str(),
+                )
+                .unwrap(),
+            )
+            .unwrap();
         let file = FileObj::new(PathBuf::from("tests/demo/demo.cpp"));
         let arc_ref = Arc::new(Mutex::new(file));
         let extra_args = vec!["-std=c++17".to_string(), "-Wall".to_string()];

cpp-linter/src/clang_tools/clang_tidy.rs

@@ -1,82 +1,106 @@
-//! This crate holds the functionality related to running clang-format and/or
+//! This module holds the functionality related to running clang-format and/or
 //! clang-tidy.
 
 use std::{
     env::current_dir,
+    fmt::{self, Display},
     fs,
     path::{Path, PathBuf},
     process::Command,
     sync::{Arc, Mutex},
 };
 
+// non-std crates
 use anyhow::{anyhow, Context, Result};
 use git2::{DiffOptions, Patch};
-// non-std crates
-use lenient_semver;
 use regex::Regex;
-use semver::Version;
 use tokio::task::JoinSet;
 use which::{which, which_in};
 
 // project-specific modules/crates
 use super::common_fs::FileObj;
 use crate::{
-    cli::ClangParams,
+    cli::{ClangParams, RequestedVersion},
     rest_api::{RestApiClient, COMMENT_MARKER, USER_OUTREACH},
 };
 pub mod clang_format;
 use clang_format::run_clang_format;
 pub mod clang_tidy;
 use clang_tidy::{run_clang_tidy, CompilationUnit};
 
-/// Fetch the path to a clang tool by `name` (ie `"clang-tidy"` or `"clang-format"`) and
-/// `version`.
-///
-/// The specified `version` can be either
-///
-/// - a full or partial semantic version specification
-/// - a path to a directory containing the executable binary `name`d
-///
-/// If the executable is not found using the specified `version`, then the tool is
-/// sought only by it's `name`.
-///
-/// The only reason this function would return an error is if the specified tool is not
-/// installed or present on the system (nor in the `$PATH` environment variable).
-pub fn get_clang_tool_exe(name: &str, version: &str) -> Result<PathBuf> {
-    if version.is_empty() {
-        // The default CLI value is an empty string.
-        // Thus, we should use whatever is installed and added to $PATH.
-        if let Ok(cmd) = which(name) {
-            return Ok(cmd);
-        } else {
-            return Err(anyhow!("Could not find clang tool by name"));
-        }
+#[derive(Debug)]
+pub enum ClangTool {
+    ClangTidy,
+    ClangFormat,
+}
+
+impl Display for ClangTool {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.as_str())
     }
-    if let Ok(semver) = lenient_semver::parse_into::<Version>(version) {
-        // `version` specified has at least a major version number
-        if let Ok(cmd) = which(format!("{}-{}", name, semver.major)) {
-            Ok(cmd)
-        } else if let Ok(cmd) = which(name) {
-            // USERS SHOULD MAKE SURE THE PROPER VERSION IS INSTALLED BEFORE USING CPP-LINTER!!!
-            // This block essentially ignores the version specified as a fail-safe.
-            //
-            // On Windows, the version's major number is typically not appended to the name of
-            // the executable (or symlink for executable), so this is useful in that scenario.
-            // On Unix systems, this block is not likely reached. Typically, installing clang
-            // will produce a symlink to the executable with the major version appended to the
-            // name.
-            Ok(cmd)
-        } else {
-            Err(anyhow!("Could not find clang tool by name and version"))
+}
+
+impl ClangTool {
+    /// Get the string representation of the clang tool's name.
+    pub const fn as_str(&self) -> &'static str {
+        match self {
+            ClangTool::ClangTidy => "clang-tidy",
+            ClangTool::ClangFormat => "clang-format",
         }
-    } else {
-        // `version` specified is not a semantic version; treat as path/to/bin
-        if let Ok(exe_path) = which_in(name, Some(version), current_dir().unwrap()) {
-            Ok(exe_path)
-        } else {
-            Err(anyhow!("Could not find clang tool by path"))
+    }
+
+    /// Fetch the path to an executable clang tool for the specified `version`.
+    ///
+    /// If the executable is not found using the specified `version`, then the tool is
+    /// sought only by it's name ([`Self::as_str()`]).
+    ///
+    /// The only reason this function would return an error is if the specified tool is not
+    /// installed or present on the system (nor in the `PATH` environment variable).
+    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 {name} 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"))
+            }
+            RequestedVersion::Requirement(req) => {
+                // `version` specified has at least a major version number.
+                for req_ver in &req.comparators {
+                    let major = req_ver.major;
+                    if let Ok(cmd) = which(format!("{name}-{major}")) {
+                        return Ok(cmd);
+                    }
+                }
+                // failed to find a binary where the major version number is suffixed to the tool name.
+
+                // USERS SHOULD MAKE SURE THE PROPER VERSION IS INSTALLED BEFORE USING CPP-LINTER!!!
+                // This line essentially ignores the version specified as a fail-safe.
+                //
+                // On Windows, the version's major number is typically not appended to the name of
+                // the executable (or symlink for executable), so this is useful in that scenario.
+                // On Unix systems, this line is not likely reached. Typically, installing clang
+                // will produce a symlink to the executable with the major version appended to the
+                // name.
+                which(name).map_err(|_| anyhow!("Could not find {name} by version"))
+            }
         }
     }
+
+    /// Run `clang-tool --version`, then extract and return the version number.
+    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 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())
+    }
 }
 
 /// This creates a task to run clang-tidy and clang-format on a single file.
@@ -146,34 +170,22 @@ pub struct ClangVersions {
     pub tidy_version: Option<String>,
 }
 
-/// 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);
-    let version_pattern = Regex::new(r"(?i)version\s*([\d.]+)").unwrap();
-    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())
-}
-
 /// Runs clang-tidy and/or clang-format and returns the parsed output from each.
 ///
 /// If `tidy_checks` is `"-*"` then clang-tidy is not executed.
 /// If `style` is a blank string (`""`), then clang-format is not executed.
 pub async fn capture_clang_tools_output(
     files: &mut Vec<Arc<Mutex<FileObj>>>,
-    version: &str,
+    version: &RequestedVersion,
     clang_params: &mut ClangParams,
     rest_api_client: &impl RestApiClient,
 ) -> Result<ClangVersions> {
     let mut clang_versions = ClangVersions::default();
     // find the executable paths for clang-tidy and/or clang-format and show version
     // info as debugging output.
     if clang_params.tidy_checks != "-*" {
-        let exe_path = get_clang_tool_exe("clang-tidy", version)?;
-        let version_found = capture_clang_version(&exe_path)?;
+        let exe_path = ClangTool::ClangTidy.get_exe_path(version)?;
+        let version_found = ClangTool::capture_version(&exe_path)?;
         log::debug!(
             "{} --version: v{version_found}",
             &exe_path.to_string_lossy()
@@ -182,8 +194,8 @@ pub async fn capture_clang_tools_output(
         clang_params.clang_tidy_command = Some(exe_path);
     }
     if !clang_params.style.is_empty() {
-        let exe_path = get_clang_tool_exe("clang-format", version)?;
-        let version_found = capture_clang_version(&exe_path)?;
+        let exe_path = ClangTool::ClangFormat.get_exe_path(version)?;
+        let version_found = ClangTool::capture_version(&exe_path)?;
         log::debug!(
             "{} --version: v{version_found}",
             &exe_path.to_string_lossy()
@@ -452,45 +464,49 @@ pub trait MakeSuggestions {
 
 #[cfg(test)]
 mod tests {
-    use std::env;
+    use std::{env, path::PathBuf, str::FromStr};
+
+    use which::which;
 
-    use super::get_clang_tool_exe;
+    use super::ClangTool;
+    use crate::cli::RequestedVersion;
 
-    const TOOL_NAME: &str = "clang-format";
+    const CLANG_FORMAT: ClangTool = ClangTool::ClangFormat;
 
     #[test]
     fn get_exe_by_version() {
         let clang_version = env::var("CLANG_VERSION").unwrap_or("16".to_string());
-        let tool_exe = get_clang_tool_exe(TOOL_NAME, clang_version.as_str());
+        let req_version = RequestedVersion::from_str(&clang_version).unwrap();
+        let tool_exe = CLANG_FORMAT.get_exe_path(&req_version);
         println!("tool_exe: {:?}", tool_exe);
         assert!(tool_exe.is_ok_and(|val| val
             .file_name()
             .unwrap()
             .to_string_lossy()
             .to_string()
-            .contains(TOOL_NAME)));
+            .contains(CLANG_FORMAT.as_str())));
     }
 
     #[test]
     fn get_exe_by_default() {
-        let tool_exe = get_clang_tool_exe(TOOL_NAME, "");
+        let tool_exe = CLANG_FORMAT.get_exe_path(&RequestedVersion::from_str("").unwrap());
         println!("tool_exe: {:?}", tool_exe);
         assert!(tool_exe.is_ok_and(|val| val
             .file_name()
             .unwrap()
             .to_string_lossy()
             .to_string()
-            .contains(TOOL_NAME)));
+            .contains(CLANG_FORMAT.as_str())));
     }
 
-    use which::which;
-
     #[test]
     fn get_exe_by_path() {
+        static TOOL_NAME: &'static str = CLANG_FORMAT.as_str();
         let clang_version = which(TOOL_NAME).unwrap();
         let bin_path = clang_version.parent().unwrap().to_str().unwrap();
         println!("binary exe path: {bin_path}");
-        let tool_exe = get_clang_tool_exe(TOOL_NAME, bin_path);
+        let tool_exe =
+            CLANG_FORMAT.get_exe_path(&RequestedVersion::from_str(bin_path).unwrap());
         println!("tool_exe: {:?}", tool_exe);
         assert!(tool_exe.is_ok_and(|val| val
             .file_name()
@@ -502,14 +518,8 @@ mod tests {
 
     #[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);
+        let tool_exe =
+            CLANG_FORMAT.get_exe_path(&RequestedVersion::Path(PathBuf::from("non-existent-path")));
         assert!(tool_exe.is_err());
     }
 }