Increase warning level and enable warning as error (#28)

BrennanConroy · web-flow · commit 7a72b7a2955b · 2020-04-14T13:05:15.000-07:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -8,6 +8,9 @@ project (signalrclient)
 
 include(CTest)
 
+set(WERROR true CACHE BOOL "Enable warnings as errors.")
+set(WALL true CACHE BOOL "Enable all warnings.")
+
 if(NOT WIN32)
   set(EXTRA_FLAGS "-std=c++11 -fPIC -DNO_SIGNALRCLIENT_EXPORTS")
 else()
diff --git a/include/signalrclient/http_client.h b/include/signalrclient/http_client.h
@@ -36,6 +36,15 @@ namespace signalr
         http_response() {}
         http_response(http_response&& rhs) noexcept : status_code(rhs.status_code), content(std::move(rhs.content)) {}
         http_response(int code, const std::string& content) : status_code(code), content(content) {}
+        http_response(const http_response& rhs) : status_code(rhs.status_code), content(rhs.content) {}
+
+        http_response& operator=(const http_response& rhs)
+        {
+            status_code = rhs.status_code;
+            content = rhs.content;
+
+            return *this;
+        }
 
         http_response& operator=(http_response&& rhs) noexcept
         {
diff --git a/include/signalrclient/log_writer.h b/include/signalrclient/log_writer.h
@@ -13,5 +13,7 @@ namespace signalr
     public:
         // NOTE: the caller does not enforce thread safety of this call
         SIGNALRCLIENT_API virtual void write(const std::string &entry) = 0;
+
+        virtual ~log_writer() {}
     };
 }
diff --git a/include/signalrclient/signalr_client_config.h b/include/signalrclient/signalr_client_config.h
@@ -5,8 +5,11 @@
 #pragma once
 
 #ifdef USE_CPPRESTSDK
-#include "cpprest/http_client.h"
-#include "cpprest/ws_client.h"
+#pragma warning (push)
+#pragma warning (disable : 5204 4355 4625 4626 4868)
+#include <cpprest/http_client.h>
+#include <cpprest/ws_client.h>
+#pragma warning (pop)
 #endif
 
 #include "_exports.h"
diff --git a/include/signalrclient/signalr_value.h b/include/signalrclient/signalr_value.h
@@ -180,8 +180,22 @@ namespace signalr
             double number;
             std::map<std::string, value> map;
 
+            // constructor of types in union are not implicitly called
+            // this is expected as we only construct a single type in the union once we know
+            // what that type is when constructing the signalr_value type.
+#pragma warning (push)
+#pragma warning (disable: 4582)
             storage() {}
+#pragma warning (pop)
+
+            storage(const storage&) = delete;
+            storage& operator=(const storage&) = delete;
+
+            // destructor is not implicitly called
+#pragma warning (push)
+#pragma warning (disable: 4583)
             ~storage() {}
+#pragma warning (pop)
         };
 
         storage mStorage;
diff --git a/src/signalrclient/CMakeLists.txt b/src/signalrclient/CMakeLists.txt
@@ -31,6 +31,40 @@ include_directories(
 
 add_library (signalrclient SHARED ${SOURCES})
 
+if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
+    if(WERROR)
+        target_compile_options(signalrclient PRIVATE /WX)
+    endif()
+    if(WALL)
+        target_compile_options(signalrclient PRIVATE /Wall)
+    endif()
+    target_compile_options(signalrclient PRIVATE
+        /wd4820 # padding added after data member
+        /wd4514 # unreferenced inline function removed
+        /wd5045 # compiler will insert Spectre mitigation if /Qspectre switch is added
+        /wd4464 # relative include paths
+        /wd4711 # function 'x' selected for automatic inline expansion
+        /wd4710 # function not inlined
+        /experimental:external /external:anglebrackets /external:templates- /external:W0
+        /GR- # disable run-time type information
+        /guard:cf # enable control-flow guard
+        /EHa # enable C++ EH (w/ SEH exceptions)
+    )
+else()
+    if(WERROR)
+        target_compile_options(signalrclient PRIVATE -Werror)
+    endif()
+    if(WALL)
+        target_compile_options(signalrclient PRIVATE -Wall)
+    endif()
+
+    # GCC on OSX has a bug with exceptions and no-rtti that can cause crashes
+    if(NOT APPLE)
+        target_compile_options(signalrclient PRIVATE -fno-rtti)
+    endif()
+    target_compile_options(signalrclient PRIVATE -Wextra -Wpedantic -Wno-unknown-pragmas)
+endif()
+
 if(NOT USE_CPPRESTSDK)
   target_link_libraries(signalrclient)
 else()
diff --git a/src/signalrclient/cancellation_token.h b/src/signalrclient/cancellation_token.h
@@ -24,6 +24,9 @@ namespace signalr
         {
         }
 
+        cancellation_token(const cancellation_token&) = delete;
+        cancellation_token& operator=(const cancellation_token&) = delete;
+
         void cancel()
         {
             std::lock_guard<std::mutex> lock(m_lock);
diff --git a/src/signalrclient/case_insensitive_comparison_utils.h b/src/signalrclient/case_insensitive_comparison_utils.h
@@ -21,7 +21,7 @@ namespace signalr
                 return false;
             }
 
-            for (int i = 0; i < s1.size(); ++i)
+            for (unsigned i = 0; i < s1.size(); ++i)
             {
                 if (std::toupper(s1[i]) != std::toupper(s2[i]))
                 {
@@ -41,7 +41,7 @@ namespace signalr
             std::hash<size_t> hasher;
             for (const auto& c : s)
             {
-                hash ^= hasher(std::toupper(c)) + (hash << 5) + (hash >> 2);
+                hash ^= hasher(static_cast<size_t>(std::toupper(c))) + (hash << 5) + (hash >> 2);
             }
 
             return hash;
diff --git a/src/signalrclient/completion_event.h b/src/signalrclient/completion_event.h
@@ -4,13 +4,19 @@
 
 #pragma once
 
+#pragma warning (push)
+#pragma warning (disable : 5204 4355)
 #include <future>
+#pragma warning (pop)
 
 namespace signalr
 {
     class completion_event_impl : public std::enable_shared_from_this<completion_event_impl>
     {
     public:
+        completion_event_impl(const completion_event_impl&) = delete;
+        completion_event_impl& operator=(const completion_event_impl&) = delete;
+
         static std::shared_ptr<completion_event_impl> create()
         {
             return std::shared_ptr<completion_event_impl>(new completion_event_impl());
diff --git a/src/signalrclient/connection_impl.cpp b/src/signalrclient/connection_impl.cpp
@@ -13,21 +13,13 @@
 #include "signalrclient/signalr_exception.h"
 #include "default_http_client.h"
 #include "case_insensitive_comparison_utils.h"
-#include "make_unique.h"
 #include "completion_event.h"
 #include <assert.h>
 #include "signalrclient/websocket_client.h"
 #include "default_websocket_client.h"
 
 namespace signalr
 {
-    // unnamed namespace makes it invisble outside this translation unit
-    namespace
-    {
-        // this is a workaround for a compiler bug where mutable lambdas won't sometimes compile
-        static void log(const logger& logger, trace_level level, const std::string& entry);
-    }
-
     std::shared_ptr<connection_impl> connection_impl::create(const std::string& url, trace_level trace_level, const std::shared_ptr<log_writer>& log_writer)
     {
         return connection_impl::create(url, trace_level, log_writer, nullptr, nullptr);
@@ -259,8 +251,7 @@ namespace signalr
                         }
                         catch (const std::exception& e)
                         {
-                            auto canceled = dynamic_cast<const canceled_exception*>(&e);
-                            if (canceled)
+                            if (token->is_canceled())
                             {
                                 connection->m_logger.log(trace_level::info,
                                     "starting the connection has been canceled.");
diff --git a/src/signalrclient/default_http_client.cpp b/src/signalrclient/default_http_client.cpp
@@ -7,7 +7,10 @@
 #ifdef USE_CPPRESTSDK
 #include "default_http_client.h"
 #include <thread>
-#include "cpprest/http_client.h"
+#pragma warning (push)
+#pragma warning (disable : 5204 4355 4625 4626 4868)
+#include <cpprest/http_client.h>
+#pragma warning (pop)
 
 namespace signalr
 {
diff --git a/src/signalrclient/default_websocket_client.cpp b/src/signalrclient/default_websocket_client.cpp
@@ -52,6 +52,7 @@ namespace signalr
             {
                 try
                 {
+                    task.get();
                     callback(nullptr);
                 }
                 catch (...)
diff --git a/src/signalrclient/default_websocket_client.h b/src/signalrclient/default_websocket_client.h
@@ -6,7 +6,10 @@
 
 #ifdef USE_CPPRESTSDK
 
-#include "cpprest/ws_client.h"
+#pragma warning (push)
+#pragma warning (disable : 5204 4355 4625 4626 4868)
+#include <cpprest/ws_client.h>
+#pragma warning (pop)
 #include "signalrclient/signalr_client_config.h"
 #include "signalrclient/websocket_client.h"
 
diff --git a/src/signalrclient/hub_connection_impl.cpp b/src/signalrclient/hub_connection_impl.cpp
@@ -7,7 +7,6 @@
 #include "signalrclient/hub_exception.h"
 #include "trace_log_writer.h"
 #include "signalrclient/signalr_exception.h"
-#include "make_unique.h"
 #include "json_hub_protocol.h"
 #include "message_type.h"
 #include "handshake_protocol.h"
@@ -40,7 +39,7 @@ namespace signalr
         : m_connection(connection_impl::create(url, trace_level, log_writer,
             http_client, websocket_factory)), m_logger(log_writer, trace_level),
         m_callback_manager(signalr::value(std::map<std::string, signalr::value> { { std::string("error"), std::string("connection went out of scope before invocation result was received") } })),
-        m_disconnected([]() noexcept {}), m_handshakeReceived(false), m_protocol(std::make_shared<json_hub_protocol>())
+        m_handshakeReceived(false), m_disconnected([]() noexcept {}), m_protocol(std::make_shared<json_hub_protocol>())
     {}
 
     void hub_connection_impl::initialize()
diff --git a/src/signalrclient/json_hub_protocol.cpp b/src/signalrclient/json_hub_protocol.cpp
@@ -7,7 +7,6 @@
 #include "message_type.h"
 #include "json_helpers.h"
 #include "signalrclient/signalr_exception.h"
-#include "json/json.h"
 
 namespace signalr
 {
@@ -84,6 +83,6 @@ namespace signalr
             break;
         }
 
-        return std::move(value);
+        return value;
     }
 }
diff --git a/src/signalrclient/logger.cpp b/src/signalrclient/logger.cpp
@@ -71,6 +71,10 @@ namespace signalr
             return "error";
         case signalr::trace_level::info:
             return "info";
+        case signalr::trace_level::none:
+            return "none";
+        case signalr::trace_level::all:
+            return "all";
         default:
             assert(false);
             return "(unknown)";
diff --git a/src/signalrclient/make_unique.h b/src/signalrclient/make_unique.h
diff --git a/src/signalrclient/signalr_client_config.cpp b/src/signalrclient/signalr_client_config.cpp
@@ -4,10 +4,6 @@
 
 #include "stdafx.h"
 #include "signalrclient/signalr_client_config.h"
-#ifdef USE_CPPRESTSDK
-#include "cpprest/http_client.h"
-#include "cpprest/ws_client.h"
-#endif
 
 namespace signalr
 {
diff --git a/src/signalrclient/signalr_value.cpp b/src/signalrclient/signalr_value.cpp
@@ -51,6 +51,7 @@ namespace signalr
         case value_type::map:
             new (&mStorage.map) std::map<std::string, value>();
             break;
+        case value_type::null:
         default:
             break;
         }
@@ -121,6 +122,7 @@ namespace signalr
         case value_type::map:
             new (&mStorage.map) std::map<std::string, value>(rhs.mStorage.map);
             break;
+        case value_type::null:
         default:
             break;
         }
@@ -146,6 +148,7 @@ namespace signalr
         case value_type::map:
             new (&mStorage.map) std::map<std::string, signalr::value>(std::move(rhs.mStorage.map));
             break;
+        case value_type::null:
         default:
             break;
         }
@@ -169,6 +172,9 @@ namespace signalr
         case value_type::map:
             mStorage.map.~map();
             break;
+        case value_type::null:
+        case value_type::float64:
+        case value_type::boolean:
         default:
             break;
         }
@@ -196,6 +202,7 @@ namespace signalr
         case value_type::map:
             new (&mStorage.map) std::map<std::string, value>(rhs.mStorage.map);
             break;
+        case value_type::null:
         default:
             break;
         }
@@ -225,6 +232,7 @@ namespace signalr
         case value_type::map:
             new (&mStorage.map) std::map<std::string, value>(std::move(rhs.mStorage.map));
             break;
+        case value_type::null:
         default:
             break;
         }
diff --git a/src/signalrclient/stdafx.h b/src/signalrclient/stdafx.h
@@ -8,11 +8,7 @@
 
 // prevents from defining min/max macros that conflict with std::min()/std::max() functions
 #define NOMINMAX
-
-#include <SDKDDKVer.h>
-
 #define WIN32_LEAN_AND_MEAN
-
 #include <windows.h>
 
 #endif
diff --git a/src/signalrclient/transport_factory.cpp b/src/signalrclient/transport_factory.cpp
@@ -4,7 +4,6 @@
 
 #include "stdafx.h"
 #include "transport_factory.h"
-#include "default_websocket_client.h"
 #include "websocket_transport.h"
 #include "signalrclient/websocket_client.h"
 
diff --git a/src/signalrclient/websocket_transport.cpp b/src/signalrclient/websocket_transport.cpp
@@ -7,7 +7,11 @@
 #include "logger.h"
 #include "signalrclient/signalr_exception.h"
 #include "base_uri.h"
+
+#pragma warning (push)
+#pragma warning (disable : 5204 4355)
 #include <future>
+#pragma warning (pop)
 
 namespace signalr
 {
@@ -20,9 +24,9 @@ namespace signalr
 
     websocket_transport::websocket_transport(const std::function<std::shared_ptr<websocket_client>(const signalr_client_config&)>& websocket_client_factory,
         const signalr_client_config& signalr_client_config, const logger& logger)
-        : transport(logger), m_websocket_client_factory(websocket_client_factory), m_close_callback([](std::exception_ptr) {}),
-        m_process_response_callback([](std::string, std::exception_ptr) {}), m_receive_loop_cts(std::make_shared<cancellation_token>()),
-        m_signalr_client_config(signalr_client_config)
+        : transport(logger), m_websocket_client_factory(websocket_client_factory), m_process_response_callback([](std::string, std::exception_ptr) {}),
+        m_close_callback([](std::exception_ptr) {}), m_signalr_client_config(signalr_client_config),
+        m_receive_loop_cts(std::make_shared<cancellation_token>())
     {
         // we use this cts to check if the receive loop is running so it should be
         // initially canceled to indicate that the receive loop is not running
diff --git a/test/signalrclienttests/connection_impl_tests.cpp b/test/signalrclienttests/connection_impl_tests.cpp
diff --git a/test/signalrclienttests/hub_connection_tests.cpp b/test/signalrclienttests/hub_connection_tests.cpp
diff --git a/test/signalrclienttests/negotiate_tests.cpp b/test/signalrclienttests/negotiate_tests.cpp
diff --git a/test/signalrclienttests/stdafx.h b/test/signalrclienttests/stdafx.h
diff --git a/test/signalrclienttests/test_utils.cpp b/test/signalrclienttests/test_utils.cpp
diff --git a/third_party_code/cpprestsdk/uri.cpp b/third_party_code/cpprestsdk/uri.cpp
diff --git a/third_party_code/jsoncpp/jsoncpp.cpp b/third_party_code/jsoncpp/jsoncpp.cpp

Original file line number	Diff line number	Diff line change
`@@ -13,5 +13,7 @@ namespace signalr`
`13`	`13`	`public:`
`14`	`14`	`// NOTE: the caller does not enforce thread safety of this call`
`15`	`15`	`SIGNALRCLIENT_API virtual void write(const std::string &entry) = 0;`
	`16`	`+`
	`17`	`+ virtual ~log_writer() {}`
`16`	`18`	`};`
`17`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,9 @@ namespace signalr`
`24`	`24`	`{`
`25`	`25`	`}`
`26`	`26`
	`27`	`+ cancellation_token(const cancellation_token&) = delete;`
	`28`	`+ cancellation_token& operator=(const cancellation_token&) = delete;`
	`29`	`+`
`27`	`30`	`void cancel()`
`28`	`31`	`{`
`29`	`32`	`std::lock_guard<std::mutex> lock(m_lock);`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ namespace signalr`
`21`	`21`	`return false;`
`22`	`22`	`}`
`23`	`23`
`24`		`- for (int i = 0; i < s1.size(); ++i)`
	`24`	`+ for (unsigned i = 0; i < s1.size(); ++i)`
`25`	`25`	`{`
`26`	`26`	`if (std::toupper(s1[i]) != std::toupper(s2[i]))`
`27`	`27`	`{`
`@@ -41,7 +41,7 @@ namespace signalr`
`41`	`41`	`std::hash<size_t> hasher;`
`42`	`42`	`for (const auto& c : s)`
`43`	`43`	`{`
`44`		`- hash ^= hasher(std::toupper(c)) + (hash << 5) + (hash >> 2);`
	`44`	`+ hash ^= hasher(static_cast<size_t>(std::toupper(c))) + (hash << 5) + (hash >> 2);`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`return hash;`
Original file line number	Diff line number	Diff line change
`@@ -4,13 +4,19 @@`
`4`	`4`
`5`	`5`	`#pragma once`
`6`	`6`
	`7`	`+#pragma warning (push)`
	`8`	`+#pragma warning (disable : 5204 4355)`
`7`	`9`	`#include <future>`
	`10`	`+#pragma warning (pop)`
`8`	`11`
`9`	`12`	`namespace signalr`
`10`	`13`	`{`
`11`	`14`	`class completion_event_impl : public std::enable_shared_from_this<completion_event_impl>`
`12`	`15`	`{`
`13`	`16`	`public:`
	`17`	`+ completion_event_impl(const completion_event_impl&) = delete;`
	`18`	`+ completion_event_impl& operator=(const completion_event_impl&) = delete;`
	`19`	`+`
`14`	`20`	`static std::shared_ptr<completion_event_impl> create()`
`15`	`21`	`{`
`16`	`22`	`return std::shared_ptr<completion_event_impl>(new completion_event_impl());`