Issue 298 clang tidy false positives

.github/workflows/ci.yml

@@ -435,7 +435,7 @@ jobs:
           style: 'file' # read .clang-format for configuration
           tidy-checks: '' # Read .clang-tidy for configuration
           database: build/Release/compile_commands.json
-          version: 12
+          version: 13
 
 
       - name: Run linters on tests
@@ -450,7 +450,7 @@ jobs:
           style: 'file' # read .clang-format for configuration
           tidy-checks: '' # Read .clang-tidy for configuration
           database: build/Release/compile_commands.json
-          version: 12
+          version: 13
 
 
       - name: Report lint failure

include/up-cpp/client/usubscription/v3/Consumer.h

@@ -9,8 +9,8 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
-#ifndef UP_CPP_CLIENT_Consumer_H
-#define UP_CPP_CLIENT_Consumer_H
+#ifndef UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H
+#define UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H
 
 #include <up-cpp/communication/NotificationSink.h>
 #include <up-cpp/communication/RpcClient.h>
@@ -23,8 +23,10 @@
 #include <utility>
 
 namespace uprotocol::client::usubscription::v3 {
-using namespace uprotocol::core::usubscription::v3;
-using namespace uprotocol::utils;
+using uprotocol::core::usubscription::v3::SubscriptionRequest;
+using uprotocol::core::usubscription::v3::UnsubscribeRequest;
+using uprotocol::core::usubscription::v3::Update;
+using uprotocol::core::usubscription::v3::uSubscription;
 
 /**
  * @struct ConsumerOptions
@@ -47,14 +49,13 @@ struct ConsumerOptions {
 	std::optional<google::protobuf::Any> subscription_details;
 };
 
-
 /// @struct uSubscriptionUUriBuilder
 /// @brief Structure to build uSubscription request URIs.
 ///
 /// This structure is used to build URIs for uSubscription service. It uses the
-/// service options from uSubscription proto to set the authority name, ue_id, ue_version_major, and
-/// the notification topic resource ID in the URI.
-struct uSubscriptionUUriBuilder {
+/// service options from uSubscription proto to set the authority name, ue_id,
+/// ue_version_major, and the notification topic resource ID in the URI.
+struct USubscriptionUUriBuilder {
 private:
 	/// URI for the uSubscription service
 	v1::UUri uri_;
@@ -63,7 +64,7 @@ struct uSubscriptionUUriBuilder {
 
 public:
 	/// @brief Constructor for uSubscriptionUUriBuilder.
-	uSubscriptionUUriBuilder() {
+	USubscriptionUUriBuilder() {
 		// Get the service descriptor
 		const google::protobuf::ServiceDescriptor* service =
 		    uSubscription::descriptor();
@@ -122,18 +123,20 @@ struct Consumer {
 	///
 	/// @param transport Transport to register with.
 	/// @param subscription_topic Topic to subscribe to.
-	/// @param callback Function that is called when publish message is received.
+	/// @param callback Function that is called when publish message is
+	/// received.
 	/// @param priority Priority of the subscription request.
 	/// @param subscribe_request_ttl Time to live for the subscription request.
 	/// @param consumer_options Additional details for uSubscription service.
 	[[nodiscard]] static ConsumerOrStatus create(
 	    std::shared_ptr<transport::UTransport> transport,
-	    const v1::UUri subscription_topic, ListenCallback&& callback,
+	    const v1::UUri& subscription_topic, ListenCallback&& callback,
 	    v1::UPriority priority,
 	    std::chrono::milliseconds subscription_request_ttl,
 	    ConsumerOptions consumer_options);
 
-	/// @brief Unsubscribe from the topic and call uSubscription service to close the subscription.
+	/// @brief Unsubscribe from the topic and call uSubscription service to
+	/// close the subscription.
 	///
 	/// @param priority Priority of the unsubscribe request.
 	/// @param request_ttl Time to live for the unsubscribe request.
@@ -156,7 +159,7 @@ struct Consumer {
 	/// @param transport Transport to register with.
 	/// @param subscriber_details Additional details about the subscriber.
 	Consumer(std::shared_ptr<transport::UTransport> transport,
-	         const v1::UUri subscription_topic,
+	         v1::UUri subscription_topic,
 	         ConsumerOptions consumer_options = {});
 
 private:
@@ -169,7 +172,7 @@ struct Consumer {
 	ConsumerOptions consumer_options_;
 
 	// URI info about the uSubscription service
-	uSubscriptionUUriBuilder uSubscriptionUUriBuilder_;
+	USubscriptionUUriBuilder uSubscriptionUUriBuilder_;
 
 	// Subscription updates
 	std::unique_ptr<communication::NotificationSink> noficationSinkHandle_;
@@ -205,13 +208,15 @@ struct Consumer {
 	/// @brief Subscribe to the topic
 	///
 	/// @param topic Topic to subscribe to.
-	/// @param subscription_request_ttl Time to live for the subscription request.
-	/// @param callback Function that is called when a published message is received.
+	/// @param subscription_request_ttl Time to live for the subscription
+	/// request.
+	/// @param callback Function that is called when a published message is
+	/// received.
 	v1::UStatus subscribe(v1::UPriority priority,
 	                      std::chrono::milliseconds subscription_request_ttl,
 	                      ListenCallback&& callback);
 };
 
 }  // namespace uprotocol::client::usubscription::v3
 
-#endif  // UP_CPP_CLIENT_Consumer_H
+#endif  // UP_CPP_CLIENT_USUBSCRIPTION_V3_CONSUMER_H

include/up-cpp/communication/NotificationSink.h

@@ -58,7 +58,7 @@ struct NotificationSink {
 	///      successfully.
 	///    * UStatus containing an error state otherwise.
 	[[nodiscard]] static SinkOrStatus create(
-	    std::shared_ptr<transport::UTransport> transport,
+	    const std::shared_ptr<transport::UTransport>& transport,
 	    ListenCallback&& callback, const v1::UUri& source_filter);
 
 	/// @note DEPRECATED
@@ -76,13 +76,12 @@ struct NotificationSink {
 	[[deprecated(
 	    "See alternate overload of "
 	    "create()")]] [[nodiscard]] static SinkOrStatus
-	create(std::shared_ptr<transport::UTransport> transport,
+	create(const std::shared_ptr<transport::UTransport>& transport,
 	       const v1::UUri& sink, ListenCallback&& callback,
 	       std::optional<v1::UUri>&& source_filter);
 
 	~NotificationSink() = default;
 
-protected:
 	/// @brief Constructs a notification listener connected to a given
 	///        transport.
 	///

include/up-cpp/communication/RpcClient.h

@@ -85,7 +85,7 @@ struct RpcClient {
 		/// @name Passthroughs for std::future
 		/// @{
 		auto get() { return future_.get(); }
-		auto valid() const noexcept { return future_.valid(); }
+		[[nodiscard]] auto valid() const noexcept { return future_.valid(); }
 		void wait() const { future_.wait(); }
 		template <typename... Args>
 		auto wait_for(Args&&... args) const {
@@ -163,7 +163,7 @@ struct RpcClient {
 	[[nodiscard]] InvokeFuture invokeMethod();
 
 	/// @brief Default move constructor (defined in RpcClient.cpp)
-	RpcClient(RpcClient&&);
+	RpcClient(RpcClient&&) noexcept;
 
 	/// @brief Default destructor (defined in RpcClient.cpp)
 	~RpcClient();

include/up-cpp/communication/RpcServer.h

@@ -90,9 +90,9 @@ struct RpcServer {
 	/// @param ttl (Optional) Time response will be valid from the moment
 	///            respond() is called. Note that the original request's TTL
 	///            may also still apply.
-	RpcServer(std::shared_ptr<transport::UTransport> transport,
-	          std::optional<v1::UPayloadFormat> format = {},
-	          std::optional<std::chrono::milliseconds> ttl = {});
+	explicit RpcServer(std::shared_ptr<transport::UTransport> transport,
+	                   std::optional<v1::UPayloadFormat> format = {},
+	                   std::optional<std::chrono::milliseconds> ttl = {});
 
 	/// @brief Allows std::make_unique to directly access RpcServer's private
 	/// constructor.

include/up-cpp/datamodel/builder/Payload.h

@@ -80,13 +80,13 @@ struct Payload {
 	///          will compile out.
 	/// @param data Data to be serialized and stored.
 	template <typename Serializer, typename ValueT>
-	Payload(Serializer s, const ValueT& data) {
-		auto serializedData = Serializer::serialize(data);
+	Payload(Serializer s [[maybe_unused]], const ValueT& data) {
+		auto serialized_data = Serializer::serialize(data);
 		if (!UPayloadFormat_IsValid(
-		        std::get<PayloadType::Format>(serializedData))) {
+		        std::get<PayloadType::Format>(serialized_data))) {
 			throw std::out_of_range("Invalid Serializer payload format");
 		}
-		payload_ = std::move(serializedData);
+		payload_ = std::move(serialized_data);
 	}
 
 	/// @brief Creates a Payload builder with a provided pre-serialized data.
@@ -95,8 +95,7 @@ struct Payload {
 	/// @param format The data format of the payload in value_bytes.
 	///
 	/// @throws std::out_of_range If format is not valid for v1::UPayloadFormat
-	Payload(const std::vector<uint8_t>& value_bytes,
-	        const v1::UPayloadFormat format);
+	Payload(const std::vector<uint8_t>& value_bytes, v1::UPayloadFormat format);
 
 	/// @brief Creates a Payload builder with a provided pre-serialized data.
 	///
@@ -107,7 +106,7 @@ struct Payload {
 	///
 	/// @note This would typically be used for UPAYLOAD_FORMAT_TEXT or
 	///       UPAYLOAD_FORMAT_JSON, but can be used for other payload formats.
-	Payload(const std::string& value, const v1::UPayloadFormat format);
+	Payload(const std::string& value, v1::UPayloadFormat format);
 
 	/// @brief Creates a Payload builder with a provided pre-serialized data.
 	///
@@ -120,7 +119,7 @@ struct Payload {
 	///
 	/// @note This would typically be used for UPAYLOAD_FORMAT_TEXT or
 	///       UPAYLOAD_FORMAT_JSON, but can be used for other payload formats.
-	Payload(std::string&& value, const v1::UPayloadFormat format);
+	Payload(std::string&& value, v1::UPayloadFormat format);
 
 	/// @brief Creates a Payload builder with a provided pre-serialized data.
 	///

include/up-cpp/datamodel/builder/UMessage.h

@@ -244,11 +244,11 @@ struct UMessageBuilder {
 private:
 	/// @brief Constructs a UMessageBuilder with the provided attributes.
 	///
-	/// @param msgType
+	/// @param msg_type
 	/// @param source
 	/// @param sink
 	/// @param request_id
-	UMessageBuilder(v1::UMessageType msgType, v1::UUri&& source,
+	UMessageBuilder(v1::UMessageType msg_type, v1::UUri&& source,
 	                std::optional<v1::UUri>&& sink = {},
 	                std::optional<v1::UUID>&& request_id = {});
 

include/up-cpp/datamodel/builder/Uuid.h

@@ -85,7 +85,7 @@ struct UuidBuilder {
 	v1::UUID build();
 
 private:
-	UuidBuilder(bool testing);
+	explicit UuidBuilder(bool testing);
 
 	const bool testing_{false};
 	std::function<std::chrono::system_clock::time_point()> time_source_;

include/up-cpp/datamodel/constants/UuidConstants.h

@@ -38,6 +38,34 @@ constexpr uint64_t MASK_32_BITS = 0xFFFFFFFF;
 constexpr uint64_t MASK_16_BITS = 0xFFFF;
 constexpr uint64_t MASK_14_BITS = 0x3FFF;
 
+// number of digits needed to represent a given number of bits in base 16
+constexpr uint64_t LEN_16_BITS_IN_HEX = 4;
+constexpr uint64_t LEN_32_BITS_IN_HEX = 8;
+constexpr uint64_t LEN_48_BITS_IN_HEX = 12;
+
+// number of characters a valid uuid
+constexpr uint64_t TOTAL_UUID_LENGTH = 36;
+constexpr uint64_t LEN_MSB_IN_HEX = 8;
+constexpr uint64_t LEN_LSB_IN_HEX = 4;
+constexpr uint64_t LEN_VCANT_IN_HEX = 4;
+constexpr uint64_t LEN_VARR_IN_HEX = 4;
+constexpr uint64_t LEN_RAND_IN_HEX = 8;
+
+// number of bits represented by a single hex character
+constexpr uint64_t LEN_HEX_TO_BIT = 4;
+
+// number of bits to represent uint64
+constexpr uint64_t LEN_UINT64_IN_BIT = sizeof(uint64_t) * 8;
+
+// expected positions of the '-' separators in a valid uuid
+constexpr uint64_t POS_FIRST_SEPARATOR = LEN_MSB_IN_HEX;
+constexpr uint64_t POS_SECOND_SEPARATOR =
+    POS_FIRST_SEPARATOR + LEN_LSB_IN_HEX + 1;
+constexpr uint64_t POS_THIRD_SEPARATOR =
+    POS_SECOND_SEPARATOR + LEN_VCANT_IN_HEX + 1;
+constexpr uint64_t POS_FOURTH_SEPARATOR =
+    POS_THIRD_SEPARATOR + LEN_VARR_IN_HEX + 1;
+
 }  // namespace uprotocol::datamodel
 
 #endif  // UP_CPP_DATAMODEL_CONSTANTS_UUIDCONSTANTS_H

include/up-cpp/datamodel/serializer/Uuid.h

@@ -27,13 +27,13 @@ namespace uprotocol::datamodel::serializer::uuid {
 
 /// @brief Converts to and from a human-readable string representation of UUID
 struct AsString {
-	[[nodiscard]] static std::string serialize(v1::UUID);
+	[[nodiscard]] static std::string serialize(const v1::UUID&);
 	[[nodiscard]] static v1::UUID deserialize(const std::string&);
 };
 
 /// @brief Converts to and from byte vector representation of UUID
 struct AsBytes {
-	[[nodiscard]] static std::vector<uint8_t> serialize(v1::UUID);
+	[[nodiscard]] static std::vector<uint8_t> serialize(const v1::UUID&);
 	[[nodiscard]] static v1::UUID deserialize(const std::vector<uint8_t>&);
 };
 

include/up-cpp/datamodel/validator/UUri.h

@@ -163,7 +163,7 @@ isValidDefaultSource(const v1::UUri&);
 struct InvalidUUri : public std::invalid_argument {
 	// Forward constructors
 	template <typename... Args>
-	InvalidUUri(Args&&... args)
+	explicit InvalidUUri(Args&&... args)
 	    : std::invalid_argument(std::forward<Args>(args)...) {}
 
 	InvalidUUri(InvalidUUri&&) noexcept;

include/up-cpp/datamodel/validator/Uuid.h

@@ -54,44 +54,44 @@ using ValidationResult = std::tuple<bool, std::optional<Reason>>;
 /// @{
 /// @brief Checks if the provided UUID contains valid uP v8 UUID data.
 /// @returns True if the UUID has valid UUID data, false otherwise.
-ValidationResult isUuid(v1::UUID);
+ValidationResult isUuid(const v1::UUID&);
 
 /// @brief Checks if the provided UUID has expired based on the given TTL.
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns True if the difference between the current system time and
 ///          the the timestamp in the UUID is greater than the TTL.
-ValidationResult isExpired(v1::UUID uuid, std::chrono::milliseconds ttl);
+ValidationResult isExpired(const v1::UUID& uuid, std::chrono::milliseconds ttl);
 /// @}
 
 /// @name Inspection utilities
 /// @{
 /// @brief Gets the version field from a UUID object
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns The UUID's version
-uint8_t getVersion(v1::UUID);
+uint8_t getVersion(const v1::UUID&);
 
 /// @brief Gets the variant field from a UUID object
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns The UUID's variant
-uint8_t getVariant(v1::UUID);
+uint8_t getVariant(const v1::UUID&);
 
 /// @brief Gets the timestamp field from a UUID object
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns The UUID's timestamp as a chrono::time_point for the system
 ///          clock.
-std::chrono::system_clock::time_point getTime(v1::UUID uuid);
+std::chrono::system_clock::time_point getTime(const v1::UUID& uuid);
 
 /// @brief Gets the difference between a UUID's timestamp and the current
 ///        time according to the system clock/
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns The age of the UUID in milliseconds
-std::chrono::milliseconds getElapsedTime(v1::UUID);
+std::chrono::milliseconds getElapsedTime(const v1::UUID& uuid);
 
 /// @brief Gets the time remaining before the UUID expires, based on the
 ///        given TTL.
 /// @throws InvalidUuid if the UUID does not contain valid UUID data
 /// @returns Remaining time (ttl - getElapsedTime(uuid)) in milliseconds
-std::chrono::milliseconds getRemainingTime(v1::UUID uuid,
+std::chrono::milliseconds getRemainingTime(const v1::UUID& uuid,
                                            std::chrono::milliseconds ttl);
 /// @}