Bunch of clang-tidy stuff

src/LogLevel.cpp

@@ -0,0 +1,55 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2013 NUClear Contributors
+ *
+ * This file is part of the NUClear codebase.
+ * See https://github.com/Fastcode/NUClear for further info.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "LogLevel.hpp"
+
+#include <ostream>
+
+namespace NUClear {
+
+std::string to_string(const LogLevel& level) {
+    switch (level) {
+        case LogLevel::TRACE: return "TRACE";
+        case LogLevel::DEBUG: return "DEBUG";
+        case LogLevel::INFO: return "INFO";
+        case LogLevel::WARN: return "WARN";
+        case LogLevel::ERROR: return "ERROR";
+        case LogLevel::FATAL: return "FATAL";
+        default:
+        case LogLevel::UNKNOWN: return "UNKNOWN";
+    }
+}
+
+LogLevel from_string(const std::string& level) {
+    return level == "TRACE"   ? LogLevel::TRACE
+           : level == "DEBUG" ? LogLevel::DEBUG
+           : level == "INFO"  ? LogLevel::INFO
+           : level == "WARN"  ? LogLevel::WARN
+           : level == "ERROR" ? LogLevel::ERROR
+           : level == "FATAL" ? LogLevel::FATAL
+                              : LogLevel::UNKNOWN;
+}
+
+std::ostream& operator<<(std::ostream& os, const LogLevel& level) {
+    return os << to_string(level);
+}
+
+}  // namespace NUClear

src/LogLevel.hpp

@@ -19,10 +19,11 @@
  * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
  * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
-
 #ifndef NUCLEAR_LOGLEVEL_HPP
 #define NUCLEAR_LOGLEVEL_HPP
 
+#include <ostream>
+
 // Why do we need to include platform.hpp here?
 // Because windows defines a bunch of things for legacy reasons, one of which is a #define for ERROR as blank
 // Of course this causes a problem when we define our own token below as error as the preprocessor removes it
@@ -37,7 +38,7 @@ namespace NUClear {
  * Log levels are used to provide different levels of detail on a per-reactor basis.
  * The logging level of a reactor can be changed by setting it in the install function.
  */
-enum LogLevel {
+enum LogLevel : uint8_t {
     /**
      * Don't use this log level when emitting logs, it is for setting reactor log level from non reactor sources.
      *
@@ -97,6 +98,33 @@ enum LogLevel {
     FATAL
 };
 
+/**
+ * This function is used to convert a LogLevel into a string
+ *
+ * @param level the LogLevel to convert
+ *
+ * @return the string representation of the LogLevel
+ */
+std::string to_string(const LogLevel& level);
+
+/**
+ * This function is used to convert a string into a LogLevel
+ *
+ * @param level the string to convert
+ *
+ * @return the LogLevel representation of the string
+ */
+LogLevel from_string(const std::string& level);
+
+/**
+ * This function is used to convert a LogLevel into a string for printing.
+ *
+ * @param os    the output stream to write to
+ * @param level the LogLevel to convert
+ * @return the output stream
+ */
+std::ostream& operator<<(std::ostream& os, const LogLevel& level);
+
 }  // namespace NUClear
 
 #endif  // NUCLEAR_LOGLEVEL_HPP

src/dsl/word/IO.hpp

@@ -97,15 +97,15 @@ namespace dsl {
 
 // On windows we use different wait events
 #ifdef _WIN32
-            // NOLINTNEXTLINE(google-runtime-int)
+            // NOLINTNEXTLINE(performance-enum-size) these have to be fixed types based on the api
             enum EventType : event_t {
                 READ  = FD_READ | FD_OOB | FD_ACCEPT,
                 WRITE = FD_WRITE,
                 CLOSE = FD_CLOSE,
                 ERROR = 0,
             };
 #else
-            // NOLINTNEXTLINE(google-runtime-int)
+            // NOLINTNEXTLINE(performance-enum-size) these have to be fixed types based on the api
             enum EventType : event_t {
                 READ  = POLLIN,
                 WRITE = POLLOUT,

src/dsl/word/Last.hpp

@@ -26,6 +26,7 @@
 #include <list>
 #include <type_traits>
 
+#include "../../dsl/trait/is_transient.hpp"
 #include "../../threading/Reaction.hpp"
 #include "../../util/MergeTransient.hpp"
 
@@ -156,6 +157,7 @@ namespace dsl {
 
         private:
             template <typename... T, int... Index>
+            // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved) the elements in it are moved
             static auto wrap(std::tuple<T...>&& data, util::Sequence<Index...> /*s*/)
                 -> decltype(std::make_tuple(LastItemStorage<n, T>(std::move(std::get<Index>(data)))...)) {
                 return std::make_tuple(LastItemStorage<n, T>(std::move(std::get<Index>(data)))...);

src/dsl/word/TCP.hpp

@@ -25,7 +25,6 @@
 
 #include <cstring>
 
-#include "../../PowerPlant.hpp"
 #include "../../threading/Reaction.hpp"
 #include "../../util/FileDescriptor.hpp"
 #include "../../util/network/resolve.hpp"

src/dsl/word/UDP.hpp

@@ -26,7 +26,6 @@
 #include <array>
 #include <stdexcept>
 
-#include "../../PowerPlant.hpp"
 #include "../../threading/Reaction.hpp"
 #include "../../util/FileDescriptor.hpp"
 #include "../../util/network/get_interfaces.hpp"
@@ -71,7 +70,7 @@ namespace dsl {
              */
             struct ConnectOptions {
                 /// The type of connection we are making
-                enum class Type { UNICAST, BROADCAST, MULTICAST };
+                enum class Type : uint8_t { UNICAST, BROADCAST, MULTICAST };
                 /// The type of connection we are making
                 Type type{};
                 /// The address we are binding to or empty for any

src/dsl/word/emit/Direct.hpp

@@ -23,7 +23,6 @@
 #ifndef NUCLEAR_DSL_WORD_EMIT_DIRECT_HPP
 #define NUCLEAR_DSL_WORD_EMIT_DIRECT_HPP
 
-#include "../../../PowerPlant.hpp"
 #include "../../store/DataStore.hpp"
 #include "../../store/ThreadStore.hpp"
 #include "../../store/TypeCallbackStore.hpp"

src/dsl/word/emit/Local.hpp

@@ -23,7 +23,6 @@
 #ifndef NUCLEAR_DSL_WORD_EMIT_LOCAL_HPP
 #define NUCLEAR_DSL_WORD_EMIT_LOCAL_HPP
 
-#include "../../../PowerPlant.hpp"
 #include "../../../util/TypeMap.hpp"
 #include "../../store/DataStore.hpp"
 #include "../../store/ThreadStore.hpp"

src/dsl/word/emit/UDP.hpp

@@ -25,7 +25,6 @@
 
 #include <stdexcept>
 
-#include "../../../PowerPlant.hpp"
 #include "../../../util/FileDescriptor.hpp"
 #include "../../../util/network/if_number_from_address.hpp"
 #include "../../../util/platform.hpp"

src/dsl/word/emit/Watchdog.hpp

@@ -25,7 +25,6 @@
 
 #include <stdexcept>
 
-#include "../../../PowerPlant.hpp"
 #include "../../../util/TypeMap.hpp"
 #include "../../../util/demangle.hpp"
 

src/extension/ChronoController.hpp

@@ -23,7 +23,6 @@
 #ifndef NUCLEAR_EXTENSION_CHRONOCONTROLLER
 #define NUCLEAR_EXTENSION_CHRONOCONTROLLER
 
-#include "../PowerPlant.hpp"
 #include "../Reactor.hpp"
 #include "../message/TimeTravel.hpp"
 #include "../util/precise_sleep.hpp"

src/extension/IOController_Posix.hpp

@@ -30,7 +30,6 @@
 #include <mutex>
 #include <system_error>
 
-#include "../PowerPlant.hpp"
 #include "../Reactor.hpp"
 #include "../dsl/word/IO.hpp"
 

src/extension/IOController_Windows.hpp

@@ -23,7 +23,6 @@
 #ifndef NUCLEAR_EXTENSION_IOCONTROLLER_WINDOWS_HPP
 #define NUCLEAR_EXTENSION_IOCONTROLLER_WINDOWS_HPP
 
-#include "../PowerPlant.hpp"
 #include "../Reactor.hpp"
 #include "../dsl/word/IO.hpp"
 #include "../util/platform.hpp"

src/extension/NetworkController.hpp

@@ -27,7 +27,6 @@
 #include <cerrno>
 #include <csignal>
 
-#include "../PowerPlant.hpp"
 #include "../Reactor.hpp"
 #include "../util/get_hostname.hpp"
 #include "network/NUClearNetwork.hpp"

src/extension/network/NUClearNetwork.cpp

@@ -23,14 +23,13 @@
 #include "NUClearNetwork.hpp"
 
 #include <algorithm>
-#include <cerrno>
 #include <cstring>
-#include <set>
+#include <iterator>
+#include <ratio>
 #include <stdexcept>
 #include <system_error>
 #include <utility>
 
-#include "../../util/network/get_interfaces.hpp"
 #include "../../util/network/if_number_from_address.hpp"
 #include "../../util/network/resolve.hpp"
 #include "../../util/platform.hpp"
@@ -117,6 +116,8 @@ namespace extension {
                     std::memcpy(key.data(), &address.ipv6.sin6_addr, sizeof(address.ipv6.sin6_addr));
                     key[8] = address.ipv6.sin6_port;
                     break;
+
+                default: throw std::invalid_argument("Unknown address family");
             }
 
             return key;

src/message/TimeTravel.hpp

@@ -34,7 +34,7 @@ namespace message {
      * to the new time and rate.
      */
     struct TimeTravel {
-        enum class Action {
+        enum class Action : uint8_t {
             /// Adjust clock and move all chrono tasks with it
             RELATIVE,
 

src/util/FunctionFusion.hpp

@@ -48,7 +48,7 @@ namespace util {
      * @return The object returned by the called subfunction
      */
     template <typename Function, int... Shared, int... Selected, typename... Arguments>
-    auto apply_function_fusion_call(std::tuple<Arguments...>&& args,
+    auto apply_function_fusion_call(const std::tuple<Arguments...>& args,
                                     const Sequence<Shared...>& /*shared*/,
                                     const Sequence<Selected...>& /*selected*/)
         -> decltype(Function::call(std::get<Shared>(args)..., std::get<Selected>(args)...)) {
@@ -57,18 +57,18 @@ namespace util {
 
     /**
      * Applies a single set of function fusion with argument ranges
+     *
      * Calls the function held in the template type Function.
-     *          for the arguments it uses the parameter packs Shared and Selected
-     *          to expand the passed tuple args and forward those selected
-     *          arguments to the function.
+     * For the arguments it uses the parameter packs Shared and Selected to expand the passed tuple args and forward
+     * those selected arguments to the function.
      *
-     * @param  args     the arguments that were passed to the superfunction
+     * @tparam Function   The struct that holds the call function wrapper to be called
+     * @tparam Shared     The number of parameters (from 0) to use in the call
+     * @tparam Start      The index of the first argument to pass to the function
+     * @tparam End        The index of the element after the last argument to pass to the function
+     * @tparam Arguments  The types of the arguments passed into the function
      *
-     * @tparam Function     the struct that holds the call function wrapper to be called
-     * @tparam Shared       the number of parameters (from 0) to use in the call
-     * @tparam Start        the index of the first argument to pass to the function
-     * @tparam End          the index of the element after the last argument to pass to the function
-     * @tparam Arguments    the types of the arguments passed into the function
+     * @param  args     the arguments that were passed to the super-function
      *
      * @return the object returned by the called subfunction
      */
@@ -95,6 +95,8 @@ namespace util {
      */
     template <int Shared, typename... Arguments>
     struct FunctionFusionCaller<std::tuple<>, Shared, std::tuple<>, std::tuple<Arguments...>> {
+        // This function is just here to satisfy the templates
+        // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved)
         static std::tuple<> call(Arguments&&... /*args*/) {
             return {};
         }
@@ -137,6 +139,8 @@ namespace util {
          * @return the result of calling this specific function
          */
         template <typename Function, int Start, int End>
+        // It is forwarded as a tuple
+        // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved)
         static auto call_one(const Sequence<Start, End>& /*e*/, Arguments&&... args)
             -> decltype(apply_function_fusion_call<Function, Shared, Start, End>(std::forward_as_tuple(args...))) {
 
@@ -153,7 +157,7 @@ namespace util {
          * @return ignore
          */
         template <typename>
-        static inline bool call_one(...);
+        static bool call_one(...);
 
         /// The FunctionFusionCaller next step in the recursion
         using NextStep =

src/util/network/get_interfaces.cpp

@@ -22,9 +22,9 @@
 
 #include "get_interfaces.hpp"
 
-#include <algorithm>
 #include <cstring>
 #include <system_error>
+#include <vector>
 
 #include "../platform.hpp"
 
@@ -177,13 +177,15 @@ namespace util {
                         switch (it->ifa_addr->sa_family) {
                             case AF_INET: std::memcpy(&iface.netmask, it->ifa_netmask, sizeof(sockaddr_in)); break;
                             case AF_INET6: std::memcpy(&iface.netmask, it->ifa_netmask, sizeof(sockaddr_in6)); break;
+                            default: break;  // We don't care about other address families
                         }
                     }
 
                     if (it->ifa_dstaddr != nullptr) {
                         switch (it->ifa_addr->sa_family) {
                             case AF_INET: std::memcpy(&iface.broadcast, it->ifa_dstaddr, sizeof(sockaddr_in)); break;
                             case AF_INET6: std::memcpy(&iface.broadcast, it->ifa_dstaddr, sizeof(sockaddr_in6)); break;
+                            default: break;  // We don't care about other address families
                         }
                     }
 

tests/networktest.cpp

@@ -168,6 +168,7 @@ class TestReactor : public NUClear::Reactor {
 
 // NOLINTNEXTLINE(bugprone-exception-escape)
 int main(int argc, const char* argv[]) {
+    // NOLINTNEXTLINE(bugprone-signal-handler,cert-msc54-cpp,cert-sig30-c)
     auto old_sigint = signal(SIGINT, [](int /*signal*/) { NUClear::PowerPlant::powerplant->shutdown(); });
     if (old_sigint == SIG_ERR) {
         std::cerr << "Failed to set SIGINT handler";

tests/tests/dsl/Priority.cpp

@@ -75,6 +75,7 @@ class TestReactor : public test_util::TestBase<TestReactor> {
                 case 4:
                     on<Trigger<Message<3>>, Priority::IDLE>().then([] { events.push_back("Idle Message<3>"); });
                     break;
+                default: throw std::invalid_argument("Should be impossible");
             }
         }
 

tests/tests/dsl/TCP.cpp

@@ -31,12 +31,12 @@ namespace {
 /// Events that occur during the test
 std::vector<std::string> events;  // NOLINT(cppcoreguidelines-avoid-non-const-global-variables)
 
-enum TestPorts {
+enum TestPorts : in_port_t {
     KNOWN_V4_PORT = 40010,
     KNOWN_V6_PORT = 40011,
 };
 
-enum TestType {
+enum TestType : uint8_t {
     V4_KNOWN,
     V4_EPHEMERAL,
     V6_KNOWN,