Browse Source

LSP: Fix file URI handling + warn about workspace project mismatch

HolonProduction 6 months ago
parent
commit
d55883b4b1

+ 10 - 3
editor/editor_file_system.cpp

@@ -1934,9 +1934,16 @@ bool EditorFileSystem::_find_file(const String &p_file, EditorFileSystemDirector
 
 	int cpos = -1;
 	for (int i = 0; i < fs->files.size(); i++) {
-		if (fs->files[i]->file == file) {
-			cpos = i;
-			break;
+		if (fs_case_sensitive) {
+			if (fs->files[i]->file == file) {
+				cpos = i;
+				break;
+			}
+		} else {
+			if (fs->files[i]->file.to_lower() == file.to_lower()) {
+				cpos = i;
+				break;
+			}
 		}
 	}
 

+ 22 - 0
modules/gdscript/language_server/gdscript_language_protocol.cpp

@@ -171,6 +171,28 @@ void GDScriptLanguageProtocol::_bind_methods() {
 Dictionary GDScriptLanguageProtocol::initialize(const Dictionary &p_params) {
 	LSP::InitializeResult ret;
 
+	{
+		// Warn if the workspace root does not match with the project that is currently open in Godot,
+		// since it might lead to unexpected behavior, like wrong warnings about duplicate class names.
+
+		String root;
+		Variant root_uri_var = p_params["rootUri"];
+		Variant root_var = p_params["rootPath"];
+		if (root_uri_var.is_string()) {
+			root = get_workspace()->get_file_path(root_uri_var);
+		} else if (root_var.is_string()) {
+			root = root_var;
+		}
+
+		if (ProjectSettings::get_singleton()->localize_path(root) != "res://") {
+			LSP::ShowMessageParams params{
+				LSP::MessageType::Warning,
+				"The GDScript Language Server might not work correctly with other projects than the one opened in Godot."
+			};
+			notify_client("window/showMessage", params.to_json());
+		}
+	}
+
 	String root_uri = p_params["rootUri"];
 	String root = p_params["rootPath"];
 	bool is_same_workspace;

+ 7 - 5
modules/gdscript/language_server/gdscript_text_document.cpp

@@ -494,12 +494,14 @@ Array GDScriptTextDocument::find_symbols(const LSP::TextDocumentPositionParams &
 	if (symbol) {
 		LSP::Location location;
 		location.uri = symbol->uri;
-		location.range = symbol->selectionRange;
-		const String &path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(symbol->uri);
-		if (file_checker->file_exists(path)) {
-			arr.push_back(location.to_json());
+		if (!location.uri.is_empty()) {
+			location.range = symbol->selectionRange;
+			const String &path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(symbol->uri);
+			if (file_checker->file_exists(path)) {
+				arr.push_back(location.to_json());
+			}
+			r_list.push_back(symbol);
 		}
-		r_list.push_back(symbol);
 	} else if (GDScriptLanguageProtocol::get_singleton()->is_smart_resolve_enabled()) {
 		List<const LSP::DocumentSymbol *> list;
 		GDScriptLanguageProtocol::get_singleton()->get_workspace()->resolve_related_symbols(p_location, list);

+ 84 - 8
modules/gdscript/language_server/gdscript_workspace.cpp

@@ -562,17 +562,93 @@ Error GDScriptWorkspace::parse_local_script(const String &p_path) {
 	return err;
 }
 
-String GDScriptWorkspace::get_file_path(const String &p_uri) const {
-	String path = p_uri.uri_file_decode();
-	String base_uri = root_uri.uri_file_decode();
-	path = path.replacen(base_uri + "/", "res://");
-	return path;
+String GDScriptWorkspace::get_file_path(const String &p_uri) {
+	int port;
+	String scheme;
+	String host;
+	String encoded_path;
+	String fragment;
+
+	// Don't use the returned error, the result isn't OK for URIs that are not valid web URLs.
+	p_uri.parse_url(scheme, host, port, encoded_path, fragment);
+
+	// TODO: Make the parsing RFC-3986 compliant.
+	ERR_FAIL_COND_V_MSG(scheme != "file" && scheme != "file:" && scheme != "file://", String(), "LSP: The language server only supports the file protocol: " + p_uri);
+
+	// Treat host like authority for now and ignore the port. It's an edge case for invalid file URI's anyway.
+	ERR_FAIL_COND_V_MSG(host != "" && host != "localhost", String(), "LSP: The language server does not support nonlocal files: " + p_uri);
+
+	// If query or fragment are present, the URI is not a valid file URI as per RFC-8089.
+	// We currently don't handle the query and it will be part of the path. However,
+	// this should not be a problem for a correct file URI.
+	ERR_FAIL_COND_V_MSG(fragment != "", String(), "LSP: Received malformed file URI: " + p_uri);
+
+	String canonical_res = ProjectSettings::get_singleton()->get_resource_path();
+	String simple_path = encoded_path.uri_file_decode().simplify_path();
+
+	// First try known paths that point to res://, to reduce file system interaction.
+	bool res_adjusted = false;
+	for (const String &res_path : absolute_res_paths) {
+		if (simple_path.begins_with(res_path)) {
+			res_adjusted = true;
+			simple_path = "res://" + simple_path.substr(res_path.size());
+			break;
+		}
+	}
+
+	// Traverse the path and compare each directory with res://
+	if (!res_adjusted) {
+		Ref<DirAccess> dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
+
+		int offset = 0;
+		while (offset <= simple_path.length()) {
+			offset = simple_path.find_char('/', offset);
+			if (offset == -1) {
+				offset = simple_path.length();
+			}
+
+			String part = simple_path.substr(0, offset);
+
+			if (!part.is_empty()) {
+				bool is_equal = dir->is_equivalent(canonical_res, part);
+
+				if (is_equal) {
+					absolute_res_paths.insert(part);
+					res_adjusted = true;
+					simple_path = "res://" + simple_path.substr(offset + 1);
+					break;
+				}
+			}
+
+			offset += 1;
+		}
+
+		// Could not resolve the path to the project.
+		if (!res_adjusted) {
+			return simple_path;
+		}
+	}
+
+	// Resolve the file inside of the project using EditorFileSystem.
+	EditorFileSystemDirectory *editor_dir;
+	int file_idx;
+	editor_dir = EditorFileSystem::get_singleton()->find_file(simple_path, &file_idx);
+	if (editor_dir) {
+		return editor_dir->get_file_path(file_idx);
+	}
+
+	return simple_path;
 }
 
 String GDScriptWorkspace::get_file_uri(const String &p_path) const {
-	String uri = p_path;
-	uri = uri.replace("res://", root_uri + "/");
-	return uri;
+	String path = ProjectSettings::get_singleton()->globalize_path(p_path).lstrip("/");
+	LocalVector<String> encoded_parts;
+	for (const String &part : path.split("/")) {
+		encoded_parts.push_back(part.uri_encode());
+	}
+
+	// Always return file URI's with authority part (encoding drive letters with leading slash), to maintain compat with RFC-1738 which required it.
+	return "file:///" + String("/").join(encoded_parts);
 }
 
 void GDScriptWorkspace::publish_diagnostics(const String &p_path) {

+ 4 - 1
modules/gdscript/language_server/gdscript_workspace.h

@@ -50,6 +50,9 @@ protected:
 	bool initialized = false;
 	HashMap<StringName, LSP::DocumentSymbol> native_symbols;
 
+	// Absolute paths that are known to point to res://
+	HashSet<String> absolute_res_paths;
+
 	const LSP::DocumentSymbol *get_native_symbol(const String &p_class, const String &p_member = "") const;
 	const LSP::DocumentSymbol *get_script_symbol(const String &p_path) const;
 	const LSP::DocumentSymbol *get_parameter_symbol(const LSP::DocumentSymbol *p_parent, const String &symbol_identifier);
@@ -78,7 +81,7 @@ public:
 	Error parse_script(const String &p_path, const String &p_content);
 	Error parse_local_script(const String &p_path);
 
-	String get_file_path(const String &p_uri) const;
+	String get_file_path(const String &p_uri);
 	String get_file_uri(const String &p_path) const;
 
 	void publish_diagnostics(const String &p_path);

+ 38 - 0
modules/gdscript/language_server/godot_lsp.h

@@ -238,6 +238,25 @@ struct ReferenceContext {
 	bool includeDeclaration = false;
 };
 
+struct ShowMessageParams {
+	/**
+	 * The message type. See {@link MessageType}.
+	 */
+	int type;
+
+	/**
+	 * The actual message.
+	 */
+	String message;
+
+	_FORCE_INLINE_ Dictionary to_json() const {
+		Dictionary dict;
+		dict["type"] = type;
+		dict["message"] = message;
+		return dict;
+	}
+};
+
 struct ReferenceParams : TextDocumentPositionParams {
 	ReferenceContext context;
 };
@@ -405,6 +424,25 @@ static const int Full = 1;
 static const int Incremental = 2;
 }; // namespace TextDocumentSyncKind
 
+namespace MessageType {
+/**
+ * An error message.
+ */
+static const int Error = 1;
+/**
+ * A warning message.
+ */
+static const int Warning = 2;
+/**
+ * An information message.
+ */
+static const int Info = 3;
+/**
+ * A log message.
+ */
+static const int Log = 4;
+}; // namespace MessageType
+
 /**
  * Completion options.
  */

+ 5 - 1
modules/gdscript/tests/test_lsp.h

@@ -334,7 +334,7 @@ void test_position_roundtrip(LSP::Position p_lsp, GodotPosition p_gd, const Pack
 // * Line & Char:
 //   * LSP: both 0-based
 //   * Godot: both 1-based
-TEST_SUITE("[Modules][GDScript][LSP]") {
+TEST_SUITE("[Modules][GDScript][LSP][Editor]") {
 	TEST_CASE("Can convert positions to and from Godot") {
 		String code = R"(extends Node
 
@@ -405,6 +405,7 @@ func f():
 		}
 	}
 	TEST_CASE("[workspace][resolve_symbol]") {
+		EditorFileSystem *efs = memnew(EditorFileSystem);
 		GDScriptLanguageProtocol *proto = initialize(root);
 		REQUIRE(proto);
 		Ref<GDScriptWorkspace> workspace = GDScriptLanguageProtocol::get_singleton()->get_workspace();
@@ -485,9 +486,11 @@ func f():
 		}
 
 		memdelete(proto);
+		memdelete(efs);
 		finish_language();
 	}
 	TEST_CASE("[workspace][document_symbol]") {
+		EditorFileSystem *efs = memnew(EditorFileSystem);
 		GDScriptLanguageProtocol *proto = initialize(root);
 		REQUIRE(proto);
 
@@ -524,6 +527,7 @@ func f():
 		}
 
 		memdelete(proto);
+		memdelete(efs);
 		finish_language();
 	}
 }