Add clang-tidy checks and fix recommendations (#13)

* Add clang-tidy, fix most issues * Fix clang tidy for generated parser files * clang-tidy GitHub Action * clang-tidy GitHub Action * clang-tidy GitHub Action
robotraconteur · Mar 17, 2024 · 11c1a08 · 11c1a08
1 parent a762249
commit 11c1a08
Show file tree

Hide file tree

Showing 64 changed files with 753 additions and 297 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -0,0 +1,103 @@
+---
+Checks: >
+    -*,
+    clang-diagnostic-*,
+    clang-analyzer-*,
+    bugprone-*,
+    cppcoreguidelines-*,
+    misc-*,
+    performance-*,
+    readability-*,
+    -clang-diagnostic-unknown-warning-option,
+    -clang-analyzer-cplusplus*,
+    -cppcoreguidelines-macro-usage,
+    -cppcoreguidelines-pro-type-static-cast-downcast,
+    -cppcoreguidelines-pro-type-vararg,
+    -cppcoreguidelines-pro-type-union-access,
+    -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+    -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+    -cppcoreguidelines-pro-bounds-constant-array-index,
+    -cppcoreguidelines-avoid-magic-numbers,
+    -cppcoreguidelines-non-private-member-variables-in-classes,
+    -cppcoreguidelines-explicit-virtual-functions,
+    -cppcoreguidelines-special-member-functions,
+    -misc-non-private-member-variables-in-classes,
+    -misc-no-recursion,
+    -readability-braces-around-statements,
+    -readability-named-parameter,
+    -readability-magic-numbers,
+    -readability-isolate-declaration,
+    -readability-function-cognitive-complexity,
+    -readability-identifier-length,
+    -readability-else-after-return,
+    -readability-const-return-type,
+    -readability-implicit-bool-conversion,
+    -readability-container-data-pointer,
+    -readability-convert-member-functions-to-static,
+    -cppcoreguidelines-special-member-functions,
+    -bugprone-easily-swappable-parameters,
+    -hicpp-*,
+    -cppcoreguidelines-prefer-member-initializer,
+    -cppcoreguidelines-pro-type-reinterpret-cast,
+    -cppcoreguidelines-avoid-non-const-global-variables,
+    -bugprone-reserved-identifier,
+    modernize-use-override
+WarningsAsErrors: >
+    -*,
+    clang-diagnostic-*,
+    clang-analyzer-*,
+    bugprone-*,
+    cppcoreguidelines-*,
+    misc-*,
+    performance-*,
+    readability-*,
+    -clang-diagnostic-unknown-warning-option,
+    -clang-analyzer-cplusplus*,
+    -cppcoreguidelines-macro-usage,
+    -cppcoreguidelines-pro-type-static-cast-downcast,
+    -cppcoreguidelines-pro-type-vararg,
+    -cppcoreguidelines-pro-type-union-access,
+    -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
+    -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+    -cppcoreguidelines-pro-bounds-constant-array-index,
+    -cppcoreguidelines-avoid-magic-numbers,
+    -cppcoreguidelines-non-private-member-variables-in-classes,
+    -cppcoreguidelines-explicit-virtual-functions,
+    -cppcoreguidelines-special-member-functions,
+    -misc-non-private-member-variables-in-classes,
+    -misc-no-recursion,
+    -readability-braces-around-statements,
+    -readability-named-parameter,
+    -readability-magic-numbers,
+    -readability-isolate-declaration,
+    -readability-function-cognitive-complexity,
+    -readability-identifier-length,
+    -readability-else-after-return,
+    -readability-const-return-type,
+    -readability-implicit-bool-conversion,
+    -readability-container-data-pointer,
+    -readability-convert-member-functions-to-static,
+    -cppcoreguidelines-special-member-functions,
+    -bugprone-easily-swappable-parameters,
+    -hicpp-*,
+    -cppcoreguidelines-prefer-member-initializer,
+    -cppcoreguidelines-pro-type-reinterpret-cast,
+    -cppcoreguidelines-avoid-non-const-global-variables,
+    -bugprone-reserved-identifier,
+    modernize-use-override
+HeaderFilterRegex: '(robotraconteurcompanion|RobotRaconteurCompanion).*'
+AnalyzeTemporaryDtors: false
+FormatStyle:     none
+CheckOptions:
+  - key:             modernize-use-override.AllowOverrideAndFinal
+    value:           '1'
+  - key:             cppcoreguidelines-explicit-virtual-functions.AllowOverrideAndFinal
+    value:           '1'
+  - key:             cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+    value:           '1'
+  - key: performance-unnecessary-value-param.AllowedTypes
+    value: MessageStringRef;RR_WEAK_PTR*;boost::weak_ptr*;boost::function*
+  - key: bugprone-signed-char-misuse.CharTypdefsToIgnore
+    value: int8_t
+  - key: modernize-use-override.OverrideSpelling
+    value: RR_OVERRIDE
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -27,7 +27,7 @@ jobs:
     - name: apt
       run: >
         sudo apt-get install 
-        zlib1g zlib1g-dev libssl-dev libusb-1.0-0
+        zlib1g zlib1g-dev libssl-dev libusb-1.0-0 ninja-build
         libusb-1.0-0-dev libdbus-1-3 libdbus-1-dev libbluetooth3 libbluetooth-dev zlib1g zlib1g-dev 
         git cmake g++ make libboost-all-dev curl libgtest-dev doxygen libyaml-cpp-dev libeigen3-dev
         ninja-build -qq    
@@ -98,4 +98,47 @@ jobs:
       run: |
         set PATH=%PATH%;%GITHUB_WORKSPACE%\build
         ctest . -C Release --output-on-failure
+  clang-tidy:
+    runs-on: ubuntu-20.04
+    steps:
+    - uses: actions/checkout@v3
+      with:
+        path: src
+        submodules: recursive
+    - uses: actions/checkout@v3
+      with:
+        path: rr-src
+        repository: robotraconteur/robotraconteur
+    - name: apt update
+      run: sudo apt update
+    - name: apt
+      run: >
+        sudo apt-get install
+        zlib1g zlib1g-dev libssl-dev libusb-1.0-0 ninja-build
+        libusb-1.0-0-dev libdbus-1-3 libdbus-1-dev libbluetooth3 libbluetooth-dev
+        git cmake g++ make libboost-all-dev autoconf libyaml-cpp-dev libeigen3-dev
+        automake libtool bison libpcre3-dev curl libgtest-dev -qq
+    - name: clang-tidy-14
+      run: |
+        wget https://apt.llvm.org/llvm.sh
+        chmod +x llvm.sh
+        sudo ./llvm.sh 14 all
+    - name: cmake rr
+      run: >
+        cmake -G Ninja -S rr-src -B rr-build 
+        -DCMAKE_BUILD_TYPE=Debug
+        -DBUILD_GEN=ON -DBUILD_TESTING=OFF
+        -DCMAKE_CXX_STANDARD=11
+    - name: cmake rr build
+      run: |
+        cmake --build rr-build --config Debug
+        sudo cmake --install rr-build --config Debug
+    - name: configure
+      run: >
+        cmake -G Ninja -DBUILD_TESTING=ON
+        -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON -DCMAKE_CXX_CLANG_TIDY=clang-tidy-14
+        -DCMAKE_BUILD_TYPE=Debug
+        -S src -B build
+    - name: build
+      run: cmake --build build --config Debug -- -j 4
 
diff --git a/include/RobotRaconteurCompanion/Converters/EigenConverters.h b/include/RobotRaconteurCompanion/Converters/EigenConverters.h
@@ -168,9 +168,9 @@ namespace Eigen
      * @param vs The Eigen vector with 2 elements
      * @return com::robotraconteur::geometry::Vector2 The result vector
      */
-    static com::robotraconteur::geometry::Vector2 ToVector2(const ::Eigen::Ref<const ::Eigen::Vector2d> vs)
+    static com::robotraconteur::geometry::Vector2 ToVector2(const ::Eigen::Ref<const ::Eigen::Vector2d>& vs)
     {
-        com::robotraconteur::geometry::Vector2 o;
+        com::robotraconteur::geometry::Vector2 o{};
         o.s.x = vs[0];
         o.s.y = vs[1];
         return o;
@@ -196,9 +196,9 @@ namespace Eigen
      * @param vs The Eigen vector with 3 elements
      * @return com::robotraconteur::geometry::Vector3 The result vector
      */
-    static com::robotraconteur::geometry::Vector3 ToVector3(const ::Eigen::Ref<const ::Eigen::Vector3d> vs)
+    static com::robotraconteur::geometry::Vector3 ToVector3(const ::Eigen::Ref<const ::Eigen::Vector3d>& vs)
     {
-        com::robotraconteur::geometry::Vector3 o;
+        com::robotraconteur::geometry::Vector3 o{};
         o.s.x = vs[0];
         o.s.y = vs[1];
         o.s.z = vs[2];
@@ -226,9 +226,9 @@ namespace Eigen
      * @param vs The Eigen vector with 6 elements
      * @return com::robotraconteur::geometry::Vector6 The result vector
      */
-    static com::robotraconteur::geometry::Vector6 ToVector6(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> > vs)
+    static com::robotraconteur::geometry::Vector6 ToVector6(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> >& vs)
     {
-        com::robotraconteur::geometry::Vector6 o;
+        com::robotraconteur::geometry::Vector6 o{};
         o.s.alpha = vs[0];
         o.s.beta = vs[1];
         o.s.gamma = vs[2];
@@ -262,9 +262,9 @@ namespace Eigen
      * @param vs The Eigen vector with 2 elements
      * @return com::robotraconteur::geometry::Point2D The result point
      */
-    static com::robotraconteur::geometry::Point2D ToPoint2D(const ::Eigen::Ref<const ::Eigen::Vector2d> vs)
+    static com::robotraconteur::geometry::Point2D ToPoint2D(const ::Eigen::Ref<const ::Eigen::Vector2d>& vs)
     {
-        com::robotraconteur::geometry::Point2D o;
+        com::robotraconteur::geometry::Point2D o{};
         o.s.x = vs[0];
         o.s.y = vs[1];
         return o;
@@ -290,9 +290,9 @@ namespace Eigen
      * @param vs The Eigen vector with 3 elements
      * @return com::robotraconteur::geometry::Point The result point
      */
-    static com::robotraconteur::geometry::Point ToPoint(const ::Eigen::Ref<const ::Eigen::Vector3d> vs)
+    static com::robotraconteur::geometry::Point ToPoint(const ::Eigen::Ref<const ::Eigen::Vector3d>& vs)
     {
-        com::robotraconteur::geometry::Point o;
+        com::robotraconteur::geometry::Point o{};
         o.s.x = vs[0];
         o.s.y = vs[1];
         o.s.z = vs[2];
@@ -320,9 +320,9 @@ namespace Eigen
      * @param vs The Eigen vector with 2 elements
      * @return com::robotraconteur::geometry::Size2D The result size
      */
-    static com::robotraconteur::geometry::Size2D ToSize2D(const ::Eigen::Ref<const ::Eigen::Vector2d> vs)
+    static com::robotraconteur::geometry::Size2D ToSize2D(const ::Eigen::Ref<const ::Eigen::Vector2d>& vs)
     {
-        com::robotraconteur::geometry::Size2D o;
+        com::robotraconteur::geometry::Size2D o{};
         o.s.width = vs[0];
         o.s.height = vs[1];
         return o;
@@ -348,9 +348,9 @@ namespace Eigen
      * @param vs The Eigen vector with 3 elements
      * @return com::robotraconteur::geometry::Size The result size
      */
-    static com::robotraconteur::geometry::Size ToSize(const ::Eigen::Ref<const ::Eigen::Vector3d> vs)
+    static com::robotraconteur::geometry::Size ToSize(const ::Eigen::Ref<const ::Eigen::Vector3d>& vs)
     {
-        com::robotraconteur::geometry::Size o;
+        com::robotraconteur::geometry::Size o{};
         o.s.width = vs[0];
         o.s.height = vs[1];
         o.s.depth = vs[2];
@@ -396,7 +396,7 @@ namespace Eigen
      */
     static com::robotraconteur::geometry::Quaternion ToQuaternion(const ::Eigen::Quaterniond& q)
     {
-        com::robotraconteur::geometry::Quaternion o;
+        com::robotraconteur::geometry::Quaternion o{};
         o.s.w = q.w();
         o.s.x = q.x();
         o.s.y = q.y();
@@ -421,9 +421,9 @@ namespace Eigen
      * @param iso The Eigen Isometry
      * @return com::robotraconteur::geometry::Transform The result com::robotraconteur::geometry::Transform
      */
-    static com::robotraconteur::geometry::Transform ToTransform(::Eigen::Isometry3d iso)
+    static com::robotraconteur::geometry::Transform ToTransform(const ::Eigen::Isometry3d& iso)
     {
-        com::robotraconteur::geometry::Transform o;
+        com::robotraconteur::geometry::Transform o{};
         ::Eigen::Quaterniond q = (::Eigen::Quaterniond)iso.linear();
         o.s.rotation = ToQuaternion(q);
         ::Eigen::Vector3d p = (::Eigen::Vector3d)iso.translation();
@@ -448,9 +448,9 @@ namespace Eigen
      * @param iso The Eigen Isometry
      * @return com::robotraconteur::geometry::Pose The result com::robotraconteur::geometry::Pose
      */
-    static com::robotraconteur::geometry::Pose ToPose(::Eigen::Isometry3d iso)
+    static com::robotraconteur::geometry::Pose ToPose(const ::Eigen::Isometry3d& iso)
     {
-        com::robotraconteur::geometry::Pose o;
+        com::robotraconteur::geometry::Pose o{};
         ::Eigen::Quaterniond q = (::Eigen::Quaterniond)iso.linear();
         o.s.orientation = ToQuaternion(q);
         ::Eigen::Vector3d p = (::Eigen::Vector3d)iso.translation();
@@ -464,9 +464,9 @@ namespace Eigen
      * @param vs The Eigen vector with 6 elements
      * @return com::robotraconteur::geometry::SpatialVelocity The result com::robotraconteur::geometry::SpatialVelocity
      */
-    static com::robotraconteur::geometry::SpatialVelocity ToSpatialVelocity(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> > vs)
+    static com::robotraconteur::geometry::SpatialVelocity ToSpatialVelocity(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> >& vs)
     {
-        com::robotraconteur::geometry::SpatialVelocity o;
+        com::robotraconteur::geometry::SpatialVelocity o{};
         o.s.angular.s.x = vs[0];
         o.s.angular.s.y = vs[1];
         o.s.angular.s.z = vs[2];
@@ -500,9 +500,9 @@ namespace Eigen
      * @param vs The Eigen vector with 6 elements
      * @return com::robotraconteur::geometry::SpatialAcceleration The result com::robotraconteur::geometry::SpatialAcceleration
      */
-    static com::robotraconteur::geometry::SpatialAcceleration ToSpatialAcceleration(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> > vs)
+    static com::robotraconteur::geometry::SpatialAcceleration ToSpatialAcceleration(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> >& vs)
     {
-        com::robotraconteur::geometry::SpatialAcceleration o;
+        com::robotraconteur::geometry::SpatialAcceleration o{};
         o.s.angular.s.x = vs[0];
         o.s.angular.s.y = vs[1];
         o.s.angular.s.z = vs[2];
@@ -536,9 +536,9 @@ namespace Eigen
      * @param vs The Eigen vector with 6 elements
      * @return com::robotraconteur::geometry::Wrench The result com::robotraconteur::geometry::Wrench
      */
-    static com::robotraconteur::geometry::Wrench ToWrench(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> > vs)
+    static com::robotraconteur::geometry::Wrench ToWrench(const ::Eigen::Ref<const ::Eigen::Matrix<double,6,1> >& vs)
     {
-        com::robotraconteur::geometry::Wrench o;
+        com::robotraconteur::geometry::Wrench o{};
         o.s.torque.s.x = vs[0];
         o.s.torque.s.y = vs[1];
         o.s.torque.s.z = vs[2];

diff --git a/include/RobotRaconteurCompanion/InfoParser/yaml/actuator_parser.h b/include/RobotRaconteurCompanion/InfoParser/yaml/actuator_parser.h
@@ -7,12 +7,13 @@ namespace YAML {
 	template<> 
 	struct convert<com::robotraconteur::actuator::ActuatorStatePtr>{
 		static Node encode(const com::robotraconteur::actuator::ActuatorStatePtr& rhs){
+			RR_UNUSED(rhs);
 			Node node;
 			return node;
 		}
 
 		static bool decode(const Node& node, com::robotraconteur::actuator::ActuatorStatePtr& rhs){
-			if (!rhs) rhs.reset(new com::robotraconteur::actuator::ActuatorState);
+			if (!rhs) rhs.reset(new com::robotraconteur::actuator::ActuatorState); // NOLINT(cppcoreguidelines-owning-memory)
 			rhs->ts = RobotRaconteur::Companion::InfoParser::yaml::parse_namedarray<com::robotraconteur::datetime::TimeSpec3>(node,"ts",true);
 			rhs->seqno = RobotRaconteur::Companion::InfoParser::yaml::parse_number<uint64_t>(node,"seqno",true);
 			rhs->actuator_state_flags = RobotRaconteur::Companion::InfoParser::yaml::parse_number<uint32_t>(node,"actuator_state_flags",true);
@@ -26,12 +27,13 @@ namespace YAML {
 	template<> 
 	struct convert<com::robotraconteur::actuator::ActuatorInfoPtr>{
 		static Node encode(const com::robotraconteur::actuator::ActuatorInfoPtr& rhs){
+			RR_UNUSED(rhs);
 			Node node;
 			return node;
 		}
 
 		static bool decode(const Node& node, com::robotraconteur::actuator::ActuatorInfoPtr& rhs){
-			if (!rhs) rhs.reset(new com::robotraconteur::actuator::ActuatorInfo);
+			if (!rhs) rhs.reset(new com::robotraconteur::actuator::ActuatorInfo); // NOLINT(cppcoreguidelines-owning-memory)
 			rhs->device_info = RobotRaconteur::Companion::InfoParser::yaml::parse_structure<com::robotraconteur::device::DeviceInfoPtr>(node,"device_info",true);
 			rhs->actuator_type = RobotRaconteur::Companion::InfoParser::yaml::parse_enum<com::robotraconteur::actuator::ActuatorTypeCode::ActuatorTypeCode>(node,"actuator_type",true);
 			rhs->command_units = RobotRaconteur::Companion::InfoParser::yaml::parse_structure_list<com::robotraconteur::units::SIUnitPtr>(node,"command_units",true);

diff --git a/include/RobotRaconteurCompanion/InfoParser/yaml/bignum_parser.h b/include/RobotRaconteurCompanion/InfoParser/yaml/bignum_parser.h
@@ -7,12 +7,13 @@ namespace YAML {
 	template<> 
 	struct convert<com::robotraconteur::bignum::BigNumPtr>{
 		static Node encode(const com::robotraconteur::bignum::BigNumPtr& rhs){
+			RR_UNUSED(rhs);
 			Node node;
 			return node;
 		}
 
 		static bool decode(const Node& node, com::robotraconteur::bignum::BigNumPtr& rhs){
-			if (!rhs) rhs.reset(new com::robotraconteur::bignum::BigNum);
+			if (!rhs) rhs.reset(new com::robotraconteur::bignum::BigNum); // NOLINT(cppcoreguidelines-owning-memory)
 			rhs->data = RobotRaconteur::Companion::InfoParser::yaml::parse_numeric_array<uint8_t>(node,"data",true,true,0);
 			return true;
 		}
@@ -23,12 +24,13 @@ namespace YAML {
 	template<> 
 	struct convert<com::robotraconteur::bignum::UnsignedBigNumPtr>{
 		static Node encode(const com::robotraconteur::bignum::UnsignedBigNumPtr& rhs){
+			RR_UNUSED(rhs);
 			Node node;
 			return node;
 		}
 
 		static bool decode(const Node& node, com::robotraconteur::bignum::UnsignedBigNumPtr& rhs){
-			if (!rhs) rhs.reset(new com::robotraconteur::bignum::UnsignedBigNum);
+			if (!rhs) rhs.reset(new com::robotraconteur::bignum::UnsignedBigNum); // NOLINT(cppcoreguidelines-owning-memory)
 			rhs->data = RobotRaconteur::Companion::InfoParser::yaml::parse_numeric_array<uint8_t>(node,"data",true,true,0);
 			return true;
 		}
@@ -39,12 +41,13 @@ namespace YAML {
 	template<> 
 	struct convert<com::robotraconteur::bignum::BigFloatPtr>{
 		static Node encode(const com::robotraconteur::bignum::BigFloatPtr& rhs){
+			RR_UNUSED(rhs);
 			Node node;
 			return node;
 		}
 
 		static bool decode(const Node& node, com::robotraconteur::bignum::BigFloatPtr& rhs){
-			if (!rhs) rhs.reset(new com::robotraconteur::bignum::BigFloat);
+			if (!rhs) rhs.reset(new com::robotraconteur::bignum::BigFloat); // NOLINT(cppcoreguidelines-owning-memory)
 			rhs->num = RobotRaconteur::Companion::InfoParser::yaml::parse_structure<com::robotraconteur::bignum::BigNumPtr>(node,"num",true);
 			rhs->den = RobotRaconteur::Companion::InfoParser::yaml::parse_structure<com::robotraconteur::bignum::BigNumPtr>(node,"den",true);
 			rhs->radix = RobotRaconteur::Companion::InfoParser::yaml::parse_structure<com::robotraconteur::bignum::BigNumPtr>(node,"radix",true);