Skip to content

feat: use concrete error types#343

Merged
2bndy5 merged 2 commits into
mainfrom
proper-capture-errors
Jun 9, 2026
Merged

feat: use concrete error types#343
2bndy5 merged 2 commits into
mainfrom
proper-capture-errors

Conversation

@2bndy5

@2bndy5 2bndy5 commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

when capturing clang tools' output.

I also exported some error types from clang-tools-manager crate.

Summary by CodeRabbit

  • New Features

    • Added structured error types for improved error reporting when clang-format and clang-tidy execution fails.
  • Refactor

    • Replaced generic error handling with specific error types for more precise error diagnostics.
    • Made the anyhow dependency optional.

when capturing clang tools' output.

I expect very low code coverage on this because simulating every little error is tedious or near impossible.
@2bndy5 2bndy5 added the enhancement New feature or request label Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@2bndy5, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 50 minutes and 37 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 04db5a4b-a3db-4d4e-8eb4-030bc8752115

📥 Commits

Reviewing files that changed from the base of the PR and between 0ab6a8c and 2950b89.

📒 Files selected for processing (1)
  • cpp-linter/src/clang_tools/clang_tidy.rs

Walkthrough

This PR replaces generic anyhow error handling with two new typed error enums (ClangCaptureError for low-level tool execution failures and ClangTaskError for orchestration-level failures) across clang-tools modules, makes anyhow an optional dependency, and expands public re-exports from the clang-tools-manager crate.

Changes

Error type migration

Layer / File(s) Summary
Error type definitions and imports
cpp-linter/src/error.rs
Two new public error enums introduce typed variants for clang tool execution and task orchestration. ClangCaptureError covers command execution, output parsing (UTF-8, XML), file I/O, regex compilation, and directory access. ClangTaskError wraps ClangCaptureError and adds tool acquisition, version lookup, JSON parsing, and async join failures. Import of GetToolError from clang_tools_manager is added.
Dependency and feature configuration
cpp-linter/Cargo.toml
anyhow dependency is changed from required to optional. The bin feature is updated to explicitly enable dep:anyhow alongside dep:clap, dep:colored, and reqwest/default-tls.
Public re-exports from clang-tools-manager
clang-tools-manager/src/lib.rs
Public re-exports are expanded to include ClangVersion, GetToolError, RequestedVersionParsingError, and RequestedVersion.

Clang tool error handling refactor

Layer / File(s) Summary
clang-format error handling refactor
cpp-linter/src/clang_tools/clang_format.rs
run_clang_format return type is changed to Result<..., ClangCaptureError>. Tool path lookup, command execution, XML output parsing, and file read failures are mapped to typed error variants (ToolPathUnknown, FailedToRunCommand, NonUtf8Output, XmlParsingFailed, ReadFileFailed) instead of generic anyhow context strings.
clang-tidy error handling refactor
cpp-linter/src/clang_tools/clang_tidy.rs
run_clang_tidy and parse_tidy_output return types are updated to use ClangCaptureError. Tool path lookup returns ToolPathUnknown. File operations during the tidy-review flow (restore/patch/restore-original) map to ReadFileFailed and WriteFileFailed. Command execution and working-directory access errors are converted to typed variants.

Clang tools orchestration error typing

Layer / File(s) Summary
Module-level error handling in clang_tools/mod
cpp-linter/src/clang_tools/mod.rs
analyze_single_file return type is narrowed to Result<_, ClangCaptureError> with mutex-poisoning mapped to ClangCaptureError::MutexPoisoned. capture_clang_tools_output return type is changed to Result<ClangVersions, ClangTaskError>. Tool discovery failures map to ClangTaskError::FindToolError. Compilation database JSON parsing error handling is simplified by removing context wrappers and relying on direct error propagation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • cpp-linter/cpp-linter-rs#242: Both PRs refactor error handling in clang_format.rs, clang_tidy.rs, and clang_tools/mod.rs around clang-tool execution (retrieved PR introduces anyhow/context, main PR replaces with typed ClangCaptureError/ClangTaskError).
  • cpp-linter/cpp-linter-rs#279: Both PRs modify src/clang_tools/mod.rs clang-tools execution and version discovery paths; #279 rewires tool installation via clang_installer::RequestedVersion::eval_tool, while main PR refactors the same functions to use typed error enums.
  • cpp-linter/cpp-linter-rs#101: Both PRs modify clang_format.rs in the run_clang_format XML output parsing and error handling path.

Suggested labels

rust

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: use concrete error types' accurately describes the main change: replacing anyhow error handling with concrete, typed error enums (ClangCaptureError and ClangTaskError) throughout the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch proper-capture-errors

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp-linter/src/clang_tools/clang_tidy.rs`:
- Around line 303-309: The read error mapping is incorrect: when calling
fs::read_to_string(&file.name) in clang_tidy.rs you should map failures to
ClangCaptureError::ReadFileFailed (not ClangCaptureError::WriteFileFailed);
update the map_err closure that currently constructs WriteFileFailed to
construct ReadFileFailed using the same file_name and source variables so the
error accurately reflects a read failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ab4608e2-229c-4936-af6e-7d518c115b9a

📥 Commits

Reviewing files that changed from the base of the PR and between 1529d72 and 0ab6a8c.

📒 Files selected for processing (6)
  • clang-tools-manager/src/lib.rs
  • cpp-linter/Cargo.toml
  • cpp-linter/src/clang_tools/clang_format.rs
  • cpp-linter/src/clang_tools/clang_tidy.rs
  • cpp-linter/src/clang_tools/mod.rs
  • cpp-linter/src/error.rs

Comment thread cpp-linter/src/clang_tools/clang_tidy.rs
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.74627% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.55%. Comparing base (1529d72) to head (2950b89).

Files with missing lines Patch % Lines
cpp-linter/src/clang_tools/clang_tidy.rs 43.33% 17 Missing ⚠️
cpp-linter/src/clang_tools/clang_format.rs 36.00% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
- Coverage   91.02%   90.55%   -0.48%     
==========================================
  Files          22       22              
  Lines        3376     3398      +22     
==========================================
+ Hits         3073     3077       +4     
- Misses        303      321      +18     

☔ View full report in Codecov by Harness.
📢 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.

@2bndy5 2bndy5 merged commit cdc871a into main Jun 9, 2026
85 checks passed
@2bndy5 2bndy5 deleted the proper-capture-errors branch June 9, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant