From 2f620db1d894533a8e859843b840801bf54cd86c Mon Sep 17 00:00:00 2001 From: HolonProduction Date: Fri, 15 Nov 2024 19:32:00 +0100 Subject: [PATCH] LSP: Fix spec violations that break the VSCode outline --- .../gdscript_extend_parser.cpp | 11 ++++++- .../tests/scripts/lsp/first_line_comment.gd | 2 ++ modules/gdscript/tests/test_lsp.h | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 modules/gdscript/tests/scripts/lsp/first_line_comment.gd diff --git a/modules/gdscript/language_server/gdscript_extend_parser.cpp b/modules/gdscript/language_server/gdscript_extend_parser.cpp index 2a3db4f508d..46ae4a88399 100644 --- a/modules/gdscript/language_server/gdscript_extend_parser.cpp +++ b/modules/gdscript/language_server/gdscript_extend_parser.cpp @@ -57,6 +57,12 @@ lsp::Position GodotPosition::to_lsp(const Vector &p_lines) const { return res; } res.line = line - 1; + + // Special case: `column = 0` -> Starts at beginning of line. + if (column <= 0) { + return res; + } + // Note: character outside of `pos_line.length()-1` is valid. res.character = column - 1; @@ -238,9 +244,12 @@ void ExtendGDScriptParser::parse_class_symbol(const GDScriptParser::ClassNode *p r_symbol.kind = lsp::SymbolKind::Class; r_symbol.deprecated = false; r_symbol.range = range_of_node(p_class); - r_symbol.range.start.line = MAX(r_symbol.range.start.line, 0); if (p_class->identifier) { r_symbol.selectionRange = range_of_node(p_class->identifier); + } else { + // No meaningful `selectionRange`, but we must ensure that it is inside of `range`. + r_symbol.selectionRange.start = r_symbol.range.start; + r_symbol.selectionRange.end = r_symbol.range.start; } r_symbol.detail = "class " + r_symbol.name; { diff --git a/modules/gdscript/tests/scripts/lsp/first_line_comment.gd b/modules/gdscript/tests/scripts/lsp/first_line_comment.gd new file mode 100644 index 00000000000..34ead5fabb3 --- /dev/null +++ b/modules/gdscript/tests/scripts/lsp/first_line_comment.gd @@ -0,0 +1,2 @@ +# Some comment +extends Node diff --git a/modules/gdscript/tests/test_lsp.h b/modules/gdscript/tests/test_lsp.h index b85c727bc5b..b49f6cce4d1 100644 --- a/modules/gdscript/tests/test_lsp.h +++ b/modules/gdscript/tests/test_lsp.h @@ -375,6 +375,18 @@ func f(): gd.to_lsp(lines); } + SUBCASE("special case: zero column for root class") { + GodotPosition gd(1, 0); + lsp::Position expected = lsp_pos(0, 0); + lsp::Position actual = gd.to_lsp(lines); + CHECK_EQ(actual, expected); + } + SUBCASE("special case: zero line and column for root class") { + GodotPosition gd(0, 0); + lsp::Position expected = lsp_pos(0, 0); + lsp::Position actual = gd.to_lsp(lines); + CHECK_EQ(actual, expected); + } SUBCASE("special case: negative line for root class") { GodotPosition gd(-1, 0); lsp::Position expected = lsp_pos(0, 0); @@ -468,6 +480,25 @@ func f(): test_resolve_symbols(uri, all_test_data, all_test_data); } + memdelete(proto); + finish_language(); + } + TEST_CASE("[workspace][document_symbol]") { + GDScriptLanguageProtocol *proto = initialize(root); + REQUIRE(proto); + + SUBCASE("selectionRange of root class must be inside range") { + String path = "res://lsp/first_line_comment.gd"; + assert_no_errors_in(path); + GDScriptLanguageProtocol::get_singleton()->get_workspace()->parse_local_script(path); + ExtendGDScriptParser *parser = GDScriptLanguageProtocol::get_singleton()->get_workspace()->parse_results[path]; + REQUIRE(parser); + lsp::DocumentSymbol cls = parser->get_symbols(); + + REQUIRE(((cls.range.start.line == cls.selectionRange.start.line && cls.range.start.character <= cls.selectionRange.start.character) || (cls.range.start.line < cls.selectionRange.start.line))); + REQUIRE(((cls.range.end.line == cls.selectionRange.end.line && cls.range.end.character >= cls.selectionRange.end.character) || (cls.range.end.line > cls.selectionRange.end.line))); + } + memdelete(proto); finish_language(); }