From 296b4a5aa460e697f19b18fb28c8700dd12118f7 Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Tue, 30 Dec 2025 12:57:34 -0500 Subject: [PATCH] [build] Expand clang-tidy coverage --- .clang-tidy | 4 ++++ src/workerd/api/analytics-engine-impl.h | 2 ++ src/workerd/api/crypto/aes.c++ | 3 ++- src/workerd/api/crypto/ec.c++ | 4 ++-- src/workerd/api/crypto/impl.c++ | 3 +++ src/workerd/api/crypto/jwk.c++ | 7 +++---- src/workerd/api/crypto/x509.c++ | 1 + src/workerd/api/form-data.c++ | 3 +-- src/workerd/api/node/exceptions.c++ | 6 +++++- src/workerd/api/url.c++ | 3 +-- src/workerd/io/trace.c++ | 8 ++++---- src/workerd/jsg/url-test.c++ | 4 ++-- src/workerd/jsg/value-test.c++ | 3 ++- src/workerd/tests/bench-api-headers.c++ | 3 +-- src/workerd/util/sqlite.c++ | 3 ++- 15 files changed, 35 insertions(+), 22 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ee293847ff2..798cbbb4e92 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,11 +16,13 @@ Checks: > bugprone-return-const-ref-from-parameter, bugprone-suspicious-*, -bugprone-suspicious-semicolon, + bugprone-switch-missing-default-case, bugprone-undefined-memory-manipulation, bugprone-unhandled-self-assignment, bugprone-unused-raii, bugprone-use-after-move, cppcoreguidelines-c-copy-assignment-signature, + cppcoreguidelines-interfaces-global-init, cppcoreguidelines-misleading-capture-default-by-value, cppcoreguidelines-noexcept-destructor, cppcoreguidelines-prefer-member-initializer, @@ -32,6 +34,7 @@ Checks: > misc-unused-alias-decls, misc-unused-using-decls, modernize-avoid-variadic-functions, + modernize-loop-convert, modernize-macro-to-enum, modernize-redundant-void-arg, modernize-type-traits, @@ -72,6 +75,7 @@ Checks: > # modernize-use-override # modernize-use-ranges # readability-avoid-return-with-void-value +# readability-convert-member-functions-to-static # readability-redundant-smartptr-get # readability-use-anyofallof diff --git a/src/workerd/api/analytics-engine-impl.h b/src/workerd/api/analytics-engine-impl.h index 31668061a11..ffd42f7d3c2 100644 --- a/src/workerd/api/analytics-engine-impl.h +++ b/src/workerd/api/analytics-engine-impl.h @@ -20,6 +20,7 @@ void setDoubles(Message msg, kj::ArrayPtr arr, kj::StringPtr errorPrefix uint index = 1; for (auto& item: arr) { + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (index) { case 1: msg.setDouble1(item); @@ -108,6 +109,7 @@ void setBlobs(Message msg, sizeSum += value.size(); JSG_REQUIRE(sizeSum <= MAX_CUMULATIVE_BYTES_IN_BLOBS, TypeError, errorPrefix, "Cumulative size of blobs exceeds ", MAX_CUMULATIVE_BYTES_IN_BLOBS, " bytes)."); + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (index) { case 1: msg.setBlob1(value); diff --git a/src/workerd/api/crypto/aes.c++ b/src/workerd/api/crypto/aes.c++ index bd46f0a65e4..c3b2747fe7e 100644 --- a/src/workerd/api/crypto/aes.c++ +++ b/src/workerd/api/crypto/aes.c++ @@ -475,8 +475,9 @@ class AesCtrKey final: public AesKeyBase { return *EVP_aes_192_ctr(); case 32: return *EVP_aes_256_ctr(); + default: + KJ_FAIL_ASSERT("CryptoKey has invalid data length"); } - KJ_FAIL_ASSERT("CryptoKey has invalid data length"); } jsg::BufferSource encryptOrDecrypt(jsg::Lock& js, diff --git a/src/workerd/api/crypto/ec.c++ b/src/workerd/api/crypto/ec.c++ index 0d3ed57af47..90cde8fea8a 100644 --- a/src/workerd/api/crypto/ec.c++ +++ b/src/workerd/api/crypto/ec.c++ @@ -1120,9 +1120,9 @@ kj::OneOf, CryptoKeyPair> EdDsaKey::generateKey(jsg::Lock& j case NID_X25519: return generateKeyImpl(js, normalizedName, nid, privateKeyUsages, publicKeyUsages, extractablePrivateKey, "X25519"_kj); + default: + KJ_FAIL_REQUIRE("ED ", normalizedName, " unimplemented", nid); } - - KJ_FAIL_REQUIRE("ED ", normalizedName, " unimplemented", nid); } } // namespace diff --git a/src/workerd/api/crypto/impl.c++ b/src/workerd/api/crypto/impl.c++ index ce866a7cc8f..8f55c6a54ae 100644 --- a/src/workerd/api/crypto/impl.c++ +++ b/src/workerd/api/crypto/impl.c++ @@ -137,6 +137,7 @@ kj::Vector> consumeAllOpenssl switch (ERR_GET_REASON(error)) { case RSA_R_DATA_LEN_NOT_EQUAL_TO_MOD_LEN: return "Invalid RSA signature."_kj; + default: } break; case ERR_LIB_EC: @@ -149,8 +150,10 @@ kj::Vector> consumeAllOpenssl return "Point is not on curve."_kj; case EC_R_UNKNOWN_GROUP: return "Unsupported elliptic curve group."_kj; + default: } break; + default: } return OpensslUntranslatedError{ diff --git a/src/workerd/api/crypto/jwk.c++ b/src/workerd/api/crypto/jwk.c++ index 1ed1e00a659..b88de3eea5b 100644 --- a/src/workerd/api/crypto/jwk.c++ +++ b/src/workerd/api/crypto/jwk.c++ @@ -285,12 +285,11 @@ JsonWebKey toJwk(const EVPKeyPointer& key, KeyType keyType) { // DSA keys are not supported for JWK export. break; } + default: + break; } } - - return JsonWebKey{ - .kty = kj::str("INVALID"), - }; + return JsonWebKey{.kty = kj::str("INVALID")}; } EVPKeyPointer fromJwk(const JsonWebKey& jwk, KeyType keyType) { diff --git a/src/workerd/api/crypto/x509.c++ b/src/workerd/api/crypto/x509.c++ index 704c4b0b350..bc4cea48e7d 100644 --- a/src/workerd/api/crypto/x509.c++ +++ b/src/workerd/api/crypto/x509.c++ @@ -820,6 +820,7 @@ jsg::JsObject X509Certificate::toLegacyObject(jsg::Lock& js) { } break; } + default: } } diff --git a/src/workerd/api/form-data.c++ b/src/workerd/api/form-data.c++ index 15098a0aeea..9e44c825b52 100644 --- a/src/workerd/api/form-data.c++ +++ b/src/workerd/api/form-data.c++ @@ -512,8 +512,7 @@ void FormData::forEach(jsg::Lock& js, // it up. Using the classic for (;;) syntax here allows for that. However, this does // mean that it's possible for a user to trigger an infinite loop here if new items // are added to the search params unconditionally on each iteration. - for (size_t i = 0; i < this->data.size(); i++) { - auto& [key, value] = this->data[i]; + for (auto& [key, value]: this->data) { callback(js, clone(js, value), key, JSG_THIS); } } diff --git a/src/workerd/api/node/exceptions.c++ b/src/workerd/api/node/exceptions.c++ index 4d0fce4b766..560f01bfbf0 100644 --- a/src/workerd/api/node/exceptions.c++ +++ b/src/workerd/api/node/exceptions.c++ @@ -63,7 +63,11 @@ namespace { case UV_##name: \ return js.str(#name##_kj); jsg::JsValue uv_err_name(jsg::Lock& js, int err) { - switch (err) { UV_ERRNO_MAP(UV_ERR_NAME_GEN) } + switch (err) { + UV_ERRNO_MAP(UV_ERR_NAME_GEN) + default: + break; + } return js.str("UNKNOWN"_kj); } #undef UV_ERR_NAME_GEN diff --git a/src/workerd/api/url.c++ b/src/workerd/api/url.c++ index 736173a157c..31a4d84ad29 100644 --- a/src/workerd/api/url.c++ +++ b/src/workerd/api/url.c++ @@ -623,8 +623,7 @@ void URLSearchParams::forEach(jsg::Lock& js, // it up. Using the classic for (;;) syntax here allows for that. However, this does // mean that it's possible for a user to trigger an infinite loop here if new items // are added to the search params unconditionally on each iteration. - for (size_t i = 0; i < this->url->query.size(); i++) { - auto& [key, value] = this->url->query[i]; + for (auto& [key, value]: this->url->query) { callback(js, value, key, JSG_THIS); } } diff --git a/src/workerd/io/trace.c++ b/src/workerd/io/trace.c++ index 8cf6f1d9080..cd38e00a5c1 100644 --- a/src/workerd/io/trace.c++ +++ b/src/workerd/io/trace.c++ @@ -1156,8 +1156,8 @@ kj::Maybe> getScriptTagsFromReader(const rpc::Trace::Onset if (reader.hasScriptTags()) { auto tags = reader.getScriptTags(); kj::Vector scriptTags(tags.size()); - for (size_t i = 0; i < tags.size(); i++) { - scriptTags.add(kj::str(tags[i])); + for (auto&& tag: tags) { + scriptTags.add(kj::str(tag)); } return kj::Maybe(scriptTags.releaseAsArray()); } @@ -1339,8 +1339,8 @@ TailEvent::Event readEventFromTailEvent(const rpc::Trace::TailEvent::Reader& rea case rpc::Trace::TailEvent::Event::ATTRIBUTE: { auto listReader = event.getAttribute(); kj::Vector attrs(listReader.size()); - for (size_t n = 0; n < listReader.size(); n++) { - attrs.add(Attribute(listReader[n])); + for (auto&& reader: listReader) { + attrs.add(Attribute(reader)); } return attrs.releaseAsArray(); } diff --git a/src/workerd/jsg/url-test.c++ b/src/workerd/jsg/url-test.c++ index 8613b161024..8c97c33ea6b 100644 --- a/src/workerd/jsg/url-test.c++ +++ b/src/workerd/jsg/url-test.c++ @@ -1215,8 +1215,8 @@ KJ_TEST("Special scheme URLS") { kj::str("file:///example"), }; - for (auto n = 0; n < kj::size(tests); n++) { - KJ_ASSERT_NONNULL(Url::tryParse(tests[n].asPtr())); + for (const auto& test: tests) { + KJ_ASSERT_NONNULL(Url::tryParse(test.asPtr())); } } diff --git a/src/workerd/jsg/value-test.c++ b/src/workerd/jsg/value-test.c++ index d6de6169378..af5e08e85b5 100644 --- a/src/workerd/jsg/value-test.c++ +++ b/src/workerd/jsg/value-test.c++ @@ -1219,8 +1219,9 @@ struct ExceptionContext: public ContextGlobalObject { return JSG_KJ_EXCEPTION(FAILED, TypeError, "boom"); case 2: return JSG_KJ_EXCEPTION(FAILED, DOMAbortError, "boom"); + default: + KJ_UNREACHABLE; } - KJ_UNREACHABLE; } JSG_RESOURCE_TYPE(ExceptionContext) { diff --git a/src/workerd/tests/bench-api-headers.c++ b/src/workerd/tests/bench-api-headers.c++ index ad1a65f6371..e305a6008ea 100644 --- a/src/workerd/tests/bench-api-headers.c++ +++ b/src/workerd/tests/bench-api-headers.c++ @@ -92,8 +92,7 @@ BENCHMARK_F(ApiHeaders, set_append)(benchmark::State& state) { for (size_t i = 0; i < 1000; ++i) { auto headers = js.alloc(); // Set common headers with various representative lengths - for (int n = 0; n < 13; n++) { - auto& h = kHeaders[n]; + for (auto& h: kHeaders) { if (h.append) { headers->append(env.js, kj::str(h.name), kj::str(h.value)); } else { diff --git a/src/workerd/util/sqlite.c++ b/src/workerd/util/sqlite.c++ index da99817344d..e2699fccfa0 100644 --- a/src/workerd/util/sqlite.c++ +++ b/src/workerd/util/sqlite.c++ @@ -1554,8 +1554,9 @@ SqliteDatabase::Query::ValuePtr SqliteDatabase::Query::getValue(uint column) { return getBlob(column); case SQLITE_NULL: return nullptr; + default: + KJ_UNREACHABLE; } - KJ_UNREACHABLE; } kj::StringPtr SqliteDatabase::Query::getColumnName(uint column) {