Modernization Phase 1.5: lint remainder — clangd-tidy from PyPI, suppressions triaged to zero

.clang-tidy

@@ -1,8 +1,3 @@
-# Note: trailing suppressions (from -modernize-use-designated-initializers
-# on) are checks added in clang-tidy versions newer than the 18 this config
-# was tuned for; they fire on existing code. Suppressed when the linter
-# moved to clangd 22 (needed to parse libc++ 22 headers) — to be triaged
-# and enabled in the lint-toolchain modernization phase.
 Checks: >
   -*,
   bugprone-*,
@@ -17,18 +12,7 @@ Checks: >
   -modernize-return-braced-init-list,
   -misc-non-private-member-variables-in-classes,
   -typecheck-expression-not-modifiable-lvalue,
-  -misc-use-internal-linkage,
-  -modernize-use-designated-initializers,
-  -bugprone-suspicious-stringview-data-usage,
-  -modernize-use-ranges,
-  -modernize-use-starts-ends-with,
-  -readability-container-contains,
-  -readability-avoid-return-with-void-value,
-  -readability-redundant-casting,
-  -readability-use-std-min-max,
-  -bugprone-unused-local-non-trivial-variable,
-  -bugprone-optional-value-conversion,
-  -performance-enum-size
+  -misc-use-internal-linkage
 
 # Turn all the warnings from the checks above into errors.
 WarningsAsErrors: "*"

.gitmodules

@@ -2,10 +2,6 @@
 	path = vcpkg
 	url = https://github.com/microsoft/vcpkg.git
 	ignore = dirty
-[submodule "clangd-tidy"]
-	path = clangd-tidy
-	url = https://github.com/lljbash/clangd-tidy.git
-	ignore = dirty
 [submodule "packages/streamr-trackerless-network/test/integration/ts-integration"]
 	path = packages/streamr-trackerless-network/test/integration/ts-integration
 	url = https://github.com/streamr-dev/native-ts-integration.git

MODERNIZATION.md

@@ -269,13 +269,35 @@ document/replace in 1.4.
   (deployment-target-26 / SDK-libc++ build, Personal Team signing);
   Android sanity via CI keyword.
 
-## Phase 1.5 — Lint stack remainder
+## Phase 1.5 — Lint stack remainder (PR pending)
 - clangd/clang-format 22 already landed in Phase 1.2 (forced by libc++ 22).
-  Remaining: bump the `clangd-tidy` submodule; triage the `.clang-tidy`
-  suppressions added in 1.2 (enable checks where cheap to satisfy, keep
-  suppressed with justification where not).
-- **Gate**: `./lint.sh` green both platforms; any format-only diff committed
-  separately.
+- **clangd-tidy: submodule → PyPI**. The submodule pinned tag 0.2.1 (a
+  single-script era); upstream 1.x is a Python package with dependencies
+  (attrs/cattrs/typing-extensions), so a bare checkout is no longer
+  runnable. The submodule is gone; `install-prerequisities.sh` does
+  `pipx install clangd-tidy==1.1.1` (version-pinned) on both platforms and
+  puts `~/.local/bin` on PATH; the 10 lint.sh call sites invoke it from
+  PATH. The unused `clang-tidy` symlink alias went with it. Gained since
+  0.2.1: `--line-filter` clang-tidy parity, diagnostic formatter fixes,
+  `clangd-tidy-diff`.
+- **All 11 post-18 check suppressions from Phase 1.2 removed — zero kept.**
+  The full-monorepo sweep fired 77 findings (plus 7 more exposed by also
+  dropping the nested test configs' suppressions, and 1 narrowing warning
+  introduced by the ranges conversion itself), all fixed in code:
+  readability-container-contains (26; includes two C++23
+  `std::string::contains` substring cases), modernize-use-designated-
+  initializers (22, incl. the two nested test configs' copies — also
+  removed), modernize-use-ranges (8), readability-avoid-return-with-void-
+  value (6, `return voidFn()` in void lambdas), bugprone-suspicious-
+  stringview-data-usage (4, string_view constants passed to `getenv()` →
+  now `const char*`), readability-redundant-casting (3, self-casts of
+  `const DhtCallContext&`), bugprone-unused-local-non-trivial-variable
+  (3 dead `debugString` debug leftovers deleted), performance-enum-size
+  (2 enums → `std::uint8_t`), bugprone-optional-value-conversion (1,
+  optional→value→optional round-trip in WebsocketServer), readability-
+  use-std-min-max (1), modernize-use-starts-ends-with (1).
+- **Gate**: `./lint.sh` green both platforms; full test suite green
+  (several fixes touch runtime code paths); format at fixed point.
 
 ## Phase 1.6 — CI/docs closeout
 - Revisit preview runner images (macos-26 / ubuntu-26.04) once GA; consider a

README.md

@@ -121,10 +121,13 @@ On the other hand,
 The root directory of the monorepo has the following structure:
 
 #### GIT submodules
-The Streamr Native SDK monorepo has two GIT submodules at its root:
+The Streamr Native SDK monorepo has one GIT submodule at its root:
 
 * `vcpkg` - [vcpkg package manager by Microsoft](https://github.com/microsoft/vcpkg) (installing as a submodule is the recommended way of installation)
-* `clangd-tidy` - [clangd-tidy](https://github.com/lljbash/clangd-tidy) A fast variant of the clang-tidy linter for C++. (not available as a brew or apt package)
+
+The [clangd-tidy](https://github.com/lljbash/clangd-tidy) linter (a fast
+variant of clang-tidy) is installed from PyPI by
+`install-prerequisities.sh` (`pipx install clangd-tidy==<pinned version>`).
 
 #### Directories
 * `build` - the main build directory for the whole monorepo.

install-prerequisities.sh

@@ -26,6 +26,7 @@ if [[ "$OSTYPE" == "darwin"* ]]; then
     TEMP_PROFILE_CONTENTS+="export HOMEBREW_PREFIX=$(brew --prefix)\n"
 
     brew install jq || true
+    brew install pipx || true
     # Latest LLVM (keg-only: not linked into $HOMEBREW_PREFIX/bin; the build
     # finds it via the LLVM_PREFIX environment variable exported below).
     brew install llvm || true
@@ -60,7 +61,7 @@ else
     # come from the same LLVM version on every platform: clangd must be able
     # to parse libc++ 22 headers, and clang-format versions must not diverge
     # between macOS and Linux or the format check flip-flops.
-    sudo apt-get install -y build-essential cmake ninja-build jq \
+    sudo apt-get install -y build-essential cmake ninja-build jq pipx \
         clang-22 lld-22 clang-tools-22 clangd-22 libc++-22-dev libc++abi-22-dev \
         clang-format-22 \
         autoconf autoconf-archive automake libtool
@@ -88,20 +89,21 @@ if [[ -n "$GITHUB_ENV" ]]; then
 fi
 TEMP_PROFILE_CONTENTS+="export CMAKE_GENERATOR=Ninja\n"
 
-cd clangd-tidy
-rm -f clang-tidy
-ln -s clangd-tidy clang-tidy
-cd ..
-
-TEMP_PROFILE_CONTENTS+="export PATH=$(pwd)/clangd-tidy:\$PATH\n"
-
-CLANGD_TIDY_PATH="$(pwd)/clangd-tidy"
-
-if [[ ":$PATH:" != *":$CLANGD_TIDY_PATH:"* ]]; then
-    export PATH="$CLANGD_TIDY_PATH:$PATH"
-    if [[ -n "$GITHUB_PATH" ]]; then
-        echo "$CLANGD_TIDY_PATH" >> $GITHUB_PATH
-    fi
+# clangd-tidy (the lint driver) comes from PyPI, version-pinned. It used to
+# be a git submodule, but since 1.x upstream ships it as a Python package
+# with dependencies, so a bare checkout is no longer runnable.
+# --force makes reruns of this script idempotent (pipx errors on an
+# already-installed package otherwise).
+pipx install --force "clangd-tidy==1.1.1"
+
+# pipx installs into ~/.local/bin, which is not on PATH everywhere.
+PIPX_BIN_DIR="$HOME/.local/bin"
+TEMP_PROFILE_CONTENTS+="export PATH=$PIPX_BIN_DIR:\$PATH\n"
+if [[ ":$PATH:" != *":$PIPX_BIN_DIR:"* ]]; then
+    export PATH="$PIPX_BIN_DIR:$PATH"
+fi
+if [[ -n "$GITHUB_PATH" ]]; then
+    echo "$PIPX_BIN_DIR" >> $GITHUB_PATH
 fi
 
 cd vcpkg

packages/streamr-dht/include/streamr-dht/Identifiers.hpp

@@ -44,7 +44,7 @@ struct Identifiers {
     static DhtAddress createRandomDhtAddress() {
         return getDhtAddressFromRaw(DhtAddressRaw{[&]() {
             std::vector<uint8_t> randomBytes(kademliaIdLengthInBytes);
-            std::generate(randomBytes.begin(), randomBytes.end(), []() {
+            std::ranges::generate(randomBytes, []() {
                 return static_cast<uint8_t>(std::rand() % 256); // NOLINT
             });
             return std::string(randomBytes.begin(), randomBytes.end());

packages/streamr-dht/include/streamr-dht/connection/ConnectionLockRpcLocal.hpp

@@ -43,9 +43,7 @@ class ConnectionLockRpcLocal : public ConnectionLockRpc<DhtCallContext> {
     LockResponse lockRequest(
         const LockRequest& request,
         const DhtCallContext& callContext) override {
-        const auto senderPeerDescriptor =
-            static_cast<const DhtCallContext&>(callContext)
-                .incomingSourceDescriptor;
+        const auto senderPeerDescriptor = callContext.incomingSourceDescriptor;
 
         if (Identifiers::areEqualPeerDescriptors(
                 senderPeerDescriptor.value(),
@@ -67,9 +65,7 @@ class ConnectionLockRpcLocal : public ConnectionLockRpc<DhtCallContext> {
     void unlockRequest(
         const UnlockRequest& request,
         const DhtCallContext& callContext) override {
-        const auto senderPeerDescriptor =
-            static_cast<const DhtCallContext&>(callContext)
-                .incomingSourceDescriptor;
+        const auto senderPeerDescriptor = callContext.incomingSourceDescriptor;
         const auto nodeId = Identifiers::getNodeIdFromPeerDescriptor(
             senderPeerDescriptor.value());
         this->options.removeRemoteLocked(nodeId, LockID{request.lockid()});
@@ -78,9 +74,7 @@ class ConnectionLockRpcLocal : public ConnectionLockRpc<DhtCallContext> {
     void gracefulDisconnect(
         const DisconnectNotice& request,
         const DhtCallContext& callContext) override {
-        const auto senderPeerDescriptor =
-            static_cast<const DhtCallContext&>(callContext)
-                .incomingSourceDescriptor;
+        const auto senderPeerDescriptor = callContext.incomingSourceDescriptor;
         SLogger::trace(
             Identifiers::getNodeIdFromPeerDescriptor(
                 senderPeerDescriptor.value()) +

packages/streamr-dht/include/streamr-dht/connection/ConnectionLockStates.hpp

@@ -45,28 +45,26 @@ class ConnectionLockStates {
         const std::optional<LockID>& lockId = std::nullopt) {
         std::scoped_lock lock(this->localLocksMutex);
         if (!lockId.has_value()) {
-            return this->localLocks.find(id) != this->localLocks.end();
+            return this->localLocks.contains(id);
         }
-        return this->localLocks.find(id) != this->localLocks.end() &&
-            this->localLocks.at(id).find(lockId.value()) !=
-            this->localLocks.at(id).end();
+        return this->localLocks.contains(id) &&
+            this->localLocks.at(id).contains(lockId.value());
     }
 
     [[nodiscard]] bool isRemoteLocked(
         const DhtAddress& id,
         const std::optional<LockID>& lockId = std::nullopt) {
         std::scoped_lock lock(this->remoteLocksMutex);
         if (!lockId.has_value()) {
-            return this->remoteLocks.find(id) != this->remoteLocks.end();
+            return this->remoteLocks.contains(id);
         }
-        return this->remoteLocks.find(id) != this->remoteLocks.end() &&
-            this->remoteLocks.at(id).find(lockId.value()) !=
-            this->remoteLocks.at(id).end();
+        return this->remoteLocks.contains(id) &&
+            this->remoteLocks.at(id).contains(lockId.value());
     }
 
     [[nodiscard]] bool isWeakLocked(const DhtAddress& id) {
         std::scoped_lock lock(this->weakLocksMutex);
-        return this->weakLocks.find(id) != this->weakLocks.end();
+        return this->weakLocks.contains(id);
     }
 
     [[nodiscard]] bool isLocked(const DhtAddress& id) {
@@ -80,31 +78,31 @@ class ConnectionLockStates {
 
     void addLocalLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->localLocksMutex);
-        if (this->localLocks.find(id) == this->localLocks.end()) {
+        if (!this->localLocks.contains(id)) {
             this->localLocks[id] = std::set<LockID>();
         }
         this->localLocks[id].insert(lockId);
     }
 
     void addRemoteLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->remoteLocksMutex);
-        if (this->remoteLocks.find(id) == this->remoteLocks.end()) {
+        if (!this->remoteLocks.contains(id)) {
             this->remoteLocks[id] = std::set<LockID>();
         }
         this->remoteLocks[id].insert(lockId);
     }
 
     void addWeakLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->weakLocksMutex);
-        if (this->weakLocks.find(id) == this->weakLocks.end()) {
+        if (!this->weakLocks.contains(id)) {
             this->weakLocks[id] = std::set<LockID>();
         }
         this->weakLocks[id].insert(lockId);
     }
 
     void removeLocalLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->localLocksMutex);
-        if (this->localLocks.find(id) != this->localLocks.end()) {
+        if (this->localLocks.contains(id)) {
             this->localLocks[id].erase(lockId);
             if (this->localLocks[id].empty()) {
                 this->localLocks.erase(id);
@@ -114,7 +112,7 @@ class ConnectionLockStates {
 
     void removeRemoteLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->remoteLocksMutex);
-        if (this->remoteLocks.find(id) != this->remoteLocks.end()) {
+        if (this->remoteLocks.contains(id)) {
             this->remoteLocks[id].erase(lockId);
             if (this->remoteLocks[id].empty()) {
                 this->remoteLocks.erase(id);
@@ -124,7 +122,7 @@ class ConnectionLockStates {
 
     void removeWeakLocked(const DhtAddress& id, const LockID& lockId) {
         std::scoped_lock lock(this->weakLocksMutex);
-        if (this->weakLocks.find(id) != this->weakLocks.end()) {
+        if (this->weakLocks.contains(id)) {
             this->weakLocks[id].erase(lockId);
             if (this->weakLocks[id].empty()) {
                 this->weakLocks.erase(id);

packages/streamr-dht/include/streamr-dht/connection/ConnectionManager.hpp

@@ -1,6 +1,7 @@
 #ifndef STREAMR_DHT_CONNECTION_CONNECTIONMANAGER_HPP
 #define STREAMR_DHT_CONNECTION_CONNECTIONMANAGER_HPP
 
+#include <cstdint>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -52,7 +53,12 @@ namespace endpointevents = streamr::dht::connection::endpoint::endpointevents;
 
 using namespace std::chrono_literals;
 
-enum class ConnectionManagerState { IDLE, RUNNING, STOPPING, STOPPED };
+enum class ConnectionManagerState : std::uint8_t {
+    IDLE,
+    RUNNING,
+    STOPPING,
+    STOPPED
+};
 
 struct ConnectionManagerOptions {
     size_t maxConnections;
@@ -89,7 +95,7 @@ class ConnectionManager : public Transport,
                         "Trying to acquire mutex lock in endpoint callback");
                     std::scoped_lock lock(this->endpointsMutex);
                     SLogger::debug("Acquired mutex lock in endpoint callback");
-                    if (this->endpoints.find(nodeId) != this->endpoints.end()) {
+                    if (this->endpoints.contains(nodeId)) {
                         this->endpoints.erase(nodeId);
                     }
                 }
@@ -129,7 +135,7 @@ class ConnectionManager : public Transport,
               [this](const Message& message, const SendOptions& sendOptions) {
                   SLogger::trace(
                       "outgoingmessagecallback() of rpcCommunicator");
-                  return this->send(message, sendOptions);
+                  this->send(message, sendOptions);
               },
               RpcCommunicatorOptions{.rpcRequestTimeout = 10s}), // NOLINT
           connectionLockRpcLocal(
@@ -166,13 +172,12 @@ class ConnectionManager : public Transport,
         this->rpcCommunicator.registerRpcNotification<UnlockRequest>(
             "unlockRequest",
             [this](const UnlockRequest& req, const DhtCallContext& context) {
-                return this->connectionLockRpcLocal.unlockRequest(req, context);
+                this->connectionLockRpcLocal.unlockRequest(req, context);
             });
         this->rpcCommunicator.registerRpcNotification<DisconnectNotice>(
             "gracefulDisconnect",
             [this](const DisconnectNotice& req, const DhtCallContext& context) {
-                return this->connectionLockRpcLocal.gracefulDisconnect(
-                    req, context);
+                this->connectionLockRpcLocal.gracefulDisconnect(req, context);
             });
         SLogger::debug("ConnectionManager constructor end");
     }
@@ -292,7 +297,7 @@ class ConnectionManager : public Transport,
             std::scoped_lock lock(this->endpointsMutex);
             SLogger::debug("Acquired mutex lock in send");
 
-            if (this->endpoints.find(nodeId) == this->endpoints.end()) {
+            if (!this->endpoints.contains(nodeId)) {
                 SLogger::debug("Node ID not found in endpoints");
                 if (sendOptions.connect) {
                     SLogger::debug("Creating new connection");
@@ -302,7 +307,7 @@ class ConnectionManager : public Transport,
                     SLogger::debug("Created new connection");
                     this->onNewConnection(connection);
                     SLogger::debug("Handled new connection");
-                    if (this->endpoints.find(nodeId) == this->endpoints.end()) {
+                    if (!this->endpoints.contains(nodeId)) {
                         SLogger::debug(
                             "Node ID not found in endpoints after creating new connection, this means that the connection failed");
                         throw SendFailed(
@@ -424,7 +429,7 @@ class ConnectionManager : public Transport,
             SLogger::debug("Trying to acquire mutex lock in unlockConnection");
             std::scoped_lock lock(this->endpointsMutex);
             SLogger::debug("Acquired mutex lock in unlockConnection");
-            if (this->endpoints.find(nodeId) == this->endpoints.end()) {
+            if (!this->endpoints.contains(nodeId)) {
                 SLogger::debug("Node ID not found in endpoints");
                 return;
             }
@@ -512,7 +517,7 @@ class ConnectionManager : public Transport,
             std::scoped_lock lock(this->endpointsMutex);
             SLogger::debug("Acquired mutex lock in acceptNewConnection");
 
-            if (this->endpoints.find(nodeId) != this->endpoints.end()) {
+            if (this->endpoints.contains(nodeId)) {
                 if (OffererHelper::getOfferer(
                         Identifiers::getNodeIdFromPeerDescriptor(
                             this->getLocalPeerDescriptor()),
@@ -580,8 +585,6 @@ class ConnectionManager : public Transport,
                 "gracefullyDisconnected() tried on a non-existing connection");
             return;
         }
-        auto debugString = targetDescriptor.DebugString();
-
         if (endpoint->isConnected()) {
             try {
                 SLogger::debug("gracefullyDisconnect() calling blockingWait()");
@@ -600,9 +603,6 @@ class ConnectionManager : public Transport,
                                      targetDescriptor,
                                      disconnectMode]()
                                         -> folly::coro::Task<void> {
-                                        auto debugString =
-                                            targetDescriptor.DebugString();
-
                                         co_return co_await this
                                             ->doGracefullyDisconnectAsync(
                                                 targetDescriptor,