From c75c55d31afcb8d5551e38b81b5f696cd8161c83 Mon Sep 17 00:00:00 2001 From: Felix Hanau Date: Tue, 30 Dec 2025 12:57:34 -0500 Subject: [PATCH] [build] Enable clang-tidy modernize-loop-convert check --- .clang-tidy | 2 ++ src/workerd/api/form-data.c++ | 3 +++ src/workerd/api/url.c++ | 3 +++ src/workerd/io/trace.c++ | 8 ++++---- src/workerd/jsg/url-test.c++ | 4 ++-- src/workerd/tests/bench-api-headers.c++ | 3 +-- src/workerd/util/ring-buffer-test.c++ | 4 ++++ 7 files changed, 19 insertions(+), 8 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ee293847ff2..d0baa3ecccc 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -32,6 +32,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 +73,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/form-data.c++ b/src/workerd/api/form-data.c++ index 15098a0aeea..0148d742707 100644 --- a/src/workerd/api/form-data.c++ +++ b/src/workerd/api/form-data.c++ @@ -512,6 +512,9 @@ 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. + // Silence clang-tidy warning, using an iterator would not work correctly if callback + // increases the array size. + // NOLINTNEXTLINE(modernize-loop-convert) for (size_t i = 0; i < this->data.size(); i++) { auto& [key, value] = this->data[i]; callback(js, clone(js, value), key, JSG_THIS); diff --git a/src/workerd/api/url.c++ b/src/workerd/api/url.c++ index 736173a157c..f9816e9a6b9 100644 --- a/src/workerd/api/url.c++ +++ b/src/workerd/api/url.c++ @@ -623,6 +623,9 @@ 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. + // Silence clang-tidy warning, using an iterator would not work correctly if callback + // increases the size of data. + // NOLINTNEXTLINE(modernize-loop-convert) for (size_t i = 0; i < this->url->query.size(); i++) { auto& [key, value] = this->url->query[i]; callback(js, value, key, JSG_THIS); diff --git a/src/workerd/io/trace.c++ b/src/workerd/io/trace.c++ index 8cf6f1d9080..c8c4114d4ad 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 (const 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 (const 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/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/ring-buffer-test.c++ b/src/workerd/util/ring-buffer-test.c++ index 4becf9af01e..cf4387c0d92 100644 --- a/src/workerd/util/ring-buffer-test.c++ +++ b/src/workerd/util/ring-buffer-test.c++ @@ -115,6 +115,8 @@ KJ_TEST("RingBuffer iterator basic") { } int expected = 0; + // Using verbose syntax to test iterator + // NOLINTNEXTLINE(modernize-loop-convert) for (auto it = buffer.begin(); it != buffer.end(); ++it) { KJ_EXPECT(*it == expected++); } @@ -165,6 +167,8 @@ KJ_TEST("RingBuffer const_iterator") { const auto& constBuffer = buffer; int expected = 0; + // Using verbose syntax to test iterator + // NOLINTNEXTLINE(modernize-loop-convert) for (auto it = constBuffer.begin(); it != constBuffer.end(); ++it) { KJ_EXPECT(*it == expected++); }