From 2e595b603118963d209ef81386dfed4a413912c2 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Thu, 26 Feb 2026 18:26:58 +0000 Subject: [PATCH 1/2] chore: Enable clang-tidy checks without issues (#6414) This change enables all clang-tidy checks that are already passing. It also modifies the clang-tidy CI job, so it runs against all files if .clang-tidy changed. --- .clang-tidy | 311 +++++++++--------- .../workflows/reusable-clang-tidy-files.yml | 4 +- .github/workflows/reusable-clang-tidy.yml | 14 +- CONTRIBUTING.md | 23 ++ 4 files changed, 196 insertions(+), 156 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index f7009c4666..5f4187b008 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,105 +1,143 @@ --- Checks: "-*, - bugprone-argument-comment + bugprone-argument-comment, + bugprone-assert-side-effect, + bugprone-bad-signal-to-kill-thread, + bugprone-bool-pointer-implicit-conversion, + bugprone-casting-through-void, + bugprone-chained-comparison, + bugprone-compare-pointer-to-member-virtual-function, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-dynamic-static-initializers, + bugprone-fold-init-type, + bugprone-forward-declaration-namespace, + bugprone-inaccurate-erase, + bugprone-incorrect-enable-if, + bugprone-incorrect-roundings, + bugprone-infinite-loop, + bugprone-integer-division, + bugprone-lambda-function-name, + bugprone-macro-parentheses, + bugprone-macro-repeated-side-effects, + bugprone-misplaced-operator-in-strlen-in-alloc, + bugprone-misplaced-pointer-arithmetic-in-alloc, + bugprone-misplaced-widening-cast, + bugprone-multi-level-implicit-pointer-conversion, + bugprone-multiple-new-in-one-expression, + bugprone-multiple-statement-macro, + bugprone-no-escape, + bugprone-non-zero-enum-to-bool-conversion, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-redundant-branch-condition, + bugprone-shared-ptr-array-mismatch, + bugprone-signal-handler, + bugprone-signed-char-misuse, + bugprone-sizeof-container, + bugprone-spuriously-wake-up-functions, + bugprone-standalone-empty, + bugprone-string-constructor, + bugprone-string-integer-assignment, + bugprone-string-literal-with-embedded-nul, + bugprone-stringview-nullptr, + bugprone-suspicious-enum-usage, + bugprone-suspicious-include, + bugprone-suspicious-memory-comparison, + bugprone-suspicious-memset-usage, + bugprone-suspicious-realloc-usage, + bugprone-suspicious-semicolon, + bugprone-suspicious-string-compare, + bugprone-swapped-arguments, + bugprone-terminating-continue, + bugprone-throw-keyword-missing, + bugprone-undefined-memory-manipulation, + bugprone-undelegated-constructor, + bugprone-unhandled-exception-at-new, + bugprone-unique-ptr-array-mismatch, + bugprone-unsafe-functions, + bugprone-virtual-near-miss, + cppcoreguidelines-no-suspend-with-lock, + cppcoreguidelines-virtual-class-destructor, + hicpp-ignored-remove-result, + misc-definitions-in-headers, + misc-header-include-cycle, + misc-misplaced-const, + misc-static-assert, + misc-throw-by-value-catch-by-reference, + misc-unused-alias-decls, + misc-unused-using-decls, + readability-duplicate-include, + readability-enum-initial-value, + readability-misleading-indentation, + readability-non-const-parameter, + readability-redundant-declaration, + readability-reference-to-constructed-temporary, + modernize-deprecated-headers, + modernize-make-shared, + modernize-make-unique, + performance-implicit-conversion-in-loop, + performance-move-constructor-init, + performance-trivially-destructible " -# bugprone-assert-side-effect, -# bugprone-bad-signal-to-kill-thread, -# bugprone-bool-pointer-implicit-conversion, -# bugprone-casting-through-void, -# bugprone-chained-comparison, -# bugprone-compare-pointer-to-member-virtual-function, -# bugprone-copy-constructor-init, -# bugprone-crtp-constructor-accessibility, -# bugprone-dangling-handle, -# bugprone-dynamic-static-initializers, +# --- +# checks that have some issues that need to be resolved: +# # bugprone-empty-catch, -# bugprone-fold-init-type, -# bugprone-forward-declaration-namespace, -# bugprone-inaccurate-erase, +# bugprone-crtp-constructor-accessibility, # bugprone-inc-dec-in-conditions, -# bugprone-incorrect-enable-if, -# bugprone-incorrect-roundings, -# bugprone-infinite-loop, -# bugprone-integer-division, -# bugprone-lambda-function-name, -# bugprone-macro-parentheses, -# bugprone-macro-repeated-side-effects, -# bugprone-misplaced-operator-in-strlen-in-alloc, -# bugprone-misplaced-pointer-arithmetic-in-alloc, -# bugprone-misplaced-widening-cast, -# bugprone-move-forwarding-reference, -# bugprone-multi-level-implicit-pointer-conversion, -# bugprone-multiple-new-in-one-expression, -# bugprone-multiple-statement-macro, -# bugprone-no-escape, -# bugprone-non-zero-enum-to-bool-conversion, -# bugprone-optional-value-conversion, -# bugprone-parent-virtual-call, -# bugprone-pointer-arithmetic-on-polymorphic-object, -# bugprone-posix-return, -# bugprone-redundant-branch-condition, # bugprone-reserved-identifier, -# bugprone-return-const-ref-from-parameter, -# bugprone-shared-ptr-array-mismatch, -# bugprone-signal-handler, -# bugprone-signed-char-misuse, -# bugprone-sizeof-container, -# bugprone-sizeof-expression, -# bugprone-spuriously-wake-up-functions, -# bugprone-standalone-empty, -# bugprone-string-constructor, -# bugprone-string-integer-assignment, -# bugprone-string-literal-with-embedded-nul, -# bugprone-stringview-nullptr, -# bugprone-suspicious-enum-usage, -# bugprone-suspicious-include, -# bugprone-suspicious-memory-comparison, -# bugprone-suspicious-memset-usage, -# bugprone-suspicious-missing-comma, -# bugprone-suspicious-realloc-usage, -# bugprone-suspicious-semicolon, -# bugprone-suspicious-string-compare, -# bugprone-suspicious-stringview-data-usage, -# bugprone-swapped-arguments, -# bugprone-switch-missing-default-case, -# bugprone-terminating-continue, -# bugprone-throw-keyword-missing, -# bugprone-too-small-loop-variable, -# bugprone-undefined-memory-manipulation, -# bugprone-undelegated-constructor, -# bugprone-unhandled-exception-at-new, -# bugprone-unhandled-self-assignment, -# bugprone-unique-ptr-array-mismatch, -# bugprone-unsafe-functions, +# bugprone-move-forwarding-reference, # bugprone-unused-local-non-trivial-variable, -# bugprone-unused-raii, +# bugprone-return-const-ref-from-parameter, +# bugprone-switch-missing-default-case, +# bugprone-sizeof-expression, +# bugprone-suspicious-stringview-data-usage, +# bugprone-suspicious-missing-comma, +# bugprone-pointer-arithmetic-on-polymorphic-object, +# bugprone-optional-value-conversion, +# bugprone-too-small-loop-variable, # bugprone-unused-return-value, # bugprone-use-after-move, -# bugprone-virtual-near-miss, -# cppcoreguidelines-init-variables, +# bugprone-unhandled-self-assignment, +# bugprone-unused-raii, +# # cppcoreguidelines-misleading-capture-default-by-value, -# cppcoreguidelines-no-suspend-with-lock, +# cppcoreguidelines-init-variables, # cppcoreguidelines-pro-type-member-init, # cppcoreguidelines-pro-type-static-cast-downcast, -# cppcoreguidelines-rvalue-reference-param-not-moved, # cppcoreguidelines-use-default-member-init, -# cppcoreguidelines-virtual-class-destructor, -# hicpp-ignored-remove-result, +# cppcoreguidelines-rvalue-reference-param-not-moved, +# # llvm-namespace-comment, # misc-const-correctness, -# misc-definitions-in-headers, -# misc-header-include-cycle, # misc-include-cleaner, -# misc-misplaced-const, # misc-redundant-expression, -# misc-static-assert, -# misc-throw-by-value-catch-by-reference, -# misc-unused-alias-decls, -# misc-unused-using-decls, +# +# readability-avoid-nested-conditional-operator, +# readability-avoid-return-with-void-value, +# readability-braces-around-statements, +# readability-container-contains, +# readability-container-size-empty, +# readability-convert-member-functions-to-static, +# readability-const-return-type, +# readability-else-after-return, +# readability-implicit-bool-conversion, +# readability-inconsistent-declaration-parameter-name, +# readability-identifier-naming, +# readability-make-member-function-const, +# readability-math-missing-parentheses, +# readability-redundant-inline-specifier, +# readability-redundant-member-init, +# readability-redundant-casting, +# readability-redundant-string-init, +# readability-simplify-boolean-expr, +# readability-static-definition-in-anonymous-namespace, +# readability-suspicious-call-argument, +# readability-use-std-min-max, +# readability-static-accessed-through-instance, +# # modernize-concat-nested-namespaces, -# modernize-deprecated-headers, -# modernize-make-shared, -# modernize-make-unique, # modernize-pass-by-value, # modernize-type-traits, # modernize-use-designated-initializers, @@ -111,79 +149,50 @@ Checks: "-*, # modernize-use-starts-ends-with, # modernize-use-std-numbers, # modernize-use-using, +# # performance-faster-string-find, # performance-for-range-copy, -# performance-implicit-conversion-in-loop, # performance-inefficient-vector-operation, # performance-move-const-arg, -# performance-move-constructor-init, # performance-no-automatic-move, -# performance-trivially-destructible, -# readability-avoid-nested-conditional-operator, -# readability-avoid-return-with-void-value, -# readability-braces-around-statements, -# readability-const-return-type, -# readability-container-contains, -# readability-container-size-empty, -# readability-convert-member-functions-to-static, -# readability-duplicate-include, -# readability-else-after-return, -# readability-enum-initial-value, -# readability-implicit-bool-conversion, -# readability-inconsistent-declaration-parameter-name, -# readability-identifier-naming, -# readability-make-member-function-const, -# readability-math-missing-parentheses, -# readability-misleading-indentation, -# readability-non-const-parameter, -# readability-redundant-casting, -# readability-redundant-declaration, -# readability-redundant-inline-specifier, -# readability-redundant-member-init, -# readability-redundant-string-init, -# readability-reference-to-constructed-temporary, -# readability-simplify-boolean-expr, -# readability-static-accessed-through-instance, -# readability-static-definition-in-anonymous-namespace, -# readability-suspicious-call-argument, -# readability-use-std-min-max +# --- # -# CheckOptions: -# readability-braces-around-statements.ShortStatementLines: 2 -# readability-identifier-naming.MacroDefinitionCase: UPPER_CASE -# readability-identifier-naming.ClassCase: CamelCase -# readability-identifier-naming.StructCase: CamelCase -# readability-identifier-naming.UnionCase: CamelCase -# readability-identifier-naming.EnumCase: CamelCase -# readability-identifier-naming.EnumConstantCase: CamelCase -# readability-identifier-naming.ScopedEnumConstantCase: CamelCase -# readability-identifier-naming.GlobalConstantCase: UPPER_CASE -# readability-identifier-naming.GlobalConstantPrefix: "k" -# readability-identifier-naming.GlobalVariableCase: CamelCase -# readability-identifier-naming.GlobalVariablePrefix: "g" -# readability-identifier-naming.ConstexprFunctionCase: camelBack -# readability-identifier-naming.ConstexprMethodCase: camelBack -# readability-identifier-naming.ClassMethodCase: camelBack -# readability-identifier-naming.ClassMemberCase: camelBack -# readability-identifier-naming.ClassConstantCase: UPPER_CASE -# readability-identifier-naming.ClassConstantPrefix: "k" -# readability-identifier-naming.StaticConstantCase: UPPER_CASE -# readability-identifier-naming.StaticConstantPrefix: "k" -# readability-identifier-naming.StaticVariableCase: UPPER_CASE -# readability-identifier-naming.StaticVariablePrefix: "k" -# readability-identifier-naming.ConstexprVariableCase: UPPER_CASE -# readability-identifier-naming.ConstexprVariablePrefix: "k" -# readability-identifier-naming.LocalConstantCase: camelBack -# readability-identifier-naming.LocalVariableCase: camelBack -# readability-identifier-naming.TemplateParameterCase: CamelCase -# readability-identifier-naming.ParameterCase: camelBack -# readability-identifier-naming.FunctionCase: camelBack -# readability-identifier-naming.MemberCase: camelBack -# readability-identifier-naming.PrivateMemberSuffix: _ -# readability-identifier-naming.ProtectedMemberSuffix: _ -# readability-identifier-naming.PublicMemberSuffix: "" -# readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*" -# bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true +CheckOptions: + # readability-braces-around-statements.ShortStatementLines: 2 + # readability-identifier-naming.MacroDefinitionCase: UPPER_CASE + # readability-identifier-naming.ClassCase: CamelCase + # readability-identifier-naming.StructCase: CamelCase + # readability-identifier-naming.UnionCase: CamelCase + # readability-identifier-naming.EnumCase: CamelCase + # readability-identifier-naming.EnumConstantCase: CamelCase + # readability-identifier-naming.ScopedEnumConstantCase: CamelCase + # readability-identifier-naming.GlobalConstantCase: UPPER_CASE + # readability-identifier-naming.GlobalConstantPrefix: "k" + # readability-identifier-naming.GlobalVariableCase: CamelCase + # readability-identifier-naming.GlobalVariablePrefix: "g" + # readability-identifier-naming.ConstexprFunctionCase: camelBack + # readability-identifier-naming.ConstexprMethodCase: camelBack + # readability-identifier-naming.ClassMethodCase: camelBack + # readability-identifier-naming.ClassMemberCase: camelBack + # readability-identifier-naming.ClassConstantCase: UPPER_CASE + # readability-identifier-naming.ClassConstantPrefix: "k" + # readability-identifier-naming.StaticConstantCase: UPPER_CASE + # readability-identifier-naming.StaticConstantPrefix: "k" + # readability-identifier-naming.StaticVariableCase: UPPER_CASE + # readability-identifier-naming.StaticVariablePrefix: "k" + # readability-identifier-naming.ConstexprVariableCase: UPPER_CASE + # readability-identifier-naming.ConstexprVariablePrefix: "k" + # readability-identifier-naming.LocalConstantCase: camelBack + # readability-identifier-naming.LocalVariableCase: camelBack + # readability-identifier-naming.TemplateParameterCase: CamelCase + # readability-identifier-naming.ParameterCase: camelBack + # readability-identifier-naming.FunctionCase: camelBack + # readability-identifier-naming.MemberCase: camelBack + # readability-identifier-naming.PrivateMemberSuffix: _ + # readability-identifier-naming.ProtectedMemberSuffix: _ + # readability-identifier-naming.PublicMemberSuffix: "" + # readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*" + bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true # bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc # misc-include-cleaner.IgnoreHeaders: '.*/(detail|impl)/.*;.*(expected|unexpected).*;.*ranges_lower_bound\.h;time.h;stdlib.h;__chrono/.*;fmt/chrono.h;boost/uuid/uuid_hash.hpp' # diff --git a/.github/workflows/reusable-clang-tidy-files.yml b/.github/workflows/reusable-clang-tidy-files.yml index 432da1d15c..d36dea747c 100644 --- a/.github/workflows/reusable-clang-tidy-files.yml +++ b/.github/workflows/reusable-clang-tidy-files.yml @@ -78,9 +78,9 @@ jobs: id: run_clang_tidy continue-on-error: true env: - FILES: ${{ inputs.files }} + TARGETS: ${{ inputs.files != '' && inputs.files || 'src tests' }} run: | - run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "$BUILD_DIR" $FILES 2>&1 | tee clang-tidy-output.txt + run-clang-tidy -j ${{ steps.nproc.outputs.nproc }} -p "${BUILD_DIR}" ${TARGETS} 2>&1 | tee clang-tidy-output.txt - name: Upload clang-tidy output if: steps.run_clang_tidy.outcome != 'success' diff --git a/.github/workflows/reusable-clang-tidy.yml b/.github/workflows/reusable-clang-tidy.yml index 7c300ee26e..7050d3509f 100644 --- a/.github/workflows/reusable-clang-tidy.yml +++ b/.github/workflows/reusable-clang-tidy.yml @@ -22,7 +22,8 @@ jobs: if: ${{ inputs.check_only_changed }} runs-on: ubuntu-latest outputs: - any_changed: ${{ steps.changed_files.outputs.any_changed }} + clang_tidy_config_changed: ${{ steps.changed_clang_tidy.outputs.any_changed }} + any_cpp_changed: ${{ steps.changed_files.outputs.any_changed }} all_changed_files: ${{ steps.changed_files.outputs.all_changed_files }} steps: - name: Checkout repository @@ -38,10 +39,17 @@ jobs: **/*.ipp separator: " " + - name: Get changed clang-tidy configuration + id: changed_clang_tidy + uses: tj-actions/changed-files@7dee1b0c1557f278e5c7dc244927139d78c0e22a # v47.0.4 + with: + files: | + .clang-tidy + run-clang-tidy: needs: [determine-files] - if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.any_changed == 'true') }} + if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.any_cpp_changed == 'true' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }} uses: ./.github/workflows/reusable-clang-tidy-files.yml with: - files: ${{ inputs.check_only_changed && needs.determine-files.outputs.all_changed_files || '' }} + files: ${{ (needs.determine-files.outputs.clang_tidy_config_changed == 'true' && '') || (inputs.check_only_changed && needs.determine-files.outputs.all_changed_files || '') }} create_issue_on_failure: ${{ inputs.create_issue_on_failure }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a928065ef2..4bb1db8689 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -251,6 +251,29 @@ pip3 install pre-commit pre-commit install ``` +## Clang-tidy + +All code must pass `clang-tidy` checks according to the settings in [`.clang-tidy`](./.clang-tidy). + +There is a Continuous Integration job that runs clang-tidy on pull requests. The CI will check: + +- All changed C++ files (`.cpp`, `.h`, `.ipp`) when only code files are modified +- **All files in the repository** when the `.clang-tidy` configuration file is changed + +This ensures that configuration changes don't introduce new warnings across the codebase. + +### Running clang-tidy locally + +Before running clang-tidy, you must build the project to generate required files (particularly protobuf headers). Refer to [`BUILD.md`](./BUILD.md) for build instructions. + +Then run clang-tidy on your local changes: + +``` +run-clang-tidy -p build src tests +``` + +This will check all source files in the `src` and `tests` directories using the compile commands from your `build` directory. + ## Contracts and instrumentation We are using [Antithesis](https://antithesis.com/) for continuous fuzzing, From 404f35d5568f60d914b9f2749fff9518cd1324cb Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 26 Feb 2026 22:01:38 -0500 Subject: [PATCH 2/2] test: Grep for failures in CI (#6339) This change adjusts the CI tests to make it easier to spot errors, without needing to sift through the thousands of lines of output. --- .github/workflows/reusable-build-test-config.yml | 15 ++++++++++++++- src/test/app/Vault_test.cpp | 8 ++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 6060a208fe..dabcc737f8 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -229,8 +229,21 @@ jobs: env: BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} run: | - ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" + set -o pipefail + ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log + - name: Show test failure summary + if: ${{ failure() && !inputs.build_only }} + working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }} + run: | + if [ ! -f unittest.log ]; then + echo "unittest.log not found; embedded tests may not have run." + exit 0 + fi + + if ! grep -E "failed" unittest.log; then + echo "Log present but no failure lines found in unittest.log." + fi - name: Debug failure (Linux) if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }} run: | diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 93ac94d7ce..7ae9faf18f 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5340,20 +5340,20 @@ class Vault_test : public beast::unit_test::suite env.close(); // 2. Mantissa larger than uint64 max + env.set_parse_failure_expected(true); try { tx[sfAssetsMaximum] = "18446744073709551617e5"; // uint64 max + 1 env(tx, THISLINE); - BEAST_EXPECT(false); + BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max"); } catch (parse_error const& e) { using namespace std::string_literals; BEAST_EXPECT( - e.what() == - "invalidParamsField 'tx_json.AssetsMaximum' has invalid " - "data."s); + e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s); } + env.set_parse_failure_expected(false); } }