Fix: clang tidy issues (#2208)

* refactor: Empty Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * refactor: log msgs Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * refactor: std::array instead C-array Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * refactor: add missed todo-labels Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: crash in ProcessAndPipes Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: use requires and concepts Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: wrong using of movement Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: redundant or missed initializer Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: c-array warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: enum size Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: clang-format-18 issues Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: naming Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: const cast warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: static downcast warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: reinterpret cast warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: const or ref data members Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: pointers arithmetic Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: missed destructor Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: non-const globals Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: narrowing conversion warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: use modern ranges algorithms Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: using namespace Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: typedef vs using Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: wrong rvalue and forward Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: else after return or break Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: avoid-endl warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: avoid-do-while warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: missing-std::forward warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: avoid making a copy Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: movement of trivially-copyable types Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: move instead once-copy Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: use designated initializers Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: container contains Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: using c-vararg calls Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: add special member-func Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: return value Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: getting value of wrapped ref Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: virtual-override warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: avoid union access Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: macro warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: replace push* by emplace* Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: var/member init warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: reserve before insert in loop Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: array element access by non-cont index Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: update .clang-tidy settings file Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: remaining various warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * Revert "refactor: Empty" This reverts commit 47a1133. * fix: mock classes Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: broken author api Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: infinite recursion in BufferView Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: decoding of version Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * rollback: unintended changes Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: review issues Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: CI's gcc steps Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * fix: newly detected clang-tidy warnings Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> * hotfix Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]> --------- Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
qdrvm · Sep 16, 2024 · 3924247 · 3924247
1 parent 4115826
commit 3924247
Show file tree

Hide file tree

Showing 344 changed files with 2,683 additions and 2,320 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -5,9 +5,50 @@
 #
 
 ---
-Checks:          '-*,clang-analyzer-*,clang-diagnostic-*,readability-*,modernize-*,boost-*,bugprone-*,cppcoreguidelines-*,google-*,hicpp-*,performance-*,readability-*,-google-readability-namespace-comments,-readability-inconsistent-declaration-parameter-name,-readability-braces-around-statements,-hicpp-signed-bitwise,-google-runtime-references,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-readability-magic-numbers,-hicpp-explicit-conversions,-hicpp-uppercase-literal-suffix,-readability-uppercase-literal-suffix,-hicpp-no-array-decay,-hicpp-special-member-functions,-bugprone-narrowing-conversions,-modernize-use-nodiscard,-google-readability-braces-around-statements,-hicpp-braces-around-statements,-bugprone-suspicious-semicolon,-readability-named-parameter,-hicpp-named-parameter,-readability-identifier-naming,-modernize-use-trailing-return-type'
-WarningsAsErrors: 'modernize-*,cppcoreguidelines-*,boost-*,performance-*,google-build-using-namespace,readability-else-after-return,google-readability-todo'
-HeaderFilterRegex: '\.(hpp|h)'
+Checks: "-*,
+  clang-analyzer-*, \
+  clang-diagnostic-*, \
+    -clang-diagnostic-missing-template-arg-list-after-template-kw, \
+  modernize-*, \
+    -modernize-use-nodiscard, \
+    -modernize-use-trailing-return-type, \
+  boost-*, \
+  bugprone-*, \
+    -bugprone-narrowing-conversions, \
+    -bugprone-suspicious-semicolon, \
+  cppcoreguidelines-*, \
+    -cppcoreguidelines-rvalue-reference-param-not-moved, \
+    -cppcoreguidelines-avoid-magic-numbers, \
+    -cppcoreguidelines-pro-bounds-array-to-pointer-decay, \
+  google-*, \
+    -google-readability-namespace-comments, \
+    -google-readability-braces-around-statements, \
+    -google-runtime-references, \
+  hicpp-*, \
+    -hicpp-explicit-conversions, \
+    -hicpp-uppercase-literal-suffix, \
+    -hicpp-signed-bitwise, \
+    -hicpp-no-array-decay, \
+    -hicpp-special-member-functions, \
+    -hicpp-braces-around-statements, \
+    -hicpp-named-parameter, \
+  performance-*, \
+  readability-*, \
+    -readability-magic-numbers, \
+    -readability-named-parameter, \
+    -readability-identifier-naming, \
+    -readability-uppercase-literal-suffix, \
+    -readability-inconsistent-declaration-parameter-name, \
+    -readability-braces-around-statements"
+WarningsAsErrors: "modernize-*, \
+  cppcoreguidelines-*, \
+  boost-*, \
+  performance-*, \
+  google-build-using-namespace, \
+  readability-else-after-return, \
+  readability-container-contains, \
+  google-readability-todo"
+HeaderFilterRegex: '\(hpp|h|pb\.h)'
 FormatStyle:     .clang-format
 User:            bogdan
 CheckOptions:
@@ -53,6 +94,8 @@ CheckOptions:
     value:           '0'
   - key:             bugprone-unused-return-value.CheckedFunctions
     value:           '::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique;::std::unique_ptr::release;::std::basic_string::empty;::std::vector::empty'
+  - key:             cppcoreguidelines-avoid-do-while.IgnoreMacros
+    value:           '1'
   - key:             cppcoreguidelines-no-malloc.Allocations
     value:           '::malloc;::calloc'
   - key:             cppcoreguidelines-no-malloc.Deallocations
@@ -243,6 +286,8 @@ CheckOptions:
     value:           '4294967295'
   - key:             readability-identifier-naming.IgnoreFailedSplit
     value:           '0'
+  - key:             readability-identifier-length.Missing
+    value:           '(it|cb|db|id|in|ts|tx|i|n|s)'
   - key:             readability-implicit-bool-conversion.AllowIntegerConditions
     value:           '0'
   - key:             readability-implicit-bool-conversion.AllowPointerConditions

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -64,12 +64,12 @@ jobs:
       env:
         KAGOME_IN_DOCKER: 0
         KAGOME_MAC_CI: 1
-        
-        
+
+
   Linux:
     if: false # Need to fix
-      # ${{ 
-      # !(github.ref == 'refs/heads/master' || 
+      # ${{
+      # !(github.ref == 'refs/heads/master' ||
       # startsWith( github.ref, 'refs/tags/' )) }}
     strategy:
       fail-fast: false
@@ -196,7 +196,7 @@ jobs:
 
   kagome_dev_docker_build_tidy:
     runs-on: ubuntu-latest
-    timeout-minutes: 180
+    timeout-minutes: 600 # TODO(xDimon): revert after merge PR#2208
     name: "Linux: clang-tidy"
 
     steps:
@@ -346,7 +346,7 @@ jobs:
           KAGOME_PACKAGE_VERSION=${{ env.package_version }} \
           DOCKER_REGISTRY_PATH=${DOCKER_REGISTRY_PATH} \
           BUILD_TYPE=${{ matrix.options.build_type }}
-          
+
       - name: "Push Docker Image to GCR"
         if: env.package_exist == 'False'
         working-directory: ./housekeeping/docker/kagome-dev
@@ -364,7 +364,7 @@ jobs:
         with:
           username: ${{ secrets.DOCKER_USERNAME }}
           password: ${{ secrets.DOCKER_TOKEN }}
-          
+
       - name: "Push Docker Image to Docker Hub"
         if: ${{ 
           (env.package_exist == 'False') && 
@@ -388,7 +388,7 @@ jobs:
         with:
           path: ${{ env.CACHE_PATH }}
           key: ${{ steps.cache-restore.outputs.cache-primary-key }}
-          
+
       - name: "Versions Info"
         if: always()
         run: |

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -36,7 +36,7 @@ set(CMAKE_TOOLCHAIN_FILE
     CACHE FILEPATH "Default toolchain")
 
 # SHA1 hash of the WasmEdge repository archive.
-# Required to separate cached runtimes 
+# Required to separate cached runtimes
 # compiled with different WasmEdge versions.
 set(WASMEDGE_ID 58aea400de9179ad3e314c7e84fd4da345b8a643)
 
@@ -195,7 +195,9 @@ include(GNUInstallDirs)
 include(cmake/install.cmake)
 
 include_directories($<INSTALL_INTERFACE:${CMAKE_INSTALL_FULL_INCLUDEDIR}/kagome>
-    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/core>)
+    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/core>
+    $<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/core> # for generated file
+)
 
 add_subdirectory(core)
 

diff --git a/cmake/Hunter/init.cmake b/cmake/Hunter/init.cmake
@@ -32,5 +32,11 @@ set(HUNTER_CACHE_SERVERS
 #     CACHE STRING "Disable binary cache"
 # )
 
+# https://hunter.readthedocs.io/en/latest/reference/user-variables.html#hunter-status-debug
+# set(
+#     HUNTER_STATUS_DEBUG ON
+#     CACHE STRING "Enable output lot of info for debugging"
+# )
+
 include(${CMAKE_CURRENT_LIST_DIR}/HunterGate.cmake)
 include(${CMAKE_CURRENT_LIST_DIR}/hunter-gate-url.cmake)
diff --git a/cmake/functions.cmake b/cmake/functions.cmake
@@ -96,10 +96,6 @@ function(compile_proto_to_cpp PROTO_LIBRARY_NAME PB_H PB_CC PROTO)
   set(${PB_CC} ${SCHEMA_OUT_DIR}/${SCHEMA_REL}/${GEN_PB} PARENT_SCOPE)
 endfunction()
 
-add_custom_target(generated
-    COMMENT "Building generated files..."
-    )
-
 function(add_proto_library NAME)
   set(SOURCES "")
   foreach(PROTO IN ITEMS ${ARGN})
@@ -119,8 +115,14 @@ function(add_proto_library NAME)
       # required for compiling proto targets
       $<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/pb/${NAME}/generated>
       )
+  kagome_install(${NAME})
 
   disable_clang_tidy(${NAME})
 
+  if (NOT TARGET generated)
+    add_custom_target(generated
+        COMMENT "Building generated files..."
+    )
+  endif()
   add_dependencies(generated ${NAME})
 endfunction()
diff --git a/core/api/jrpc/jrpc_handle_batch.cpp b/core/api/jrpc/jrpc_handle_batch.cpp
@@ -15,7 +15,7 @@ namespace kagome::api {
    */
   template <typename Cb>
   struct Parser {
-    Cb cb;
+    std::remove_reference_t<Cb> cb;
 
     rapidjson::MemoryStream stream{nullptr, 0};
     size_t level = 0;
@@ -85,6 +85,7 @@ namespace kagome::api {
       --level;
       if (level == 1) {
         const size_t end = stream.Tell();
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
         std::string_view request(stream.begin_ + begin, end - begin);
         cb(request);
       }
@@ -96,7 +97,7 @@ namespace kagome::api {
                                    std::string_view request) {
     std::string request_string;
     if (!request.empty() && request[0] == '[') {
-      const auto cb = [&](std::string_view request) {
+      auto cb = [&](std::string_view request) {
         request_string = request;
         auto formatted = handler.HandleRequest(request_string);
         if (formatted->GetSize() == 0) {
@@ -109,7 +110,7 @@ namespace kagome::api {
         }
         batch_.append(formatted->GetData(), formatted->GetSize());
       };
-      Parser<decltype(cb) &> parser{cb};
+      Parser<decltype(cb) &> parser{.cb = std::move(cb)};
       if (parser.parse(request)) {
         if (!batch_.empty()) {
           batch_.push_back(']');

diff --git a/core/api/jrpc/value_converter.hpp b/core/api/jrpc/value_converter.hpp
@@ -206,15 +206,13 @@ namespace kagome::api {
     data["stateVersion"] = makeValue(val.state_version);
 
     jArray apis;
-    std::transform(val.apis.begin(),
-                   val.apis.end(),
-                   std::back_inserter(apis),
-                   [](const auto &api) {
-                     jArray api_data;
-                     api_data.emplace_back(hex_lower_0x(api.first));
-                     api_data.emplace_back(makeValue(api.second));
-                     return api_data;
-                   });
+    std::ranges::transform(
+        val.apis, std::back_inserter(apis), [](const auto &api) {
+          jArray api_data;
+          api_data.emplace_back(hex_lower_0x(api.first));
+          api_data.emplace_back(makeValue(api.second));
+          return api_data;
+        });
 
     data["apis"] = std::move(apis);
     return data;
@@ -313,11 +311,10 @@ namespace kagome::api {
             -> jsonrpc::Value {
           jArray peers;
           peers.resize(params.peers.size());
-          std::transform(
-              params.peers.begin(),
-              params.peers.end(),
-              peers.begin(),
-              [](const auto &peer_id) { return makeValue(peer_id.toHex()); });
+          std::ranges::transform(
+              params.peers, peers.begin(), [](const auto &peer_id) {
+                return makeValue(peer_id.toHex());
+              });
           return jStruct{std::pair{"broadcast", std::move(peers)}};
         },
         [](const primitives::events::InBlockEventParams &params) {

diff --git a/core/api/service/author/impl/author_api_impl.cpp b/core/api/service/author/impl/author_api_impl.cpp
@@ -43,8 +43,8 @@ namespace kagome::api {
         store_{std::move(store)},
         keys_{std::move(keys)},
         key_store_{std::move(key_store)},
-        api_service_{std::move(api_service)},
-        block_tree_{std::move(block_tree)},
+        api_service_{api_service},
+        block_tree_{block_tree},
         logger_{log::createLogger("AuthorApi", "author_api")} {
     BOOST_ASSERT_MSG(keys_api_ != nullptr, "session keys api is nullptr");
     BOOST_ASSERT_MSG(pool_ != nullptr, "transaction pool is nullptr");
@@ -63,8 +63,7 @@ namespace kagome::api {
   outcome::result<void> AuthorApiImpl::insertKey(crypto::KeyType key_type_id,
                                                  crypto::SecureBuffer<> seed,
                                                  const BufferView &public_key) {
-    if (std::find(kKeyTypes.begin(), kKeyTypes.end(), key_type_id)
-        == kKeyTypes.end()) {
+    if (std::ranges::find(kKeyTypes, key_type_id) == kKeyTypes.end()) {
       std::string types;
       for (auto &type : kKeyTypes) {
         types.append(type.toString());
@@ -112,25 +111,26 @@ namespace kagome::api {
   // it could be extended by reading config from chainspec palletSession/keys
   // value
   outcome::result<bool> AuthorApiImpl::hasSessionKeys(const BufferView &keys) {
-    scale::ScaleDecoderStream stream(keys);
-    std::array<uint8_t, 32> key;
     if (keys.size() < 32 || keys.size() > 32 * 6 || (keys.size() % 32) != 0) {
       SL_WARN(logger_,
               "not valid key sequence, author_hasSessionKeys RPC call expects "
               "no more than 6 public keys in concatenated string, keys should "
               "be 32 byte in size");
       return false;
     }
+    scale::ScaleDecoderStream stream(keys);
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,hicpp-member-init)
+    std::array<uint8_t, 32> key;
     stream >> key;
     if (store_->ed25519().findKeypair(
             crypto::KeyTypes::GRANDPA,
             crypto::Ed25519PublicKey(common::Blob<32>(key)))) {
-      unsigned count = 1;
+      auto it = crypto::polkadot_key_order.begin();
       while (stream.currentIndex() < keys.size()) {
+        ++it;
         stream >> key;
         if (not store_->sr25519().findKeypair(
-                crypto::polkadot_key_order[count++],
-                crypto::Sr25519PublicKey(common::Blob<32>(key)))) {
+                *it, crypto::Sr25519PublicKey(common::Blob<32>(key)))) {
           return false;
         }
       }

diff --git a/core/api/service/beefy/rpc.cpp b/core/api/service/beefy/rpc.cpp
@@ -17,8 +17,8 @@ namespace kagome::api {
       LazySPtr<network::Beefy> beefy,
       LazySPtr<blockchain::BlockHeaderRepository> block_header_repository)
       : server_{std::move(server)},
-        beefy_{std::move(beefy)},
-        block_header_repository_{std::move(block_header_repository)} {}
+        beefy_{beefy},
+        block_header_repository_{block_header_repository} {}
 
   void BeefyRpc::registerHandlers() {
     server_->registerHandler(

diff --git a/core/api/service/chain/impl/chain_api_impl.cpp b/core/api/service/chain/impl/chain_api_impl.cpp
@@ -34,7 +34,7 @@ namespace kagome::api {
       LazySPtr<api::ApiService> api_service)
       : header_repo_{std::move(block_repo)},
         block_tree_{std::move(block_tree)},
-        api_service_{std::move(api_service)},
+        api_service_{api_service},
         block_storage_{std::move(block_storage)} {
     BOOST_ASSERT_MSG(header_repo_ != nullptr,
                      "block repo parameter is nullptr");

diff --git a/core/api/service/chain/requests/get_block_hash.cpp b/core/api/service/chain/requests/get_block_hash.cpp
@@ -39,9 +39,9 @@ namespace kagome::api::chain::request {
       param.reserve(array.size());
       for (const auto &v : array) {
         if (v.IsInteger32()) {
-          param.emplace_back(VectorParam(v.AsInteger32()));
+          param.emplace_back(v.AsInteger32());
         } else if (v.IsString()) {
-          param.emplace_back(VectorParam(v.AsString()));
+          param.emplace_back(v.AsString());
         } else {
           throw jsonrpc::InvalidParametersFault("invalid argument");
         }

diff --git a/core/api/service/child_state/impl/child_state_api_impl.cpp b/core/api/service/child_state/impl/child_state_api_impl.cpp
@@ -132,7 +132,7 @@ namespace kagome::api {
     auto res = child_storage_trie_reader->tryGet(key);
     return common::map_result_optional(
         std::move(res),
-        [](common::BufferOrView &&r) { return r.intoBuffer(); });
+        [](common::BufferOrView &&r) { return std::move(r).intoBuffer(); });
   }
 
   outcome::result<std::optional<primitives::BlockHash>>