chore: Fix a number of SonarQube reliability issues

Core/include/Acts/AmbiguityResolution/ScoreBasedAmbiguityResolution.ipp

@@ -413,7 +413,7 @@ std::vector<double> Acts::ScoreBasedAmbiguityResolution::ambiguityScore(
       score = score * fac;
       ACTS_DEBUG("Modifier for chi2 = " << chi2 << " and NDF = " << indf
                                         << " is : " << fac
-                                        << "  New score now: " << score)
+                                        << "  New score now: " << score);
     }
     iTrack++;
 

Core/include/Acts/EventData/SourceLink.hpp

@@ -44,7 +44,7 @@ class SourceLink final {
                   "Cannot wrap SourceLink in SourceLink");
 
     if constexpr (std::is_same_v<T, std::decay_t<T>>) {
-      m_upstream = any_type{std::move(upstream)};
+      m_upstream = any_type{std::forward<T>(upstream)};
     } else {
       m_upstream = any_type{static_cast<std::decay_t<T>>(upstream)};
     }

Core/include/Acts/Propagator/DirectNavigator.hpp

@@ -295,7 +295,7 @@ class DirectNavigator {
         // Set the current surface
         state.navigation.currentSurface = *state.navigation.navSurfaceIter;
         ACTS_VERBOSE("Current surface set to  "
-                     << state.navigation.currentSurface->geometryId())
+                     << state.navigation.currentSurface->geometryId());
         // Move the sequence to the next surface
         ++state.navigation.navSurfaceIter;
         if (state.navigation.navSurfaceIter !=

Core/include/Acts/Seeding/EstimateTrackParamsFromSeed.hpp

@@ -55,7 +55,7 @@ std::optional<BoundVector> estimateTrackParamsFromSeed(
   // Check the number of provided space points
   std::size_t numSP = std::distance(spBegin, spEnd);
   if (numSP < 3) {
-    ACTS_ERROR("At least three space points are required.")
+    ACTS_ERROR("At least three space points are required.");
     return std::nullopt;
   }
 
@@ -67,7 +67,7 @@ std::optional<BoundVector> estimateTrackParamsFromSeed(
 
   for (spacepoint_iterator_t it = spBegin; it != spEnd; it++) {
     if (*it == nullptr) {
-      ACTS_ERROR("Empty space point found. This should not happen.")
+      ACTS_ERROR("Empty space point found. This should not happen.");
       return std::nullopt;
     }
     const auto& sp = *it;
@@ -158,7 +158,7 @@ std::optional<BoundVector> estimateTrackParamsFromSeed(
   // Check the number of provided space points
   std::size_t numSP = std::distance(spBegin, spEnd);
   if (numSP != 3) {
-    ACTS_ERROR("There should be exactly three space points provided.")
+    ACTS_ERROR("There should be exactly three space points provided.");
     return std::nullopt;
   }
 
@@ -171,7 +171,7 @@ std::optional<BoundVector> estimateTrackParamsFromSeed(
     // case?
     ACTS_WARNING("The magnetic field at the bottom space point: B = "
                  << bFieldInTesla << " T is smaller than |B|_min = "
-                 << bFieldMinInTesla << " T. Estimation is not performed.")
+                 << bFieldMinInTesla << " T. Estimation is not performed.");
     return std::nullopt;
   }
 
@@ -185,7 +185,7 @@ std::optional<BoundVector> estimateTrackParamsFromSeed(
   for (std::size_t isp = 0; isp < 3; ++isp) {
     spacepoint_iterator_t it = std::next(spBegin, isp);
     if (*it == nullptr) {
-      ACTS_ERROR("Empty space point found. This should not happen.")
+      ACTS_ERROR("Empty space point found. This should not happen.");
       return std::nullopt;
     }
     const auto& sp = *it;

Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp

@@ -768,14 +768,14 @@ class CombinatorialKalmanFilter {
 
         if (!tsRes.ok()) {
           ACTS_ERROR(
-              "Processing of selected track states failed: " << tsRes.error())
+              "Processing of selected track states failed: " << tsRes.error());
           return tsRes.error();
         }
         Result<std::tuple<unsigned int, bool>> procRes = processNewTrackStates(
             state.geoContext, prevTipState, *tsRes, result);
         if (!procRes.ok()) {
-          ACTS_ERROR(
-              "Processing of selected track states failed: " << procRes.error())
+          ACTS_ERROR("Processing of selected track states failed: "
+                     << procRes.error());
           return procRes.error();
         }
         auto [nNewBranchesOnSurface, isOutlier] = *procRes;

Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp

@@ -606,12 +606,14 @@ class Gx2Fitter {
             // MaterialUpdateStage::FullUpdate);
           }
         } else {
-          ACTS_INFO("Surface " << geoId << " has no measurement/material/hole.")
+          ACTS_INFO("Surface " << geoId
+                               << " has no measurement/material/hole.");
         }
       }
       ACTS_VERBOSE("result.processedMeasurements: "
                    << result.processedMeasurements << "\n"
-                   << "inputMeasurements.size(): " << inputMeasurements->size())
+                   << "inputMeasurements.size(): "
+                   << inputMeasurements->size());
       if (result.processedMeasurements >= inputMeasurements->size()) {
         ACTS_INFO("Actor: finish: all measurements found.");
         result.finished = true;
@@ -831,17 +833,17 @@ class Gx2Fitter {
                              trackState, *m_addToSumLogger);
           } else {
             ACTS_ERROR("Can not process state with measurement with "
-                       << measDim << " dimensions.")
+                       << measDim << " dimensions.");
             throw std::domain_error(
                 "Found measurement with less than 1 or more than 6 "
                 "dimension(s).");
           }
         } else if (typeFlags.test(TrackStateFlag::HoleFlag)) {
           // Handle hole
           // TODO: write hole handling
-          ACTS_VERBOSE("Placeholder: Handle hole.")
+          ACTS_VERBOSE("Placeholder: Handle hole.");
         } else {
-          ACTS_WARNING("Unknown state encountered")
+          ACTS_WARNING("Unknown state encountered");
         }
         // TODO: Material handling. Should be there for hole and measurement
       }
@@ -1005,8 +1007,9 @@ class Gx2Fitter {
 
     if (trackContainer.hasColumn(
             Acts::hashString(Gx2fConstants::gx2fnUpdateColumn))) {
-      ACTS_DEBUG("Add nUpdate to track")
-      track.template component<std::uint32_t>("Gx2fnUpdateColumn") = nUpdate;
+      ACTS_DEBUG("Add nUpdate to track");
+      track.template component<std::uint32_t>("Gx2fnUpdateColumn") =
+          static_cast<std::uint32_t>(nUpdate);
     }
 
     // TODO write test for calculateTrackQuantities

Core/include/Acts/TrackFitting/detail/GsfActor.hpp

@@ -152,7 +152,7 @@ struct GsfActor {
 
     // Return is we found an error earlier
     if (!result.result.ok()) {
-      ACTS_WARNING("result.result not ok, return!")
+      ACTS_WARNING("result.result not ok, return!");
       return;
     }
 

Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp

@@ -105,7 +105,7 @@ auto kalmanHandleMeasurement(
     } else {
       ACTS_VERBOSE(
           "Filtering step successful. But measurement is determined "
-          "to be an outlier. Stepping state is not updated.")
+          "to be an outlier. Stepping state is not updated.");
       // Set the outlier type flag
       typeFlags.set(TrackStateFlag::OutlierFlag);
       trackStateProxy.shareFrom(trackStateProxy, TrackStatePropMask::Predicted,

Core/include/Acts/Utilities/Axis.hpp

@@ -160,7 +160,8 @@ class Axis<AxisType::Equidistant, bdt> final : public IAxis {
   /// @return Set of neighboring bin indices (global)
   NeighborHoodIndices neighborHoodIndices(std::size_t idx,
                                           std::size_t size = 1) const {
-    return neighborHoodIndices(idx, std::make_pair(-size, size));
+    return neighborHoodIndices(idx,
+                               std::make_pair(-static_cast<int>(size), size));
   }
 
   /// @brief Get #size bins which neighbor the one given
@@ -418,7 +419,8 @@ class Axis<AxisType::Variable, bdt> final : public IAxis {
   /// Create a binning structure with @c nBins variable-sized bins from the
   /// given bin boundaries. @c nBins is given by the number of bin edges
   /// reduced by one.
-  Axis(std::vector<ActsScalar> binEdges) : m_binEdges(std::move(binEdges)) {}
+  explicit Axis(std::vector<ActsScalar> binEdges)
+      : m_binEdges(std::move(binEdges)) {}
 
   /// @param [in] typeTag boundary type tag
   /// @param [in] binEdges vector of bin edges
@@ -461,7 +463,8 @@ class Axis<AxisType::Variable, bdt> final : public IAxis {
   /// @return Set of neighboring bin indices (global)
   NeighborHoodIndices neighborHoodIndices(std::size_t idx,
                                           std::size_t size = 1) const {
-    return neighborHoodIndices(idx, std::make_pair(-size, size));
+    return neighborHoodIndices(idx,
+                               std::make_pair(-static_cast<int>(size), size));
   }
 
   /// @brief Get #size bins which neighbor the one given

Core/include/Acts/Utilities/BinUtility.hpp

@@ -18,6 +18,7 @@
 #include <iostream>
 #include <iterator>
 #include <memory>
+#include <stdexcept>
 #include <string>
 #include <vector>
 
@@ -122,7 +123,7 @@ class BinUtility {
     m_transform = m_transform * gbu.transform();
     m_itransform = m_transform.inverse();
     if (m_binningData.size() + bData.size() > 3) {
-      throw "BinUtility does not support dim > 3";
+      throw std::runtime_error{"BinUtility does not support dim > 3"};
     }
     m_binningData.insert(m_binningData.end(), bData.begin(), bData.end());
     return (*this);
@@ -268,7 +269,7 @@ class BinUtility {
   /// @return the binning value of the accessor entry
   BinningValue binningValue(std::size_t ba = 0) const {
     if (ba >= m_binningData.size()) {
-      throw "dimension out of bounds";
+      throw std::runtime_error{"Dimension out of bounds"};
     }
     return (m_binningData[ba].binvalue);
   }

Core/include/Acts/Utilities/BinningData.hpp

@@ -244,9 +244,6 @@ class BinningData {
         binvalue == binH) {
       return lposition[0];
     }
-    if (binvalue == binPhi) {
-      return lposition[1];
-    }
     return lposition[1];
   }
 

Core/include/Acts/Utilities/Delegate.hpp

@@ -74,33 +74,33 @@ class Delegate<R(Args...), H, O> {
  public:
   Delegate() = default;
 
-  Delegate(Delegate &&) = default;
-  Delegate &operator=(Delegate &&) = default;
-  Delegate(const Delegate &) = default;
-  Delegate &operator=(const Delegate &) = default;
+  Delegate(Delegate &&) noexcept = default;
+  Delegate &operator=(Delegate &&) noexcept = default;
+  Delegate(const Delegate &) noexcept = default;
+  Delegate &operator=(const Delegate &) noexcept = default;
 
   /// Constructor with an explicit runtime callable
   /// @param callable The runtime value of the callable
   /// @note The function signature requires the first argument of the callable is `const void*`.
   ///       i.e. if the signature of the delegate is `void(int)`, the
   ///       callable's signature has to be `void(const void*, int)`.
-  Delegate(function_type callable) { connect(callable); }
+  explicit Delegate(function_type callable) { connect(callable); }
 
   /// Constructor with a possibly stateful function object.
   /// @tparam Callable Type of the callable
   /// @param callable The callable (function object or lambda)
   /// @note @c Delegate does not assume owner ship over @p callable. You need to ensure
   ///       it's lifetime is longer than that of @c Delegate.
   template <typename Callable, typename = isNoFunPtr<Callable>>
-  Delegate(Callable &callable) {
+  explicit Delegate(Callable &callable) {
     connect(callable);
   }
 
   /// Constructor with a compile-time free function pointer
   /// @tparam Callable The compile-time free function pointer
   /// @note @c DelegateFuncTag is used to communicate the callable type
   template <auto Callable>
-  Delegate(DelegateFuncTag<Callable> /*tag*/) {
+  explicit Delegate(DelegateFuncTag<Callable> /*tag*/) {
     connect<Callable>();
   }
 
@@ -223,7 +223,7 @@ class Delegate<R(Args...), H, O> {
   template <auto Callable, typename Type, DelegateType T = kOwnership,
             typename = std::enable_if_t<T == DelegateType::Owning>>
   void connect(std::unique_ptr<const Type> instance) {
-    using member_ptr_type = return_type (Type::*)(Args...) const;
+    using member_ptr_type = return_type (Type::*)(Args && ...) const;
     static_assert(Concepts::is_detected<isSignatureCompatible, member_ptr_type,
                                         decltype(Callable)>::value,
                   "Callable given does not correspond exactly to required call "
@@ -235,7 +235,7 @@ class Delegate<R(Args...), H, O> {
           delete concretePayload;
         });
 
-    m_function = [](const holder_type *payload, Args... args) -> return_type {
+    m_function = [](const holder_type *payload, Args &&...args) -> return_type {
       assert(payload != nullptr && "Payload is required, but not set");
       const auto *concretePayload = static_cast<const Type *>(payload);
       return std::invoke(Callable, concretePayload,
@@ -246,10 +246,10 @@ class Delegate<R(Args...), H, O> {
   /// The call operator that exposes the functionality of the @c Delegate type.
   /// @param args The arguments to call the contained function with
   /// @return Return value of the contained function
-  return_type operator()(Args... args) const {
+  template <typename... Ts>
+  return_type operator()(Ts &&...args) const {
     assert(connected() && "Delegate is not connected");
-    return std::invoke(m_function, m_payload.ptr(),
-                       std::forward<Args>(args)...);
+    return std::invoke(m_function, m_payload.ptr(), std::forward<Ts>(args)...);
   }
 
   /// Return whether this delegate is currently connected
@@ -258,7 +258,7 @@ class Delegate<R(Args...), H, O> {
 
   /// Return whether this delegate is currently connected
   /// @return True if this delegate is connected
-  operator bool() const { return connected(); }
+  explicit operator bool() const { return connected(); }
 
   /// Disconnect this delegate, meaning it cannot be called anymore
   void disconnect() {
@@ -274,7 +274,9 @@ class Delegate<R(Args...), H, O> {
 
  private:
   // Deleter that does not do anything
-  static void noopDeleter(const holder_type * /*unused*/) {}
+  static void noopDeleter(const holder_type * /*unused*/) {
+    // we do not own the payload
+  }
 
   /// @cond
 

Core/include/Acts/Utilities/GridBinFinder.hpp

@@ -37,8 +37,17 @@ class GridBinFinder {
   /// @pre The provided paramers must be of type 'int', 'std::pair<int, int>' or 'std::vector<std::pair<int, int>>'
   /// no other type is allowed. The order of these parameters must correspond to
   /// the same ordering of the axes in the grid
-  template <typename... args>
-  GridBinFinder(args&&... vals);
+  template <typename... args,
+            typename = std::enable_if_t<
+                sizeof...(args) == DIM &&
+                std::conjunction_v<std::disjunction<
+                    std::is_same<int, std::decay_t<args>>,
+                    std::is_same<std::pair<int, int>, std::decay_t<args>>,
+                    std::is_same<std::vector<std::pair<int, int>>,
+                                 std::decay_t<args>>>...>>>
+  explicit GridBinFinder(args&&... vals) {
+    storeValue(std::forward<args>(vals)...);
+  }
 
   /// @brief Retrieve the neighbouring bins given a local position in the grid
   ///

Core/include/Acts/Utilities/GridBinFinder.ipp

@@ -6,19 +6,6 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-template <std::size_t DIM>
-template <typename... args>
-Acts::GridBinFinder<DIM>::GridBinFinder(args&&... vals) {
-  static_assert(sizeof...(args) == DIM);
-  static_assert(
-      std::conjunction<std::disjunction<
-          std::is_same<int, typename std::decay<args>::type>,
-          std::is_same<std::pair<int, int>, typename std::decay<args>::type>,
-          std::is_same<std::vector<std::pair<int, int>>,
-                       typename std::decay<args>::type>>...>::value);
-  storeValue(std::forward<args>(vals)...);
-}
-
 template <std::size_t DIM>
 template <typename first_value_t, typename... vals>
 void Acts::GridBinFinder<DIM>::storeValue(first_value_t&& fv,

Core/include/Acts/Utilities/Logger.hpp

@@ -66,11 +66,14 @@
 
 // Debug level agnostic implementation of the ACTS_XYZ logging macros
 #define ACTS_LOG(level, x)                                                     \
-  if (logger().doPrint(level)) {                                               \
-    std::ostringstream os;                                                     \
-    os << x;                                                                   \
-    logger().log(level, os.str());                                             \
-  }
+  do {                                                                         \
+    if (logger().doPrint(level)) {                                             \
+      std::ostringstream os;                                                   \
+      os << x;                                                                 \
+      logger().log(level, os.str());                                           \
+    }                                                                          \
+  }                                                                            \
+  while(0)
 
 /// @brief macro for verbose debug output
 /// @ingroup Logging
@@ -239,13 +242,16 @@ class ThresholdFailure : public std::runtime_error {
 /// lifetime.
 class ScopedFailureThreshold {
  public:
-  ScopedFailureThreshold(Level level) : m_previousLevel(getFailureThreshold()) {
-    setFailureThreshold(level);
-  }
-  ~ScopedFailureThreshold() { setFailureThreshold(m_previousLevel); }
+  explicit ScopedFailureThreshold(Level level) { setFailureThreshold(level); }
+  ScopedFailureThreshold(const ScopedFailureThreshold&) = delete;
+  ScopedFailureThreshold& operator=(const ScopedFailureThreshold&) = delete;
+  ScopedFailureThreshold(ScopedFailureThreshold&&) = delete;
+  ScopedFailureThreshold& operator=(ScopedFailureThreshold&&) = delete;
+
+  ~ScopedFailureThreshold() noexcept;
 
  private:
-  Level m_previousLevel;
+  Level m_previousLevel{getFailureThreshold()};
 };
 
 /// @brief abstract base class for printing debug output