Apply static analysis suggestions

.clang-tidy

@@ -7,13 +7,15 @@ Checks: >
     -bugprone-too-small-loop-variable,
     -clang-analyzer-*,
     cppcoreguidelines-*,
-    - cppcoreguidelines-init-variables,
-    - cppcoreguidelines-narrowing-conversions,
-    - cppcoreguidelines-non-private-member-variables-in-classes,
-    - cppcoreguidelines-owning-memory,
-    - cppcoreguidelines-pro-bounds-pointer-arithmetic,
-    - cppcoreguidelines-pro-type-member-init,
-    - cppcoreguidelines-special-member-functions,
+    -cppcoreguidelines-avoid-const-or-ref-data-members,
+    -cppcoreguidelines-avoid-do-while,
+    -cppcoreguidelines-init-variables,
+    -cppcoreguidelines-narrowing-conversions,
+    -cppcoreguidelines-non-private-member-variables-in-classes,
+    -cppcoreguidelines-owning-memory,
+    -cppcoreguidelines-pro-bounds-pointer-arithmetic,
+    -cppcoreguidelines-pro-type-member-init,
+    -cppcoreguidelines-special-member-functions,
     google-*,
     -google-default-arguments,
     -google-explicit-constructor,
@@ -23,21 +25,24 @@ Checks: >
     llvm-*,
     -llvm-header-guard,
     misc-*,
+    -misc-include-cleaner,
     -misc-non-private-member-variables-in-classes,
     modernize-*,
     -modernize-return-braced-init-list,
     -modernize-use-trailing-return-type,
     -modernize-use-nodiscard,
     performance-*,
-    - performance-move-const-arg,
+    -performance-avoid-endl,
+    -performance-move-const-arg,
     readability-*,
     -readability-function-size,
     -readability-identifier-length,
     -readability-function-cognitive-complexity,
     -readability-named-parameter,
     -readability-convert-member-functions-to-static,
     -readability-implicit-bool-conversion,
-    -readability-avoid-const-params-in-decls
+    -readability-avoid-const-params-in-decls,
+    -readability-uppercase-literal-suffix,
 
 WarningsAsErrors: '*'
 HeaderFilterRegex: '.*'

CHANGELOG.md

@@ -48,6 +48,7 @@
 #### Internals
 
 - Iterator type required by `TWRoute::replace` function (#1103)
+- Address some of the SonaQube and clang-tidy reports (#1200)
 
 #### CI
 

libvroom_examples/libvroom.cpp

@@ -64,7 +64,7 @@ void log_solution(const vroom::Solution& sol, bool geometry) {
       std::cout << type;
 
       // Add job/pickup/delivery/break ids.
-      if (step.step_type != vroom::STEP_TYPE::START and
+      if (step.step_type != vroom::STEP_TYPE::START &&
           step.step_type != vroom::STEP_TYPE::END) {
         std::cout << " " << step.id;
       }

src/algorithms/heuristics/heuristics.cpp

@@ -44,7 +44,7 @@ inline void seed_route(const Input& input,
       continue;
     }
 
-    bool is_pickup = (current_job.type == JOB_TYPE::PICKUP);
+    const bool is_pickup = (current_job.type == JOB_TYPE::PICKUP);
 
     if (route.size() + (is_pickup ? 2 : 1) > vehicle.max_tasks) {
       continue;
@@ -57,9 +57,9 @@ inline void seed_route(const Input& input,
                       higher_amount < current_job.delivery);
     }
     if (init == INIT::EARLIEST_DEADLINE) {
-      Duration current_deadline = is_pickup
-                                    ? input.jobs[job_rank + 1].tws.back().end
-                                    : current_job.tws.back().end;
+      const Duration current_deadline =
+        is_pickup ? input.jobs[job_rank + 1].tws.back().end
+                  : current_job.tws.back().end;
       try_validity = (current_deadline < earliest_deadline);
     }
     if (init == INIT::FURTHEST) {
@@ -177,8 +177,9 @@ inline Eval fill_route(const Input& input,
           const auto current_eval =
             utils::addition_cost(input, job_rank, vehicle, route.route, r);
 
-          double current_cost = static_cast<double>(current_eval.cost) -
-                                lambda * static_cast<double>(regrets[job_rank]);
+          const double current_cost =
+            static_cast<double>(current_eval.cost) -
+            lambda * static_cast<double>(regrets[job_rank]);
 
           if (current_cost < best_cost &&
               (vehicle.ok_for_range_bounds(route_eval + current_eval)) &&
@@ -266,15 +267,15 @@ inline Eval fill_route(const Input& input,
               current_eval = p_add + d_adds[delivery_r];
             }
 
-            double current_cost =
+            const double current_cost =
               current_eval.cost -
               lambda * static_cast<double>(regrets[job_rank]);
 
             if (current_cost < best_cost) {
               modified_with_pd.push_back(job_rank + 1);
 
               // Update best cost depending on validity.
-              bool valid =
+              const bool valid =
                 (vehicle.ok_for_range_bounds(route_eval + current_eval)) &&
                 route
                   .is_valid_addition_for_capacity_inclusion(input,

src/algorithms/kruskal.cpp

@@ -36,11 +36,11 @@ UndirectedGraph<T> minimum_spanning_tree(const UndirectedGraph<T>& graph) {
   std::iota(representative.begin(), representative.end(), 0);
 
   for (const auto& edge : edges) {
-    Index first_vertex = edge.get_first_vertex();
-    Index second_vertex = edge.get_second_vertex();
+    const Index first_vertex = edge.get_first_vertex();
+    const Index second_vertex = edge.get_second_vertex();
 
-    Index first_rep = representative[first_vertex];
-    Index second_rep = representative[second_vertex];
+    const Index first_rep = representative[first_vertex];
+    const Index second_rep = representative[second_vertex];
     if (first_rep != second_rep) {
       // Adding current edge won't create a cycle as vertices are in
       // separate connected components.

src/algorithms/local_search/insertion_search.h

@@ -43,7 +43,7 @@ compute_best_insertion_single(const Input& input,
     for (Index rank = sol_state.insertion_ranks_begin[v][j];
          rank < sol_state.insertion_ranks_end[v][j];
          ++rank) {
-      Eval current_eval =
+      const Eval current_eval =
         utils::addition_cost(input, j, v_target, route.route, rank);
       if (current_eval.cost < result.eval.cost &&
           v_target.ok_for_range_bounds(sol_state.route_evals[v] +
@@ -120,7 +120,7 @@ RouteInsertion compute_best_insertion_pd(const Input& input,
       valid_delivery_insertions[d_rank] =
         route.is_valid_addition_for_tw_without_max_load(input, j + 1, d_rank);
     }
-    found_valid |= valid_delivery_insertions[d_rank];
+    found_valid |= static_cast<bool>(valid_delivery_insertions[d_rank]);
   }
 
   if (!found_valid) {
@@ -131,7 +131,7 @@ RouteInsertion compute_best_insertion_pd(const Input& input,
   for (Index pickup_r = sol_state.insertion_ranks_begin[v][j];
        pickup_r < sol_state.insertion_ranks_end[v][j];
        ++pickup_r) {
-    Eval p_add =
+    const Eval p_add =
       utils::addition_cost(input, j, v_target, route.route, pickup_r);
     if (result.eval < p_add) {
       // Even without delivery insertion more expensive than current best.

src/algorithms/local_search/local_search.cpp

@@ -423,7 +423,7 @@ void LocalSearch<Route,
           continue;
         }
 
-        Priority u_priority = _input.jobs[u].priority;
+        const Priority u_priority = _input.jobs[u].priority;
         const auto& u_pickup = _input.jobs[u].pickup;
         const auto& u_delivery = _input.jobs[u].delivery;
 
@@ -504,7 +504,7 @@ void LocalSearch<Route,
                                      t_rank,
                                      u);
 
-                bool better_if_valid =
+                const bool better_if_valid =
                   (best_priorities[source] < priority_gain) ||
                   (best_priorities[source] == priority_gain &&
                    best_gains[source][source] < r.gain());
@@ -552,11 +552,11 @@ void LocalSearch<Route,
 
         const auto& job_s_type = _input.jobs[s_job_rank].type;
 
-        bool both_s_single =
+        const bool both_s_single =
           (job_s_type == JOB_TYPE::SINGLE) &&
           (_input.jobs[s_next_job_rank].type == JOB_TYPE::SINGLE);
 
-        bool is_s_pickup =
+        const bool is_s_pickup =
           (job_s_type == JOB_TYPE::PICKUP) &&
           (_sol_state.matching_delivery_rank[source][s_rank] == s_rank + 1);
 
@@ -596,11 +596,11 @@ void LocalSearch<Route,
 
           const auto& job_t_type = _input.jobs[t_job_rank].type;
 
-          bool both_t_single =
+          const bool both_t_single =
             (job_t_type == JOB_TYPE::SINGLE) &&
             (_input.jobs[t_next_job_rank].type == JOB_TYPE::SINGLE);
 
-          bool is_t_pickup =
+          const bool is_t_pickup =
             (job_t_type == JOB_TYPE::PICKUP) &&
             (_sol_state.matching_delivery_rank[target][t_rank] == t_rank + 1);
 
@@ -718,11 +718,11 @@ void LocalSearch<Route,
             const auto t_next_job_rank = _sol[target].route[t_rank + 1];
             const auto& job_t_type = _input.jobs[t_job_rank].type;
 
-            bool both_t_single =
+            const bool both_t_single =
               (job_t_type == JOB_TYPE::SINGLE) &&
               (_input.jobs[t_next_job_rank].type == JOB_TYPE::SINGLE);
 
-            bool is_t_pickup =
+            const bool is_t_pickup =
               (job_t_type == JOB_TYPE::PICKUP) &&
               (_sol_state.matching_delivery_rank[target][t_rank] == t_rank + 1);
 
@@ -1223,11 +1223,11 @@ void LocalSearch<Route,
         const auto job_s_type = _input.jobs[_sol[source].route[s_rank]].type;
         const auto s_next_job_rank = _sol[source].route[s_rank + 1];
 
-        bool both_s_single =
+        const bool both_s_single =
           (job_s_type == JOB_TYPE::SINGLE) &&
           (_input.jobs[s_next_job_rank].type == JOB_TYPE::SINGLE);
 
-        bool is_s_pickup =
+        const bool is_s_pickup =
           (job_s_type == JOB_TYPE::PICKUP) &&
           (_sol_state.matching_delivery_rank[source][s_rank] == s_rank + 1);
 
@@ -1244,12 +1244,12 @@ void LocalSearch<Route,
         for (unsigned t_rank = s_rank + 3; t_rank < end_t_rank; ++t_rank) {
           const auto& job_t_type = _input.jobs[_sol[target].route[t_rank]].type;
 
-          bool both_t_single =
+          const bool both_t_single =
             (job_t_type == JOB_TYPE::SINGLE) &&
             (_input.jobs[_sol[target].route[t_rank + 1]].type ==
              JOB_TYPE::SINGLE);
 
-          bool is_t_pickup =
+          const bool is_t_pickup =
             (job_t_type == JOB_TYPE::PICKUP) &&
             (_sol_state.matching_delivery_rank[target][t_rank] == t_rank + 1);
 
@@ -1316,12 +1316,12 @@ void LocalSearch<Route,
 
           const auto& job_t_type = _input.jobs[_sol[target].route[t_rank]].type;
 
-          bool both_t_single =
+          const bool both_t_single =
             (job_t_type == JOB_TYPE::SINGLE) &&
             (_input.jobs[_sol[source].route[t_rank + 1]].type ==
              JOB_TYPE::SINGLE);
 
-          bool is_t_pickup =
+          const bool is_t_pickup =
             (job_t_type == JOB_TYPE::PICKUP) &&
             (_sol_state.matching_delivery_rank[target][t_rank] == t_rank + 1);
 
@@ -1375,7 +1375,7 @@ void LocalSearch<Route,
           _sol_state.weak_insertion_ranks_begin[source][s_job_rank];
         if (_input.jobs[s_job_rank].type == JOB_TYPE::DELIVERY) {
           // Don't move a delivery before its matching pickup.
-          Index begin_candidate =
+          const Index begin_candidate =
             _sol_state.matching_pickup_rank[source][s_rank] + 1;
           begin_t_rank = std::max(begin_t_rank, begin_candidate);
         }
@@ -1425,11 +1425,12 @@ void LocalSearch<Route,
       for (unsigned s_rank = 0; s_rank < _sol[source].size() - 1; ++s_rank) {
         const auto& job_type = _input.jobs[_sol[source].route[s_rank]].type;
 
-        bool both_single = (job_type == JOB_TYPE::SINGLE) &&
-                           (_input.jobs[_sol[source].route[s_rank + 1]].type ==
-                            JOB_TYPE::SINGLE);
+        const bool both_single =
+          (job_type == JOB_TYPE::SINGLE) &&
+          (_input.jobs[_sol[source].route[s_rank + 1]].type ==
+           JOB_TYPE::SINGLE);
 
-        bool is_pickup =
+        const bool is_pickup =
           (job_type == JOB_TYPE::PICKUP) &&
           (_sol_state.matching_delivery_rank[source][s_rank] == s_rank + 1);
 
@@ -1548,7 +1549,7 @@ void LocalSearch<Route,
           }
 
           // Matching delivery rank in source route.
-          unsigned s_d_rank =
+          const Index s_d_rank =
             _sol_state.matching_delivery_rank[source][s_p_rank];
 
           if (!_input.vehicle_ok_with_job(target,

src/algorithms/local_search/route_split_utils.h

@@ -88,28 +88,26 @@ compute_best_route_split_choice(const Input& input,
         continue;
       }
 
-      if (current_end_eval < second_best_end_eval) {
-        // Worth checking end route full validity.
-
-        if (empty_routes[v_rank].is_valid_addition_for_tw(input,
-                                                          end_delivery,
-                                                          source.route.begin() +
-                                                            r,
-                                                          source.route.end(),
-                                                          0,
-                                                          0)) {
-          if (current_end_eval < first_best_end_eval) {
-            // We have a new first choice.
-            second_v_end = first_v_end;
-            second_best_end_eval = first_best_end_eval;
-
-            first_v_end = v_rank;
-            first_best_end_eval = current_end_eval;
-          } else {
-            // We have a new second choice.
-            second_v_end = v_rank;
-            second_best_end_eval = current_end_eval;
-          }
+      if (current_end_eval < second_best_end_eval &&
+          // Worth checking end route full validity.
+          empty_routes[v_rank].is_valid_addition_for_tw(input,
+                                                        end_delivery,
+                                                        source.route.begin() +
+                                                          r,
+                                                        source.route.end(),
+                                                        0,
+                                                        0)) {
+        if (current_end_eval < first_best_end_eval) {
+          // We have a new first choice.
+          second_v_end = first_v_end;
+          second_best_end_eval = first_best_end_eval;
+
+          first_v_end = v_rank;
+          first_best_end_eval = current_end_eval;
+        } else {
+          // We have a new second choice.
+          second_v_end = v_rank;
+          second_best_end_eval = current_end_eval;
         }
       }
     }
@@ -160,28 +158,26 @@ compute_best_route_split_choice(const Input& input,
         continue;
       }
 
-      if (current_begin_eval < second_best_begin_eval) {
-        // Worth checking begin route full validity.
-
-        if (empty_routes[v_rank].is_valid_addition_for_tw(input,
-                                                          begin_delivery,
-                                                          source.route.begin(),
-                                                          source.route.begin() +
-                                                            r,
-                                                          0,
-                                                          0)) {
-          if (current_begin_eval < first_best_begin_eval) {
-            // We have a new first choice.
-            second_v_begin = first_v_begin;
-            second_best_begin_eval = first_best_begin_eval;
-
-            first_v_begin = v_rank;
-            first_best_begin_eval = current_begin_eval;
-          } else {
-            // We have a new second choice.
-            second_v_begin = v_rank;
-            second_best_begin_eval = current_begin_eval;
-          }
+      if (current_begin_eval < second_best_begin_eval &&
+          // Worth checking begin route full validity.
+          empty_routes[v_rank].is_valid_addition_for_tw(input,
+                                                        begin_delivery,
+                                                        source.route.begin(),
+                                                        source.route.begin() +
+                                                          r,
+                                                        0,
+                                                        0)) {
+        if (current_begin_eval < first_best_begin_eval) {
+          // We have a new first choice.
+          second_v_begin = first_v_begin;
+          second_best_begin_eval = first_best_begin_eval;
+
+          first_v_begin = v_rank;
+          first_best_begin_eval = current_begin_eval;
+        } else {
+          // We have a new second choice.
+          second_v_begin = v_rank;
+          second_best_begin_eval = current_begin_eval;
         }
       }
     }