Merge branch 'main' into houliston/sonar

Fastcode · Sep 22, 2023 · 6ce07e9 · 6ce07e9
2 parents 641ebc0 + 8e46684
commit 6ce07e9
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 42 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -9,6 +9,7 @@ Checks: >
   cppcoreguidelines-*,
   -cppcoreguidelines-avoid-magic-numbers,
   -cppcoreguidelines-non-private-member-variables-in-classes,
+  -cppcoreguidelines-owning-memory,
   -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
   -cppcoreguidelines-pro-bounds-constant-array-index,
   -cppcoreguidelines-pro-bounds-pointer-arithmetic,

diff --git a/.cmake-format.py b/.cmake-format.py
@@ -75,9 +75,6 @@
     # Format keywords consistently as 'lower' or 'upper' case
     keyword_case = "upper"
 
-    # A list of command names which should always be wrapped
-    always_wrap = ["NUCLEAR_ROLE", "nuclear_role"]
-
     # If true, the argument lists which are known to be sortable will be sorted
     # lexicographicall
     enable_sort = True
@@ -216,6 +213,6 @@
     # A dictionary containing any per-command configuration overrides. Currently
     # only `command_case` is supported.
     per_command = {
-        "ToolchainLibraryFinder": {"command_case": "unchanged"},
         "HeaderLibrary": {"command_case": "unchanged"},
+        "ToolchainLibraryFinder": {"command_case": "unchanged"},
     }
diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml
@@ -45,7 +45,7 @@ jobs:
           for f in build/tests/individual/*; do echo "Testing $f"; ./$f; done
 
       - name: Collect coverage into one XML report
-        run: gcovr --sonarqube > coverage.xml
+        run: gcovr --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml
 
       - name: Run sonar-scanner
         env:

diff --git a/cmake/Modules/FindCATCH.cmake b/cmake/Modules/FindCATCH.cmake
@@ -22,7 +22,8 @@ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 
 include(HeaderLibrary)
 
-header_library(
+HeaderLibrary(
   NAME CATCH
-  HEADER catch.hpp URL "https://raw.githubusercontent.com/catchorg/Catch2/v2.x/single_include/catch2/catch.hpp"
+  HEADER catch.hpp
+  URL "https://raw.githubusercontent.com/catchorg/Catch2/v2.x/single_include/catch2/catch.hpp"
 )
diff --git a/cmake/Modules/FindSphinx.cmake b/cmake/Modules/FindSphinx.cmake
@@ -21,4 +21,4 @@ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 ]]
 
 include(ToolchainLibraryFinder)
-toolchainlibraryfinder(NAME Sphinx BINARY sphinx-build)
+ToolchainLibraryFinder(NAME Sphinx BINARY sphinx-build)
diff --git a/cmake/Modules/HeaderLibrary.cmake b/cmake/Modules/HeaderLibrary.cmake
@@ -21,7 +21,7 @@ OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 ]]
 
 include(CMakeParseArguments)
-function(header_library)
+function(HeaderLibrary)
   # Extract the arguments from our function call
   set(options, "")
   set(oneValueArgs "NAME")
@@ -69,22 +69,25 @@ function(header_library)
 
   # Setup and export our variables
   set(required_vars ${required_vars} "${PACKAGE_NAME}_INCLUDE_DIR")
-  set(${PACKAGE_NAME}_INCLUDE_DIRS ${${PACKAGE_NAME}_INCLUDE_DIR} PARENT_SCOPE)
+  set(${PACKAGE_NAME}_INCLUDE_DIRS
+      ${${PACKAGE_NAME}_INCLUDE_DIR}
+      PARENT_SCOPE
+  )
   mark_as_advanced(${PACKAGE_NAME}_INCLUDE_DIR ${PACKAGE_NAME}_INCLUDE_DIRS)
 
   # Find the package
   include(FindPackageHandleStandardArgs)
   find_package_handle_standard_args(
     ${PACKAGE_NAME}
-    FOUND_VAR
-    ${PACKAGE_NAME}_FOUND
-    REQUIRED_VARS
-    ${required_vars}
-    VERSION_VAR
-    ${PACKAGE_NAME}_VERSION
+    FOUND_VAR ${PACKAGE_NAME}_FOUND
+    REQUIRED_VARS ${required_vars}
+    VERSION_VAR ${PACKAGE_NAME}_VERSION
   )
 
   # Export our found variable to parent scope
-  set(${PACKAGE_NAME}_FOUND ${PACKAGE_NAME}_FOUND PARENT_SCOPE)
+  set(${PACKAGE_NAME}_FOUND
+      ${PACKAGE_NAME}_FOUND
+      PARENT_SCOPE
+  )
 
-endfunction(header_library)
+endfunction(HeaderLibrary)
diff --git a/cmake/Modules/ToolchainLibraryFinder.cmake b/cmake/Modules/ToolchainLibraryFinder.cmake
@@ -26,15 +26,14 @@ function(ToolchainLibraryFinder)
   # Extract the arguments from our function call
   set(options, "")
   set(oneValueArgs "NAME")
-  set(
-    multiValueArgs
-    "HEADER"
-    "LIBRARY"
-    "PATH_SUFFIX"
-    "BINARY"
-    "VERSION_FILE"
-    "VERSION_BINARY_ARGUMENTS"
-    "VERSION_REGEX"
+  set(multiValueArgs
+      "HEADER"
+      "LIBRARY"
+      "PATH_SUFFIX"
+      "BINARY"
+      "VERSION_FILE"
+      "VERSION_BINARY_ARGUMENTS"
+      "VERSION_REGEX"
   )
   cmake_parse_arguments(PACKAGE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
 
@@ -54,7 +53,10 @@ function(ToolchainLibraryFinder)
 
     # Setup and export our variables
     set(required_vars ${required_vars} "${PACKAGE_NAME}_INCLUDE_DIR")
-    set(${PACKAGE_NAME}_INCLUDE_DIRS ${${PACKAGE_NAME}_INCLUDE_DIR} PARENT_SCOPE)
+    set(${PACKAGE_NAME}_INCLUDE_DIRS
+        ${${PACKAGE_NAME}_INCLUDE_DIR}
+        PARENT_SCOPE
+    )
     mark_as_advanced(${PACKAGE_NAME}_INCLUDE_DIR ${PACKAGE_NAME}_INCLUDE_DIRS)
 
   endif(PACKAGE_HEADER)
@@ -70,7 +72,10 @@ function(ToolchainLibraryFinder)
 
     # Setup and export our variables
     set(required_vars ${required_vars} "${PACKAGE_NAME}_LIBRARY")
-    set(${PACKAGE_NAME}_LIBRARIES ${${PACKAGE_NAME}_LIBRARY} PARENT_SCOPE)
+    set(${PACKAGE_NAME}_LIBRARIES
+        ${${PACKAGE_NAME}_LIBRARY}
+        PARENT_SCOPE
+    )
     mark_as_advanced(${PACKAGE_NAME}_LIBRARY ${PACKAGE_NAME}_LIBRARIES)
 
   endif(PACKAGE_LIBRARY)
@@ -86,7 +91,10 @@ function(ToolchainLibraryFinder)
 
     # Setup and export our variables
     set(required_vars ${required_vars} "${PACKAGE_NAME}_BINARY")
-    set(${PACKAGE_NAME}_BINARY ${${PACKAGE_NAME}_BINARY} PARENT_SCOPE)
+    set(${PACKAGE_NAME}_BINARY
+        ${${PACKAGE_NAME}_BINARY}
+        PARENT_SCOPE
+    )
     mark_as_advanced(${PACKAGE_NAME}_BINARY)
 
   endif(PACKAGE_BINARY)
@@ -103,7 +111,9 @@ function(ToolchainLibraryFinder)
     # Execute our binary to get a version string
     if(PACKAGE_VERSION_BINARY_ARGUMENTS AND PACKAGE_BINARY)
       exec_program(
-        ${${PACKAGE_NAME}_BINARY} ARGS ${PACKAGE_VERSION_BINARY_ARGUMENTS} OUTPUT_VARIABLE full_version_string
+        ${${PACKAGE_NAME}_BINARY} ARGS
+        ${PACKAGE_VERSION_BINARY_ARGUMENTS}
+        OUTPUT_VARIABLE full_version_string
       )
     endif(PACKAGE_VERSION_BINARY_ARGUMENTS AND PACKAGE_BINARY)
 
@@ -120,16 +130,15 @@ function(ToolchainLibraryFinder)
   include(FindPackageHandleStandardArgs)
   find_package_handle_standard_args(
     ${PACKAGE_NAME}
-    FOUND_VAR
-    ${PACKAGE_NAME}_FOUND
-    REQUIRED_VARS
-    ${required_vars}
-    VERSION_VAR
-    ${PACKAGE_NAME}_VERSION
-    # VERSION_VAR "${MAJOR}.${MINOR}.${PATCH}")
+    FOUND_VAR ${PACKAGE_NAME}_FOUND
+    REQUIRED_VARS ${required_vars}
+    VERSION_VAR ${PACKAGE_NAME}_VERSION # VERSION_VAR "${MAJOR}.${MINOR}.${PATCH}")
   )
 
   # Export our found variable to parent scope
-  set(${PACKAGE_NAME}_FOUND ${PACKAGE_NAME}_FOUND PARENT_SCOPE)
+  set(${PACKAGE_NAME}_FOUND
+      ${PACKAGE_NAME}_FOUND
+      PARENT_SCOPE
+  )
 
 endfunction(ToolchainLibraryFinder)
diff --git a/src/util/demangle.cpp b/src/util/demangle.cpp
@@ -112,10 +112,19 @@ namespace util {
      */
     std::string demangle(const char* symbol) {
 
+        if (symbol == nullptr) {
+            return {};
+        }
+
         int status = -1;
-        const std::unique_ptr<char, void (*)(void*)> res{abi::__cxa_demangle(symbol, nullptr, nullptr, &status),
-                                                         std::free};
-        if (status == 0) {
+        const std::unique_ptr<char, void (*)(char*)> res{abi::__cxa_demangle(symbol, nullptr, nullptr, &status),
+                                                         [](char* ptr) {
+                                                             // The API for __cxa_demangle REQUIRES us to call free
+                                                             // No choice here so suppress the linter
+                                                             // NOLINTNEXTLINE(cppcoreguidelines-no-malloc)
+                                                             std::free(ptr);  // NOSONAR
+                                                         }};
+        if (res != nullptr) {
             std::string demangled = res.get();
             demangled             = std::regex_replace(demangled, std::regex(R"(\s+)"), "");
             return demangled;

diff --git a/tests/util/demangle.cpp b/tests/util/demangle.cpp
@@ -72,6 +72,18 @@ SCENARIO("Test the demangle function correctly demangles symbols", "[util][deman
         }
     }
 
+    GIVEN("A nullptr as the symbol") {
+        const char* symbol = nullptr;
+
+        WHEN("Demangle is called") {
+            std::string result = NUClear::util::demangle(symbol);
+
+            THEN("It should return an empty string") {
+                REQUIRE(result.empty());
+            }
+        }
+    }
+
     GIVEN("A symbol from a struct") {
         const char* symbol         = typeid(TestSymbol).name();
         const std::string expected = "TestSymbol";