Skip to content

Commit

Permalink
Add a lint for toplevel "using namespace" in headers. (project-chip#3…
Browse files Browse the repository at this point in the history
…2757)

We had all sorts of things using the wrong namespaces because everything was
being imported into multiple namespaces if things happened to include certain
headers.  Remove the "using namespace" bits in core headers, fix the resulting
compile issues, add a lint so people stop doing that.
  • Loading branch information
bzbarsky-apple authored Mar 29, 2024
1 parent 2f17606 commit c89090c
Show file tree
Hide file tree
Showing 43 changed files with 134 additions and 108 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,18 @@ jobs:
if: always()
run: |
git grep -I -n 'SuccessOrExit([^=)]*(' -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match.
- name: Check for use of "using namespace" outside of a class/function in headers.
if: always()
run: |
# Various platforms have `using namespace chip::Ble` in their BLEManager* headers; just exclude those for now.
#
# Exclude platform openiotsdk bits that do this in their persistent storage header.
#
# Also exclude examples (for now) and third_party, which have various instances of this.
#
# Ignore uses of `System::Clock::Literals`, because that's the only way to have things using _ms32 or whatnot
# in a header file.
git grep -I -n -e '^using namespace' --and --not -e 'System::Clock::Literals' -- './**/*.h' ':(exclude)src/platform/*/BLEManager*.h' ':(exclude)src/platform/openiotsdk/KVPsaPsStore.h' ':(exclude)./examples' ':(exclude)./third_party' && exit 1 || exit 0
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/IssueNOCChainCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class IssueNOCChainCommand : public CHIPCommand

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const chip::ByteSpan & noc,
const chip::ByteSpan & icac, const chip::ByteSpan & rcac,
chip::Optional<chip::IdentityProtectionKeySpan> ipk,
chip::Optional<chip::Crypto::IdentityProtectionKeySpan> ipk,
chip::Optional<chip::NodeId> adminSubject)
{
auto command = static_cast<IssueNOCChainCommand *>(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class OpenCommissioningWindowCommand : public CHIPCommand
"1 to use Enhanced Commissioning Method.\n 0 to use Basic Commissioning Method.");
AddArgument("window-timeout", 0, UINT16_MAX, &mCommissioningWindowTimeout,
"Time, in seconds, before the commissioning window closes.");
AddArgument("iteration", chip::kSpake2p_Min_PBKDF_Iterations, chip::kSpake2p_Max_PBKDF_Iterations, &mIteration,
"Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0.");
AddArgument("iteration", chip::Crypto::kSpake2p_Min_PBKDF_Iterations, chip::Crypto::kSpake2p_Max_PBKDF_Iterations,
&mIteration, "Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0.");
AddArgument("discriminator", 0, 4096, &mDiscriminator, "Discriminator to use for advertising. Ignored if 'option' is 0.");
AddArgument("timeout", 0, UINT16_MAX, &mTimeout, "Time, in seconds, before this command is considered to have timed out.");
}
Expand Down
5 changes: 3 additions & 2 deletions examples/common/pigweed/rpc_services/Device.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "platform/ConfigurationManager.h"
#include "platform/DiagnosticDataProvider.h"
#include "platform/PlatformManager.h"
#include <crypto/CHIPCryptoPAL.h>
#include <platform/DeviceInstanceInfoProvider.h>
#include <setup_payload/QRCodeSetupPayloadGenerator.h>

Expand Down Expand Up @@ -188,9 +189,9 @@ class CommissionableDataProviderRpcWrapper : public DeviceLayer::CommissionableD
private:
std::optional<uint16_t> mDiscriminatorOverride;
std::optional<uint32_t> mPasscodeOverride;
Spake2pVerifierSerialized mVerifierBuf;
Crypto::Spake2pVerifierSerialized mVerifierBuf;
std::optional<ByteSpan> mVerifierOverride;
uint8_t mSaltBuf[kSpake2p_Max_PBKDF_Salt_Length];
uint8_t mSaltBuf[Crypto::kSpake2p_Max_PBKDF_Salt_Length];
std::optional<ByteSpan> mSaltOverride;
std::optional<uint32_t> mIterationCountOverride;
DeviceLayer::CommissionableDataProvider * mCommissionableDataProvider = nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ using namespace chip::app;
using namespace chip::app::DataModel;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::ElectricalPowerMeasurement;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Attributes;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Structs;

CHIP_ERROR ElectricalPowerMeasurementInstance::Init()
{
Expand Down
1 change: 1 addition & 0 deletions examples/platform/linux/CommissionerMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ using namespace chip::DeviceLayer;
using namespace chip::Inet;
using namespace chip::Transport;
using namespace chip::app::Clusters;
using namespace chip::Protocols::UserDirectedCommissioning;

using namespace ::chip::Messaging;
using namespace ::chip::Controller;
Expand Down
7 changes: 4 additions & 3 deletions examples/tv-app/android/java/TVApp-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::AppPlatform;
using namespace chip::Credentials;
using namespace chip::Protocols::UserDirectedCommissioning;

#define JNI_METHOD(RETURN, METHOD_NAME) extern "C" JNIEXPORT RETURN JNICALL Java_com_matter_tv_server_tvapp_TvApp_##METHOD_NAME

Expand Down Expand Up @@ -204,15 +205,15 @@ class MyPincodeService : public PasscodeService
bool foundApp = ContentAppPlatform::GetInstance().HasTargetContentApp(vendorId, productId, rotatingId, info, passcode);
if (!foundApp)
{
info.checkState = chip::Controller::TargetAppCheckState::kAppNotFound;
info.checkState = TargetAppCheckState::kAppNotFound;
}
else if (passcode != 0)
{
info.checkState = chip::Controller::TargetAppCheckState::kAppFoundPasscodeReturned;
info.checkState = TargetAppCheckState::kAppFoundPasscodeReturned;
}
else
{
info.checkState = chip::Controller::TargetAppCheckState::kAppFoundNoPasscode;
info.checkState = TargetAppCheckState::kAppFoundNoPasscode;
}
CommissionerDiscoveryController * cdc = GetCommissionerDiscoveryController();
if (cdc != nullptr)
Expand Down
1 change: 1 addition & 0 deletions examples/tv-app/tv-common/src/AppTv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ extern CommissionerDiscoveryController * GetCommissionerDiscoveryController();
using namespace chip;
using namespace chip::AppPlatform;
using namespace chip::app::Clusters;
using namespace chip::Protocols::UserDirectedCommissioning;

#if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
class MyUserPrompter : public UserPrompter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::AdministratorCommissioning;
using namespace chip::Protocols;
using namespace chip::Crypto;
using chip::Protocols::InteractionModel::Status;

class AdministratorCommissioningAttrAccess : public AttributeAccessInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ namespace app {
namespace Clusters {
namespace DeviceEnergyManagement {

using namespace chip::app::Clusters::DeviceEnergyManagement::Attributes;

class Delegate
{
public:
Expand Down Expand Up @@ -160,24 +158,24 @@ class Delegate

// ------------------------------------------------------------------
// Get attribute methods
virtual ESATypeEnum GetESAType() = 0;
virtual bool GetESACanGenerate() = 0;
virtual ESAStateEnum GetESAState() = 0;
virtual int64_t GetAbsMinPower() = 0;
virtual int64_t GetAbsMaxPower() = 0;
virtual PowerAdjustmentCapability::TypeInfo::Type GetPowerAdjustmentCapability() = 0;
virtual DataModel::Nullable<Structs::ForecastStruct::Type> GetForecast() = 0;
virtual OptOutStateEnum GetOptOutState() = 0;
virtual ESATypeEnum GetESAType() = 0;
virtual bool GetESACanGenerate() = 0;
virtual ESAStateEnum GetESAState() = 0;
virtual int64_t GetAbsMinPower() = 0;
virtual int64_t GetAbsMaxPower() = 0;
virtual Attributes::PowerAdjustmentCapability::TypeInfo::Type GetPowerAdjustmentCapability() = 0;
virtual DataModel::Nullable<Structs::ForecastStruct::Type> GetForecast() = 0;
virtual OptOutStateEnum GetOptOutState() = 0;

// ------------------------------------------------------------------
// Set attribute methods
virtual CHIP_ERROR SetESAType(ESATypeEnum) = 0;
virtual CHIP_ERROR SetESACanGenerate(bool) = 0;
virtual CHIP_ERROR SetESAState(ESAStateEnum) = 0;
virtual CHIP_ERROR SetAbsMinPower(int64_t) = 0;
virtual CHIP_ERROR SetAbsMaxPower(int64_t) = 0;
virtual CHIP_ERROR SetPowerAdjustmentCapability(PowerAdjustmentCapability::TypeInfo::Type) = 0;
virtual CHIP_ERROR SetForecast(DataModel::Nullable<Structs::ForecastStruct::Type>) = 0;
virtual CHIP_ERROR SetESAType(ESATypeEnum) = 0;
virtual CHIP_ERROR SetESACanGenerate(bool) = 0;
virtual CHIP_ERROR SetESAState(ESAStateEnum) = 0;
virtual CHIP_ERROR SetAbsMinPower(int64_t) = 0;
virtual CHIP_ERROR SetAbsMaxPower(int64_t) = 0;
virtual CHIP_ERROR SetPowerAdjustmentCapability(Attributes::PowerAdjustmentCapability::TypeInfo::Type) = 0;
virtual CHIP_ERROR SetForecast(DataModel::Nullable<Structs::ForecastStruct::Type>) = 0;

protected:
EndpointId mEndpointId = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ namespace app {
namespace Clusters {
namespace ElectricalPowerMeasurement {

using namespace chip::app::Clusters::ElectricalPowerMeasurement::Attributes;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Structs;

class Delegate
{
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::OperationalCredentials;
using namespace chip::Credentials;
using namespace chip::Crypto;
using namespace chip::Protocols::InteractionModel;

namespace {
Expand Down
2 changes: 2 additions & 0 deletions src/app/icd/client/CheckInHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

#include <protocols/secure_channel/Constants.h>

using namespace chip::Protocols::SecureChannel;

namespace chip {
namespace app {

Expand Down
2 changes: 0 additions & 2 deletions src/app/icd/client/DefaultCheckInDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
namespace chip {
namespace app {

using namespace std;

class InteractionModelEngine;

/// Callbacks for check in protocol
Expand Down
2 changes: 1 addition & 1 deletion src/app/icd/client/DefaultICDClientStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ CHIP_ERROR DefaultICDClientStorage::DeleteAllEntries(FabricIndex fabricIndex)
}

CHIP_ERROR DefaultICDClientStorage::ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
CounterType & counter)
Protocols::SecureChannel::CounterType & counter)
{
uint8_t appDataBuffer[kAppDataLength];
MutableByteSpan appData(appDataBuffer);
Expand Down
3 changes: 2 additions & 1 deletion src/app/icd/client/DefaultICDClientStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ class DefaultICDClientStorage : public ICDClientStorage
*/
CHIP_ERROR DeleteAllEntries(FabricIndex fabricIndex);

CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo, CounterType & counter) override;
CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Protocols::SecureChannel::CounterType & counter) override;

protected:
enum class ClientInfoTag : uint8_t
Expand Down
4 changes: 2 additions & 2 deletions src/app/icd/client/ICDClientStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
namespace chip {
namespace app {

using namespace Protocols::SecureChannel;
/**
* The ICDClientStorage class is an abstract interface that defines the operations
* for storing, retrieving and deleting ICD client information in persistent storage.
Expand Down Expand Up @@ -81,7 +80,8 @@ class ICDClientStorage
* @param[out] clientInfo retrieved matched clientInfo from storage
* @param[out] counter counter value received in the check-in message
*/
virtual CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo, CounterType & counter) = 0;
virtual CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Protocols::SecureChannel::CounterType & counter) = 0;

// 4 bytes for counter + 2 bytes for ActiveModeThreshold
static inline constexpr uint8_t kAppDataLength = 6;
Expand Down
1 change: 1 addition & 0 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

using namespace chip::app::Clusters;
using namespace chip::System::Clock;
using namespace chip::Crypto;

using AdministratorCommissioning::CommissioningWindowStatusEnum;
using chip::app::DataModel::MakeNullable;
Expand Down
7 changes: 4 additions & 3 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app/data-model/Nullable.h>
#include <app/server/AppDelegate.h>
#include <app/server/CommissioningModeProvider.h>
#include <crypto/CHIPCryptoPAL.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <lib/core/ClusterEnums.h>
#include <lib/core/DataModelTypes.h>
Expand Down Expand Up @@ -93,7 +94,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
FabricIndex fabricIndex, VendorId vendorId);

CHIP_ERROR OpenEnhancedCommissioningWindow(System::Clock::Seconds16 commissioningTimeout, uint16_t discriminator,
Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt,
Crypto::Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt,
FabricIndex fabricIndex, VendorId vendorId);

void CloseCommissioningWindow();
Expand Down Expand Up @@ -204,7 +205,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
uint8_t mFailedCommissioningAttempts = 0;

bool mUseECM = false;
Spake2pVerifier mECMPASEVerifier;
Crypto::Spake2pVerifier mECMPASEVerifier;
uint16_t mECMDiscriminator = 0;
// mListeningForPASE is true only when we are listening for
// PBKDFParamRequest messages or when we're in the middle of a PASE
Expand All @@ -214,7 +215,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
bool mCommissioningTimeoutTimerArmed = false;
uint32_t mECMIterations = 0;
uint32_t mECMSaltLength = 0;
uint8_t mECMSalt[kSpake2p_Max_PBKDF_Salt_Length];
uint8_t mECMSalt[Crypto::kSpake2p_Max_PBKDF_Salt_Length];

// For tests only, so that we can test the commissioning window timeout
// without having to wait 3 minutes.
Expand Down
8 changes: 5 additions & 3 deletions src/app/tests/TestCommissionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

#include <nlunit-test.h>

using namespace chip::Crypto;

using chip::CommissioningWindowAdvertisement;
using chip::CommissioningWindowManager;
using chip::Server;
Expand Down Expand Up @@ -328,9 +330,9 @@ void CheckCommissioningWindowManagerEnhancedWindowTask(intptr_t context)
CHIP_ERROR err = chip::DeviceLayer::GetCommissionableDataProvider()->GetSetupDiscriminator(originDiscriminator);
NL_TEST_ASSERT(suite, err == CHIP_NO_ERROR);
uint16_t newDiscriminator = static_cast<uint16_t>(originDiscriminator + 1);
chip::Spake2pVerifier verifier;
constexpr uint32_t kIterations = chip::kSpake2p_Min_PBKDF_Iterations;
uint8_t salt[chip::kSpake2p_Min_PBKDF_Salt_Length];
Spake2pVerifier verifier;
constexpr uint32_t kIterations = kSpake2p_Min_PBKDF_Iterations;
uint8_t salt[kSpake2p_Min_PBKDF_Salt_Length];
chip::ByteSpan saltData(salt);

NL_TEST_ASSERT(suite, !sWindowStatusDirty);
Expand Down
1 change: 1 addition & 0 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace chip {
namespace Controller {

using namespace chip::app::Clusters;
using namespace chip::Crypto;
using chip::app::DataModel::MakeNullable;
using chip::app::DataModel::NullNullable;

Expand Down
4 changes: 3 additions & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <controller/CommissioneeDeviceProxy.h>
#include <controller/CommissioningDelegate.h>
#include <credentials/DeviceAttestationConstructor.h>
#include <crypto/CHIPCryptoPAL.h>
#include <protocols/secure_channel/RendezvousParameters.h>

namespace chip {
Expand Down Expand Up @@ -70,7 +71,8 @@ class AutoCommissioner : public CommissioningDelegate
ByteSpan GetDAC() const { return ByteSpan(mDAC, mDACLen); }
ByteSpan GetPAI() const { return ByteSpan(mPAI, mPAILen); }

CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, IdentityProtectionKeySpan ipk, NodeId adminSubject);
CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, Crypto::IdentityProtectionKeySpan ipk,
NodeId adminSubject);
EndpointId GetEndpoint(const CommissioningStage & stage) const;
CommissioningStage GetNextCommissioningStageInternal(CommissioningStage currentStage, CHIP_ERROR & lastErr);

Expand Down
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ using namespace chip::System;
using namespace chip::Transport;
using namespace chip::Credentials;
using namespace chip::app::Clusters;
using namespace chip::Crypto;

namespace chip {
namespace Controller {
Expand Down
Loading

0 comments on commit c89090c

Please sign in to comment.