Skip to content

fix: avoid appending --extra-arg= for clang-tidy #172

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
2bndy5 merged 1 commit into from
Dec 23, 2025
Merged

fix: avoid appending --extra-arg= for clang-tidy #172

2bndy5 merged 1 commit into from
Dec 23, 2025

Conversation

@2bndy5
Copy link
Contributor

@2bndy5 2bndy5 commented Dec 22, 2025 •
edited by coderabbitai bot
Loading

This avoids adding a --extra-arg= (with no arg value) to clang-tidy invocations.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a bug where empty argument values could be inadvertently passed to clang-tidy during execution. The argument construction logic now properly validates and filters out empty values before passing them to the tool, resulting in more reliable linting operations.

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

@2bndy5
fix: avoid appending --extra-arg= for clang-tidy
a9c8ada 
This avoids adding a `--extra-arg=` (with no arg value) to clang-tidy invocations.
@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

A minor bug fix in the run_clang_tidy function within the clang-tidy wrapper module. The change modifies extra argument construction to filter out empty argument values before appending them as --extra-arg parameters. This prevents malformed empty arguments from being passed to the clang-tidy tool.

Changes

Cohort / File(s) Change Summary
Extra argument filtering in clang-tidy wrapper
cpp_linter/clang_tools/clang_tidy.py		 Modified run_clang_tidy to only append --extra-arg={arg} when arg is non-empty, preventing empty argument values from being passed to clang-tidy

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

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 accurately describes the main bug fix: avoiding empty --extra-arg= arguments when constructing extra arguments for clang-tidy invocations.
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 empty-extra-args

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[bot]
coderabbitai bot reviewed Dec 22, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cpp_linter/clang_tools/clang_tidy.py (1)

249-252: LGTM! The fix correctly prevents empty arguments from being passed to clang-tidy.

The conditional check successfully filters out empty strings after quote stripping, which addresses the issue described in the PR.

Optional: Consider also stripping whitespace for more defensive handling

If you want to handle whitespace-only arguments more defensively (e.g., " " after quote removal), you could also strip whitespace:

 for extra_arg in extra_args:
-    arg = extra_arg.strip('"')
+    arg = extra_arg.strip('"').strip()
     if arg:  # avoid adding empty arg values
         cmds.append(f"--extra-arg={arg}")

This would prevent --extra-arg= (with only whitespace) from being added. However, this is likely an edge case and the current implementation is sufficient for the stated goal.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7885064 and a9c8ada.

📒 Files selected for processing (1)
  • cpp_linter/clang_tools/clang_tidy.py
⏰ 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). (21)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: test (ubuntu-latest, 9)
  • GitHub Check: test (ubuntu-latest, 10)
  • GitHub Check: test (ubuntu-latest, 15)
  • GitHub Check: test (windows-latest, 18)
  • GitHub Check: test (windows-latest, 13)
  • GitHub Check: test (windows-latest, 16)
  • GitHub Check: test (windows-latest, 17)
  • GitHub Check: test (windows-latest, 15)
  • GitHub Check: test (ubuntu-latest, 17)
  • GitHub Check: test (windows-latest, 9)
  • GitHub Check: test (ubuntu-latest, 18)
  • GitHub Check: test (windows-latest, 11)
  • GitHub Check: test (ubuntu-latest, 14)
  • GitHub Check: test (ubuntu-latest, 21)
  • GitHub Check: test (ubuntu-latest, 16)
  • GitHub Check: test (windows-latest, 14)
  • GitHub Check: test (windows-latest, 19)
  • GitHub Check: test (windows-latest, 20)
  • GitHub Check: test (windows-latest, 10)
  • GitHub Check: test (windows-latest, 21)

@github-actions github-actions bot added the bug Something isn't working label Dec 22, 2025
@2bndy5 2bndy5 mentioned this pull request Dec 22, 2025
Closed
@codecov
Copy link

codecov bot commented Dec 22, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.26%. Comparing base (7885064) to head (a9c8ada).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #172   +/-   ##
=======================================
  Coverage   98.25%   98.26%           
=======================================
  Files          23       23           
  Lines        1896     1897    +1     
=======================================
+ Hits         1863     1864    +1     
  Misses         33       33

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

@2bndy5 2bndy5 merged commit 7e2a47a into main Dec 23, 2025
43 checks passed
@2bndy5 2bndy5 deleted the empty-extra-args branch December 23, 2025 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@shenxianpeng shenxianpeng shenxianpeng approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@2bndy5 @shenxianpeng