fix: skip parsing clang-tidy diagnostic rationale

[2bndy5]

Specifically when the rationale line has added context in square brackets.

This includes a test based on the actual bug reported in cpp-linter/cpp-linter-action#389.

Corresponds to cpp-linter/cpp-linter#176

Summary by CodeRabbit

• Bug Fixes

  • Fixed diagnostic parsing to properly distinguish between actual diagnostics and diagnostic rationale text in output.
  • Improved path normalization handling for compilation database and standard scenarios.

• Tests

  • Added test coverage for diagnostic rationale parsing.

• Chores

  • Updated spelling configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR modifies clang-tidy diagnostic parsing to distinguish real diagnostics from false-positive brackets by introducing a validation guard, adjusts path normalization logic to be conditional on compilation database presence and findings, and adds a test case to verify the new diagnostic detection behavior.

Changes

Cohort / File(s)	Summary
Diagnostic parsing refinements cpp-linter/src/clang_tools/clang_tidy.rs	Introduces a guard to start new TidyNotification only when diagnostic name contains no spaces and includes a dash, preventing false positives like "[with auto ...]"; adjusts path normalization to check compilation database for matching units and normalize relative to found directory or CWD; finalizes current notification after parsing completes
Test coverage cpp-linter/src/clang_tools/clang_tidy.rs	Adds test case skip_parse_tidy_diagnostic_rationale to verify diagnostic rationale brackets are ignored; adjusts test imports for parse_tidy_output access
Spell-check dictionary cspell.config.yml	Adds "declval" to recognized words list

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: parse clang-tidy output when WarningsAsErrors is asserted #190: Modifies diagnostic/header detection logic in parse_tidy_output through regex and start-condition changes, addressing similar diagnostic identification concerns.
• feat: capture and output clang tool's version number #54: Changes to parse_tidy_output function signature and return handling that affect notification processing flow.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: skipping parsing of clang-tidy diagnostic rationale lines, which is the primary focus of the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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 skip-tidy-rationale

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4575ec and 72b949f.

📒 Files selected for processing (2)

• cpp-linter/src/clang_tools/clang_tidy.rs
• cspell.config.yml

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

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.

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.870Z
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.

📚 Learning: 2024-09-29T18:00:02.286Z

Learnt from: 2bndy5
Repo: cpp-linter/cpp-linter-rs PR: 50
File: justfile:37-42
Timestamp: 2024-09-29T18:00:02.286Z
Learning: The word 'mdbook' is still present in `CHANGELOG.md`, so `cspell.config.yml` should continue to include `mdbook` in the spelling exceptions list.

Applied to files:

• cspell.config.yml

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

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.870Z
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/clang_tidy.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/clang_tidy.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/clang_tidy.rs

🧬 Code graph analysis (1)

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

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

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

🔇 Additional comments (5)

cspell.config.yml (1)

22-22: LGTM!

The addition of declval is appropriate—it's a valid C++ standard library identifier (std::declval) that appears in the new test case's clang-tidy output.

cpp-linter/src/clang_tools/clang_tidy.rs (4)

153-159: Nice fix for the false-positive diagnostic detection.

The hyphen check correctly filters out non-diagnostic bracket content like [with auto = ...]. Valid clang-tidy check names always follow a category-name pattern containing hyphens.

Minor note: The space check (!contains(' ')) is technically redundant since the regex group 6 pattern [a-zA-Z\d\-\.]+ already prohibits spaces. It's harmless as a defensive check, but could be simplified if desired.

---

161-215: LGTM!

The restructuring correctly:

1. Pushes the previous notification before starting a new one (preventing data loss)
2. Moves path normalization inside the guard to only execute for valid diagnostics
3. Uses debug_assert! appropriately to catch unexpected non-absolute paths during development

The conditional flow for database vs. no-database path normalization is clear and maintains the expected behavior.

---

361-365: LGTM!

Import update correctly brings parse_tidy_output into scope for the new test. Access to the private function from the in-file test module follows standard Rust conventions.

---

479-528: Excellent test case derived from the actual bug report.

The test correctly exercises the fix:

• Contains 4 actual clang-diagnostic-error diagnostics
• Contains problematic note: lines with brackets like [with file:auto = typename ...] that previously triggered false positives
• Assertions verify that only valid diagnostics (containing -, no spaces) are captured

This provides good regression coverage for the issue reported in cpp-linter/cpp-linter-action#389.

---

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

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.98%. Comparing base (a4575ec) to head (72b949f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
+ Coverage   96.93%   96.98%   +0.05%     
==========================================
  Files          14       14              
  Lines        3033     3085      +52     
==========================================
+ Hits         2940     2992      +52     
  Misses         93       93              

☔ 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.