Apply clang-tidy readability-else-after-return (#17125)

* Apply clang-tidy readability-else-after-return

* Apply changes from mac

* Restyle

* Fix some wrongly added whitespace in comments and log strings

* Restyle

* One more pass at removing else after return - this seems to be incremental

* One more pass at removing else after return - this seems to be incremental

* One more pass on darwin of running tidy fix

* Restyle

* Remove odd error usage in CommissioneeShellCommands.cpp

* Remove odd error usage in ControllerShellCommands.cpp

* Take out return from PUBLISH_RECORDS

* Restyle

* Add 2 more fixes for else after return

* Restyle

.clang-tidy

@@ -1,4 +1,4 @@
 ---
-Checks: 'modernize-use-nullptr,bugprone-*,-bugprone-not-null-terminated-result,-bugprone-suspicious-memory-comparison,-bugprone-argument-comment,-bugprone-unused-return-value,-bugprone-branch-clone,-bugprone-easily-swappable-parameters,-bugprone-reserved-identifier,-bugprone-macro-parentheses,-bugprone-forward-declaration-namespace,-bugprone-forwarding-reference-overload,-bugprone-undelegated-constructor,-bugprone-sizeof-expression,-bugprone-implicit-widening-of-multiplication-result,-bugprone-too-small-loop-variable,-bugprone-narrowing-conversions,-bugprone-misplaced-widening-cast,-bugprone-suspicious-include,-bugprone-signed-char-misuse,-bugprone-copy-constructor-init,-clang-analyzer-core.CallAndMessage,-clang-analyzer-core.UndefinedBinaryOperatorResult,-clang-analyzer-core.NullDereference,-clang-analyzer-optin.cplusplus.UninitializedObject,-clang-analyzer-core.uninitialized.Branch,-clang-analyzer-optin.performance,-clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-cplusplus.Move,-clang-analyzer-optin.cplusplus.VirtualCall,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-nullability.NullablePassedToNonnull,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-unix.cstring.NullArg,-clang-analyzer-security.insecureAPI.rand,-clang-analyzer-core.NonNullParamChecker,-clang-analyzer-nullability.NullPassedToNonnull,-clang-analyzer-unix.Malloc,-clang-analyzer-valist.Unterminated,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-diagnostic-implicit-int-conversion'
+Checks: 'readability-else-after-return,modernize-use-nullptr,bugprone-*,-bugprone-not-null-terminated-result,-bugprone-suspicious-memory-comparison,-bugprone-argument-comment,-bugprone-unused-return-value,-bugprone-branch-clone,-bugprone-easily-swappable-parameters,-bugprone-reserved-identifier,-bugprone-macro-parentheses,-bugprone-forward-declaration-namespace,-bugprone-forwarding-reference-overload,-bugprone-undelegated-constructor,-bugprone-sizeof-expression,-bugprone-implicit-widening-of-multiplication-result,-bugprone-too-small-loop-variable,-bugprone-narrowing-conversions,-bugprone-misplaced-widening-cast,-bugprone-suspicious-include,-bugprone-signed-char-misuse,-bugprone-copy-constructor-init,-clang-analyzer-core.CallAndMessage,-clang-analyzer-core.UndefinedBinaryOperatorResult,-clang-analyzer-core.NullDereference,-clang-analyzer-optin.cplusplus.UninitializedObject,-clang-analyzer-core.uninitialized.Branch,-clang-analyzer-optin.performance,-clang-analyzer-optin.osx.cocoa.localizability.EmptyLocalizationContextChecker,-clang-analyzer-deadcode.DeadStores,-clang-analyzer-cplusplus.Move,-clang-analyzer-optin.cplusplus.VirtualCall,-clang-analyzer-security.insecureAPI.strcpy,-clang-analyzer-nullability.NullablePassedToNonnull,-clang-analyzer-optin.performance.Padding,-clang-analyzer-security.insecureAPI.bzero,-clang-analyzer-unix.cstring.NullArg,-clang-analyzer-security.insecureAPI.rand,-clang-analyzer-core.NonNullParamChecker,-clang-analyzer-nullability.NullPassedToNonnull,-clang-analyzer-unix.Malloc,-clang-analyzer-valist.Unterminated,-clang-analyzer-cplusplus.NewDeleteLeaks,-clang-diagnostic-implicit-int-conversion'
 WarningsAsErrors: '*'
 HeaderFilterRegex: '(src|examples|zzz_generated|credentials)'

examples/bridge-app/linux/main.cpp

@@ -225,7 +225,7 @@ int AddDeviceEndpoint(Device * dev, EmberAfEndpointType * ep, const Span<const E
                                     gCurrentEndpointId, index);
                     return index;
                 }
-                else if (ret != EMBER_ZCL_STATUS_DUPLICATE_EXISTS)
+                if (ret != EMBER_ZCL_STATUS_DUPLICATE_EXISTS)
                 {
                     return -1;
                 }

examples/chip-tool/commands/clusters/CustomArgument.h

@@ -56,19 +56,19 @@ class CustomArgumentParser
             {
                 return CustomArgumentParser::PutOctetString(writer, tag, value);
             }
-            else if (IsUnsignedNumberPrefix(value))
+            if (IsUnsignedNumberPrefix(value))
             {
                 return CustomArgumentParser::PutUnsignedFromString(writer, tag, value);
             }
-            else if (IsSignedNumberPrefix(value))
+            if (IsSignedNumberPrefix(value))
             {
                 return CustomArgumentParser::PutSignedFromString(writer, tag, value);
             }
-            else if (IsFloatNumberPrefix(value))
+            if (IsFloatNumberPrefix(value))
             {
                 return CustomArgumentParser::PutFloatFromString(writer, tag, value);
             }
-            else if (IsDoubleNumberPrefix(value))
+            if (IsDoubleNumberPrefix(value))
             {
                 return CustomArgumentParser::PutDoubleFromString(writer, tag, value);
             }

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

@@ -49,10 +49,8 @@ const chip::Credentials::AttestationTrustStore * GetTestFileAttestationTrustStor
     {
         return &attestationTrustStore;
     }
-    else
-    {
-        return nullptr;
-    }
+
+    return nullptr;
 }
 } // namespace
 

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

@@ -313,20 +313,18 @@ bool Command::InitArgument(size_t argIndex, char * argValue)
                 *value = chip::ByteSpan(chip::Uint8::from_char(argValue), octetCount);
                 return true;
             }
-            else
+
+            // Just ASCII.  Check for the "str:" prefix.
+            static constexpr char strPrefix[] = "str:";
+            constexpr size_t strPrefixLen     = ArraySize(strPrefix) - 1; // Don't         count the null
+            if (strncmp(argValue, strPrefix, strPrefixLen) == 0)
             {
-                // Just ASCII.  Check for the "str:" prefix.
-                static constexpr char strPrefix[] = "str:";
-                constexpr size_t strPrefixLen     = ArraySize(strPrefix) - 1; // Don't count the null
-                if (strncmp(argValue, strPrefix, strPrefixLen) == 0)
-                {
-                    // Skip the prefix
-                    argValue += strPrefixLen;
-                    argLen -= strPrefixLen;
-                }
-                *value = chip::ByteSpan(chip::Uint8::from_char(argValue), argLen);
-                return true;
+                // Skip the prefix
+                argValue += strPrefixLen;
+                argLen -= strPrefixLen;
             }
+            *value = chip::ByteSpan(chip::Uint8::from_char(argValue), argLen);
+            return true;
         });
         break;
     }
@@ -347,10 +345,8 @@ bool Command::InitArgument(size_t argIndex, char * argValue)
                 uint64_t max = arg.max;
                 return (!ss.fail() && ss.eof() && *value >= min && *value <= max);
             }
-            else
-            {
-                return false;
-            }
+
+            return false;
         });
         break;
     }
@@ -409,10 +405,8 @@ bool Command::InitArgument(size_t argIndex, char * argValue)
                 int64_t max = chip::CanCastTo<int64_t>(arg.max) ? static_cast<int64_t>(arg.max) : INT64_MAX;
                 return (!ss.fail() && ss.eof() && *value >= min && *value <= max);
             }
-            else
-            {
-                return false;
-            }
+
+            return false;
         });
         break;
     }

examples/platform/linux/CommissioneeShellCommands.cpp

@@ -79,8 +79,6 @@ static CHIP_ERROR PrintAllCommands()
 
 static CHIP_ERROR CommissioneeHandler(int argc, char ** argv)
 {
-    CHIP_ERROR error = CHIP_NO_ERROR;
-
     if (argc == 0 || strcmp(argv[0], "help") == 0)
     {
         return PrintAllCommands();
@@ -92,10 +90,10 @@ static CHIP_ERROR CommissioneeHandler(int argc, char ** argv)
         chip::Inet::IPAddress commissioner;
         chip::Inet::IPAddress::FromString(argv[1], commissioner);
         uint16_t port = (uint16_t) strtol(argv[2], &eptr, 10);
-        return error  = SendUDC(true, chip::Transport::PeerAddress::UDP(commissioner, port));
+        return SendUDC(true, chip::Transport::PeerAddress::UDP(commissioner, port));
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
-    else if (strcmp(argv[0], "setdiscoverytimeout") == 0)
+    if (strcmp(argv[0], "setdiscoverytimeout") == 0)
     {
         char * eptr;
         int16_t timeout = (int16_t) strtol(argv[1], &eptr, 10);
@@ -111,7 +109,7 @@ static CHIP_ERROR CommissioneeHandler(int argc, char ** argv)
         return CHIP_NO_ERROR;
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY
-    else if (strcmp(argv[0], "restartmdns") == 0)
+    if (strcmp(argv[0], "restartmdns") == 0)
     {
         if (argc < 2)
         {
@@ -127,18 +125,15 @@ static CHIP_ERROR CommissioneeHandler(int argc, char ** argv)
             chip::app::DnssdServer::Instance().StartServer(chip::Dnssd::CommissioningMode::kEnabledBasic);
             return CHIP_NO_ERROR;
         }
-        else if (strcmp(argv[1], "enabled_enhanced") == 0)
+        if (strcmp(argv[1], "enabled_enhanced") == 0)
         {
             chip::app::DnssdServer::Instance().StartServer(chip::Dnssd::CommissioningMode::kEnabledEnhanced);
             return CHIP_NO_ERROR;
         }
         return PrintAllCommands();
     }
-    else
-    {
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-    return error;
+
+    return CHIP_ERROR_INVALID_ARGUMENT;
 }
 
 void RegisterCommissioneeCommands()

examples/platform/linux/ControllerShellCommands.cpp

@@ -202,34 +202,32 @@ static CHIP_ERROR PrintAllCommands()
 
 static CHIP_ERROR ControllerHandler(int argc, char ** argv)
 {
-    CHIP_ERROR error = CHIP_NO_ERROR;
-
     if (argc == 0 || strcmp(argv[0], "help") == 0)
     {
         return PrintAllCommands();
     }
 #if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
     else if (strcmp(argv[0], "udc-reset") == 0)
     {
-        return error = ResetUDC(true);
+        return ResetUDC(true);
     }
     else if (strcmp(argv[0], "udc-print") == 0)
     {
-        return error = PrintUDC(true);
+        return PrintUDC(true);
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     else if (strcmp(argv[0], "discover-commissionable") == 0)
     {
-        return error = discover(true);
+        return discover(true);
     }
     else if (strcmp(argv[0], "discover-commissionable-instance") == 0)
     {
-        return error = discover(true, argv[1]);
+        return discover(true, argv[1]);
     }
     else if (strcmp(argv[0], "discover-display") == 0)
     {
-        return error = display(true);
+        return display(true);
     }
     else if (strcmp(argv[0], "commission-onnetwork") == 0)
     {
@@ -247,7 +245,7 @@ static CHIP_ERROR ControllerHandler(int argc, char ** argv)
 
         uint16_t port = (uint16_t) strtol(argv[4], &eptr, 10);
 
-        return error = pairOnNetwork(true, pincode, disc, Transport::PeerAddress::UDP(address, port));
+        return pairOnNetwork(true, pincode, disc, Transport::PeerAddress::UDP(address, port));
     }
     else if (strcmp(argv[0], "ux") == 0)
     {
@@ -289,14 +287,11 @@ static CHIP_ERROR ControllerHandler(int argc, char ** argv)
         char * eptr;
         uint32_t pincode = (uint32_t) strtol(argv[1], &eptr, 10);
         size_t index     = (size_t) strtol(argv[2], &eptr, 10);
-        return error     = pairUDC(true, pincode, index);
+        return pairUDC(true, pincode, index);
     }
 #endif // CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
-    else
-    {
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-    return error;
+
+    return CHIP_ERROR_INVALID_ARGUMENT;
 }
 
 void RegisterControllerCommands()

examples/platform/linux/LinuxCommissionableDataProvider.cpp

@@ -94,7 +94,7 @@ CHIP_ERROR LinuxCommissionableDataProvider::Init(chip::Optional<std::vector<uint
         ChipLogError(Support, "PASE salt length invalid: %u", static_cast<unsigned>(spake2pSaltLength));
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
-    else if (!havePaseSalt)
+    if (!havePaseSalt)
     {
         ChipLogProgress(Support, "LinuxCommissionableDataProvider didn't get a PASE salt, generating one.");
         std::vector<uint8_t> spake2pSaltVector;

examples/platform/linux/Options.cpp

@@ -321,8 +321,7 @@ bool HandleOption(const char * aProgram, OptionSet * aOptions, int aIdentifier,
             retval = false;
             break;
         }
-        else if ((iterCount < chip::Crypto::kSpake2p_Min_PBKDF_Iterations) ||
-                 (iterCount > chip::Crypto::kSpake2p_Max_PBKDF_Iterations))
+        if ((iterCount < chip::Crypto::kSpake2p_Min_PBKDF_Iterations) || (iterCount > chip::Crypto::kSpake2p_Max_PBKDF_Iterations))
         {
             PrintArgError("%s: ERROR: argument %s not in range [%zu, %zu]\n", aProgram, aName,
                           chip::Crypto::kSpake2p_Min_PBKDF_Iterations, chip::Crypto::kSpake2p_Max_PBKDF_Iterations);

examples/shell/shell_common/cmd_ping.cpp

@@ -140,12 +140,10 @@ Transport::PeerAddress GetEchoPeerAddress()
     {
         return Transport::PeerAddress::TCP(gDestAddr, gPingArguments.GetEchoPort());
     }
-    else
+
 #endif
-    {
 
-        return Transport::PeerAddress::UDP(gDestAddr, gPingArguments.GetEchoPort(), ::chip::Inet::InterfaceId::Null());
-    }
+    return Transport::PeerAddress::UDP(gDestAddr, gPingArguments.GetEchoPort(), ::chip::Inet::InterfaceId::Null());
 }
 
 void Shutdown()

examples/tv-app/linux/include/account-login/AccountLoginManager.cpp

@@ -40,11 +40,9 @@ bool AccountLoginManager::HandleLogin(const CharSpan & tempAccountIdentifier, co
         ChipLogProgress(Zcl, "AccountLoginManager::HandleLogin success");
         return true;
     }
-    else
-    {
-        ChipLogProgress(Zcl, "AccountLoginManager::HandleLogin failed expected pin %s", mSetupPin);
-        return false;
-    }
+
+    ChipLogProgress(Zcl, "AccountLoginManager::HandleLogin failed expected pin %s", mSetupPin);
+    return false;
 }
 
 bool AccountLoginManager::HandleLogout()

examples/tv-casting-app/linux/main.cpp

@@ -86,17 +86,16 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
             gDiscoveryFilter = Dnssd::DiscoveryFilter(Dnssd::DiscoveryFilterType::kDeviceType, static_cast<uint16_t>(deviceType));
             return true;
         }
-        else
+
+        for (int i = 0; i < kKnownDeviceTypesCount; i++)
         {
-            for (int i = 0; i < kKnownDeviceTypesCount; i++)
+            if (strcasecmp(aValue, kKnownDeviceTypes[i].name) == 0)
             {
-                if (strcasecmp(aValue, kKnownDeviceTypes[i].name) == 0)
-                {
-                    gDiscoveryFilter = Dnssd::DiscoveryFilter(Dnssd::DiscoveryFilterType::kDeviceType, kKnownDeviceTypes[i].id);
-                    return true;
-                }
+                gDiscoveryFilter = Dnssd::DiscoveryFilter(Dnssd::DiscoveryFilterType::kDeviceType, kKnownDeviceTypes[i].id);
+                return true;
             }
         }
+
         ChipLogError(AppServer, "%s: INTERNAL ERROR: Unhandled option value: %s %s", aProgram, aName, aValue);
         return false;
     }

src/app/EventManagement.cpp

@@ -405,15 +405,13 @@ CHIP_ERROR EventManagement::CopyAndAdjustDeltaTime(const TLVReader & aReader, si
         return ctx->mpWriter->Put(TLV::ContextTag(to_underlying(EventDataIB::Tag::kDeltaSystemTimestamp)),
                                   ctx->mpContext->mCurrentTime.mValue - ctx->mpContext->mPreviousTime.mValue);
     }
-    else if ((aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kEpochTimestamp))) && !(ctx->mpContext->mFirst))
+    if ((aReader.GetTag() == TLV::ContextTag(to_underlying(EventDataIB::Tag::kEpochTimestamp))) && !(ctx->mpContext->mFirst))
     {
         return ctx->mpWriter->Put(TLV::ContextTag(to_underlying(EventDataIB::Tag::kDeltaEpochTimestamp)),
                                   ctx->mpContext->mCurrentTime.mValue - ctx->mpContext->mPreviousTime.mValue);
     }
-    else
-    {
-        return ctx->mpWriter->CopyElement(reader);
-    }
+
+    return ctx->mpWriter->CopyElement(reader);
 }
 
 void EventManagement::VendEventNumber()

src/app/clusters/barrier-control-server/barrier-control-server.cpp

@@ -235,11 +235,9 @@ static uint32_t calculateDelayMs(EndpointId endpoint, uint8_t targetPosition, bo
     {
         return MIN_POSITION_CHANGE_DELAY_MS;
     }
-    else
-    {
-        uint32_t delayMs = openOrClosePeriodMs / positionDelta;
-        return (delayMs < MIN_POSITION_CHANGE_DELAY_MS ? MIN_POSITION_CHANGE_DELAY_MS : delayMs);
-    }
+
+    uint32_t delayMs = openOrClosePeriodMs / positionDelta;
+    return (delayMs < MIN_POSITION_CHANGE_DELAY_MS ? MIN_POSITION_CHANGE_DELAY_MS : delayMs);
 }
 
 void emberAfBarrierControlClusterServerTickCallback(EndpointId endpoint)

src/app/clusters/bindings/bindings.cpp

@@ -196,7 +196,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
         LogErrorOnFailure(NotifyBindingsChanged());
         return CHIP_NO_ERROR;
     }
-    else if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
+    if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
     {
         TargetStructType target;
         ReturnErrorOnFailure(decoder.Decode(target));