Skip to content

chore: Enable most bugprone checks#7643

Open
mathbunnyru wants to merge 4 commits into
XRPLF:developfrom
mathbunnyru:all_bugprone
Open

chore: Enable most bugprone checks#7643
mathbunnyru wants to merge 4 commits into
XRPLF:developfrom
mathbunnyru:all_bugprone

Conversation

@mathbunnyru

@mathbunnyru mathbunnyru commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This enables most bugrpone- checks, except where it might be difficult to fix, and they probably better be addressed one by one.

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@mathbunnyru mathbunnyru added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Jun 26, 2026
@mathbunnyru mathbunnyru changed the title chore: Enable all bugprone checks by default chore: Enable all bugprone checks Jun 26, 2026
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.18182% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.0%. Comparing base (50fdb38) to head (99ad457).

Files with missing lines Patch % Lines
src/xrpld/rpc/detail/Pathfinder.cpp 60.0% 12 Missing ⚠️
src/libxrpl/nodestore/backend/RocksDBFactory.cpp 75.0% 1 Missing ⚠️
src/xrpld/core/detail/Config.cpp 66.7% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #7643   +/-   ##
=======================================
  Coverage     82.0%   82.0%           
=======================================
  Files         1007    1007           
  Lines        76888   76875   -13     
  Branches      8971    8972    +1     
=======================================
- Hits         63042   63033    -9     
+ Misses       13837   13833    -4     
  Partials         9       9           
Files with missing lines Coverage Δ
include/xrpl/config/BasicConfig.h 91.0% <100.0%> (+0.1%) ⬆️
include/xrpl/protocol/detail/token_errors.h 18.5% <ø> (+1.3%) ⬆️
src/libxrpl/conditions/Condition.cpp 63.0% <ø> (+6.9%) ⬆️
src/libxrpl/conditions/Fulfillment.cpp 82.2% <ø> (+2.6%) ⬆️
src/libxrpl/ledger/helpers/TokenHelpers.cpp 95.3% <100.0%> (-<0.1%) ⬇️
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp 93.4% <100.0%> (-<0.1%) ⬇️
src/xrpld/rpc/detail/PathRequestManager.cpp 81.6% <100.0%> (+0.1%) ⬆️
src/libxrpl/nodestore/backend/RocksDBFactory.cpp 52.8% <75.0%> (-0.3%) ⬇️
src/xrpld/core/detail/Config.cpp 77.6% <66.7%> (+0.1%) ⬆️
src/xrpld/rpc/detail/Pathfinder.cpp 88.2% <60.0%> (-1.2%) ⬇️

... and 4 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions

Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@mathbunnyru mathbunnyru force-pushed the all_bugprone branch 2 times, most recently from a121a3e to 440ad09 Compare June 26, 2026 13:28
@mathbunnyru mathbunnyru changed the title chore: Enable all bugprone checks chore: Enable most bugprone checks Jun 26, 2026
@mathbunnyru mathbunnyru removed the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Jun 26, 2026
@mathbunnyru mathbunnyru requested review from bthomee and godexsoft June 26, 2026 15:10
@mathbunnyru mathbunnyru marked this pull request as ready for review June 26, 2026 15:10

Copilot AI 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.

Pull request overview

This PR updates the repository’s .clang-tidy configuration to enable most bugprone-* checks and applies a set of small refactors / annotations across core code and tests to satisfy (or intentionally suppress) the newly enabled diagnostics.

Changes:

  • Enable most bugprone-* clang-tidy checks by removing several per-check exclusions from .clang-tidy.
  • Refactor a number of “assignment-in-condition”, duplicated-branch, and empty-branch patterns into clearer control flow.
  • Add targeted NOLINTNEXTLINE(...) suppressions in tests where deterministic behavior is intentional (e.g., fixed RNG seeds, float loop counters).

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/xrpld/rpc/detail/PathRequestManager.cpp Removes assignment-in-if pattern when sending path_find updates.
src/xrpld/rpc/detail/Pathfinder.cpp Refactors path ranking comparisons and replaces empty branches with continue/simplified flow.
src/xrpld/core/detail/Config.cpp Simplifies validators file existence/type validation condition.
src/tests/libxrpl/tx/AccountSet.cpp Adds NOLINTNEXTLINE for float loop counter in a test.
src/test/unit_test/SuiteJournal.h Simplifies switch handling to avoid redundant default/break pattern.
src/test/rpc/LedgerEntry_test.cpp Removes duplicated returns by using intentional fallthrough for UInt32/UInt64 cases.
src/test/nodestore/TestBase.h Removes redundant switch case returning same default value.
src/test/csf/Sim.h Adds a bugprone-random-generator-seed suppression comment for deterministic behavior.
src/test/consensus/LedgerTrie_test.cpp Adds bugprone-random-generator-seed suppression for fixed-seed RNG in test.
src/test/app/XChain_test.cpp Removes redundant if/else branches; adds RNG seed suppression in sim test.
src/test/app/NFTokenBurn_test.cpp Adds RNG seed suppression for deterministic test behavior.
src/test/app/MPToken_test.cpp Combines feature-flag conditional branches with identical expectations.
src/test/app/LoanBroker_test.cpp Removes assignment-in-if pattern by splitting assignment and condition.
src/test/app/AccountSet_test.cpp Adds NOLINTNEXTLINE for float loop counter in a test.
src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp Combines two flag branches with identical validation logic.
src/libxrpl/nodestore/backend/RocksDBFactory.cpp Replaces std::bit_cast pointer casts with reinterpret_cast for RocksDB slices.
src/libxrpl/ledger/helpers/TokenHelpers.cpp Simplifies conditional clearing of amounts with short-circuit logic.
src/libxrpl/conditions/Fulfillment.cpp Removes unreachable break statements after return.
src/libxrpl/conditions/Condition.cpp Removes unreachable code in unsupported-type switch cases.
include/xrpl/protocol/detail/token_errors.h Removes redundant switch case where default already returns same string.
include/xrpl/config/BasicConfig.h Removes assignment-in-if by separating assignment from condition.
.clang-tidy Enables more bugprone-* checks by removing multiple exclusions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/csf/Sim.h
Comment thread src/libxrpl/nodestore/backend/RocksDBFactory.cpp
Comment thread src/test/app/XChain_test.cpp
Comment thread src/test/csf/Sim.h
Comment thread src/xrpld/rpc/detail/Pathfinder.cpp
Comment thread src/xrpld/rpc/detail/Pathfinder.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants