GDScript LSP: Fix crash in notify_client

`latest_client_id` now defaults to `-1` (invalid ID) instead of `0`. Also fix typo in notification `gdscrip_client/changeWorkspace`, and fix argument names in method binds. Fixes godotengine#39375. (cherry picked from commit e34f337)
akien-mga · Jun 10, 2020 · 3edae03 · 3edae03
1 parent 0b6a410
commit 3edae03
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 14 deletions.
diff --git a/modules/gdscript/language_server/gdscript_extend_parser.cpp b/modules/gdscript/language_server/gdscript_extend_parser.cpp
@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "gdscript_extend_parser.h"
+
 #include "../gdscript.h"
 #include "core/io/json.h"
 #include "gdscript_language_protocol.h"

diff --git a/modules/gdscript/language_server/gdscript_language_protocol.cpp b/modules/gdscript/language_server/gdscript_language_protocol.cpp
@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "gdscript_language_protocol.h"
+
 #include "core/io/json.h"
 #include "core/os/copymem.h"
 #include "core/project_settings.h"
@@ -160,7 +161,7 @@ void GDScriptLanguageProtocol::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("initialized", "params"), &GDScriptLanguageProtocol::initialized);
 	ClassDB::bind_method(D_METHOD("on_client_connected"), &GDScriptLanguageProtocol::on_client_connected);
 	ClassDB::bind_method(D_METHOD("on_client_disconnected"), &GDScriptLanguageProtocol::on_client_disconnected);
-	ClassDB::bind_method(D_METHOD("notify_client", "p_method", "p_params"), &GDScriptLanguageProtocol::notify_client, DEFVAL(Variant()), DEFVAL(-1));
+	ClassDB::bind_method(D_METHOD("notify_client", "method", "params"), &GDScriptLanguageProtocol::notify_client, DEFVAL(Variant()), DEFVAL(-1));
 	ClassDB::bind_method(D_METHOD("is_smart_resolve_enabled"), &GDScriptLanguageProtocol::is_smart_resolve_enabled);
 	ClassDB::bind_method(D_METHOD("get_text_document"), &GDScriptLanguageProtocol::get_text_document);
 	ClassDB::bind_method(D_METHOD("get_workspace"), &GDScriptLanguageProtocol::get_workspace);
@@ -188,8 +189,12 @@ Dictionary GDScriptLanguageProtocol::initialize(const Dictionary &p_params) {
 
 		Dictionary params;
 		params["path"] = workspace->root;
-		Dictionary request = make_notification("gdscrip_client/changeWorkspace", params);
+		Dictionary request = make_notification("gdscript_client/changeWorkspace", params);
 
+		ERR_FAIL_COND_V_MSG(latest_client_id == -1, ret.to_json(),
+				"GDScriptLanguageProtocol: Can't initialize as no client is connected.");
+		ERR_FAIL_INDEX_V_MSG((uint64_t)latest_client_id, clients.size(), ret.to_json(),
+				vformat("GDScriptLanguageProtocol: Can't initialize invalid peer '%d'.", latest_client_id));
 		Ref<LSPeer> peer = clients.get(latest_client_id);
 		if (peer != NULL) {
 			String msg = JSON::print(request);
@@ -271,8 +276,11 @@ void GDScriptLanguageProtocol::stop() {
 
 void GDScriptLanguageProtocol::notify_client(const String &p_method, const Variant &p_params, int p_client_id) {
 	if (p_client_id == -1) {
+		ERR_FAIL_COND_MSG(latest_client_id == -1,
+				"GDScript LSP: Can't notify client as none was connected.");
 		p_client_id = latest_client_id;
 	}
+	ERR_FAIL_INDEX((uint64_t)p_client_id, clients.size());
 	Ref<LSPeer> peer = clients.get(p_client_id);
 	ERR_FAIL_COND(peer == NULL);
 
@@ -293,13 +301,10 @@ bool GDScriptLanguageProtocol::is_goto_native_symbols_enabled() const {
 GDScriptLanguageProtocol::GDScriptLanguageProtocol() {
 	server.instance();
 	singleton = this;
-	_initialized = false;
 	workspace.instance();
 	text_document.instance();
 	set_scope("textDocument", text_document.ptr());
 	set_scope("completionItem", text_document.ptr());
 	set_scope("workspace", workspace.ptr());
 	workspace->root = ProjectSettings::get_singleton()->get_resource_path();
-	latest_client_id = 0;
-	next_client_id = 0;
 }
diff --git a/modules/gdscript/language_server/gdscript_language_protocol.h b/modules/gdscript/language_server/gdscript_language_protocol.h
@@ -70,8 +70,8 @@ class GDScriptLanguageProtocol : public JSONRPC {
 
 	HashMap<int, Ref<LSPeer> > clients;
 	Ref<TCP_Server> server;
-	int latest_client_id;
-	int next_client_id;
+	int latest_client_id = -1;
+	int next_client_id = 0;
 
 	Ref<GDScriptTextDocument> text_document;
 	Ref<GDScriptWorkspace> workspace;
@@ -82,7 +82,7 @@ class GDScriptLanguageProtocol : public JSONRPC {
 	String process_message(const String &p_text);
 	String format_output(const String &p_text);
 
-	bool _initialized;
+	bool _initialized = false;
 
 protected:
 	static void _bind_methods();

diff --git a/modules/gdscript/language_server/gdscript_language_server.cpp b/modules/gdscript/language_server/gdscript_language_server.cpp
@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "gdscript_language_server.h"
+
 #include "core/os/file_access.h"
 #include "core/os/os.h"
 #include "editor/editor_log.h"

diff --git a/modules/gdscript/language_server/gdscript_text_document.cpp b/modules/gdscript/language_server/gdscript_text_document.cpp
@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "gdscript_text_document.h"
+
 #include "../gdscript.h"
 #include "core/os/os.h"
 #include "editor/editor_settings.h"

diff --git a/modules/gdscript/language_server/gdscript_workspace.cpp b/modules/gdscript/language_server/gdscript_workspace.cpp
@@ -29,6 +29,7 @@
 /*************************************************************************/
 
 #include "gdscript_workspace.h"
+
 #include "../gdscript.h"
 #include "../gdscript_parser.h"
 #include "core/project_settings.h"
@@ -41,12 +42,12 @@
 
 void GDScriptWorkspace::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("symbol"), &GDScriptWorkspace::symbol);
-	ClassDB::bind_method(D_METHOD("parse_script", "p_path", "p_content"), &GDScriptWorkspace::parse_script);
-	ClassDB::bind_method(D_METHOD("parse_local_script", "p_path"), &GDScriptWorkspace::parse_local_script);
-	ClassDB::bind_method(D_METHOD("get_file_path", "p_uri"), &GDScriptWorkspace::get_file_path);
-	ClassDB::bind_method(D_METHOD("get_file_uri", "p_path"), &GDScriptWorkspace::get_file_uri);
-	ClassDB::bind_method(D_METHOD("publish_diagnostics", "p_path"), &GDScriptWorkspace::publish_diagnostics);
-	ClassDB::bind_method(D_METHOD("generate_script_api", "p_path"), &GDScriptWorkspace::generate_script_api);
+	ClassDB::bind_method(D_METHOD("parse_script", "path", "content"), &GDScriptWorkspace::parse_script);
+	ClassDB::bind_method(D_METHOD("parse_local_script", "path"), &GDScriptWorkspace::parse_local_script);
+	ClassDB::bind_method(D_METHOD("get_file_path", "uri"), &GDScriptWorkspace::get_file_path);
+	ClassDB::bind_method(D_METHOD("get_file_uri", "path"), &GDScriptWorkspace::get_file_uri);
+	ClassDB::bind_method(D_METHOD("publish_diagnostics", "path"), &GDScriptWorkspace::publish_diagnostics);
+	ClassDB::bind_method(D_METHOD("generate_script_api", "path"), &GDScriptWorkspace::generate_script_api);
 }
 
 void GDScriptWorkspace::remove_cache_parser(const String &p_path) {