refactor: improve CLI value parsing/docs

[2bndy5]

A follow-up to #204

• Allow boolean options to be treated as flags (no value passed) if their default value is false (--file-changes-only, --step-summary, --*-review, etc).
• 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 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.

Summary by CodeRabbit

• Refactor

  • Unified clang tool discovery and a flexible CLI version option that accepts a path, system default, semantic requirement, or a special “no value”; several CLI flags now accept optional values with sensible defaults.

• Documentation

  • CLI help now includes possible values and aliases for arguments (including explicit boolean mappings).

• Chores

  • Removed an unused dependency.

[coderabbitai]

Walkthrough

Replaces string-based version handling with a new RequestedVersion enum, introduces a public ClangTool enum and executable-resolution API, removes the lenient_semver dependency, updates CLI option parsing to support optional values, and enhances CLI documentation generation for possible values.

Changes

Cohort / File(s)	Summary
Dependency Management cpp-linter/Cargo.toml	Removed lenient_semver = "0.4.2" from [dependencies].
Clang tools API cpp-linter/src/clang_tools/mod.rs, cpp-linter/src/clang_tools/clang_tidy.rs	Added public ClangTool enum (ClangTidy, ClangFormat) with as_str, get_exe_path(&RequestedVersion), and capture_version methods. Replaced free-function get_clang_tool_exe usage with ClangTool API; updated signatures and tests to use RequestedVersion.
CLI parsing & types cpp-linter/src/cli/structs.rs, cpp-linter/src/cli/mod.rs	Introduced public RequestedVersion enum (Path, SystemDefault, Requirement, NoValue) with FromStr. Changed GeneralOptions.version type to RequestedVersion. Replaced ValueEnum derives for LinesChangedOnly and ThreadComments with manual ValueEnum impls. Added default_missing_value/0..=1 arity adjustments for multiple CLI flags (verbosity, files_changed_only, style, tidy_checks, thread_comments, and various feedback flags).
Runtime integration & tests cpp-linter/src/run.rs, cpp-linter/src/clang_tools/* (tests)	Updated run entrypoint to treat RequestedVersion::NoValue as a version path, pass &RequestedVersion into clang-tool capture functions, removed NO-VERSION error path, and adjusted tests/flag names/usages to match new CLI semantics and ClangTool API.
Docs generation docs/src/lib.rs	Enhanced CLI doc generator to emit a "Possible values" section for arguments when present, including boolean mappings and enumerated values with aliases and per-value help text.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Files/areas needing extra attention:
  • cpp-linter/src/clang_tools/mod.rs — verify version-resolution logic and error paths around RequestedVersion.
  • cpp-linter/src/cli/structs.rs — confirm RequestedVersion::FromStr parsing (VersionReq vs path canonicalization vs magic token).
  • CLI parsing changes in cpp-linter/src/cli/mod.rs and integration in src/run.rs — ensure new default_missing_value and arity semantics behave as intended.
  • Updated tests under cpp-linter/src/clang_tools/* — ensure they reflect public API and edge cases.

Possibly related PRs

• fix: clang-tidy diagnostic comments in PR review #77 — Edits to clang_tools/mod.rs and related capture/output logic; directly overlaps with the ClangTool/version-resolution changes.
• fix: propagate errors #47 — Prior updates to get_clang_tool_exe and tool lookup behavior; related to replacing that function with ClangTool::get_exe_path.
• feat: capture and output clang tool's version number #54 — Implements/relocates clang tool version capture and lookup; likely shares motivation and code paths with these changes.

Suggested labels

dependencies

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: refactoring CLI value parsing and documentation improvements, which aligns with the PR's core objectives of enhancing CLI option handling and updating documentation.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch review-cli

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1984565 and 3608b72.

📒 Files selected for processing (2)

• cpp-linter/src/clang_tools/mod.rs (6 hunks)
• cpp-linter/src/cli/structs.rs (6 hunks)

🧰 Additional context used

🧠 Learnings (6)

📓 Common learnings

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 208
File: cpp-linter/src/clang_tools/mod.rs:60-115
Timestamp: 2025-11-04T06:50:10.842Z
Learning: In the cpp-linter-rs project, path validation (such as checking whether a path is a file or directory) should be performed in CLI parsing (cpp-linter/src/cli/structs.rs) rather than in the tool lookup logic (cpp-linter/src/clang_tools/mod.rs). This maintains proper separation of concerns.

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 54
File: cpp-linter/src/clang_tools/mod.rs:149-159
Timestamp: 2024-11-23T14:10:31.760Z
Learning: Clang tools' `--version` output is consistent across all released versions, so adding extra error handling or unit tests for version extraction may be unnecessary. Tests are run after the clang tools are installed.

📚 Learning: 2025-11-04T06:50:10.842Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 208
File: cpp-linter/src/clang_tools/mod.rs:60-115
Timestamp: 2025-11-04T06:50:10.842Z
Learning: In the cpp-linter-rs project, path validation (such as checking whether a path is a file or directory) should be performed in CLI parsing (cpp-linter/src/cli/structs.rs) rather than in the tool lookup logic (cpp-linter/src/clang_tools/mod.rs). This maintains proper separation of concerns.

Applied to files:

• cpp-linter/src/clang_tools/mod.rs
• cpp-linter/src/cli/structs.rs

📚 Learning: 2024-11-23T06:20:11.698Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 54
File: cpp-linter/src/rest_api/mod.rs:253-254
Timestamp: 2024-11-23T06:20:11.698Z
Learning: In `cpp-linter/src/rest_api/mod.rs`, it is safe to use `unwrap()` on `clang_versions.tidy_version` and `clang_versions.format_version` because they are guaranteed to be `Some` at that point in the code.

Applied to files:

• cpp-linter/src/clang_tools/mod.rs
• cpp-linter/src/cli/structs.rs

📚 Learning: 2024-10-02T07:55:08.948Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 52
File: cpp-linter/src/clang_tools/mod.rs:146-146
Timestamp: 2024-10-02T07:55:08.948Z
Learning: In the `cpp-linter` project, the function `capture_clang_tools_output` is only called in `cpp-linter/src/run.rs`, and there are no other calls to this function elsewhere in the codebase.

Applied to files:

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

📚 Learning: 2024-11-23T14:10:31.760Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 54
File: cpp-linter/src/clang_tools/mod.rs:149-159
Timestamp: 2024-11-23T14:10:31.760Z
Learning: Clang tools' `--version` output is consistent across all released versions, so adding extra error handling or unit tests for version extraction may be unnecessary. Tests are run after the clang tools are installed.

Applied to files:

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

📚 Learning: 2025-01-21T09:56:32.771Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 101
File: cpp-linter/src/clang_tools/clang_format.rs:155-161
Timestamp: 2025-01-21T09:56:32.771Z
Learning: In cpp-linter-rs, the XML output being parsed is generated programmatically by clang-format tool. The only failure case for XML parsing is when clang-format produces a blank XML document, in which case falling back to empty results (using unwrap_or) is the desired behavior.

Applied to files:

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

🧬 Code graph analysis (2)

cpp-linter/src/clang_tools/mod.rs (1)

cpp-linter/src/cli/structs.rs (5)

• from_str (37-70)
• from_str (116-127)
• from_str (190-201)
• from (234-257)
• from (275-286)

cpp-linter/src/cli/structs.rs (1)

cpp-linter/src/common_fs/mod.rs (3)

• normalize_path (237-262)
• from (56-70)
• new (44-53)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: s390x-unknown-linux-gnu
• GitHub Check: aarch64-unknown-linux-musl
• GitHub Check: x86_64-unknown-linux-musl
• GitHub Check: arm-unknown-linux-gnueabi
• GitHub Check: arm-unknown-linux-gnueabihf
• GitHub Check: armv7-unknown-linux-gnueabihf
• GitHub Check: x86_64-unknown-linux-gnu
• GitHub Check: powerpc-unknown-linux-gnu
• GitHub Check: aarch64-unknown-linux-gnu
• GitHub Check: test (ubuntu-latest)
• GitHub Check: Build armv7-unknown-linux-gnueabihf
• GitHub Check: Build x86_64-pc-windows-msvc
• GitHub Check: Build aarch64-apple-darwin
• GitHub Check: Build x86_64-unknown-linux-musl
• GitHub Check: Build current binary
• GitHub Check: Build x86_64-apple-darwin
• GitHub Check: Build aarch64-unknown-linux-gnu
• GitHub Check: Build aarch64-pc-windows-msvc
• GitHub Check: Build x86_64-unknown-linux-gnu
• GitHub Check: Build FreeBSD

🔇 Additional comments (9)

cpp-linter/src/cli/structs.rs (4)

34-71: LGTM! Path validation correctly implemented in CLI parsing.

The FromStr implementation properly handles all cases:

• Empty string → SystemDefault
• Sentinel value → NoValue
• Version requirements (with flexible parsing for bare versions like "16" → "=16")
• Paths (with existence check, parent directory extraction for files, and canonicalization)

The path handling ensures that RequestedVersion::Path always contains a directory, not a file path, which aligns with how it's used in get_exe_path. This maintains the proper separation of concerns as per the architectural principle learned.

Based on learnings.

---

85-128: LGTM! Well-designed ValueEnum implementation with helpful defaults.

The manual implementation provides excellent UX improvements:

• Clear display values with help text for CLI --help output
• Aliases for flexibility ("true"/"on"/"1", "false"/"off"/"0")
• Case-insensitive parsing with sensible default fallback to Off

The default fallback ensures the parser never fails, which is appropriate for this use case.

---

162-202: LGTM! Consistent implementation with good UX.

Follows the same pattern as LinesChangedOnly with appropriate display values, aliases, and case-insensitive parsing with default fallback.

---

354-367: LGTM! Good test coverage for path validation.

The test verifies two key behaviors:

1. File paths have their parent directory extracted and canonicalized
2. Non-existent paths return an error

This confirms the from_str implementation works as designed.

cpp-linter/src/clang_tools/mod.rs (5)

32-51: LGTM! Clean type-safe abstraction for clang tools.

The ClangTool enum provides type safety by replacing string-based tool name passing with explicit variants. The Display implementation and as_str() method provide convenient string access when needed.

---

60-115: LGTM! Version requirement logic correctly implemented.

The method properly handles all RequestedVersion variants:

1. Path variant: Uses which_in to search in the specified directory. Since RequestedVersion::from_str ensures the Path variant always contains a directory (extracting parent for file paths), this works correctly.

2. SystemDefault/NoValue: Falls back to system PATH lookup.

3. Requirement variant: The version matching logic is now correct (past review concern addressed):

   • Finds the highest major version from all comparators using n.major > highest_major (line 80)
   • Decrements through versions, checking which satisfy the requirement
   • Searches for versioned binaries (e.g., clang-format-15) before falling back to unversioned

For example, with ">=14, <16": starts at 17, checks 15 and 14 (both match), searches for clang-format-15 and clang-format-14.

Based on learnings.

---

117-128: LGTM! Clean version extraction implementation.

The method runs --version and extracts the version number using a regex pattern. Per the learnings, clang tools' version output is consistent, so the straightforward approach is appropriate.

Based on learnings.

---

201-261: LGTM! Clean integration of the new ClangTool API.

The function has been successfully updated to use:

• ClangTool::{ClangTidy,ClangFormat}.get_exe_path(&RequestedVersion) for executable lookup
• ClangTool::capture_version for version extraction

The signature change from version: &str to version: &RequestedVersion provides better type safety and enables the flexible version handling (path, requirement, or default).

---

489-548: LGTM! Comprehensive test coverage for the new API.

The tests cover all RequestedVersion variants:

• Version requirement (">=9, <22") - exercises the corrected logic
• System default (empty string)
• Path-based lookup
• Error case (non-existent path)

The tests confirm the implementation works correctly across all scenarios.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 95.45455% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.53%. Comparing base (106a0e9) to head (3608b72).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cpp-linter/src/cli/structs.rs	95.19%	5 Missing ⚠️
cpp-linter/src/clang_tools/mod.rs	94.73%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #208      +/-   ##
==========================================
- Coverage   96.61%   96.53%   -0.08%     
==========================================
  Files          14       14              
  Lines        2891     2999     +108     
==========================================
+ Hits         2793     2895     +102     
- Misses         98      104       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.