Skip to content

Commit a1edb9c

Browse files
LSP: Fix file URI handling + warn about workspace project mismatch
1 parent 019ab87 commit a1edb9c

File tree

7 files changed

+176
-18
lines changed

7 files changed

+176
-18
lines changed

editor/editor_file_system.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1934,9 +1934,16 @@ bool EditorFileSystem::_find_file(const String &p_file, EditorFileSystemDirector
19341934

19351935
int cpos = -1;
19361936
for (int i = 0; i < fs->files.size(); i++) {
1937-
if (fs->files[i]->file == file) {
1938-
cpos = i;
1939-
break;
1937+
if (fs_case_sensitive) {
1938+
if (fs->files[i]->file == file) {
1939+
cpos = i;
1940+
break;
1941+
}
1942+
} else {
1943+
if (fs->files[i]->file.to_lower() == file.to_lower()) {
1944+
cpos = i;
1945+
break;
1946+
}
19401947
}
19411948
}
19421949

modules/gdscript/language_server/gdscript_language_protocol.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,28 @@ void GDScriptLanguageProtocol::_bind_methods() {
171171
Dictionary GDScriptLanguageProtocol::initialize(const Dictionary &p_params) {
172172
LSP::InitializeResult ret;
173173

174+
{
175+
// Warn if the workspace root does not match with the project that is currently open in Godot,
176+
// since it might lead to unexpected behavior, like wrong warnings about duplicate class names.
177+
178+
String root;
179+
Variant root_uri_var = p_params["rootUri"];
180+
Variant root_var = p_params["rootPath"];
181+
if (root_uri_var.is_string()) {
182+
root = get_workspace()->get_file_path(root_uri_var);
183+
} else if (root_var.is_string()) {
184+
root = root_var;
185+
}
186+
187+
if (ProjectSettings::get_singleton()->localize_path(root) != "res://") {
188+
LSP::ShowMessageParams params{
189+
LSP::MessageType::Warning,
190+
"The GDScript Language Server might not work correctly with other projects than the one opened in Godot."
191+
};
192+
notify_client("window/showMessage", params.to_json());
193+
}
194+
}
195+
174196
String root_uri = p_params["rootUri"];
175197
String root = p_params["rootPath"];
176198
bool is_same_workspace;

modules/gdscript/language_server/gdscript_text_document.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -494,12 +494,14 @@ Array GDScriptTextDocument::find_symbols(const LSP::TextDocumentPositionParams &
494494
if (symbol) {
495495
LSP::Location location;
496496
location.uri = symbol->uri;
497-
location.range = symbol->selectionRange;
498-
const String &path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(symbol->uri);
499-
if (file_checker->file_exists(path)) {
500-
arr.push_back(location.to_json());
497+
if (!location.uri.is_empty()) {
498+
location.range = symbol->selectionRange;
499+
const String &path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(symbol->uri);
500+
if (file_checker->file_exists(path)) {
501+
arr.push_back(location.to_json());
502+
}
503+
r_list.push_back(symbol);
501504
}
502-
r_list.push_back(symbol);
503505
} else if (GDScriptLanguageProtocol::get_singleton()->is_smart_resolve_enabled()) {
504506
List<const LSP::DocumentSymbol *> list;
505507
GDScriptLanguageProtocol::get_singleton()->get_workspace()->resolve_related_symbols(p_location, list);

modules/gdscript/language_server/gdscript_workspace.cpp

Lines changed: 90 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -562,17 +562,99 @@ Error GDScriptWorkspace::parse_local_script(const String &p_path) {
562562
return err;
563563
}
564564

565-
String GDScriptWorkspace::get_file_path(const String &p_uri) const {
566-
String path = p_uri.uri_file_decode();
567-
String base_uri = root_uri.uri_file_decode();
568-
path = path.replacen(base_uri + "/", "res://");
569-
return path;
565+
String GDScriptWorkspace::get_file_path(const String &p_uri) {
566+
int port;
567+
String scheme, host, encoded_path, fragment;
568+
569+
// Don't use the returned error, the result isn't OK for URIs that are not valid web URLs.
570+
p_uri.parse_url(scheme, host, port, encoded_path, fragment);
571+
572+
// TODO: Make the parsing RFC-3986 compliant.
573+
if (scheme != "file" && scheme != "file:" && scheme != "file://") {
574+
ERR_PRINT("LSP: The language server only supports the file protocol: " + p_uri);
575+
return String();
576+
}
577+
578+
// Treat host like authority for now and ignore the port. It's an edge case for invalid file URI's anyway.
579+
if (host != "" && host != "localhost") {
580+
ERR_PRINT("LSP: The language server does not support nonlocal files: " + p_uri);
581+
return String();
582+
}
583+
584+
// If query or fragment are present, the URI is not a valid file URI as per RFC-8089.
585+
// We currently don't handle the query and it will be part of the path. However,
586+
// this should not be a problem for a correct file URI.
587+
if (fragment != "") {
588+
ERR_PRINT("LSP: Received malformed file URI: " + p_uri);
589+
return String();
590+
}
591+
592+
String canonical_res = ProjectSettings::get_singleton()->get_resource_path();
593+
String simple_path = encoded_path.uri_file_decode().simplify_path();
594+
595+
// First try known paths that point to res://, to reduce file system interaction.
596+
bool res_adjusted = false;
597+
for (const String &res_path : absolute_res_paths) {
598+
if (simple_path.begins_with(res_path)) {
599+
res_adjusted = true;
600+
simple_path = "res://" + simple_path.substr(res_path.size());
601+
break;
602+
}
603+
}
604+
605+
// Traverse the path and compare each directory with res://
606+
if (!res_adjusted) {
607+
Ref<DirAccess> dir = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
608+
609+
int offset = 0;
610+
while (offset <= simple_path.length()) {
611+
offset = simple_path.find_char('/', offset);
612+
if (offset == -1) {
613+
offset = simple_path.length();
614+
}
615+
616+
String part = simple_path.substr(0, offset);
617+
618+
if (!part.is_empty()) {
619+
bool is_equal = dir->is_equivalent(canonical_res, part);
620+
621+
if (is_equal) {
622+
absolute_res_paths.insert(part);
623+
res_adjusted = true;
624+
simple_path = "res://" + simple_path.substr(offset + 1);
625+
break;
626+
}
627+
}
628+
629+
offset += 1;
630+
}
631+
632+
// Could not resolve the path to the project.
633+
if (!res_adjusted) {
634+
return simple_path;
635+
}
636+
}
637+
638+
// Resolve the file inside of the project using EditorFileSystem.
639+
EditorFileSystemDirectory *editor_dir;
640+
int file_idx;
641+
editor_dir = EditorFileSystem::get_singleton()->find_file(simple_path, &file_idx);
642+
if (editor_dir) {
643+
return editor_dir->get_file_path(file_idx);
644+
}
645+
646+
return simple_path;
570647
}
571648

572649
String GDScriptWorkspace::get_file_uri(const String &p_path) const {
573-
String uri = p_path;
574-
uri = uri.replace("res://", root_uri + "/");
575-
return uri;
650+
String path = ProjectSettings::get_singleton()->globalize_path(p_path).lstrip("/");
651+
LocalVector<String> encoded_parts;
652+
for (const String &part : path.split("/")) {
653+
encoded_parts.push_back(part.uri_encode());
654+
}
655+
656+
// Always return file URI's with authority part (encoding drive letters with leading slash), to maintain compat with RFC-1738 which required it.
657+
return "file:///" + String("/").join(encoded_parts);
576658
}
577659

578660
void GDScriptWorkspace::publish_diagnostics(const String &p_path) {

modules/gdscript/language_server/gdscript_workspace.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ class GDScriptWorkspace : public RefCounted {
5050
bool initialized = false;
5151
HashMap<StringName, LSP::DocumentSymbol> native_symbols;
5252

53+
// Absolute paths that are known to point to res://
54+
HashSet<String> absolute_res_paths;
55+
5356
const LSP::DocumentSymbol *get_native_symbol(const String &p_class, const String &p_member = "") const;
5457
const LSP::DocumentSymbol *get_script_symbol(const String &p_path) const;
5558
const LSP::DocumentSymbol *get_parameter_symbol(const LSP::DocumentSymbol *p_parent, const String &symbol_identifier);
@@ -78,7 +81,7 @@ class GDScriptWorkspace : public RefCounted {
7881
Error parse_script(const String &p_path, const String &p_content);
7982
Error parse_local_script(const String &p_path);
8083

81-
String get_file_path(const String &p_uri) const;
84+
String get_file_path(const String &p_uri);
8285
String get_file_uri(const String &p_path) const;
8386

8487
void publish_diagnostics(const String &p_path);

modules/gdscript/language_server/godot_lsp.h

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,25 @@ struct ReferenceContext {
238238
bool includeDeclaration = false;
239239
};
240240

241+
struct ShowMessageParams {
242+
/**
243+
* The message type. See {@link MessageType}.
244+
*/
245+
int type;
246+
247+
/**
248+
* The actual message.
249+
*/
250+
String message;
251+
252+
_FORCE_INLINE_ Dictionary to_json() const {
253+
Dictionary dict;
254+
dict["type"] = type;
255+
dict["message"] = message;
256+
return dict;
257+
}
258+
};
259+
241260
struct ReferenceParams : TextDocumentPositionParams {
242261
ReferenceContext context;
243262
};
@@ -405,6 +424,25 @@ static const int Full = 1;
405424
static const int Incremental = 2;
406425
}; // namespace TextDocumentSyncKind
407426

427+
namespace MessageType {
428+
/**
429+
* An error message.
430+
*/
431+
static const int Error = 1;
432+
/**
433+
* A warning message.
434+
*/
435+
static const int Warning = 2;
436+
/**
437+
* An information message.
438+
*/
439+
static const int Info = 3;
440+
/**
441+
* A log message.
442+
*/
443+
static const int Log = 4;
444+
}; // namespace MessageType
445+
408446
/**
409447
* Completion options.
410448
*/

modules/gdscript/tests/test_lsp.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ void test_position_roundtrip(LSP::Position p_lsp, GodotPosition p_gd, const Pack
334334
// * Line & Char:
335335
// * LSP: both 0-based
336336
// * Godot: both 1-based
337-
TEST_SUITE("[Modules][GDScript][LSP]") {
337+
TEST_SUITE("[Modules][GDScript][LSP][Editor]") {
338338
TEST_CASE("Can convert positions to and from Godot") {
339339
String code = R"(extends Node
340340
@@ -405,6 +405,7 @@ func f():
405405
}
406406
}
407407
TEST_CASE("[workspace][resolve_symbol]") {
408+
EditorFileSystem *efs = memnew(EditorFileSystem);
408409
GDScriptLanguageProtocol *proto = initialize(root);
409410
REQUIRE(proto);
410411
Ref<GDScriptWorkspace> workspace = GDScriptLanguageProtocol::get_singleton()->get_workspace();
@@ -485,9 +486,11 @@ func f():
485486
}
486487

487488
memdelete(proto);
489+
memdelete(efs);
488490
finish_language();
489491
}
490492
TEST_CASE("[workspace][document_symbol]") {
493+
EditorFileSystem *efs = memnew(EditorFileSystem);
491494
GDScriptLanguageProtocol *proto = initialize(root);
492495
REQUIRE(proto);
493496

@@ -524,6 +527,7 @@ func f():
524527
}
525528

526529
memdelete(proto);
530+
memdelete(efs);
527531
finish_language();
528532
}
529533
}

0 commit comments

Comments
 (0)