test: Improve test coverage of Language Server component (#61)

* refactoring & dispatcher tests

* dispatcher test

* add feature launch test (so far launch, stackTrace and setBreakpoints tested)

* test: finished launch feature tests

* refactor: refactor register methods so virtual method is not called in constructors

* refactor: fix sonarcloud code smells

* refactor: fix code smells

* test: add dap server test

* test: improve dispatcher and lsp_erver tests

* test: add malformed message test for dap_server

* refactor: fix exception thowing

* test: variable initialization

* test: fix never ending test

* avoid waiting infinitely when there are no requests

* test: add finish requests test

* test: fix finish requests test

* fix possible race condition

* remove unnecessary lock

* unify visual studio build folders

* code smell fixes

CMakeSettings.json

@@ -5,8 +5,8 @@
       "generator": "Ninja",
       "configurationType": "Debug",
       "inheritEnvironments": [ "msvc_x86" ],
-      "buildRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\build\\${name}",
-      "installRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeCommandArgs": "",
       "buildCommandArgs": "-v",
       "ctestCommandArgs": "",
@@ -17,8 +17,8 @@
       "generator": "Ninja",
       "configurationType": "RelWithDebInfo",
       "inheritEnvironments": [ "msvc_x86" ],
-      "buildRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\build\\${name}",
-      "installRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeCommandArgs": "",
       "buildCommandArgs": "-v",
       "ctestCommandArgs": "",
@@ -29,8 +29,8 @@
       "generator": "Ninja",
       "configurationType": "Debug",
       "inheritEnvironments": [ "msvc_x64_x64" ],
-      "buildRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\build\\${name}",
-      "installRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeCommandArgs": "",
       "buildCommandArgs": "-v",
       "ctestCommandArgs": "",
@@ -41,8 +41,8 @@
       "generator": "Ninja",
       "configurationType": "RelWithDebInfo",
       "inheritEnvironments": [ "msvc_x64_x64" ],
-      "buildRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\build\\${name}",
-      "installRoot": "${env.USERPROFILE}\\CMakeBuilds\\${workspaceHash}\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeCommandArgs": "",
       "buildCommandArgs": "-v",
       "ctestCommandArgs": "",
@@ -52,8 +52,8 @@
       "name": "WSL-GNU-Release",
       "generator": "Ninja",
       "configurationType": "RelWithDebInfo",
-      "buildRoot": "${projectDir}\\out\\build\\${name}",
-      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeExecutable": "/usr/bin/cmake",
       "cmakeCommandArgs": "-DCMAKE_CXX_COMPILER=\"g++-8\" -DCMAKE_C_COMPILER=\"gcc-8\"",
       "buildCommandArgs": "",
@@ -67,8 +67,8 @@
       "name": "WSL-Clang-Release",
       "generator": "Ninja",
       "configurationType": "RelWithDebInfo",
-      "buildRoot": "${projectDir}\\out\\build\\${name}",
-      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeExecutable": "/usr/bin/cmake",
       "cmakeCommandArgs": "-DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8",
       "buildCommandArgs": "",
@@ -82,8 +82,8 @@
       "name": "WSL-Clang-Debug-ASAN",
       "generator": "Ninja",
       "configurationType": "Debug",
-      "buildRoot": "${projectDir}\\out\\build\\${name}",
-      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeExecutable": "/usr/bin/cmake",
       "cmakeCommandArgs": "-DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_FLAGS=\"-fsanitize=address,undefined\"",
       "buildCommandArgs": "",
@@ -93,12 +93,27 @@
       "addressSanitizerRuntimeFlags": "detect_leaks=0",
       "variables": []
     },
+    {
+      "name": "WSL-Clang-Release-ASAN",
+      "generator": "Ninja",
+      "configurationType": "Release",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
+      "cmakeExecutable": "/usr/bin/cmake",
+      "cmakeCommandArgs": "-DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_FLAGS=\"-fsanitize=address,undefined,fuzzer-no-link\"",
+      "buildCommandArgs": "",
+      "ctestCommandArgs": "",
+      "inheritEnvironments": [ "linux_x64" ],
+      "wslPath": "${defaultWSLPath}",
+      "addressSanitizerRuntimeFlags": "detect_leaks=0",
+      "variables": []
+    },
     {
       "name": "WSL-Clang-Fuzzer",
       "generator": "Ninja",
       "configurationType": "Debug",
-      "buildRoot": "${projectDir}\\out\\build\\${name}",
-      "installRoot": "${projectDir}\\out\\install\\${name}",
+      "buildRoot": "${projectDir}\\build\\${name}",
+      "installRoot": "${projectDir}\\install\\${name}",
       "cmakeExecutable": "/usr/bin/cmake",
       "cmakeCommandArgs": "-DCMAKE_CXX_COMPILER=clang++-8 -DCMAKE_C_COMPILER=clang-8 -DBUILD_FUZZER=On -DBUILD_VSIX=Off -DWITH_LIBCXX=Off -DCMAKE_CXX_FLAGS=\"-fsanitize=address,undefined\"",
       "buildCommandArgs": "-v",

language_server/src/dap/dap_feature.h

@@ -45,7 +45,7 @@ class dap_feature : public feature
         : feature(ws_mngr, response_provider)
     {}
 
-    std::string convert_path(const std::string& path)
+    std::string convert_path(const std::string& path) const
     {
         if (path_format_ == path_format::URI)
             return uri_to_path(path);
@@ -69,6 +69,7 @@ class dap_feature : public feature
 
     int column_1_based_;
     int line_1_based_;
+private:
     path_format path_format_;
 };
 

language_server/src/dap/dap_server.cpp

@@ -27,6 +27,7 @@ server::server(parser_library::workspace_manager& ws_mngr)
     : language_server::server(ws_mngr)
 {
     features_.push_back(std::make_unique<feature_launch>(ws_mngr_, *this));
+    register_feature_methods();
     register_methods();
 }
 
@@ -64,24 +65,23 @@ void server::message_received(const json& message)
 {
     try
     {
-        assert(message["type"] == "request");
-        last_seq_ = message["seq"].get<json::number_unsigned_t>();
+        assert(message.at("type") == "request");
+        last_seq_ = message.at("seq").get<json::number_unsigned_t>();
         auto arguments = message.find("arguments");
         if (arguments == message.end())
-            call_method(message["command"].get<std::string>(), message["seq"], json());
+            call_method(message.at("command").get<std::string>(), message.at("seq"), json());
         else
-            call_method(message["command"].get<std::string>(), message["seq"], arguments.value());
+            call_method(message.at("command").get<std::string>(), message.at("seq"), arguments.value());
     }
     catch (const nlohmann::json::exception& e)
     {
+        (void)e;
         LOG_WARNING(std::string("There was an error with received request:") + e.what());
     }
 }
 
 void server::register_methods()
 {
-    language_server::server::register_methods();
-
     methods_.emplace(
         "initialize", std::bind(&server::on_initialize, this, std::placeholders::_1, std::placeholders::_2));
     methods_.emplace(

language_server/src/dap/dap_server.h

@@ -33,7 +33,7 @@ namespace hlasm_plugin::language_server::dap {
 class server : public hlasm_plugin::language_server::server
 {
 public:
-    server(parser_library::workspace_manager& ws_mngr);
+    explicit server(parser_library::workspace_manager& ws_mngr);
 
     // Inherited via server
     virtual void respond(const json& id, const std::string& requested_method, const json& args) override;
@@ -48,14 +48,16 @@ class server : public hlasm_plugin::language_server::server
 
     virtual void message_received(const json& message) override;
 
-    virtual void register_methods() override;
+
 
 
 private:
     uint64_t last_seq_ = 0;
 
     void on_initialize(const json& requested_seq, const json& args);
     void on_disconnect(const json& request_seq, const json& args);
+
+    void register_methods();
 };
 
 } // namespace hlasm_plugin::language_server::dap

language_server/src/dap/feature_launch.cpp

@@ -76,8 +76,7 @@ void feature_launch::on_set_breakpoints(const json& request_seq, const json& arg
     {
         for (auto& bp_json : bpoints_found.value())
         {
-            // parser_library::breakpoint bp();
-            breakpoints.emplace_back((size_t)(bp_json["line"].get<json::number_unsigned_t>() - line_1_based_));
+            breakpoints.emplace_back(bp_json["line"].get<json::number_unsigned_t>() - line_1_based_);
             breakpoints_verified.push_back(json { { "verified", true } });
         }
     }
@@ -192,7 +191,6 @@ void feature_launch::on_variables(const json& request_seq, const json& args)
                 type = "B_TYPE";
                 break;
             case parser_library::set_type::C_TYPE:
-
                 type = "C_TYPE";
                 break;
             default:

language_server/src/dap/feature_launch.h

@@ -19,7 +19,7 @@
 
 namespace hlasm_plugin::language_server::dap {
 // Implements all the events and requests from DAP protocol.
-class feature_launch : public dap_feature, parser_library::debug_event_consumer
+class feature_launch : public dap_feature, public parser_library::debug_event_consumer
 {
 public:
     feature_launch(parser_library::workspace_manager& ws_mngr, response_provider& response_provider);

language_server/src/dap/tcp_handler.cpp

@@ -52,10 +52,9 @@ void tcp_handler::async_accept()
         newline_is_space::imbue_stream(*stream_);
         acceptor_.async_accept(*stream_->rdbuf(), std::bind(&tcp_handler::handle_accept, this, std::placeholders::_1));
     }
-    catch (asio::system_error& e)
+    catch (const asio::system_error& e)
     {
         std::string message = "Warning: Macro tracer asio exception. " + std::string(e.what()) + "\n";
-        std::cout << message;
         LOG_WARNING(message);
         return;
     }
@@ -72,10 +71,9 @@ void tcp_handler::cancel()
         acceptor_.cancel();
         acceptor_.close();
     }
-    catch (asio::system_error& e)
+    catch (const asio::system_error& e)
     {
         std::string message = "Warning: Macro tracer asio exception. " + std::string(e.what());
-        std::cout << message;
         LOG_WARNING(message);
     }
 }

language_server/src/dispatcher.cpp

@@ -23,8 +23,7 @@
 
 #include "logger.h"
 
-namespace hlasm_plugin {
-namespace language_server {
+namespace hlasm_plugin::language_server {
 
 dispatcher::dispatcher(std::istream& in, std::ostream& out, server& server, request_manager& req_mngr)
     : server_(server)
@@ -160,7 +159,6 @@ int dispatcher::run_server_loop()
         if (read_message(message))
         {
             LOG_INFO(message);
-            LOG_INFO(message);
 
             json message_json = 0;
             try
@@ -193,5 +191,4 @@ int dispatcher::run_server_loop()
     return ret;
 }
 
-} // namespace language_server
-} // namespace hlasm_plugin
+} // namespace hlasm_plugin::language_server

language_server/src/dispatcher.h

@@ -41,16 +41,17 @@ class dispatcher : public send_message_provider
     // Reads messages from in_ in infinite loop, deserializes it and notifies the server.
     // Returns return value according to LSP: 0 if server was shut down apropriately
     int run_server_loop();
-    bool read_message(std::string& out);
-
-    void write_message(const std::string& in);
 
     // Serializes the json and sends it as message.
     void reply(const json& result) override;
 
 
 
 private:
+    bool read_message(std::string& out);
+
+    void write_message(const std::string& in);
+
     server& server_;
     std::istream& in_;
     std::ostream& out_;

language_server/src/feature.cpp

@@ -105,12 +105,12 @@ parser_library::position feature::parse_position(const json& position_json)
         position_json["character"].get<nlohmann::json::number_unsigned_t>() };
 }
 
-json feature::range_to_json(parser_library::range range)
+json feature::range_to_json(const parser_library::range & range)
 {
     return json { { "start", position_to_json(range.start) }, { "end", position_to_json(range.end) } };
 }
 
-json feature::position_to_json(parser_library::position position)
+json feature::position_to_json(const parser_library::position & position)
 {
     return json { { "line", position.line }, { "character", position.column } };
 }

language_server/src/feature.h

@@ -23,8 +23,7 @@
 #include "common_types.h"
 #include "workspace_manager.h"
 
-namespace hlasm_plugin {
-namespace language_server {
+namespace hlasm_plugin::language_server {
 
 // Provides methods to send notification, respond to request and respond with error respond
 class response_provider
@@ -37,6 +36,7 @@ class response_provider
         int err_code,
         const std::string& err_message,
         const json& error) = 0;
+    virtual ~response_provider() = default;
 };
 
 // Abstract class for group of methods that add functionality to server.
@@ -45,14 +45,14 @@ class feature
 public:
     // Constructs the feature with workspace_manager.
     // All the requests and notification are passed to the workspace manager
-    feature(parser_library::workspace_manager& ws_mngr)
+    explicit feature(parser_library::workspace_manager& ws_mngr)
         : ws_mngr_(ws_mngr)
-    {}
+    { }
     // Constructs the feature with workspace_manager and response_provider through which the feature can send messages.
     feature(parser_library::workspace_manager& ws_mngr, response_provider& response_provider)
         : ws_mngr_(ws_mngr)
         , response_(&response_provider)
-    {}
+    { }
 
     // Implement to add methods to server.
     void virtual register_methods(std::map<std::string, method>& methods) = 0;
@@ -74,8 +74,8 @@ class feature
     // Converts LSP json representation of position into parse_library::position.
     static parser_library::position parse_position(const json& position_json);
 
-    static json range_to_json(parser_library::range range);
-    static json position_to_json(parser_library::position position);
+    static json range_to_json(const parser_library::range & range);
+    static json position_to_json(const parser_library::position & position);
 
     virtual ~feature() = default;
 
@@ -85,7 +85,6 @@ class feature
     response_provider* response_ = nullptr;
 };
 
-} // namespace language_server
-} // namespace hlasm_plugin
+} // namespace hlasm_plugin::language_server
 
 #endif // !HLASMPLUGIN_LANGUAGESERVER_FEATURE_H

language_server/src/lsp/feature_language_features.cpp

@@ -48,7 +48,10 @@ json feature_language_features::register_capabilities()
             { { "resolveProvider", false }, { "triggerCharacters", { "&", ".", "_", "$", "#", "@", "*" } } } } };
 }
 
-void feature_language_features::initialize_feature(const json&) {}
+void feature_language_features::initialize_feature(const json&)
+{
+    //No need for initialization in this feature.
+}
 
 void feature_language_features::definition(const json& id, const json& params)
 {

language_server/src/lsp/feature_text_synchronization.cpp

@@ -51,7 +51,10 @@ json feature_text_synchronization::register_capabilities()
 }
 
 
-void feature_text_synchronization::initialize_feature(const json&) {}
+void feature_text_synchronization::initialize_feature(const json&)
+{
+    //No need for initialization in this feature.
+}
 
 void feature_text_synchronization::on_did_open(const json&, const json& params)
 {
@@ -164,6 +167,9 @@ void feature_text_synchronization::consume_highlighting_info(parser_library::all
                 case parser_library::semantics::hl_scopes::data_def_extension:
                     scope = "data_def_extension";
                     break;
+                default:
+                    assert(false);
+                    break;
             }
             tokens_array.push_back(json { { "lineStart", fi.token(j).token_range.start.line },
                 { "columnStart", fi.token(j).token_range.start.column },