chore: Fix most possible clang-tidy autofixes by godexsoft · Pull Request #6422 · XRPLF/rippled

godexsoft · 2026-02-25T18:31:20Z

High Level Overview of Change

This PR enabled all checks in clang-tidy (other than include cleaner and identifier-naming) and ran -fix on all of them. Then i fixed all compilation errors, sometimes bringing back old code.
Note that the clang-tidy config is not modified in this PR. The idea is to be a bit more prepared for the upcoming one-by-one fixes of each clang-tidy check.

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

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)

…enerated files

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

…sues

codecov · 2026-02-27T14:07:57Z

Codecov Report

❌ Patch coverage is 90.73924% with 114 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.7%. Comparing base (2e595b6) to head (4d7d753).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/beast/insight/StatsDCollector.cpp	0.0%	16 Missing ⚠️
src/libxrpl/shamap/SHAMapDelta.cpp	59.0%	16 Missing ⚠️
src/libxrpl/shamap/SHAMapSync.cpp	75.5%	12 Missing ⚠️
src/libxrpl/basics/Archive.cpp	0.0%	7 Missing ⚠️
src/libxrpl/json/json_reader.cpp	79.4%	7 Missing ⚠️
src/libxrpl/nodestore/backend/RocksDBFactory.cpp	54.5%	5 Missing ⚠️
src/libxrpl/rdb/SociDB.cpp	37.5%	5 Missing ⚠️
src/libxrpl/nodestore/DatabaseRotatingImp.cpp	55.6%	4 Missing ⚠️
src/libxrpl/basics/Log.cpp	66.7%	3 Missing ⚠️
src/libxrpl/beast/utility/beast_PropertyStream.cpp	81.2%	3 Missing ⚠️
... and 24 more

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6422     +/-   ##
=========================================
- Coverage     79.8%   79.7%   -0.1%     
=========================================
  Files          848     848             
  Lines        67757   67948    +191     
  Branches      7554    7561      +7     
=========================================
+ Hits         54073   54163     +90     
- Misses       13684   13785    +101

Files with missing lines	Coverage Δ
include/xrpl/basics/BasicConfig.h	`89.7% <100.0%> (ø)`
include/xrpl/basics/Log.h	`71.4% <ø> (ø)`
include/xrpl/beast/net/IPEndpoint.h	`94.4% <ø> (ø)`
include/xrpl/beast/utility/PropertyStream.h	`11.5% <ø> (ø)`
include/xrpl/core/JobQueue.h	`100.0% <ø> (ø)`
include/xrpl/core/detail/Workers.h	`100.0% <ø> (ø)`
include/xrpl/json/json_value.h	`98.5% <ø> (ø)`
include/xrpl/json/json_writer.h	`100.0% <ø> (ø)`
include/xrpl/ledger/OpenView.h	`100.0% <ø> (ø)`
include/xrpl/protocol/PublicKey.h	`80.5% <ø> (ø)`
... and 193 more

... and 111 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

kuznetsss

So far got to the src/libxrpl/beast/net/IPAddressConversion.cpp, will continue later 😄
A few questions:

kuznetsss · 2026-02-27T17:00:19Z

src/libxrpl/basics/ResolverAsio.cpp

 class AsyncObject
 {
 protected:
+private:


Is it correct?

I don't see this code in my version anymore.. i think it's outdated.

kuznetsss · 2026-02-27T17:00:28Z

src/libxrpl/basics/ResolverAsio.cpp

    {
    }

+protected:


kuznetsss · 2026-02-27T17:46:09Z

src/libxrpl/beast/core/SemanticVersion.cpp

 compare(SemanticVersion const& lhs, SemanticVersion const& rhs)
 {
    if (lhs.majorVersion > rhs.majorVersion)
+    {


Looks like curly brackets are not needed here anymore, but it's ok to leave tham

I don't have them already in my code.

kuznetsss · 2026-02-27T17:46:52Z

src/libxrpl/beast/insight/Groups.cpp

    }

-    ~GroupsImp() = default;
+    ~GroupsImp() override = default;


Do we need to define this destructor at all?

kuznetsss · 2026-02-27T17:47:27Z

src/libxrpl/beast/insight/NullCollector.cpp

 private:
    NullHookImpl&
-    operator=(NullHookImpl const&);
+    operator=(NullHookImpl const&) = delete;


Then it doesn't need to be private

kuznetsss · 2026-02-27T17:47:56Z

src/libxrpl/beast/insight/StatsDCollector.cpp

 private:
    StatsDCounterImpl&
-    operator=(StatsDCounterImpl const&);
+    operator=(StatsDCounterImpl const&) = delete;


Same, doesn't need to be private

src/libxrpl/beast/insight/StatsDCollector.cpp

bthomee

I think I only got through 10% but your responses to my comments will help me review the rest better.

bthomee · 2026-03-01T21:06:31Z

include/xrpl/protocol/XChainAttestations.h

@@ -169,7 +169,7 @@ struct AttestationCreateAccount : AttestationBase
        Buffer signature_,
        AccountID const& sendingAccount_,
        STAmount const& sendingAmount_,


Shouldn't this STAmount const& sendingAmount_ also be changed like rewardAmount_?

I don't understand the question. Renaming variables is not something clang-tidy would do by itself :)

bthomee · 2026-03-01T21:08:29Z

include/xrpl/tx/paths/BookTip.h

    uint256 m_dir;
    uint256 m_index;


Shouldn't these bool and uint256 variables be initialized similarly? e.g. uint256 m_index{};

Since uint256 is a user type (vector) so it does not need explicit initialization, unlike say an int would. I think clang-tidy only adds those when otherwise you would have something unintialized.

bthomee · 2026-03-01T21:15:35Z

src/libxrpl/basics/Archive.cpp

+    int result = 0;
+    struct archive_entry* entry = nullptr;


Here and elsewhere, given the modern C++ convention of uniform initialization, wouldn't it be better to have something like the following instead of using copy initialization?

int result{}; struct archive_entry* entry{}

or if you want to be explicit:

int result{0}; struct archive_entry* entry{nullptr}

It's the same from language perspective (no copy). I suspect clang-tidy prefers to use curlies for members but assignment for normal variables. I'm not aware if we can control this. Worth checking when we are working on the relevant clang-tidy check.

bthomee · 2026-03-01T21:21:57Z

src/libxrpl/basics/base64.cpp

    auto const inverse = base64::get_inverse();

-    while (len-- && *in != '=')
+    while (((len--) != 0u) && *in != '=')


Suggested change

while (((len--) != 0u) && *in != '=')

while ((len--) != 0u && *in != '=')

Unnecessary extra parentheses.

clang-tidy isolates the refactored condition, hence the extra parens. We can use https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-parentheses.html to clean up later on i think. Don't think we should do this manually.

bthomee · 2026-03-01T21:27:13Z

src/libxrpl/basics/Number.cpp


    // Quadratic least squares curve fit of f^(1/d) in the range [0, 1]
-    auto const D = ((6 * di + 11) * di + 6) * di + 1;
+    auto const D = (((6 * di + 11) * di + 6) * di) + 1;


Were these parentheses really added by clang-tidy auto-fixes? They are totally unnecessary from a functional point of view.

I believe this is to do with arithmetic operators precedence. There was a clang-tidy check that makes sure there are parens around math where you have to know/guess what comes first. Parens make it obvious. Yes, no functional difference - it's a refactoring 👍

godexsoft and others added 10 commits February 24, 2026 14:53

Enable bugprone checks that have no issues atm

a92331d

Enable all checks that work already

f9b392e

Enable clang-tidy in CI if config changed

b5c8747

Add documentation about running clang-tidy and don't check protobuf/g…

ce5cea9

…enerated files

Update .github/workflows/reusable-clang-tidy.yml

cdb8add

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fix CI logic

17f87a6

Update .github/workflows/reusable-clang-tidy-files.yml

8c14fae

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

Update CONTRIBUTING.md

64cec52

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

Update CONTRIBUTING.md

b4dabb3

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>

Merge branch 'develop' into chore/enable-clang-tidy-checks-without-is…

4b10afc

…sues

godexsoft added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Feb 25, 2026

godexsoft added 2 commits February 26, 2026 18:43

Fix compilation after clang-tidy

2d6598d

Fix strange double const

c6ff6b6

godexsoft force-pushed the chore/clang-tidy-checks-autofixes branch from 9f0a5bd to c6ff6b6 Compare February 26, 2026 18:58

godexsoft added 5 commits February 26, 2026 19:20

Merge develop and fix issues

c3633ca

Add flag to ignore missing-field-initializers outside of MSVC

7be1454

Fix a logical-not-parentheses issue

92ac183

Fix issue specific to running in ci

10e61d1

Fix move issue

70b5e07

godexsoft added 2 commits February 27, 2026 14:43

Fix warnings turning errors

14805e1

Bring back size() to fix windows compilation

4d7d753

godexsoft marked this pull request as ready for review February 27, 2026 16:48

Copilot AI review requested due to automatic review settings February 27, 2026 16:48

Copilot AI reviewed Feb 27, 2026

View reviewed changes

godexsoft requested review from bthomee and kuznetsss February 27, 2026 16:48

godexsoft removed the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Feb 27, 2026

kuznetsss reviewed Feb 27, 2026

View reviewed changes

bthomee reviewed Mar 1, 2026

View reviewed changes

	while (((len--) != 0u) && *in != '=')
	while ((len--) != 0u && *in != '=')

Conversation

godexsoft commented Feb 25, 2026

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

kuznetsss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bthomee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Feb 27, 2026 •

edited

Loading