clang-tidy: apply modernize-loop-convert over a set of files (#23443)

* Modernize using:

```
./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-chip-tool-clang/compile_commands.json --export-fixes out/fixes.xml fix
```

* Fix using:

```
./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-all-clusters-clang/compile_commands.json --export-fixes out/fixes.xml fix
```

* Modernize using:

```
./scripts/run-clang-tidy-on-compile-commands.py --checks modernize-loop-convert --compile-database out/linux-x64-tests-clang/compile_commands.json --export-fixes out/fixes.xml fix
```

* Restyle and remove unused variable

* Remove more unused variables for array sizes

* Restyle

* Start some manual renames of iteration values ... automated tool is quite silly

* More manual updates for naming of things. Code should be cleaner now

* Restyle

* Enforce lowercase naming for cdSigningKey iterator variable

* One more pass for cleanups

Co-authored-by: Andrei Litvin <[email protected]>

examples/chip-tool/commands/common/CHIPCommand.cpp

@@ -166,9 +166,9 @@ void CHIPCommand::MaybeTearDownStack()
     // since the CHIP thread and event queue have been stopped, preventing any thread
     // races.
     //
-    for (auto it = mCommissioners.begin(); it != mCommissioners.end(); it++)
+    for (auto & commissioner : mCommissioners)
     {
-        ShutdownCommissioner(it->first);
+        ShutdownCommissioner(commissioner.first);
     }
 
     StopTracing();

examples/chip-tool/commands/common/Command.cpp

@@ -42,9 +42,9 @@ bool Command::InitArguments(int argc, char ** argv)
     size_t argvExtraArgsCount = (size_t) argc;
     size_t mandatoryArgsCount = 0;
     size_t optionalArgsCount  = 0;
-    for (size_t i = 0; i < mArgs.size(); i++)
+    for (auto & arg : mArgs)
     {
-        if (mArgs[i].isOptional())
+        if (arg.isOptional())
         {
             optionalArgsCount++;
         }
@@ -877,9 +877,8 @@ void ResetOptionalArg(const Argument & arg)
 
 void Command::ResetArguments()
 {
-    for (size_t i = 0; i < mArgs.size(); i++)
+    for (const auto & arg : mArgs)
     {
-        const Argument arg      = mArgs[i];
         const ArgumentType type = arg.type;
         if (arg.isOptional())
         {

src/app/BufferedReadCallback.cpp

@@ -65,9 +65,9 @@ CHIP_ERROR BufferedReadCallback::GenerateListTLV(TLV::ScopedBufferTLVReader & aR
     // To avoid that, a single contiguous buffer is the best likely approach for now.
     //
     uint32_t totalBufSize = 0;
-    for (size_t i = 0; i < mBufferedList.size(); i++)
+    for (const auto & packetBuffer : mBufferedList)
     {
-        totalBufSize += mBufferedList[i]->TotalLength();
+        totalBufSize += packetBuffer->TotalLength();
     }
 
     //

src/app/clusters/general-diagnostics-server/general-diagnostics-server.cpp

@@ -58,9 +58,9 @@ bool IsTestEventTriggerEnabled()
 
 bool IsByteSpanAllZeros(const ByteSpan & byteSpan)
 {
-    for (auto * it = byteSpan.begin(); it != byteSpan.end(); ++it)
+    for (unsigned char it : byteSpan)
     {
-        if (*it != 0)
+        if (it != 0)
         {
             return false;
         }

src/app/tests/TestAttributeValueEncoder.cpp

@@ -261,9 +261,9 @@ void TestEncodeFabricScoped(nlTestSuite * aSuite, void * aContext)
 
     // We tried to encode three items, however, the encoder should only put the item with matching fabric index into the final list.
     CHIP_ERROR err = test.encoder.EncodeList([items](const auto & encoder) -> CHIP_ERROR {
-        for (size_t i = 0; i < 3; i++)
+        for (const auto & item : items)
         {
-            ReturnErrorOnFailure(encoder.Encode(items[i]));
+            ReturnErrorOnFailure(encoder.Encode(item));
         }
         return CHIP_NO_ERROR;
     });

src/app/tests/TestPendingNotificationMap.cpp

@@ -80,10 +80,10 @@ void TestAddRemove(nlTestSuite * aSuite, void * aContext)
     pendingMap.RemoveAllEntriesForNode(chip::ScopedNodeId());
     uint8_t expectedEntryIndecies[] = { 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19 };
     iter                            = pendingMap.begin();
-    for (size_t i = 0; i < sizeof(expectedEntryIndecies); i++)
+    for (uint8_t ch : expectedEntryIndecies)
     {
         PendingNotificationEntry entry = *iter;
-        NL_TEST_ASSERT(aSuite, entry.mBindingEntryId == expectedEntryIndecies[i]);
+        NL_TEST_ASSERT(aSuite, entry.mBindingEntryId == ch);
         ++iter;
     }
     NL_TEST_ASSERT(aSuite, iter == pendingMap.end());

src/app/tests/TestReadInteraction.cpp

@@ -1225,11 +1225,11 @@ void TestReadInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
     chip::app::AttributePathParams attributePathParams[2];
-    for (int i = 0; i < 2; i++)
+    for (auto & attributePathParam : attributePathParams)
     {
-        attributePathParams[i].mEndpointId  = Test::kMockEndpoint3;
-        attributePathParams[i].mClusterId   = Test::MockClusterId(2);
-        attributePathParams[i].mAttributeId = Test::MockAttributeId(4);
+        attributePathParam.mEndpointId  = Test::kMockEndpoint3;
+        attributePathParam.mClusterId   = Test::MockClusterId(2);
+        attributePathParam.mAttributeId = Test::MockAttributeId(4);
     }
 
     ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
@@ -2213,10 +2213,10 @@ void TestReadInteraction::TestReadShutdown(nlTestSuite * apSuite, void * apConte
     //
     // Allocate a number of clients
     //
-    for (int i = 0; i < 4; i++)
+    for (auto & client : pClients)
     {
-        pClients[i] = Platform::New<app::ReadClient>(engine, &ctx.GetExchangeManager(), delegate,
-                                                     chip::app::ReadClient::InteractionType::Subscribe);
+        client = Platform::New<app::ReadClient>(engine, &ctx.GetExchangeManager(), delegate,
+                                                chip::app::ReadClient::InteractionType::Subscribe);
     }
 
     //

src/app/tests/suites/pics/PICSBooleanExpressionParser.cpp

@@ -39,9 +39,8 @@ void PICSBooleanExpressionParser::Tokenize(std::string & expression, std::vector
 
     std::string s;
 
-    for (size_t i = 0; i < expression.size(); i++)
+    for (char c : expression)
     {
-        char c = expression[i];
         switch (c)
         {
         case ' ':

src/ble/BleLayer.cpp

@@ -203,9 +203,11 @@ CHIP_ERROR BleTransportCapabilitiesRequestMessage::Decode(const PacketBufferHand
     VerifyOrReturnError(CAPABILITIES_MSG_CHECK_BYTE_1 == chip::Encoding::Read8(p), BLE_ERROR_INVALID_MESSAGE);
     VerifyOrReturnError(CAPABILITIES_MSG_CHECK_BYTE_2 == chip::Encoding::Read8(p), BLE_ERROR_INVALID_MESSAGE);
 
-    for (size_t i = 0; i < kCapabilitiesRequestSupportedVersionsLength; i++)
+    static_assert(kCapabilitiesRequestSupportedVersionsLength == sizeof(msg.mSupportedProtocolVersions),
+                  "Expected capability sizes and storage must match");
+    for (unsigned char & version : msg.mSupportedProtocolVersions)
     {
-        msg.mSupportedProtocolVersions[i] = chip::Encoding::Read8(p);
+        version = chip::Encoding::Read8(p);
     }
 
     msg.mMtu        = chip::Encoding::LittleEndian::Read16(p);

src/controller/tests/TestWriteChunking.cpp

@@ -289,9 +289,9 @@ void TestWriteChunking::TestBadChunking(nlTestSuite * apSuite, void * apContext)
         app::WriteClient writeClient(&ctx.GetExchangeManager(), &writeCallback, Optional<uint16_t>::Missing());
 
         ByteSpan list[kTestListLength];
-        for (uint8_t j = 0; j < kTestListLength; j++)
+        for (auto & item : list)
         {
-            list[j] = ByteSpan(sByteSpanData, static_cast<uint32_t>(i));
+            item = ByteSpan(sByteSpanData, static_cast<uint32_t>(i));
         }
 
         err = writeClient.EncodeAttribute(attributePath, app::DataModel::List<ByteSpan>(list, kTestListLength));

src/controller/tests/data_model/TestRead.cpp

@@ -427,11 +427,11 @@ void TestReadInteraction::TestReadSubscribeAttributeResponseWithCache(nlTestSuit
     chip::app::ClusterStateCache cache(delegate);
 
     chip::app::EventPathParams eventPathParams[100];
-    for (uint32_t index = 0; index < 100; index++)
+    for (auto & eventPathParam : eventPathParams)
     {
-        eventPathParams[index].mEndpointId = Test::kMockEndpoint3;
-        eventPathParams[index].mClusterId  = Test::MockClusterId(2);
-        eventPathParams[index].mEventId    = 0;
+        eventPathParam.mEndpointId = Test::kMockEndpoint3;
+        eventPathParam.mClusterId  = Test::MockClusterId(2);
+        eventPathParam.mEventId    = 0;
     }
 
     chip::app::ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());

src/credentials/CHIPCert.cpp

@@ -616,9 +616,9 @@ ChipDN::~ChipDN() {}
 
 void ChipDN::Clear()
 {
-    for (uint8_t i = 0; i < CHIP_CONFIG_CERT_MAX_RDN_ATTRIBUTES; i++)
+    for (auto & dn : rdn)
     {
-        rdn[i].Clear();
+        dn.Clear();
     }
 }
 

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp

@@ -645,11 +645,11 @@ CHIP_ERROR CsaCdKeysTrustStore::AddTrustedKey(const ByteSpan & derCertBytes)
 CHIP_ERROR CsaCdKeysTrustStore::LookupVerifyingKey(const ByteSpan & kid, Crypto::P256PublicKey & outPubKey) const
 {
     // First, search for the well known keys
-    for (size_t keyIdx = 0; keyIdx < gCdSigningKeys.size(); keyIdx++)
+    for (auto & cdSigningKey : gCdSigningKeys)
     {
-        if (kid.data_equal(gCdSigningKeys[keyIdx].mKid))
+        if (kid.data_equal(cdSigningKey.mKid))
         {
-            outPubKey = gCdSigningKeys[keyIdx].mPubkey;
+            outPubKey = cdSigningKey.mPubkey;
             return CHIP_NO_ERROR;
         }
     }

src/credentials/tests/TestCertificationDeclaration.cpp

@@ -262,14 +262,10 @@ static constexpr TestCase sTestCases[] = {
       ByteSpan(sTestCMS_CDContent02), ByteSpan(sTestCMS_SignedMessage02) },
 };
 
-static constexpr size_t sNumTestCases = ArraySize(sTestCases);
-
 static void TestCD_EncodeDecode(nlTestSuite * inSuite, void * inContext)
 {
-    for (size_t i = 0; i < sNumTestCases; i++)
+    for (const auto & testCase : sTestCases)
     {
-        const TestCase & testCase = sTestCases[i];
-
         uint8_t encodedCertElemBuf[kCertificationElements_TLVEncodedMaxLength];
         MutableByteSpan encodedCDPayload(encodedCertElemBuf);
 
@@ -379,10 +375,8 @@ static void TestCD_CMSSignAndVerify(nlTestSuite * inSuite, void * inContext)
 
 static void TestCD_CMSVerifyAndExtract(nlTestSuite * inSuite, void * inContext)
 {
-    for (size_t i = 0; i < sNumTestCases; i++)
+    for (const auto & testCase : sTestCases)
     {
-        const TestCase & testCase = sTestCases[i];
-
         // Verify using signer P256PublicKey
         ByteSpan cdContentOut;
         NL_TEST_ASSERT(inSuite,
@@ -412,10 +406,8 @@ static void TestCD_CMSVerifyAndExtract(nlTestSuite * inSuite, void * inContext)
 
 static void TestCD_CertificationElementsDecoder(nlTestSuite * inSuite, void * inContext)
 {
-    for (size_t i = 0; i < sNumTestCases; i++)
+    for (const auto & testCase : sTestCases)
     {
-        const TestCase & testCase = sTestCases[i];
-
         uint8_t encodedCertElemBuf[kCertificationElements_TLVEncodedMaxLength];
         MutableByteSpan encodedCDPayload(encodedCertElemBuf);
 

src/credentials/tests/TestChipCert.cpp

@@ -428,22 +428,18 @@ static void TestChipCert_CertValidation(nlTestSuite * inSuite, void * inContext)
                                                                          { TestCert::kNode01_01, sGenTBSHashFlag,  sNullLoadFlag       } } },
     };
     // clang-format on
-    static const size_t sNumValidationTestCases = ArraySize(sValidationTestCases);
 
-    for (unsigned i = 0; i < sNumValidationTestCases; i++)
+    for (const auto & testCase : sValidationTestCases)
     {
         const ChipCertificateData * resultCert = nullptr;
-        const ValidationTestCase & testCase    = sValidationTestCases[i];
-
-        err = certSet.Init(kMaxCertsPerTestCase);
+        err                                    = certSet.Init(kMaxCertsPerTestCase);
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
-        for (size_t i2 = 0; i2 < kMaxCertsPerTestCase; i2++)
+        for (auto inputCert : testCase.InputCerts)
         {
-            if (testCase.InputCerts[i2].Type != TestCert::kNone)
+            if (inputCert.Type != TestCert::kNone)
             {
-                err = LoadTestCert(certSet, testCase.InputCerts[i2].Type, testCase.InputCerts[i2].LoadFlags,
-                                   testCase.InputCerts[i2].DecodeFlags);
+                err = LoadTestCert(certSet, inputCert.Type, inputCert.LoadFlags, inputCert.DecodeFlags);
                 NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
             }
         }
@@ -1137,11 +1133,8 @@ static void TestChipCert_CertType(nlTestSuite * inSuite, void * inContext)
         {  TestCert::kNode02_02,       kCertType_Node            },
     };
     // clang-format on
-    static const size_t sNumTestCases = ArraySize(sTestCases);
-
-    for (unsigned i = 0; i < sNumTestCases; i++)
+    for (const auto & testCase : sTestCases)
     {
-        const TestCase & testCase = sTestCases[i];
         uint8_t certType;
 
         err = certSet.Init(1);
@@ -1186,11 +1179,8 @@ static void TestChipCert_CertId(nlTestSuite * inSuite, void * inContext)
         {  TestCert::kNode02_02,       0xDEDEDEDE00020002 },
     };
     // clang-format on
-    static const size_t sNumTestCases = ArraySize(sTestCases);
-
-    for (unsigned i = 0; i < sNumTestCases; i++)
+    for (const auto & testCase : sTestCases)
     {
-        const TestCase & testCase = sTestCases[i];
         uint64_t chipId;
 
         err = certSet.Init(certData, 1);

src/crypto/tests/CHIPCryptoPALTest.cpp

@@ -204,8 +204,6 @@ const AesCtrTestEntry theAesCtrTestVector[] = {
     }
 };
 
-constexpr size_t kAesCtrTestVectorSize = sizeof(theAesCtrTestVector) / sizeof(theAesCtrTestVector[0]);
-
 constexpr size_t KEY_LENGTH   = Crypto::kAES_CCM128_Key_Length;
 constexpr size_t NONCE_LENGTH = Crypto::kAES_CCM128_Nonce_Length;
 
@@ -249,14 +247,13 @@ static void TestAES_CTR_128CryptTestVectors(nlTestSuite * inSuite, void * inCont
 {
     HeapChecker heapChecker(inSuite);
     int numOfTestsRan = 0;
-    for (size_t vectorIndex = 0; vectorIndex < kAesCtrTestVectorSize; vectorIndex++)
+    for (const auto & vector : theAesCtrTestVector)
     {
-        const AesCtrTestEntry * vector = &theAesCtrTestVector[vectorIndex];
-        if (vector->plaintextLen > 0)
+        if (vector.plaintextLen > 0)
         {
             numOfTestsRan++;
-            TestAES_CTR_128_Encrypt(inSuite, vector);
-            TestAES_CTR_128_Decrypt(inSuite, vector);
+            TestAES_CTR_128_Encrypt(inSuite, &vector);
+            TestAES_CTR_128_Decrypt(inSuite, &vector);
         }
     }
     NL_TEST_ASSERT(inSuite, numOfTestsRan > 0);

src/crypto/tests/TestGroupOperationalCredentials.cpp

@@ -80,22 +80,19 @@ struct GroupKeySetTestEntry theGroupKeySetTestVector[] = {
     },
 };
 
-const uint16_t theGroupKeySetTestVectorLength = sizeof(theGroupKeySetTestVector) / sizeof(theGroupKeySetTestVector[0]);
-
 void TestDeriveGroupOperationalCredentials(nlTestSuite * apSuite, void * apContext)
 {
     GroupOperationalCredentials opCreds;
 
-    for (unsigned i = 0; i < theGroupKeySetTestVectorLength; i++)
+    for (const auto & testVector : theGroupKeySetTestVector)
     {
-        const ByteSpan epochKey(theGroupKeySetTestVector[i].epochKey, KEY_LENGTH);
+        const ByteSpan epochKey(testVector.epochKey, KEY_LENGTH);
         NL_TEST_ASSERT(apSuite,
                        CHIP_NO_ERROR == Crypto::DeriveGroupOperationalCredentials(epochKey, kCompressedFabricId1, opCreds));
 
-        NL_TEST_ASSERT(apSuite, opCreds.hash == theGroupKeySetTestVector[i].groupKeys->hash);
-        NL_TEST_ASSERT(apSuite,
-                       0 == memcmp(opCreds.encryption_key, theGroupKeySetTestVector[i].groupKeys->encryption_key, KEY_LENGTH));
-        NL_TEST_ASSERT(apSuite, 0 == memcmp(opCreds.privacy_key, theGroupKeySetTestVector[i].groupKeys->privacy_key, KEY_LENGTH));
+        NL_TEST_ASSERT(apSuite, opCreds.hash == testVector.groupKeys->hash);
+        NL_TEST_ASSERT(apSuite, 0 == memcmp(opCreds.encryption_key, testVector.groupKeys->encryption_key, KEY_LENGTH));
+        NL_TEST_ASSERT(apSuite, 0 == memcmp(opCreds.privacy_key, testVector.groupKeys->privacy_key, KEY_LENGTH));
     }
 }
 

src/lib/address_resolve/AddressResolve_DefaultImpl.cpp

@@ -292,9 +292,9 @@ void Resolver::ReArmTimer()
     System::Clock::Timestamp now = mTimeSource.GetMonotonicTimestamp();
 
     System::Clock::Timeout nextTimeout = kInvalidTimeout;
-    for (auto it = mActiveLookups.begin(); it != mActiveLookups.end(); it++)
+    for (auto & activeLookup : mActiveLookups)
     {
-        System::Clock::Timeout timeout = it->NextEventTimeout(now);
+        System::Clock::Timeout timeout = activeLookup.NextEventTimeout(now);
 
         if (timeout < nextTimeout)
         {