iox-eclipse-iceoryx#889 Fix clang-tidy violations for iceoryx_hoofs

.clang-tidy

@@ -1,4 +1,38 @@
-Checks: '-*,readability-*,-readability-named-parameter,-readability-avoid-const-params-in-decls,-readability-else-after-return,performance-*,-readability-redundant-access-specifiers,hicpp-*,-hicpp-named-parameter,-hicpp-avoid-c-arrays,cppcoreguidelines-*,-hicpp-no-array-decay,-hicpp-signed-bitwise,-hicpp-vararg,-cppcoreguidelines-avoid-c-arrays,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-type-vararg,concurrency-*,clang-analyzer-*,cert-*,bugprone-*'
+# NOTE: following checks are disabled, because they have duplicates in other group:
+#
+# - readability-magic-numbers (duplicate of cppcoreguidelines-avoid-magic-numbers)
+# - hicpp-no-malloc (duplicate of cppcoreguidelines-no-malloc)
+# - hicpp-member-init (duplicate of cppcoreguidelines-pro-type-member-init)
+# - performance-move-const-arg (duplicate of hicpp-move-const-arg)
+# - bugprone-use-after-move (duplicate of hicpp-move-const-arg)
+
+Checks: '
+-*,
+readability-*,
+clang-analyzer-*,
+cert-*,
+bugprone-*,
+-readability-named-parameter,
+-readability-avoid-const-params-in-decls,
+-readability-else-after-return,performance-*,
+-readability-redundant-access-specifiers,hicpp-*,
+-readability-magic-numbers,
+-hicpp-named-parameter,
+-hicpp-avoid-c-arrays,cppcoreguidelines-*,
+-hicpp-no-array-decay,-hicpp-signed-bitwise,
+-hicpp-vararg,
+-hicpp-no-malloc,
+-hicpp-member-init,
+-performance-move-const-arg,
+-cppcoreguidelines-avoid-c-arrays,
+-cppcoreguidelines-pro-bounds-constant-array-index,
+-cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+-cppcoreguidelines-pro-type-vararg,concurrency-*,
+-cppcoreguidelines-macro-usage,
+-cppcoreguidelines-avoid-non-const-global-variables,
+-cppcoreguidelines-pro-type-reinterpret-cast,
+-bugprone-use-after-move'
+
 CheckOptions:
   - { key: readability-identifier-naming.ClassCase,               value: CamelCase }
   - { key: readability-identifier-naming.EnumCase,                value: CamelCase }
@@ -11,7 +45,7 @@ CheckOptions:
   - { key: readability-identifier-naming.ProtectedMemberPrefix,   value: m_ }
   - { key: readability-identifier-naming.MemberCase,              value: camelBack }
   - { key: readability-identifier-naming.ConstexprVariableCase,   value: UPPER_CASE }
-  - { key: readability-identifier-naming.EnumCase,                value: UPPER_CASE }
+  - { key: readability-identifier-naming.EnumConstantCase,        value: UPPER_CASE }
   - { key: readability-identifier-naming.GlobalConstantCase,      value: UPPER_CASE }
   - { key: readability-identifier-naming.TemplateParameterCase,   value: CamelCase }
   - { key: readability-function-size.LineThreshold,               value: 200 }

.github/workflows/build-test.yml

@@ -16,6 +16,12 @@ jobs:
       - uses: actions/checkout@v2
       - run: ./tools/check_formatting.sh
 
+  run-clang-tidy:
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - run: ./tools/ci/run-clang-tidy.sh
+
   build-test-ubuntu:
     runs-on: ubuntu-18.04
     needs: pre-flight-check

CHANGELOG.md

@@ -23,6 +23,7 @@
 
 **Refactoring:**
 
+- Add clang-tidy rules for iceoryx_hoofs[\#889](https://github.com/eclipse-iceoryx/iceoryx/issues/889)
 - Move all tests into an anonymous namespace[\#563](https://github.com/eclipse-iceoryx/iceoryx/issues/563)
 - Refactor smart_c to use contract by design and expected[\#418](https://github.com/eclipse-iceoryx/iceoryx/issues/418)
 - PoshRuntime Mock[\#449](https://github.com/eclipse-iceoryx/iceoryx/issues/449)

iceoryx_binding_c/test/moduletests/test_c2cpp_enum_translation.cpp

@@ -57,7 +57,7 @@ TEST(c2cpp_enum_translation_test, SubscriberState)
 // the clang sanitizer detects this successfully and this leads to termination, and with this the test fails
 #if !defined(__clang__)
     iox::Error errorValue = iox::Error::kNO_ERROR;
-    auto errorHandlerGuard = iox::ErrorHandler::SetTemporaryErrorHandler(
+    auto errorHandlerGuard = iox::ErrorHandler::setTemporaryErrorHandler(
         [&](const iox::Error e, const std::function<void()>, const iox::ErrorLevel) { errorValue = e; });
     EXPECT_EQ(c2cpp::queueFullPolicy(static_cast<iox_QueueFullPolicy>(-1)),
               iox::popo::QueueFullPolicy::DISCARD_OLDEST_DATA);
@@ -93,7 +93,7 @@ TEST(c2cpp_enum_translation_test, SubscriberEvent)
 // the clang sanitizer detects this successfully and this leads to termination, and with this the test fails
 #if !defined(__clang__)
     iox::Error errorValue = iox::Error::kNO_ERROR;
-    auto errorHandlerGuard = iox::ErrorHandler::SetTemporaryErrorHandler(
+    auto errorHandlerGuard = iox::ErrorHandler::setTemporaryErrorHandler(
         [&](const iox::Error e, const std::function<void()>, const iox::ErrorLevel) { errorValue = e; });
     EXPECT_EQ(c2cpp::subscriberTooSlowPolicy(static_cast<iox_SubscriberTooSlowPolicy>(-1)),
               iox::popo::SubscriberTooSlowPolicy::DISCARD_OLDEST_DATA);

iceoryx_dds/include/iceoryx_dds/internal/log/logging.hpp

@@ -26,15 +26,23 @@ namespace dds
 {
 struct DDSLoggingComponent
 {
+    // NOLINTNEXTLINE(readability-identifier-naming)
     static constexpr char Ctx[] = "DDS";
+    // NOLINTNEXTLINE(readability-identifier-naming)
     static constexpr char Description[] = "Log context of the DDS module.";
 };
 
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogFatal = iox::log::ffbb::LogFatal<DDSLoggingComponent>;
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogError = iox::log::ffbb::LogError<DDSLoggingComponent>;
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogWarn = iox::log::ffbb::LogWarn<DDSLoggingComponent>;
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogInfo = iox::log::ffbb::LogInfo<DDSLoggingComponent>;
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogDebug = iox::log::ffbb::LogDebug<DDSLoggingComponent>;
+// NOLINTNEXTLINE(readability-identifier-naming)
 static constexpr auto LogVerbose = iox::log::ffbb::LogVerbose<DDSLoggingComponent>;
 
 } // namespace dds

iceoryx_hoofs/include/iceoryx_hoofs/cxx/attributes.hpp

@@ -27,7 +27,8 @@ namespace cxx
 ///     uint32_t foo();
 ///     IOX_DISCARD_RESULT(foo()); // suppress compiler warning for unused return value
 /// @endcode
-#define IOX_DISCARD_RESULT(expr) static_cast<void>(expr) // NOLINT
+// NOLINTNEXTLINE
+#define IOX_DISCARD_RESULT(expr) static_cast<void>(expr)
 
 /// @brief IOX_NO_DISCARD adds the [[nodiscard]] keyword if it is available for the current compiler.
 ///        If additionally the keyword [[gnu::warn_unused]] is present it will be added as well.
@@ -37,14 +38,18 @@ namespace cxx
 ///   activate keywords for gcc>=5 or clang>=4
 #if defined(_WIN32)
 // On WIN32 we are using C++17 which makes the keyword [[nodiscard]] available
-#define IOX_NO_DISCARD [[nodiscard]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_NO_DISCARD [[nodiscard]]
 #elif defined(__APPLE__) && defined(__clang__)
 // On APPLE we are using C++17 which makes the keyword [[nodiscard]] available
-#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]]
 #elif (defined(__clang__) && __clang_major__ >= 4)
-#define IOX_NO_DISCARD [[gnu::warn_unused]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_NO_DISCARD [[gnu::warn_unused]]
 #elif (defined(__GNUC__) && __GNUC__ >= 5)
-#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_NO_DISCARD [[nodiscard, gnu::warn_unused]]
 #else
 // on an unknown platform we use for now nothing since we do not know what is supported there
 #define IOX_NO_DISCARD
@@ -57,13 +62,16 @@ namespace cxx
 ///   activate keywords for gcc>=7 or clang>=4
 #if defined(_WIN32)
 // On WIN32 we are using C++17 which makes the keyword [[fallthrough]] available
-#define IOX_FALLTHROUGH [[fallthrough]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_FALLTHROUGH [[fallthrough]]
 #elif defined(__APPLE__) && defined(__clang__)
 // On APPLE we are using C++17 which makes the keyword [[fallthrough]] available
-#define IOX_FALLTHROUGH [[fallthrough]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_FALLTHROUGH [[fallthrough]]
 #elif (defined(__GNUC__) && __GNUC__ >= 7) && !defined(__clang__)
 // clang prints a warning therefore we exclude it here
-#define IOX_FALLTHROUGH [[fallthrough]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_FALLTHROUGH [[fallthrough]]
 #else
 // on an unknown platform we use for now nothing since we do not know what is supported there
 #define IOX_FALLTHROUGH
@@ -74,10 +82,12 @@ namespace cxx
 /// @note
 ///   activate attribute for gcc or clang
 #if defined(__GNUC__) || defined(__clang__)
-#define IOX_MAYBE_UNUSED [[gnu::unused]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_MAYBE_UNUSED [[gnu::unused]]
 #elif defined(_WIN32)
 // On WIN32 we are using C++17 which makes the attribute [[maybe_unused]] available
-#define IOX_MAYBE_UNUSED [[maybe_unused]] // NOLINT
+// NOLINTNEXTLINE
+#define IOX_MAYBE_UNUSED [[maybe_unused]]
 // on an unknown platform we use for now nothing since we do not know what is supported there
 #else
 #define IOX_MAYBE_UNUSED

iceoryx_hoofs/include/iceoryx_hoofs/cxx/deadline_timer.hpp

@@ -45,7 +45,7 @@ class DeadlineTimer
   public:
     /// @brief Constructor
     /// @param[in] timeToWait duration until the timer expires
-    DeadlineTimer(const iox::units::Duration timeToWait) noexcept;
+    explicit DeadlineTimer(const iox::units::Duration timeToWait) noexcept;
 
     /// @brief Checks if the timer has expired compared to its absolute end time
     /// @return false if the timer is still active and true if it is expired
@@ -63,10 +63,10 @@ class DeadlineTimer
 
     /// @brief calculates the remaining time before the timer goes off
     /// @return the time duration before the timer expires
-    const iox::units::Duration remainingTime() const noexcept;
+    iox::units::Duration remainingTime() const noexcept;
 
   private:
-    iox::units::Duration getCurrentMonotonicTime() const noexcept;
+    static iox::units::Duration getCurrentMonotonicTime() noexcept;
 
     iox::units::Duration m_timeToWait;
     iox::units::Duration m_endTime;

iceoryx_hoofs/include/iceoryx_hoofs/cxx/generic_raii.hpp

@@ -47,13 +47,13 @@ class GenericRAII
   public:
     /// @brief constructor which creates GenericRAII that calls only the cleanupFunction on destruction
     /// @param[in] cleanupFunction callable which will be called in the destructor
-    GenericRAII(const std::function<void()> cleanupFunction) noexcept;
+    explicit GenericRAII(const std::function<void()>& cleanupFunction) noexcept;
 
     /// @brief constructor which calls initFunction and stores the cleanupFunction which will be
     ///           called in the destructor
     /// @param[in] initFunction callable which will be called in the constructor
     /// @param[in] cleanupFunction callable which will be called in the destructor
-    GenericRAII(const function_ref<void()> initFunction, const std::function<void()> cleanupFunction) noexcept;
+    GenericRAII(const function_ref<void()>& initFunction, const std::function<void()>& cleanupFunction) noexcept;
 
     /// @brief calls m_cleanupFunction callable if it was set in the constructor
     ~GenericRAII() noexcept;

iceoryx_hoofs/include/iceoryx_hoofs/design_pattern/creation.hpp

@@ -137,11 +137,20 @@ class Creation
     /// @brief returns true if the object was constructed successfully, otherwise false
     bool isInitialized() const noexcept;
 
+    // void creationMove() noexcept;
+
   protected:
     bool m_isInitialized{false};
     ErrorType m_errorValue;
 };
 
+template <typename Class, typename Error>
+void creationMove(Creation<Class, Error>& lhs, Creation<Class, Error>&& rhs) noexcept
+{
+    using CreationPattern_t = Creation<Class, Error>;
+    lhs.operator=(std::forward<CreationPattern_t>(rhs));
+}
+
 } // namespace DesignPattern
 
 #include "iceoryx_hoofs/internal/design_pattern/creation.inl"

iceoryx_hoofs/include/iceoryx_hoofs/error_handling/error_handling.hpp

@@ -20,7 +20,7 @@
 #include "iceoryx_hoofs/cxx/generic_raii.hpp"
 #include "iceoryx_hoofs/cxx/vector.hpp"
 
-#include <assert.h>
+#include <cassert>
 #include <functional>
 #include <iostream>
 #include <mutex>
@@ -215,22 +215,22 @@ using HandlerFunction = std::function<void(const Error error, const std::functio
 ///         error handling.
 class ErrorHandler
 {
-    friend void errorHandler(const Error error, const std::function<void()> errorCallBack, const ErrorLevel level);
+    friend void errorHandler(const Error error, const std::function<void()>& errorCallBack, const ErrorLevel level);
 
   public:
-    static cxx::GenericRAII SetTemporaryErrorHandler(const HandlerFunction& newHandler);
+    static cxx::GenericRAII setTemporaryErrorHandler(const HandlerFunction& newHandler);
 
-    static const char* ToString(const Error error);
+    static const char* toString(const Error error);
 
   protected:
-    static void ReactOnErrorLevel(const ErrorLevel level, const char* errorText);
+    static void reactOnErrorLevel(const ErrorLevel level, const char* errorText);
 
   private:
-    static void DefaultHandler(const Error error,
-                               const std::function<void()> errorCallBack,
+    static void defaultHandler(const Error error,
+                               const std::function<void()>& errorCallBack,
                                const ErrorLevel level = ErrorLevel::FATAL);
 
-    static const char* errorNames[];
+    static const char* ERROR_NAMES[];
     static iox::HandlerFunction handler;
     /// Needed, if you want to exchange the handler. Remember the old one and call it if it is not your error. The error
     /// mock needs to be the last one exchanging the handler in tests.
@@ -266,7 +266,7 @@ class ErrorHandler
 ///
 /// @code
 /// bool called = false;
-/// auto temporaryErrorHandler = ErrorHandler::SetTemporaryErrorHandler(
+/// auto temporaryErrorHandler = ErrorHandler::setTemporaryErrorHandler(
 ///     [&](const Error e, std::function<void()>, const ErrorLevel) {
 ///         called = true;
 ///     });
@@ -275,7 +275,7 @@ class ErrorHandler
 /// ASSERT_TRUE(called);
 /// @endcode
 void errorHandler(const Error error,
-                  const std::function<void()> errorCallBack = std::function<void()>(),
+                  const std::function<void()>& errorCallBack = std::function<void()>(),
                   const ErrorLevel level = ErrorLevel::FATAL);
 } // namespace iox
 

iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/active_object.hpp

@@ -32,7 +32,7 @@ class ActiveObject
   protected:
     ActiveObject() noexcept;
     virtual ~ActiveObject() noexcept;
-    void addTask(const std::function<void()> f) noexcept;
+    void addTask(const std::function<void()>& f) noexcept;
     void mainLoop() noexcept;
     void stopRunning() noexcept;
 

iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/creation.inl

@@ -79,6 +79,12 @@ inline iox::cxx::expected<ErrorType> Creation<DerivedClass, ErrorType>::placemen
     return iox::cxx::success<>();
 }
 
+// template <typename DerivedClass, typename ErrorType>
+// inline void Creation<DerivedClass, ErrorType>::creationMove() noexcept
+// {
+//     CreationPattern_t::operator=(std::forward<CreationPattern_t>(rhs.base));
+// }
+
 
 } // namespace DesignPattern
 #endif

iceoryx_hoofs/include/iceoryx_hoofs/internal/file_reader/file_reader.hpp

@@ -31,7 +31,7 @@ namespace cxx
 /// @code
 ///     cxx::FileReader reader("filename");
 ///     std::string str;
-///     if(reader.IsOpen()) {
+///     if(reader.isOpen()) {
 ///         reader.ReadeLine(str);
 ///     }
 ///
@@ -64,9 +64,9 @@ class FileReader
     ~FileReader() = default;
 
     /// Check if the associated file is open.
-    bool IsOpen() const;
+    bool isOpen() const;
     /// Read one line from the file and store the result in f_string.
-    bool ReadLine(std::string& f_string);
+    bool readLine(std::string& f_string);
 
   private:
 #ifdef _WIN32

iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/access_control.hpp

@@ -113,9 +113,9 @@ class AccessController
 
     cxx::vector<PermissionEntry, MaxNumOfPermissions> m_permissions;
 
-    cxx::expected<smartAclPointer_t, AccessControllerError> createACL(const int32_t f_numEntries) const;
-    bool createACLEntry(const acl_t f_ACL, const PermissionEntry& f_entry) const;
-    bool addAclPermission(acl_permset_t f_permset, acl_perm_t f_perm) const;
+    static cxx::expected<smartAclPointer_t, AccessControllerError> createACL(const int32_t f_numEntries);
+    static bool createACLEntry(const acl_t f_ACL, const PermissionEntry& f_entry);
+    static bool addAclPermission(acl_permset_t f_permset, acl_perm_t f_perm);
 
     bool m_useACLMask{false};
 };

iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/message_queue.hpp

@@ -63,9 +63,9 @@ class MessageQueue : public DesignPattern::Creation<MessageQueue, IpcChannelErro
     MessageQueue();
 
     MessageQueue(const MessageQueue& other) = delete;
-    MessageQueue(MessageQueue&& other);
+    MessageQueue(MessageQueue&& other) noexcept;
     MessageQueue& operator=(const MessageQueue& other) = delete;
-    MessageQueue& operator=(MessageQueue&& other);
+    MessageQueue& operator=(MessageQueue&& other) noexcept;
 
     ~MessageQueue();
 
@@ -98,7 +98,7 @@ class MessageQueue : public DesignPattern::Creation<MessageQueue, IpcChannelErro
     MessageQueue(const IpcChannelName_t& name,
                  const IpcChannelSide channelSide,
                  const size_t maxMsgSize = MAX_MESSAGE_SIZE,
-                 const uint64_t maxMsgNumber = 10u);
+                 const uint64_t maxMsgNumber = 10U);
 
     cxx::expected<int32_t, IpcChannelError> open(const IpcChannelName_t& name, const IpcChannelSide channelSide);
 
@@ -111,9 +111,9 @@ class MessageQueue : public DesignPattern::Creation<MessageQueue, IpcChannelErro
 
   private:
     IpcChannelName_t m_name;
-    struct mq_attr m_attributes;
+    mq_attr m_attributes{};
     mqd_t m_mqDescriptor = INVALID_DESCRIPTOR;
-    IpcChannelSide m_channelSide;
+    IpcChannelSide m_channelSide = IpcChannelSide::CLIENT;
 
 #ifdef __QNX__
     static constexpr int TIMEOUT_ERRNO = EINTR;