PublicArchive/godot
2
0
Fork 0
mirror of https://github.com/godotengine/godot.git synced 2026-05-06 07:56:56 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #118979 from HolonProduction/lsp/annoation-identifier-crash

Browse Source 
LSP: Fix crash in `find_usages_in_file` by correctly resolving annotation names
This commit is contained in:
Thaddeus Crews
2026-05-04 12:40:41 -05:00
parent 176b795177 0813b357d2
commit f1e8491c16
7 changed files with 126 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/gdscript/gdscript_editor.cpp
+1 -1
View File
@@ -4631,7 +4631,7 @@ static Error _lookup_symbol_from_base(const GDScriptParser::DataType &p_base, co
}
} break;
case GDScriptParser::COMPLETION_ANNOTATION: {
const String annotation_symbol = "@" + p_symbol;
const String annotation_symbol = "@" + p_symbol.trim_prefix("@");
if (parser.annotation_exists(annotation_symbol)) {
r_result.type = ScriptLanguage::LOOKUP_RESULT_CLASS_ANNOTATION;
r_result.class_name = "@GDScript";
modules/gdscript/language_server/gdscript_extend_parser.cpp
+44 -26
View File
@@ -632,14 +632,29 @@ String ExtendGDScriptParser::get_text_for_lookup_symbol(const LSP::Position &p_c
return longthing;
}
String ExtendGDScriptParser::get_identifier_under_position(const LSP::Position &p_position, LSP::Range &r_range) const {
String ExtendGDScriptParser::get_symbol_name_under_position(const LSP::Position &p_position, LSP::Range &r_range) const {
r_range = LSP::Range(p_position, p_position); // Default for error macros.
ERR_FAIL_INDEX_V(p_position.line, lines.size(), "");
String line = lines[p_position.line];
if (line.is_empty()) {
return "";
}
// Checks against line.size(), which includes a terminating NUL. This is to allow a cursor after the last character.
ERR_FAIL_INDEX_V(p_position.character, line.size(), "");
LSP::Position pos = p_position;
// Cursor after last character.
if (pos.character >= line.length()) {
pos.character--;
}
// If on the start of an annotation move the position into the identifier part. We account for "@" at the end.
if (line[pos.character] == '@' && pos.character + 1 < line.size() && is_unicode_identifier_start(line[pos.character + 1])) {
pos.character++;
}
// `p_position` cursor is BETWEEN chars, not ON chars.
// ->
// ```gdscript
@@ -651,51 +666,54 @@ String ExtendGDScriptParser::get_identifier_under_position(const LSP::Position &
// |
// | cursor on `member`, pos on ` ` (space)
// ```
// -> Move position to previous character if:
// * Position not on valid identifier char.
// * Prev position is valid identifier char.
LSP::Position pos = p_position;
if (
pos.character >= line.length() // Cursor at end of line.
|| (!is_unicode_identifier_continue(line[pos.character]) // Not on valid identifier char.
&& (pos.character > 0 // Not line start -> there is a prev char.
&& is_unicode_identifier_continue(line[pos.character - 1]) // Prev is valid identifier char.
))) {
if (!is_unicode_identifier_continue(line[pos.character])) {
if (pos.character == 0 || !is_unicode_identifier_continue(line[pos.character - 1])) {
// In between two non-identifier chars.
return "";
}
// Move position to previous character if not on valid char and the previous char is valid.
pos.character--;
}
int start_pos = pos.character;
// Iterate forward till we have a start. Symbol starts require lookahead, so we save the latest valid start and track back to it once we can stop looking.
// E.g. ?0nly
// ^
// | could be an identifier start (since 0 can't start identifiers), but if there was another symbol in front of 0 the identifier would be longer.
int last_valid_start_pos = -1;
for (int c = pos.character; c >= 0; c--) {
start_pos = c;
char32_t ch = line[c];
bool valid_char = is_unicode_identifier_continue(ch);
if (!valid_char) {
if (is_unicode_identifier_start(ch)) {
last_valid_start_pos = c;
}
if (!is_unicode_identifier_continue(ch)) {
break;
}
}
int end_pos = pos.character;
// Iterate backwards till we have an end. No lookahead required. Uses +1 since the end of a range is exclusive.
int end_pos = pos.character + 1;
for (int c = pos.character; c < line.length(); c++) {
char32_t ch = line[c];
bool valid_char = is_unicode_identifier_continue(ch);
if (!valid_char) {
if (!is_unicode_identifier_continue(ch)) {
break;
}
end_pos = c;
end_pos = c + 1;
}
if (!is_unicode_identifier_start(line[start_pos + 1])) {
if (last_valid_start_pos == -1) {
return "";
}
if (start_pos < end_pos) {
r_range.start.line = r_range.end.line = pos.line;
r_range.start.character = start_pos + 1;
r_range.end.character = end_pos + 1;
return line.substr(start_pos + 1, end_pos - start_pos);
// For annotations we include the @, since it is included in the symbol name used by other parts of the LSP code.
int start_pos = last_valid_start_pos;
if (start_pos > 0 && line[start_pos - 1] == '@') {
start_pos -= 1;
}
return "";
r_range.start.character = start_pos;
r_range.end.character = end_pos;
return line.substr(start_pos, end_pos - start_pos);
}
String ExtendGDScriptParser::get_uri() const {
modules/gdscript/language_server/gdscript_extend_parser.h
+7 -1
View File
@@ -139,7 +139,13 @@ public:
String get_text_for_completion(const LSP::Position &p_cursor) const;
String get_text_for_lookup_symbol(const LSP::Position &p_cursor, const String &p_symbol = "", bool p_func_required = false) const;
String get_identifier_under_position(const LSP::Position &p_position, LSP::Range &r_range) const;
/**
* Parses the symbol name at the given position. Returns that name and its full range.
*
* The returned name might be a false positive and not translate to an actual symbol e.g. it might be a language keyword.
* The results of this method are not equivalent to identifier AST nodes. Instead it returns results that are compatible with `LSP::DocumentSymbol::name` i.e. includes `@` for annotations.
*/
String get_symbol_name_under_position(const LSP::Position &p_position, LSP::Range &r_range) const;
String get_uri() const;
/**
modules/gdscript/language_server/gdscript_language_protocol.cpp
+5 -5
View File
@@ -629,12 +629,12 @@ void GDScriptLanguageProtocol::resolve_related_symbols(const LSP::TextDocumentPo
return;
}
String symbol_identifier;
String symbol_name;
LSP::Range range;
symbol_identifier = parser->get_identifier_under_position(p_doc_pos.position, range);
symbol_name = parser->get_symbol_name_under_position(p_doc_pos.position, range);
for (const KeyValue<StringName, ClassMembers> &E : workspace->native_members) {
if (const LSP::DocumentSymbol *const *symbol = E.value.getptr(symbol_identifier)) {
if (const LSP::DocumentSymbol *const *symbol = E.value.getptr(symbol_name)) {
r_list.push_back(*symbol);
}
}
@@ -642,13 +642,13 @@ void GDScriptLanguageProtocol::resolve_related_symbols(const LSP::TextDocumentPo
for (const KeyValue<String, ExtendGDScriptParser *> &E : client->parse_results) {
const ExtendGDScriptParser *scr = E.value;
const ClassMembers &members = scr->get_members();
if (const LSP::DocumentSymbol *const *symbol = members.getptr(symbol_identifier)) {
if (const LSP::DocumentSymbol *const *symbol = members.getptr(symbol_name)) {
r_list.push_back(*symbol);
}
for (const KeyValue<String, ClassMembers> &F : scr->get_inner_classes()) {
const ClassMembers *inner_class = &F.value;
if (const LSP::DocumentSymbol *const *symbol = inner_class->getptr(symbol_identifier)) {
if (const LSP::DocumentSymbol *const *symbol = inner_class->getptr(symbol_name)) {
r_list.push_back(*symbol);
}
}
modules/gdscript/language_server/gdscript_workspace.cpp
+29 -21
View File
@@ -423,7 +423,7 @@ bool GDScriptWorkspace::can_rename(const LSP::TextDocumentPositionParams &p_doc_
String path = get_file_path(p_doc_pos.textDocument.uri);
const ExtendGDScriptParser *parser = GDScriptLanguageProtocol::get_singleton()->get_parse_result(path);
if (parser) {
_ALLOW_DISCARD_ parser->get_identifier_under_position(p_doc_pos.position, r_range);
_ALLOW_DISCARD_ parser->get_symbol_name_under_position(p_doc_pos.position, r_range);
r_symbol = *reference_symbol;
return true;
}
@@ -453,7 +453,7 @@ Vector<LSP::Location> GDScriptWorkspace::find_usages_in_file(const LSP::Document
params.position.character = character;
LSP::Range range;
String identifier_under_cursor = parser->get_identifier_under_position(params.position, range);
String identifier_under_cursor = parser->get_symbol_name_under_position(params.position, range);
if (identifier_under_cursor == identifier) {
const LSP::DocumentSymbol *other_symbol = resolve_symbol(params);
@@ -468,7 +468,14 @@ Vector<LSP::Location> GDScriptWorkspace::find_usages_in_file(const LSP::Document
}
}
character = line.find(identifier, range.end.character);
if (identifier_under_cursor.length() < identifier.length()) {
// `get_symbol_name_under_position` is supposed to recognize all possible symbol names. Since a simple string search already confirmed
// the presence of `p_symbol.name` in the text, this case has to be a bug.
ERR_PRINT(vformat("LSP Bug, please report. \"get_symbol_name_under_position\" did not correctly resolve \"%s\"", identifier));
character = line.find(identifier, character + 1);
} else {
character = line.find(identifier, range.end.character);
}
}
}
}
@@ -641,29 +648,30 @@ const LSP::DocumentSymbol *GDScriptWorkspace::resolve_symbol(const LSP::TextDocu
const ExtendGDScriptParser *parser = GDScriptLanguageProtocol::get_singleton()->get_parse_result(path);
if (parser) {
String symbol_identifier = p_symbol_name;
if (symbol_identifier.get_slice_count("(") > 0) {
symbol_identifier = symbol_identifier.get_slicec('(', 0);
String symbol_name = p_symbol_name;
if (symbol_name.get_slice_count("(") > 0) {
symbol_name = symbol_name.get_slicec('(', 0);
}
LSP::Position pos = p_doc_pos.position;
if (symbol_identifier.is_empty()) {
if (symbol_name.is_empty()) {
LSP::Range range;
symbol_identifier = parser->get_identifier_under_position(p_doc_pos.position, range);
symbol_name = parser->get_symbol_name_under_position(p_doc_pos.position, range);
pos.character = range.end.character;
}
if (!symbol_identifier.is_empty()) {
if (ScriptServer::is_global_class(symbol_identifier)) {
String class_path = ScriptServer::get_global_class_path(symbol_identifier);
if (!symbol_name.is_empty()) {
if (ScriptServer::is_global_class(symbol_name)) {
String class_path = ScriptServer::get_global_class_path(symbol_name);
symbol = get_script_symbol(class_path);
} else {
ScriptLanguage::LookupResult ret;
if (symbol_identifier == "new" && parser->get_lines()[p_doc_pos.position.line].remove_chars(" \t").contains("new(")) {
symbol_identifier = "_init";
// TODO: `lookup_code` should already account for this. We might be able to simplify code here.
if (symbol_name == "new" && parser->get_lines()[p_doc_pos.position.line].remove_chars(" \t").contains("new(")) {
symbol_name = "_init";
}
if (OK == GDScriptLanguage::get_singleton()->lookup_code(parser->get_text_for_lookup_symbol(pos, symbol_identifier, p_func_required), symbol_identifier, path, nullptr, ret)) {
if (OK == GDScriptLanguage::get_singleton()->lookup_code(parser->get_text_for_lookup_symbol(pos, symbol_name, p_func_required), symbol_name, path, nullptr, ret)) {
if (ret.location >= 0) {
String target_script_path = path;
if (ret.script.is_valid()) {
@@ -674,13 +682,13 @@ const LSP::DocumentSymbol *GDScriptWorkspace::resolve_symbol(const LSP::TextDocu
const ExtendGDScriptParser *target_parser = GDScriptLanguageProtocol::get_singleton()->get_parse_result(target_script_path);
if (target_parser) {
symbol = target_parser->get_symbol_defined_at_line(LINE_NUMBER_TO_INDEX(ret.location), symbol_identifier);
symbol = target_parser->get_symbol_defined_at_line(LINE_NUMBER_TO_INDEX(ret.location), symbol_name);
if (symbol) {
switch (symbol->kind) {
case LSP::SymbolKind::Function: {
if (symbol->name != symbol_identifier) {
symbol = get_parameter_symbol(symbol, symbol_identifier);
if (symbol->name != symbol_name) {
symbol = get_parameter_symbol(symbol, symbol_name);
}
} break;
}
@@ -688,15 +696,15 @@ const LSP::DocumentSymbol *GDScriptWorkspace::resolve_symbol(const LSP::TextDocu
}
} else {
String member = ret.class_member;
if (member.is_empty() && symbol_identifier != ret.class_name) {
member = symbol_identifier;
if (member.is_empty() && symbol_name != ret.class_name) {
member = symbol_name;
}
symbol = get_native_symbol(ret.class_name, member);
}
} else {
symbol = get_local_symbol_at(parser, symbol_identifier, p_doc_pos.position);
symbol = get_local_symbol_at(parser, symbol_name, p_doc_pos.position);
if (!symbol) {
symbol = parser->get_member_symbol(symbol_identifier);
symbol = parser->get_member_symbol(symbol_name);
}
}
}
modules/gdscript/language_server/godot_lsp.h
+7
View File
@@ -110,6 +110,9 @@ struct Position {
dict["character"] = character;
return dict;
}
Position() = default;
Position(int p_line, int p_character) : line(p_line), character(p_character) {}
};
/**
@@ -160,6 +163,10 @@ struct Range {
dict["end"] = end.to_json();
return dict;
}
Range() = default;
Range(Position p_start, Position p_end) : start(p_start), end(p_end) {}
Range(int p_start_line, int p_start_column, int p_end_line, int p_end_column) : start(p_start_line, p_start_column), end(p_end_line, p_end_column) {}
};
/**
modules/gdscript/tests/test_lsp.h
+33
View File
@@ -515,6 +515,39 @@ TEST_SUITE("[Modules][GDScript][LSP][Editor]") {
CHECK_EQ(LSP::marked_documentation("Class [Sprite2D] with [url=https://godotengine.org]link[/url]"),
"Class `Sprite2D` with [link](https://godotengine.org)");
}
TEST_CASE("get_symbol_name_under_position") {
const String code = U"we_do suPPort Unicöde and @annotations, numb3rs \n0nly for-continuation #comment\n@start@\na\n b \n";
const HashMap<String, LSP::Range> symbols = {
{ "we_do", LSP::Range(0, 0, 0, 5) },
{ U"suPPort", LSP::Range(0, 6, 0, 13) },
{ U"Unicöde", LSP::Range(0, 14, 0, 21) },
{ U"and", LSP::Range(0, 23, 0, 26) },
{ "@annotations", LSP::Range(0, 27, 0, 39) },
{ U"numb3rs", LSP::Range(0, 41, 0, 48) },
{ U"nly", LSP::Range(1, 1, 1, 4) },
{ U"for", LSP::Range(1, 5, 1, 8) },
{ U"continuation", LSP::Range(1, 9, 1, 21) },
{ U"comment", LSP::Range(1, 23, 1, 30) },
{ U"@start", LSP::Range(2, 0, 2, 6) },
{ U"a", LSP::Range(3, 0, 3, 1) },
{ U"b", LSP::Range(4, 1, 4, 2) },
};
ExtendGDScriptParser *parser = memnew(ExtendGDScriptParser);
parser->parse(code, "res://dummy.gd");
for (KeyValue<String, LSP::Range> symbol : symbols) {
for (int i = symbol.value.start.character; i <= symbol.value.end.character; i++) {
LSP::Range range;
const String symbol_name = parser->get_symbol_name_under_position(LSP::Position(symbol.value.start.line, i), range);
CHECK_EQ(symbol.key, symbol_name);
CHECK_EQ(range, symbol.value);
}
}
memdelete(parser);
}
}
} // namespace GDScriptTests