Merge pull request PointCloudLibrary#5584 from gnawme/norm.evangelisat/add-readability-checks

Added initial set of readability-* clang-tidy checks

Author: mvieth

.clang-tidy

@@ -1,5 +1,38 @@
 ---
-Checks: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast,google-readability-casting'
-WarningsAsErrors: '-*,modernize-use-auto,modernize-deprecated-headers,modernize-redundant-void-arg,modernize-replace-random-shuffle,modernize-use-equals-default,modernize-use-equals-delete,modernize-use-nullptr,modernize-use-override,modernize-use-using,performance-faster-string-find,performance-for-range-copy,performance-implicit-conversion-in-loop,performance-inefficient-algorithm,performance-inefficient-vector-operation,performance-move-const-arg,performance-move-constructor-init,performance-no-automatic-move,performance-noexcept-move-constructor,performance-type-promotion-in-math-fn,cppcoreguidelines-pro-type-cstyle-cast,cppcoreguidelines-pro-type-static-cast-downcast,google-readability-casting'
+Checks: >
+    -*,
+    cppcoreguidelines-pro-type-cstyle-cast,
+    cppcoreguidelines-pro-type-static-cast-downcast,
+    google-readability-casting,
+    modernize-deprecated-headers,
+    modernize-redundant-void-arg,
+    modernize-replace-random-shuffle,
+    modernize-use-auto,
+    modernize-use-equals-default,
+    modernize-use-equals-delete,
+    modernize-use-nullptr,
+    modernize-use-override,
+    modernize-use-using,
+    performance-faster-string-find,
+    performance-for-range-copy,
+    performance-implicit-conversion-in-loop,
+    performance-inefficient-algorithm,
+    performance-inefficient-vector-operation,
+    performance-move-const-arg,
+    performance-move-constructor-init,
+    performance-no-automatic-move,
+    performance-noexcept-move-constructor,
+    performance-type-promotion-in-math-fn,
+    readability-container-size-empty,
+    readability-delete-null-pointer,
+    readability-duplicate-include,
+    readability-redundant-declaration,
+    readability-redundant-smartptr-get,
+    readability-redundant-string-cstr,
+    readability-redundant-string-init,
+    readability-simplify-boolean-expr,
+    readability-simplify-subscript-expr,
+WarningsAsErrors: '*'
 CheckOptions:
 - {key: modernize-use-auto.MinTypeNameLength, value: 7}
+UseColor: true

common/include/pcl/PolygonMesh.h

@@ -32,7 +32,7 @@ namespace pcl
       const auto point_offset = mesh1.cloud.width * mesh1.cloud.height;
 
       bool success = pcl::PCLPointCloud2::concatenate(mesh1.cloud, mesh2.cloud);
-      if (success == false) {
+      if (!success) {
         return false;
       }
       // Make the resultant polygon mesh take the newest stamp

common/src/io.cpp

@@ -201,10 +201,7 @@ pcl::concatenateFields (const pcl::PCLPointCloud2 &cloud1,
     point_offset += field_offset;
   }
 
-  if (!cloud1.is_dense || !cloud2.is_dense)
-    cloud_out.is_dense = false;
-  else
-    cloud_out.is_dense = true;
+  cloud_out.is_dense = cloud1.is_dense && cloud2.is_dense;
 
   return (true);
 }

features/include/pcl/features/impl/brisk_2d.hpp

@@ -564,6 +564,7 @@ BRISK2DEstimation<PointInT, PointOutT, KeypointT, IntensityT>::compute (
     int* pvalues = values;
     const float& x = (kp.x);
     const float& y = (kp.y);
+    // NOLINTNEXTLINE(readability-simplify-boolean-expr)
     if (true) // kp.angle==-1
     {
       if (!rotation_invariance_enabled_)

features/include/pcl/features/impl/range_image_border_extractor.hpp

@@ -334,10 +334,7 @@ bool RangeImageBorderExtractor::calculateMainPrincipalCurvature(int x, int y, in
         bool& beam_valid = beams_valid[beam_idx++];
         if (step==1)
         {
-          if (x2==x && y2==y)
-            beam_valid = false;
-          else
-            beam_valid = true;
+          beam_valid = !(x2==x && y2==y);
         }
         else
           if (!beam_valid)

filters/include/pcl/filters/impl/conditional_removal.hpp

@@ -624,7 +624,7 @@ pcl::ConditionalRemoval<PointT>::setCondition (ConditionBasePtr condition)
 template <typename PointT> void
 pcl::ConditionalRemoval<PointT>::applyFilter (PointCloud &output)
 {
-  if (capable_ == false)
+  if (!capable_)
   {
     PCL_WARN ("[pcl::%s::applyFilter] not capable!\n", getClassName ().c_str ());
     return;

filters/include/pcl/filters/impl/radius_outlier_removal.hpp

@@ -109,10 +109,7 @@ pcl::RadiusOutlierRemoval<PointT>::applyFilterIndices (Indices &indices)
       }
       else
       {
-        if (negative_)
-          chk_neighbors = true;
-        else
-          chk_neighbors = false;
+        chk_neighbors = negative_;
       }
 
       // Points having too few neighbors are outliers and are passed to removed indices

filters/include/pcl/filters/passthrough.h

@@ -97,7 +97,7 @@ namespace pcl
         */
       PassThrough (bool extract_removed_indices = false) :
         FilterIndices<PointT> (extract_removed_indices),
-        filter_field_name_ (""),
+        
         filter_limit_min_ (std::numeric_limits<float>::lowest()),
         filter_limit_max_ (std::numeric_limits<float>::max())
       {
@@ -212,8 +212,8 @@ namespace pcl
     public:
       /** \brief Constructor. */
       PassThrough (bool extract_removed_indices = false) :
-        FilterIndices<pcl::PCLPointCloud2>::FilterIndices (extract_removed_indices), filter_field_name_("")
-      , filter_limit_min_(std::numeric_limits<float>::lowest())
+        FilterIndices<pcl::PCLPointCloud2>::FilterIndices (extract_removed_indices), 
+       filter_limit_min_(std::numeric_limits<float>::lowest())
       , filter_limit_max_(std::numeric_limits<float>::max())
       {
         filter_name_ = "PassThrough";

filters/include/pcl/filters/voxel_grid.h

@@ -202,7 +202,7 @@ namespace pcl
         max_b_ (Eigen::Vector4i::Zero ()),
         div_b_ (Eigen::Vector4i::Zero ()),
         divb_mul_ (Eigen::Vector4i::Zero ()),
-        filter_field_name_ (""),
+        
         filter_limit_min_ (std::numeric_limits<float>::lowest()),
         filter_limit_max_ (std::numeric_limits<float>::max()),
         filter_limit_negative_ (false),
@@ -528,7 +528,7 @@ namespace pcl
         max_b_ (Eigen::Vector4i::Zero ()),
         div_b_ (Eigen::Vector4i::Zero ()),
         divb_mul_ (Eigen::Vector4i::Zero ()),
-        filter_field_name_ (""),
+        
         filter_limit_min_ (std::numeric_limits<float>::lowest()),
         filter_limit_max_ (std::numeric_limits<float>::max()),
         filter_limit_negative_ (false),

io/src/openni2/openni2_device.cpp

@@ -163,7 +163,7 @@ pcl::io::openni2::OpenNI2Device::getStringID () const
 bool
 pcl::io::openni2::OpenNI2Device::isValid () const
 {
-  return (openni_device_.get () != nullptr) && openni_device_->isValid ();
+  return (openni_device_ != nullptr) && openni_device_->isValid ();
 }
 
 float
@@ -332,7 +332,7 @@ pcl::io::openni2::OpenNI2Device::stopAllStreams ()
 void
 pcl::io::openni2::OpenNI2Device::stopIRStream ()
 {
-  if (ir_video_stream_.get () != nullptr)
+  if (ir_video_stream_ != nullptr)
   {
     ir_video_stream_->stop ();
     ir_video_started_ = false;
@@ -341,7 +341,7 @@ pcl::io::openni2::OpenNI2Device::stopIRStream ()
 void
 pcl::io::openni2::OpenNI2Device::stopColorStream ()
 {
-  if (color_video_stream_.get () != nullptr)
+  if (color_video_stream_ != nullptr)
   {
     color_video_stream_->stop ();
     color_video_started_ = false;
@@ -350,7 +350,7 @@ pcl::io::openni2::OpenNI2Device::stopColorStream ()
 void
 pcl::io::openni2::OpenNI2Device::stopDepthStream ()
 {
-  if (depth_video_stream_.get () != nullptr)
+  if (depth_video_stream_ != nullptr)
   {
     depth_video_stream_->stop ();
     depth_video_started_ = false;
@@ -360,13 +360,13 @@ pcl::io::openni2::OpenNI2Device::stopDepthStream ()
 void
 pcl::io::openni2::OpenNI2Device::shutdown ()
 {
-  if (ir_video_stream_.get () != nullptr)
+  if (ir_video_stream_ != nullptr)
     ir_video_stream_->destroy ();
 
-  if (color_video_stream_.get () != nullptr)
+  if (color_video_stream_ != nullptr)
     color_video_stream_->destroy ();
 
-  if (depth_video_stream_.get () != nullptr)
+  if (depth_video_stream_ != nullptr)
     depth_video_stream_->destroy ();
 
 }
@@ -747,7 +747,7 @@ bool OpenNI2Device::setPlaybackSpeed (double speed)
 std::shared_ptr<openni::VideoStream>
 pcl::io::openni2::OpenNI2Device::getIRVideoStream () const
 {
-  if (ir_video_stream_.get () == nullptr)
+  if (ir_video_stream_ == nullptr)
   {
     if (hasIRSensor ())
     {
@@ -764,7 +764,7 @@ pcl::io::openni2::OpenNI2Device::getIRVideoStream () const
 std::shared_ptr<openni::VideoStream>
 pcl::io::openni2::OpenNI2Device::getColorVideoStream () const
 {
-  if (color_video_stream_.get () == nullptr)
+  if (color_video_stream_ == nullptr)
   {
     if (hasColorSensor ())
     {
@@ -781,7 +781,7 @@ pcl::io::openni2::OpenNI2Device::getColorVideoStream () const
 std::shared_ptr<openni::VideoStream>
 pcl::io::openni2::OpenNI2Device::getDepthVideoStream () const
 {
-  if (depth_video_stream_.get () == nullptr)
+  if (depth_video_stream_ == nullptr)
   {
     if (hasDepthSensor ())
     {