diff --git a/Cargo.lock b/Cargo.lock index 446a59b8..767faacc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -288,7 +288,6 @@ dependencies = [ "fast-glob", "futures", "git2", - "lenient_semver", "log", "mockito", "quick-xml", @@ -939,35 +938,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "lenient_semver" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de8de3f4f3754c280ce1c8c42ed8dd26a9c8385c2e5ad4ec5a77e774cea9c1ec" -dependencies = [ - "lenient_semver_parser", - "semver", -] - -[[package]] -name = "lenient_semver_parser" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f650c1d024ddc26b4bb79c3076b30030f2cf2b18292af698c81f7337a64d7d6" -dependencies = [ - "lenient_semver_version_builder", - "semver", -] - -[[package]] -name = "lenient_semver_version_builder" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9049f8ff49f75b946f95557148e70230499c8a642bf2d6528246afc7d0282d17" -dependencies = [ - "semver", -] - [[package]] name = "libc" version = "0.2.175" diff --git a/cpp-linter/Cargo.toml b/cpp-linter/Cargo.toml index 38135966..389cb3b5 100644 --- a/cpp-linter/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -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" diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index 81bfae0a..1adf9bde 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -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; @@ -346,14 +346,15 @@ mod test { use std::{ 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 +422,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()]; diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index bc985863..7a1cc233 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -1,18 +1,18 @@ -//! 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; @@ -21,7 +21,7 @@ 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; @@ -29,54 +29,102 @@ 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 { - 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` 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 { + 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")) + } + // 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) => { + // `req.comparators` has at least a major version number for each comparator. + // We need to start with the highest major version number first, then + // decrement to the lowest that satisfies the requirement. + + // find the highest major version from requirement's boundaries. + let mut it = req.comparators.iter(); + let mut highest_major = it.next().map(|v| v.major).unwrap_or_default() + 1; + for n in it { + if n.major > highest_major { + // +1 because we aren't checking the comparator's operator here. + highest_major = n.major + 1; + } + } + + // aggregate by decrementing through major versions that satisfy the requirement. + let mut majors = vec![]; + while highest_major > 0 { + // check if the current major version satisfies the requirement. + if req.matches(&Version::new(highest_major, 0, 0)) { + majors.push(highest_major); + } + highest_major -= 1; + } + + // now we're ready to search for the binary exe with the major version suffixed. + for major in majors { + if let Ok(cmd) = which(format!("{self}-{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 {self} by version")) + } } } + + /// Run `clang-tool --version`, then extract and return the version number. + fn capture_version(clang_tool: &PathBuf) -> Result { + 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,25 +194,13 @@ pub struct ClangVersions { pub tidy_version: Option, } -/// Run `clang-tool --version`, then extract and return the version number. -fn capture_clang_version(clang_tool: &PathBuf) -> Result { - 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>>, - version: &str, + version: &RequestedVersion, clang_params: &mut ClangParams, rest_api_client: &impl RestApiClient, ) -> Result { @@ -172,8 +208,8 @@ pub async fn capture_clang_tools_output( // 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 +218,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 +488,48 @@ pub trait MakeSuggestions { #[cfg(test)] mod tests { - use std::env; + use std::{path::PathBuf, str::FromStr}; - use super::get_clang_tool_exe; + use which::which; + + 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 requirement = ">=9, <22"; + let req_version = RequestedVersion::from_str(requirement).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 +541,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()); } } diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index 4f2fb951..211d9849 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -3,15 +3,12 @@ use std::{path::PathBuf, str::FromStr}; // non-std crates use clap::{ - builder::{ - ArgPredicate, FalseyValueParser, NonEmptyStringValueParser, PossibleValuesParser, - TypedValueParser, - }, + builder::{FalseyValueParser, NonEmptyStringValueParser}, value_parser, ArgAction, Args, Parser, Subcommand, ValueEnum, }; mod structs; -pub use structs::{ClangParams, FeedbackInput, LinesChangedOnly, ThreadComments}; +pub use structs::{ClangParams, FeedbackInput, LinesChangedOnly, RequestedVersion, ThreadComments}; #[derive(Debug, Clone, PartialEq, Eq, ValueEnum)] pub enum Verbosity { @@ -74,23 +71,25 @@ pub struct GeneralOptions { /// /// Accepted options are: /// - /// - A semantic major version number from `10` to `21`. + /// - A semantic version specifier, eg. `>=10, <13`, `=12.0.1`, or simply `16`. /// - A blank string (`''`) to use the platform's default /// installed version. /// - A path to where the clang tools are /// installed (if using a custom install location). /// All paths specified here are converted to absolute. + /// - If this option is specified without a value, then + /// the cpp-linter version is printed and the program exits. #[arg( short = 'V', long, - default_missing_value = "NO-VERSION", - default_value = "", + default_missing_value = "CPP-LINTER-VERSION", num_args = 0..=1, - require_equals = true, + value_parser = RequestedVersion::from_str, + default_value = "", help_heading = "General options", - verbatim_doc_comment, + verbatim_doc_comment )] - pub version: String, + pub version: RequestedVersion, /// This controls the action's verbosity in the workflow's logs. /// @@ -100,6 +99,8 @@ pub struct GeneralOptions { short = 'v', long, default_value = "info", + default_missing_value = "debug", + num_args = 0..=1, help_heading = "General options" )] pub verbosity: Verbosity, @@ -128,22 +129,13 @@ pub struct SourceOptions { pub repo_root: String, /// This controls what part of the files are analyzed. - /// - /// The following values are accepted: - /// - /// - `false`: All lines in a file are analyzed. - /// - `true`: Only lines in the diff that contain additions are analyzed. - /// - `diff`: All lines in the diff are analyzed (including unchanged - /// lines but not subtractions). #[arg( short, long, default_value = "true", - value_parser = PossibleValuesParser::new( - ["true", "on", "1", "false", "off", "0", "diff"], - ).map(|s| ::from_str(&s).unwrap()), help_heading = "Source options", - verbatim_doc_comment, + ignore_case = true, + verbatim_doc_comment )] pub lines_changed_only: LinesChangedOnly, @@ -163,7 +155,15 @@ pub struct SourceOptions { short, long, default_value = "false", - default_value_if("lines-changed-only", ArgPredicate::Equals("true".into()), "true"), + default_missing_value = "true", + default_value_ifs = [ + ("lines-changed-only", "true", "true"), + ("lines-changed-only", "on", "true"), + ("lines-changed-only", "1", "true"), + ("lines-changed-only", "diff", "true"), + ], + num_args = 0..=1, + action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Source options", verbatim_doc_comment, @@ -200,7 +200,7 @@ pub struct FormatOptions { /// The style rules to use. /// /// - Set this to `file` to have clang-format use the closest relative - /// .clang-format file. + /// .clang-format file. Same as passing no value to this option. /// - Set this to a blank string (`''`) to disable using clang-format /// entirely. /// @@ -213,6 +213,8 @@ pub struct FormatOptions { short, long, default_value = "llvm", + default_missing_value = "file", + num_args = 0..=1, help_heading = "Clang-format options", verbatim_doc_comment )] @@ -260,6 +262,8 @@ pub struct TidyOptions { short = 'c', long, default_value = "boost-*,bugprone-*,performance-*,readability-*,portability-*,modernize-*,clang-analyzer-*,cppcoreguidelines-*", + default_missing_value = "", + num_args = 0..=1, help_heading = "Clang-tidy options", verbatim_doc_comment )] @@ -306,10 +310,6 @@ pub struct TidyOptions { pub struct FeedbackOptions { /// Set this option to true to enable the use of thread comments as feedback. /// - /// Set this to `update` to update an existing comment if one exists; - /// the value 'true' will always delete an old comment and post a new one - /// if necessary. - /// /// > [!NOTE] /// > To use thread comments, the `GITHUB_TOKEN` (provided by /// > Github to each repository) must be declared as an environment @@ -321,11 +321,11 @@ pub struct FeedbackOptions { short = 'g', long, default_value = "false", - value_parser = PossibleValuesParser::new( - ["true", "on", "1", "false", "off", "0", "update"], - ).map(|s| ::from_str(&s).unwrap()), + default_missing_value = "update", + num_args = 0..=1, help_heading = "Feedback options", - verbatim_doc_comment, + ignore_case = true, + verbatim_doc_comment )] pub thread_comments: ThreadComments, @@ -352,6 +352,8 @@ pub struct FeedbackOptions { short = 'w', long, default_value_t = false, + default_missing_value = "true", + num_args = 0..=1, action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", @@ -375,6 +377,8 @@ pub struct FeedbackOptions { short = 'd', long, default_value_t = false, + default_missing_value = "true", + num_args = 0..=1, action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", @@ -386,6 +390,8 @@ pub struct FeedbackOptions { short = 'm', long, default_value_t = false, + default_missing_value = "true", + num_args = 0..=1, action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", @@ -398,6 +404,8 @@ pub struct FeedbackOptions { short = 'R', long, default_value_t = false, + default_missing_value = "true", + num_args = 0..=1, action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 318601a5..f4cf3d27 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -1,12 +1,77 @@ use std::{fmt::Display, path::PathBuf, str::FromStr}; -use clap::ValueEnum; +use anyhow::{anyhow, Error}; +use clap::{builder::PossibleValue, ValueEnum}; +use semver::VersionReq; use super::Cli; -use crate::{clang_tools::clang_tidy::CompilationUnit, common_fs::FileFilter}; +use crate::{ + clang_tools::clang_tidy::CompilationUnit, + common_fs::{normalize_path, FileFilter}, +}; + +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub enum RequestedVersion { + /// A specific path to the clang tool binary. + Path(PathBuf), + + /// Whatever the system default uses (if any). + #[default] + SystemDefault, + + /// A specific version requirement for the clang tool binary. + /// + /// For example, `=12.0.1`, `>=10.0.0, <13.0.0`. + Requirement(VersionReq), + + /// A sentinel when no value is given. + /// + /// This is used internally to differentiate when the user intended + /// to invoke the `version` subcommand instead. + NoValue, +} + +impl FromStr for RequestedVersion { + type Err = Error; + + fn from_str(input: &str) -> Result { + if input.is_empty() { + Ok(Self::SystemDefault) + } else if input == "CPP-LINTER-VERSION" { + Ok(Self::NoValue) + } else if let Ok(req) = VersionReq::parse(input) { + Ok(Self::Requirement(req)) + } else if let Ok(req) = VersionReq::parse(format!("={input}").as_str()) { + Ok(Self::Requirement(req)) + } else { + let path = PathBuf::from(input); + if !path.exists() { + return Err(anyhow!( + "The specified version is not a proper requirement or a valid path: {}", + input + )); + } + let path = if !path.is_dir() { + path.parent() + .ok_or(anyhow!( + "Unknown parent directory of the given file path for `--version`: {}", + input + ))? + .to_path_buf() + } else { + path + }; + let path = match path.canonicalize() { + Ok(p) => Ok(normalize_path(&p)), + Err(e) => Err(anyhow!("Failed to canonicalize path '{input}': {e:?}")), + }?; + Ok(Self::Path(path)) + } + } +} /// An enum to describe `--lines-changed-only` CLI option's behavior. -#[derive(PartialEq, Clone, Debug, Default, ValueEnum)] +#[derive(PartialEq, Clone, Debug, Default)] pub enum LinesChangedOnly { /// All lines are scanned #[default] @@ -17,11 +82,44 @@ pub enum LinesChangedOnly { On, } -impl FromStr for LinesChangedOnly { - type Err = (); +impl ValueEnum for LinesChangedOnly { + /// Get a list possible value variants for display in `--help` output. + fn value_variants<'a>() -> &'a [Self] { + &[ + LinesChangedOnly::Off, + LinesChangedOnly::Diff, + LinesChangedOnly::On, + ] + } + + /// Get a display value (for `--help` output) of the enum variant. + fn to_possible_value(&self) -> Option { + match self { + LinesChangedOnly::Off => Some( + PossibleValue::new("false") + .help("All lines in a file are analyzed.") + .aliases(["off", "0"]), + ), + LinesChangedOnly::Diff => Some(PossibleValue::new("diff").help( + "All lines in the diff are analyzed \ + (including unchanged lines but not subtractions).", + )), + LinesChangedOnly::On => Some( + PossibleValue::new("true") + .help("Only lines in the diff that contain additions are analyzed.") + .aliases(["on", "1"]), + ), + } + } - fn from_str(val: &str) -> Result { - match val { + /// Parse a string into a [`LinesChangedOnly`] enum variant. + fn from_str(val: &str, ignore_case: bool) -> Result { + let val = if ignore_case { + val.to_lowercase() + } else { + val.to_string() + }; + match val.as_str() { "true" | "on" | "1" => Ok(LinesChangedOnly::On), "diff" => Ok(LinesChangedOnly::Diff), _ => Ok(LinesChangedOnly::Off), @@ -50,7 +148,7 @@ impl Display for LinesChangedOnly { } /// An enum to describe `--thread-comments` CLI option's behavior. -#[derive(PartialEq, Clone, Debug, ValueEnum)] +#[derive(PartialEq, Clone, Debug)] pub enum ThreadComments { /// Always post a new comment and delete any outdated ones. On, @@ -61,11 +159,41 @@ pub enum ThreadComments { Update, } -impl FromStr for ThreadComments { - type Err = (); +impl ValueEnum for ThreadComments { + /// Get a list possible value variants for display in `--help` output. + fn value_variants<'a>() -> &'a [Self] { + &[Self::On, Self::Off, Self::Update] + } - fn from_str(val: &str) -> Result { - match val { + /// Get a display value (for `--help` output) of the enum variant. + fn to_possible_value(&self) -> Option { + match self { + ThreadComments::On => Some( + PossibleValue::new("true") + .help("Always post a new comment and delete any outdated ones.") + .aliases(["on", "1"]), + ), + ThreadComments::Off => Some( + PossibleValue::new("false") + .help("Do not post thread comments.") + .aliases(["off", "0"]), + ), + ThreadComments::Update => { + Some(PossibleValue::new("update").help( + "Only update existing thread comments. If none exist, then post a new one.", + )) + } + } + } + + /// Parse a string into a [`ThreadComments`] enum variant. + fn from_str(val: &str, ignore_case: bool) -> Result { + let val = if ignore_case { + val.to_lowercase() + } else { + val.to_string() + }; + match val.as_str() { "true" | "on" | "1" => Ok(ThreadComments::On), "update" => Ok(ThreadComments::Update), _ => Ok(ThreadComments::Off), @@ -178,9 +306,12 @@ impl Default for FeedbackInput { mod test { // use crate::cli::get_arg_parser; + use std::{path::PathBuf, str::FromStr}; + + use crate::{cli::RequestedVersion, common_fs::normalize_path}; + use super::{Cli, LinesChangedOnly, ThreadComments}; - use clap::Parser; - use std::str::FromStr; + use clap::{Parser, ValueEnum}; #[test] fn parse_positional() { @@ -193,21 +324,45 @@ mod test { #[test] fn display_lines_changed_only_enum() { - let input = "diff".to_string(); + let input = "Diff"; assert_eq!( - LinesChangedOnly::from_str(&input).unwrap(), + LinesChangedOnly::from_str(&input, true).unwrap(), LinesChangedOnly::Diff ); - assert_eq!(format!("{}", LinesChangedOnly::Diff), input); + assert_eq!(format!("{}", LinesChangedOnly::Diff), input.to_lowercase()); + + assert_eq!( + LinesChangedOnly::from_str(&input, false).unwrap(), + LinesChangedOnly::Off + ); } #[test] fn display_thread_comments_enum() { - let input = "false".to_string(); + let input = "Update"; + assert_eq!( + ThreadComments::from_str(input, true).unwrap(), + ThreadComments::Update + ); + assert_eq!(format!("{}", ThreadComments::Update), input.to_lowercase()); assert_eq!( - ThreadComments::from_str(&input).unwrap(), + ThreadComments::from_str(input, false).unwrap(), ThreadComments::Off ); - assert_eq!(format!("{}", ThreadComments::Off), input); + } + + #[test] + fn validate_version_path() { + let this_path_str = "src/cli/structs.rs"; + let this_path = PathBuf::from(this_path_str); + let this_canonical = this_path.canonicalize().unwrap(); + let parent = this_canonical.parent().unwrap(); + let expected = normalize_path(parent); + let req_ver = RequestedVersion::from_str(this_path_str).unwrap(); + if let RequestedVersion::Path(parsed) = req_ver { + assert_eq!(&parsed, &expected); + } + + assert!(RequestedVersion::from_str("file.rs").is_err()); } } diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index e20bdb3b..c22e862d 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -3,9 +3,11 @@ //! In python, this module is exposed as `cpp_linter.run` that has 1 function exposed: //! `main()`. -use std::env; -use std::path::Path; -use std::sync::{Arc, Mutex}; +use std::{ + env, + path::Path, + sync::{Arc, Mutex}, +}; // non-std crates use anyhow::{anyhow, Result}; @@ -13,11 +15,13 @@ use clap::Parser; use log::{set_max_level, LevelFilter}; // project specific modules/crates -use crate::clang_tools::capture_clang_tools_output; -use crate::cli::{ClangParams, Cli, CliCommand, FeedbackInput, LinesChangedOnly}; -use crate::common_fs::FileFilter; -use crate::logger; -use crate::rest_api::{github::GithubApiClient, RestApiClient}; +use crate::{ + clang_tools::capture_clang_tools_output, + cli::{ClangParams, Cli, CliCommand, FeedbackInput, LinesChangedOnly, RequestedVersion}, + common_fs::FileFilter, + logger, + rest_api::{github::GithubApiClient, RestApiClient}, +}; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -43,19 +47,15 @@ const VERSION: &str = env!("CARGO_PKG_VERSION"); pub async fn run_main(args: Vec) -> Result<()> { let cli = Cli::parse_from(args); - if matches!(cli.commands, Some(CliCommand::Version)) { + if matches!(cli.commands, Some(CliCommand::Version)) + || cli.general_options.version == RequestedVersion::NoValue + { println!("cpp-linter v{}", VERSION); return Ok(()); } logger::try_init(); - if cli.general_options.version == "NO-VERSION" { - log::error!("The `--version` arg is used to specify which version of clang to use."); - log::error!("To get the cpp-linter version, use `cpp-linter version` sub-command."); - return Err(anyhow!("Clang version not specified.")); - } - if cli.source_options.repo_root != "." { env::set_current_dir(Path::new(&cli.source_options.repo_root)).map_err(|e| { anyhow!( @@ -139,7 +139,7 @@ pub async fn run_main(args: Vec) -> Result<()> { let user_inputs = FeedbackInput::from(&cli); let clang_versions = capture_clang_tools_output( &mut arc_files, - cli.general_options.version.as_str(), + &cli.general_options.version, &mut clang_params, &rest_api_client, ) @@ -190,7 +190,6 @@ mod test { "-l".to_string(), "false".to_string(), "-v".to_string(), - "debug".to_string(), "-i=target|benches/libgit2".to_string(), ]) .await; @@ -198,7 +197,7 @@ mod test { } #[tokio::test] - async fn bad_version_input() { + async fn no_version_input() { env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests let result = run_main(vec![ "cpp-linter".to_string(), @@ -207,7 +206,7 @@ mod test { "-V".to_string(), ]) .await; - assert!(result.is_err()); + assert!(result.is_ok()); } #[tokio::test] @@ -216,9 +215,9 @@ mod test { env::set_var("PRE_COMMIT", "1"); let result = run_main(vec![ "cpp-linter".to_string(), - "-l".to_string(), + "--lines-changed-only".to_string(), "false".to_string(), - "-i=target|benches/libgit2".to_string(), + "--ignore=target|benches/libgit2".to_string(), ]) .await; assert!(result.is_err()); @@ -246,7 +245,7 @@ mod test { env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests let result = run_main(vec![ "cpp-linter".to_string(), - "-r".to_string(), + "--repo-root".to_string(), "some-non-existent-dir".to_string(), ]) .await; diff --git a/docs/src/lib.rs b/docs/src/lib.rs index 109f74ca..304c22d7 100644 --- a/docs/src/lib.rs +++ b/docs/src/lib.rs @@ -1,6 +1,6 @@ //! This exposes a function in Python, so an mkdocs plugin can use it to generate the CLI document. //! For actual library/binary source code look in cpp-linter folder. -use std::collections::HashMap; +use std::{any::TypeId, collections::HashMap}; use clap::CommandFactory; use cpp_linter::cli::Cli; @@ -116,6 +116,38 @@ fn generate_cli_doc(metadata: HashMap>>) -> Py if let Some(help) = &arg.get_long_help().or(short_help) { out.push_str(format!("{}\n", help.to_string().trim()).as_str()); } + let possible_vals = arg.get_possible_values(); + let is_boolean = arg.get_value_parser().type_id() == TypeId::of::(); + if !possible_vals.is_empty() { + out.push_str("\nPossible values:\n\n"); + if is_boolean { + out.push_str("- `true` | `on` | `1`\n"); + out.push_str("- `false` | `off` | `0`\n"); + } else { + for val in possible_vals { + let name = format!( + "`{}`", + val.get_name_and_aliases() + .collect::>() + .join("` | `") + ); + let help = val + .get_help() + .map(|v| { + let v = v.to_string(); + let trimmed = v.trim(); + if !trimmed.is_empty() { + format!(": {trimmed}") + } else { + trimmed.to_string() + } + }) + .unwrap_or_default(); + + out.push_str(format!("- {name}{help}\n").as_str()); + } + } + } } } Ok(out)