Fix issues spotted by clang-tidy

ComputationalBiomechanicsLab · Sep 14, 2024 · b0a90a0 · b0a90a0
1 parent 6559704
commit b0a90a0
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 28 deletions.
diff --git a/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp b/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp
@@ -477,7 +477,7 @@ namespace
         for (const OpenSim::Body& b : m.getComponentList<OpenSim::Body>())
         {
             const std::string name = b.getName();
-            const Transform xform = to<Transform>(b.getTransformInGround(st));
+            const auto xform = to<Transform>(b.getTransformInGround(st));
 
             auto& el = rv.emplace<Body>(UID{}, name, xform);
             el.setMass(b.getMass());
@@ -537,7 +537,7 @@ namespace
                 continue;
             }
 
-            const Transform xform = to<Transform>(parentFrame.getTransformInGround(st));
+            const auto xform = to<Transform>(parentFrame.getTransformInGround(st));
 
             auto& jointEl = rv.emplace<Joint>(UID{}, j.getConcreteClassName(), j.getName(), parent, child, xform);
             jointLookup.emplace(&j, jointEl.getID());
@@ -598,7 +598,7 @@ namespace
             }
 
             auto& el = rv.emplace<Mesh>(UID{}, attachment, meshData, realLocation);
-            Transform newTransform = to<Transform>(frame.getTransformInGround(st));
+            auto newTransform = to<Transform>(frame.getTransformInGround(st));
             newTransform.scale = to<Vec3>(mesh.get_scale_factors());
 
             el.setXform(newTransform);

diff --git a/src/OpenSimCreator/UI/PreviewExperimentalData/PreviewExperimentalDataTab.cpp b/src/OpenSimCreator/UI/PreviewExperimentalData/PreviewExperimentalDataTab.cpp
@@ -148,7 +148,7 @@ namespace
             (std::constructible_from<CStringView, ColumnHeaderStrings> && ...)
         static DataSeriesPattern forDatatype(ColumnHeaderStrings&&... headerSuffixes)
         {
-            return DataSeriesPattern{DataType, {CStringView{std::forward<ColumnHeaderStrings>(headerSuffixes)}...}};
+            return DataSeriesPattern{DataType, std::initializer_list<CStringView>{CStringView{std::forward<ColumnHeaderStrings>(headerSuffixes)}...}};  // NOLINT(cppcoreguidelines-pro-bounds-array-to-pointer-decay,hicpp-no-array-decay)
         }
 
         // Returns the `DataPointType` matched by this pattern.
@@ -183,7 +183,7 @@ namespace
     private:
         DataSeriesPattern(DataPointType type, std::initializer_list<CStringView> header_suffxes) :
             m_Type{type},
-            m_HeaderSuffixes{std::move(header_suffxes)}
+            m_HeaderSuffixes{header_suffxes}
         {}
 
         DataPointType m_Type;
@@ -392,10 +392,10 @@ namespace
         OpenSim_DECLARE_PROPERTY(column_offset, int, "index of the first column that contains this data series")
 
         explicit DataSeries(
-            const std::shared_ptr<const OpenSim::Storage>& storage,
+            std::shared_ptr<const OpenSim::Storage> storage,
             const DataSeriesAnnotation& annotation) :
 
-            m_Storage{storage},
+            m_Storage{std::move(storage)},
             m_Annotation{annotation}
         {
             setName(annotation.label);
@@ -440,7 +440,7 @@ namespace
             setName(m_Storage->getName());
 
             for (const auto& annotation : m_Schema.annotations()) {
-                addComponent(new DataSeries{m_Storage, annotation});
+                addComponent(std::make_unique<DataSeries>(m_Storage, annotation).release());
             }
         }
         std::shared_ptr<OpenSim::Storage> m_Storage;
@@ -469,7 +469,7 @@ namespace
             m_Model->commit("loaded model");
         }
 
-        void loadMotionFiles(std::vector<std::filesystem::path> paths)
+        void loadMotionFiles(std::span<const std::filesystem::path> paths)
         {
             if (paths.empty()) {
                 return;
@@ -478,8 +478,9 @@ namespace
             OpenSim::Model& model = m_Model->updModel();
             AnnotatedMotion* lastMotion = nullptr;
             for (const std::filesystem::path& path : paths) {
-                lastMotion = new AnnotatedMotion{path};
-                model.addModelComponent(lastMotion);
+                auto ptr = std::make_unique<AnnotatedMotion>(path);
+                lastMotion = ptr.get();
+                model.addModelComponent(ptr.release());
             }
             m_Model->setSelected(lastMotion);
             InitializeModel(model);
@@ -488,7 +489,7 @@ namespace
             m_Model->commit("loaded motions");
         }
 
-        void loadExternalLoads(std::vector<std::filesystem::path> paths)
+        void loadExternalLoads(std::span<const std::filesystem::path> paths)
         {
             if (paths.empty()) {
                 return;
@@ -497,12 +498,13 @@ namespace
             OpenSim::Model& model = m_Model->updModel();
             OpenSim::ExternalLoads* lastLoads = nullptr;
             for (const std::filesystem::path& path : paths) {
-                lastLoads = new OpenSim::ExternalLoads{path.string(), true};
+                auto ptr = std::make_unique<OpenSim::ExternalLoads>(path.string(), true);
                 for (int i = 0; i < lastLoads->getSize(); ++i) {
                     // don't actually apply the forces, we're only visualizing them
                     //(*lastLoads)[i].set_appliesForce(false);
                 }
-                model.addModelComponent(lastLoads);
+                lastLoads = ptr.get();
+                model.addModelComponent(ptr.release());
             }
             m_Model->setSelected(lastLoads);
             InitializeModel(model);
@@ -530,7 +532,7 @@ namespace
         ReadonlyPropertiesEditorPanel(
             std::string_view panelName,
             IPopupAPI* api,
-            std::shared_ptr<const UndoableModelStatePair> targetModel) :
+            const std::shared_ptr<const UndoableModelStatePair>& targetModel) :
 
             StandardPanelImpl{panelName},
             m_PropertiesEditor{api, targetModel, [model = targetModel](){ return model->getSelected(); }}

diff --git a/src/OpenSimCreator/UI/Shared/BasicWidgets.cpp b/src/OpenSimCreator/UI/Shared/BasicWidgets.cpp
@@ -185,7 +185,7 @@ namespace
         const OpenSim::Frame& frame,
         const SimTK::State& state)
     {
-        Transform rv = to<Transform>(mesh.getFrame().findTransformBetween(state, frame));
+        auto rv = to<Transform>(mesh.getFrame().findTransformBetween(state, frame));
         rv.scale = to<Vec3>(mesh.get_scale_factors());
         return rv;
     }

diff --git a/src/OpenSimCreator/Utils/OpenSimHelpers.cpp b/src/OpenSimCreator/Utils/OpenSimHelpers.cpp
@@ -1292,7 +1292,7 @@ std::vector<GeometryPathPoint> osc::GetAllPathPoints(const OpenSim::GeometryPath
         }
         else if (const auto* pwp = dynamic_cast<const OpenSim::PathWrapPoint*>(ap)) {
             // special case: it's a wrapping point, so add each part of the wrap
-            const Transform body2ground = to<Transform>(pwp->getParentFrame().getTransformInGround(st));
+            const auto body2ground = to<Transform>(pwp->getParentFrame().getTransformInGround(st));
             const OpenSim::Array<SimTK::Vec3>& wrapPath = pwp->getWrapPath(st);
 
             rv.reserve(rv.size() + size(wrapPath));
@@ -1387,8 +1387,8 @@ namespace
         //
         // - if there's a plane, then the plane's location+normal are needed in order
         //   to figure out where the force is exherted
-        const Transform body2ground = to<Transform>(halfSpace.getFrame().getTransformInGround(state));
-        const Transform geom2body = to<Transform>(halfSpace.getTransform());
+        const auto body2ground = to<Transform>(halfSpace.getFrame().getTransformInGround(state));
+        const auto geom2body = to<Transform>(halfSpace.getTransform());
 
         const Vec3 originInGround = body2ground * to<Vec3>(halfSpace.get_location());
         const Vec3 normalInGround = normalize(body2ground.rotation * geom2body.rotation) * c_ContactHalfSpaceUpwardsNormal;

diff --git a/src/oscar/Graphics/ColorHSLA.cpp b/src/oscar/Graphics/ColorHSLA.cpp
@@ -73,9 +73,9 @@ osc::ColorHSLA::ColorHSLA(const Color& color)
     const float delta = max - min;
 
     this->hue = calc_normalized_hsla_hue(r, g, b, min, max, delta);
-    this->lightness = 0.5f*(min + max);
+    this->lightness = 0.5f*(min + max);  // NOLINT(cppcoreguidelines-prefer-member-initializer)
     this->saturation = calc_hsla_saturation(lightness, min, max);
-    this->alpha = a;
+    this->alpha = a;  // NOLINT(cppcoreguidelines-prefer-member-initializer)
 }
 
 osc::ColorHSLA::operator Color() const

diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp
@@ -21,7 +21,6 @@
 #include <oscar/Maths/Mat4.h>
 #include <oscar/Maths/MathHelpers.h>
 #include <oscar/Maths/Quat.h>
-#include <oscar/Maths/Transform.h>
 #include <oscar/Maths/VecFunctions.h>
 #include <oscar/Maths/Vec3.h>
 #include <oscar/Platform/App.h>
@@ -34,7 +33,7 @@
 #include <oscar/Utils/ScopeGuard.h>
 #include <oscar/Utils/UID.h>
 
-#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>
+#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>  // NOLINT(bugprone-macro-parentheses)
 #include <imgui.h>
 #include <imgui/imgui_internal.h>
 #include <imgui/misc/cpp/imgui_stdlib.h>
@@ -1213,7 +1212,7 @@ void osc::ui::table_setup_scroll_freeze(int cols, int rows)
 bool osc::ui::table_column_sort_specs_are_dirty()
 {
     const ImGuiTableSortSpecs* specs = ImGui::TableGetSortSpecs();
-    return specs and specs->SpecsDirty;
+    return (specs != nullptr) and specs->SpecsDirty;
 }
 
 std::vector<ui::TableColumnSortSpec> osc::ui::get_table_column_sort_specs()

diff --git a/src/oscar/UI/oscimgui.h b/src/oscar/UI/oscimgui.h
@@ -580,7 +580,7 @@ namespace osc::ui
     void table_setup_column(CStringView label, ColumnFlags = {}, float init_width_or_weight = 0.0f, ID = ID{});
     void end_table();
 
-    void push_style_color(ColorVar index, const Color&);
+    void push_style_color(ColorVar, const Color&);
     void pop_style_color(int count = 1);
 
     Color get_color(ColorVar);

diff --git a/src/oscar/UI/ui_context.cpp b/src/oscar/UI/ui_context.cpp
@@ -13,7 +13,7 @@
 #include <oscar/Utils/Perf.h>
 #include <SDL_events.h>
 
-#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>
+#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>  // NOLINT(bugprone-macro-parentheses)
 #include <imgui.h>
 #include <ImGuizmo.h>
 #include <implot.h>

diff --git a/src/oscar/UI/ui_graphics_backend.cpp b/src/oscar/UI/ui_graphics_backend.cpp
@@ -34,7 +34,7 @@
 #include <oscar/Utils/UID.h>
 
 #include <ankerl/unordered_dense.h>
-#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>
+#define IMGUI_USER_CONFIG <oscar/UI/oscimgui_config.h>  // NOLINT(bugprone-macro-parentheses)
 #include <imgui/imgui.h>
 
 #include <array>

diff --git a/tests/TestOpenSimCreator/Utils/TestOpenSimHelpers.cpp b/tests/TestOpenSimCreator/Utils/TestOpenSimHelpers.cpp
@@ -317,7 +317,7 @@ TEST(OpenSimHelpers, WriteComponentTopologyGraphAsDotViz)
 TEST(OpenSimHelpers, WriteModelMultibodySystemGraphAsDotViz)
 {
     OpenSim::Model model;
-    model.addBody(new OpenSim::Body("somebody", 1.0, SimTK::Vec3(0.0), SimTK::Inertia{SimTK::Vec3(1.0), SimTK::Vec3(1.0)}));
+    model.addBody(std::make_unique<OpenSim::Body>("somebody", 1.0, SimTK::Vec3(0.0), SimTK::Inertia{SimTK::Vec3(1.0)}).release());
     model.buildSystem();
 
     std::stringstream ss;