Fix: resolve_symbol resolves to wrong symbol in certain cases

Handled here:
* in getter/setter
* in lambdas inside functions
* parameters in signals
* in different scopes
* inside match
* when script contains multiple variables with same name

Though not all cases are fixed. For example:
* lambdas in fields
* match cases
* shadowed variables  
  Though: shadowing is discouraged in gdscript

modules/gdscript/language_server/gdscript_extend_parser.cpp

@@ -264,6 +264,21 @@ void ExtendGDScriptParser::parse_class_symbol(const GDScriptParser::ClassNode *p
 				symbol.uri = uri;
 				symbol.script_path = path;
 
+				if (m.variable->getter) {
+					lsp::DocumentSymbol get_symbol;
+					parse_function_symbol(m.variable->getter, get_symbol);
+					//TODO: adjust name?
+					get_symbol.local = true;
+					symbol.children.push_back(get_symbol);
+				}
+				if (m.variable->setter) {
+					lsp::DocumentSymbol set_symbol;
+					parse_function_symbol(m.variable->setter, set_symbol);
+					//TODO: adjust name?
+					set_symbol.local = true;
+					symbol.children.push_back(set_symbol);
+				}
+
 				r_symbol.children.push_back(symbol);
 			} break;
 			case ClassNode::Member::CONSTANT: {
@@ -342,6 +357,22 @@ void ExtendGDScriptParser::parse_class_symbol(const GDScriptParser::ClassNode *p
 				}
 				symbol.detail += ")";
 
+				for (GDScriptParser::ParameterNode *param : m.signal->parameters) {
+					lsp::DocumentSymbol param_symbol;
+					param_symbol.name = param->identifier->name;
+					param_symbol.kind = lsp::SymbolKind::Variable;
+					param_symbol.deprecated = false;
+					param_symbol.local = true;
+					param_symbol.range = range_of_node(param);
+					param_symbol.selectionRange = range_of_node(param->identifier);
+					param_symbol.uri = uri;
+					param_symbol.script_path = path;
+					param_symbol.detail = "var " + param_symbol.name;
+					if (param->get_datatype().is_hard_type()) {
+						param_symbol.detail += ": " + param->get_datatype().to_string();
+					}
+					symbol.children.push_back(param_symbol);
+				}
 				r_symbol.children.push_back(symbol);
 			} break;
 			case ClassNode::Member::ENUM: {
@@ -385,12 +416,24 @@ void ExtendGDScriptParser::parse_class_symbol(const GDScriptParser::ClassNode *p
 void ExtendGDScriptParser::parse_function_symbol(const GDScriptParser::FunctionNode *p_func, lsp::DocumentSymbol &r_symbol) {
 	const String uri = get_uri();
 
-	r_symbol.name = p_func->identifier->name;
-	r_symbol.kind = p_func->is_static ? lsp::SymbolKind::Function : lsp::SymbolKind::Method;
-	r_symbol.detail = "func " + String(p_func->identifier->name) + "(";
+	//TODO: merge with parent when lambda? (-> variable)
+	bool is_lambda = p_func->source_lambda != nullptr;
+
+	r_symbol.name = is_lambda ? "λ" : p_func->identifier->name;
+	r_symbol.kind = (is_lambda || p_func->is_static) ? lsp::SymbolKind::Function : lsp::SymbolKind::Method;
+	r_symbol.detail = "func";
+	if (!is_lambda) {
+		r_symbol.detail += " " + String(p_func->identifier->name);
+	}
+	r_symbol.detail += "(";
 	r_symbol.deprecated = false;
 	r_symbol.range = range_of_node(p_func);
-	r_symbol.selectionRange = range_of_node(p_func->identifier);
+	if (is_lambda) {
+		//TODO: range of `func`?
+		r_symbol.selectionRange.start = r_symbol.selectionRange.end = r_symbol.range.start;
+	} else {
+		r_symbol.selectionRange = range_of_node(p_func->identifier);
+	}
 	r_symbol.documentation = parse_documentation(LINE_NUMBER_TO_INDEX(p_func->start_line));
 	r_symbol.uri = uri;
 	r_symbol.script_path = path;
@@ -442,8 +485,17 @@ void ExtendGDScriptParser::parse_function_symbol(const GDScriptParser::FunctionN
 				node_stack.push_back(while_node->loop);
 			} break;
 
+			case GDScriptParser::TypeNode::MATCH: {
+				GDScriptParser::MatchNode *match_node = (GDScriptParser::MatchNode *)node;
+				for (GDScriptParser::MatchBranchNode *branch_node : match_node->branches) {
+					node_stack.push_back(branch_node);
+				}
+
+			} break;
+
 			case GDScriptParser::TypeNode::MATCH_BRANCH: {
 				GDScriptParser::MatchBranchNode *match_node = (GDScriptParser::MatchBranchNode *)node;
+				//TODO: patterns?
 				node_stack.push_back(match_node->block);
 			} break;
 
@@ -475,6 +527,12 @@ void ExtendGDScriptParser::parse_function_symbol(const GDScriptParser::FunctionN
 				case SuiteNode::Local::VARIABLE:
 					symbol.range = range_of_node(local.variable);
 					symbol.selectionRange = range_of_node(local.variable->identifier);
+					if (local.variable->initializer && local.variable->initializer->type == GDScriptParser::Node::LAMBDA) {
+						GDScriptParser::LambdaNode *lambda_node = (GDScriptParser::LambdaNode *)local.variable->initializer;
+						lsp::DocumentSymbol lambda;
+						parse_function_symbol(lambda_node->function, lambda);
+						symbol.children.push_back(lambda);
+					}
 					break;
 				case SuiteNode::Local::PARAMETER:
 					symbol.range = range_of_node(local.parameter);

modules/gdscript/language_server/gdscript_workspace.cpp

@@ -181,35 +181,36 @@ const lsp::DocumentSymbol *GDScriptWorkspace::get_parameter_symbol(const lsp::Do
 	return nullptr;
 }
 
-const lsp::DocumentSymbol *GDScriptWorkspace::get_local_symbol(const ExtendGDScriptParser *p_parser, const String &p_symbol_identifier) {
+const lsp::DocumentSymbol *GDScriptWorkspace::get_local_symbol_at(const ExtendGDScriptParser *p_parser, const String &p_symbol_identifier, const lsp::Position p_position) {
 	const lsp::DocumentSymbol *class_symbol = &p_parser->get_symbols();
 
-	for (int i = 0; i < class_symbol->children.size(); ++i) {
-		int kind = class_symbol->children[i].kind;
-		switch (kind) {
-			case lsp::SymbolKind::Function:
-			case lsp::SymbolKind::Method:
-			case lsp::SymbolKind::Class: {
-				const lsp::DocumentSymbol *function_symbol = &class_symbol->children[i];
-
-				for (int l = 0; l < function_symbol->children.size(); ++l) {
-					const lsp::DocumentSymbol *local = &function_symbol->children[l];
-					if (!local->detail.is_empty() && local->name == p_symbol_identifier) {
-						return local;
-					}
-				}
-			} break;
+	// go down and pick closest `DocumentSymbol` with `p_symbol_identifier`
 
-			case lsp::SymbolKind::Variable: {
-				const lsp::DocumentSymbol *variable_symbol = &class_symbol->children[i];
-				if (variable_symbol->name == p_symbol_identifier) {
-					return variable_symbol;
-				}
-			} break;
+	const lsp::DocumentSymbol *current = &p_parser->get_symbols();
+	const lsp::DocumentSymbol *best_match = nullptr;
+
+	while (current) {
+		if (current->name == p_symbol_identifier) {
+			if (current->selectionRange.contains(p_position)) {
+				// exact match: pos is ON symbol decl identifier
+				return current;
+			}
+
+			best_match = current;
+		}
+
+		const lsp::DocumentSymbol *parent = current;
+		current = nullptr;
+		for (const lsp::DocumentSymbol &child : parent->children) {
+			//TODO: use scope instead range: might be usage in initializer (-> ref outer)
+			if (child.range.contains(p_position)) {
+				current = &child;
+				break;
+			}
 		}
 	}
 
-	return nullptr;
+	return best_match;
 }
 
 void GDScriptWorkspace::reload_all_workspace_scripts() {
@@ -711,12 +712,6 @@ const lsp::DocumentSymbol *GDScriptWorkspace::resolve_symbol(const lsp::TextDocu
 											symbol = get_parameter_symbol(symbol, symbol_identifier);
 										}
 									} break;
-
-									case lsp::SymbolKind::Variable: {
-										if (symbol->local) {
-											symbol = get_local_symbol(parser, symbol_identifier);
-										}
-									} break;
 								}
 							}
 						}
@@ -732,7 +727,7 @@ const lsp::DocumentSymbol *GDScriptWorkspace::resolve_symbol(const lsp::TextDocu
 					symbol = parser->get_member_symbol(symbol_identifier);
 
 					if (!symbol) {
-						symbol = get_local_symbol(parser, symbol_identifier);
+						symbol = get_local_symbol_at(parser, symbol_identifier, p_doc_pos.position);
 					}
 				}
 			}

modules/gdscript/language_server/gdscript_workspace.h

@@ -54,7 +54,7 @@ class GDScriptWorkspace : public RefCounted {
 	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);
-	const lsp::DocumentSymbol *get_local_symbol(const ExtendGDScriptParser *p_parser, const String &p_symbol_identifier);
+	const lsp::DocumentSymbol *get_local_symbol_at(const ExtendGDScriptParser *p_parser, const String &p_symbol_identifier, const lsp::Position p_position);
 
 	void reload_all_workspace_scripts();
 

modules/gdscript/language_server/godot_lsp.h

@@ -124,6 +124,19 @@ struct Range {
 		return start == p_other.start && end == p_other.end;
 	}
 
+	bool contains(const Position p_pos) const {
+		// inside line range
+		if (start.line <= p_pos.line && p_pos.line <= end.line) {
+			// if on start line: must come after start char
+			bool start_ok = p_pos.line == start.line ? start.character <= p_pos.character : true;
+			// if on end line: must come before end char
+			bool end_ok = p_pos.line == end.line ? p_pos.character <= end.character : true;
+			return start_ok && end_ok;
+		} else {
+			return false;
+		}
+	}
+
 	String to_string() const {
 		return vformat("[%s:%s]", start.to_string(), end.to_string());
 	}

modules/gdscript/tests/lsp_project/scopes.gd

@@ -0,0 +1,106 @@
+extends Node
+
+var member := 2
+#   ^^^^^^ public -> public
+
+signal some_changed(new_value)
+#      |          | ^^^^^^^^^ signal:parameter -> signal:parameter
+#      ^^^^^^^^^^^^ signal -> signal
+var some_value := 42:
+#   ^^^^^^^^^^ property -> property
+	get:
+		return some_value
+		#      ^^^^^^^^^^ -> property
+	set(value):
+	#   ^^^^^ property:set:value -> property:set:value
+		some_changed.emit(value)
+		#          |      ^^^^^ -> property:set:value
+		#<^^^^^^^^^^ -> signal
+		some_value = value
+		#        |   ^^^^^ -> property:set:value
+		#<^^^^^^^^ -> property
+
+func v():
+	var value := member + 2
+	#   |   |    ^^^^^^ -> public
+	#   ^^^^^ v:value -> v:value
+	print(value)
+	#     ^^^^^ -> v:value
+	if value > 0:
+	#  ^^^^^ -> v:value
+		var beta := value + 2
+		#   |  |    ^^^^^ -> v:value
+		#   ^^^^ v:if:beta -> v:if:beta
+		print(beta)
+		#     ^^^^ -> v:if:beta
+
+		for counter in beta:
+		#   |     |    ^^^^ -> v:if:beta
+		#   ^^^^^^^ v:if:counter -> v:if:counter
+			print (counter)
+			#      ^^^^^^^ -> v:if:counter
+
+	else:
+		for counter in value:
+		#   |     |    ^^^^^ -> v:value
+		#   ^^^^^^^ v:else:counter -> v:else:counter
+			print(counter)
+			#     ^^^^^^^ -> v:else:counter
+
+func f():
+	var func1 = func(value): print(value + 13)
+	#   |   |        |   |         ^^^^^ -> f:func1:value
+	#   |   |        ^^^^^ f:func1:value -> f:func1:value
+	#   ^^^^^ f:func1 -> f:func1
+	var func2 = func(value): print(value + 42)
+	#   |   |        |   |         ^^^^^ -> f:func2:value
+	#   |   |        ^^^^^ f:func2:value -> f:func2:value
+	#   ^^^^^ f:func2 -> f:func2
+
+	func1.call(1)
+	#<^^^ -> f:func1
+	func2.call(2)
+	#<^^^ -> f:func2
+
+func m():
+	var value = 42
+	#   ^^^^^ m:value -> m:value
+
+	match value:
+	#     ^^^^^ -> m:value
+		13: 
+			print(value)
+			#     ^^^^^ -> m:value
+		[var start, _, var end]: 
+		#    |   |         ^^^ m:match:array:end -> m:match:array:end
+		#    ^^^^^  m:match:array:start -> m:match:array:start
+			print(start + end)
+			#     |   |   ^^^ -> m:match:array:end
+			#     ^^^^^ -> m:match:array:start
+		{ "name": var name }: 
+		#             ^^^^ m:match:dict:var -> m:match:dict:var
+			print(name)
+			#     ^^^^ -> m:match:dict:var
+		var whatever:
+		#   ^^^^^^^^ m:match:var -> m:match:var
+			print(whatever)
+			#     ^^^^^^^^ -> m:match:var
+
+func m2():
+	var value = 42 
+	#   ^^^^^ m2:value -> m2:value
+
+	match value: 
+	#     ^^^^^ -> m2:value
+		{ "name": var name }:
+		#             ^^^^ m2:match:dict:var -> m2:match:dict:var
+			print(name)
+			#     ^^^^ -> m2:match:dict:var
+		[var name, ..]:
+		#    ^^^^ m2:match:array:var -> m2:match:array:var
+			print(name)
+			#     ^^^^ -> m2:match:array:var
+		var name:
+		#   ^^^^ m2:match:var -> m2:match:var
+			print(name)
+			#     ^^^^ -> m2:match:var

modules/gdscript/tests/test_lsp.h

@@ -43,8 +43,8 @@
 #include "core/os/os.h"
 #include "editor/editor_help.h"
 #include "editor/editor_node.h"
-#include "regex/regex.h"
 #include "gdscript/gdscript_analyzer.h"
+#include "regex/regex.h"
 
 #include "thirdparty/doctest/doctest.h"
 
@@ -315,6 +315,14 @@ TEST_SUITE("[Modules][GDScript][LSP]") {
 			test_resolve_symbols(uri, all_test_data, all_test_data);
 		}
 
+		SUBCASE("Can get correct ranges for scopes") {
+			String path = "res://scopes.gd";
+			assert_no_errors_in(path);
+			String uri = workspace->get_file_uri(path);
+			Vector<InlineTestData> all_test_data = read_tests(path);
+			test_resolve_symbols(uri, all_test_data, all_test_data);
+		}
+
 		memdelete(proto);
 	}
 }