refactor: cleaning up the methods and config reading

4 files changed, 68 insertions, 36 deletions

diff --git a/src/server/definition.odin b/src/server/definition.odin
index 9994afa..869816c 100644
--- a/src/server/definition.odin
+++ b/src/server/definition.odin
@@ -42,7 +42,7 @@ get_all_package_file_locations :: proc(
 	return true
 }
 
-get_definition_location :: proc(document: ^Document, position: common.Position) -> ([]common.Location, bool) {
+get_definition_location :: proc(document: ^Document, position: common.Position, config: ^common.Config) -> ([]common.Location, bool) {
 	locations := make([dynamic]common.Location, context.temp_allocator)
 
 	location: common.Location
@@ -100,7 +100,7 @@ get_definition_location :: proc(document: ^Document, position: common.Position)
 		}
 
 		if resolved, ok := resolve_location_selector(&ast_context, position_context.selector_expr); ok {
-			if common.config.enable_overload_resolution {
+			if config.enable_overload_resolution {
 				resolved = try_resolve_proc_group_overload(
 					&ast_context,
 					&position_context,
@@ -147,7 +147,7 @@ get_definition_location :: proc(document: ^Document, position: common.Position)
 			&ast_context,
 			position_context.identifier.derived.(^ast.Ident)^,
 		); ok {
-			if common.config.enable_overload_resolution {
+			if config.enable_overload_resolution {
 				resolved = try_resolve_proc_group_overload(&ast_context, &position_context, resolved)
 			}
 			if v, ok := resolved.value.(SymbolAggregateValue); ok {
@@ -198,36 +198,10 @@ try_resolve_proc_group_overload :: proc(
 
 	// For selector expressions, we need to look up the full symbol to check if it's a proc group
 	full_symbol := symbol
-	if selector_expr != nil {
-		if selector, ok := selector_expr.derived.(^ast.Selector_Expr); ok {
-			if _, is_pkg := symbol.value.(SymbolPackageValue); is_pkg || symbol.value == nil {
-				if selector.field != nil {
-					if ident, ok := selector.field.derived.(^ast.Ident); ok {
-						if pkg_symbol, ok := lookup(ident.name, symbol.pkg, ast_context.fullpath); ok {
-							full_symbol = pkg_symbol
-						}
-					}
-				}
-			}
-		}
-	} else if position_context.identifier != nil && symbol.value == nil {
-		// For identifiers (non-selector), the symbol from resolve_location_identifier may not have
-		// value set (e.g., for globals). We need to do a lookup to get the full symbol.
-		if ident, ok := position_context.identifier.derived.(^ast.Ident); ok {
-			pkg := symbol.pkg
-			if pkg == "" do pkg = ast_context.document_package
-
-			if pkg_symbol, ok := lookup(ident.name, pkg, ast_context.fullpath); ok {
-				full_symbol = pkg_symbol
-			} else if global, ok := ast_context.globals[ident.name]; ok {
-				// If lookup fails (e.g., in tests without full indexing), try checking if it's a proc group
-				if proc_group, is_proc_group := global.expr.derived.(^ast.Proc_Group); is_proc_group {
-					full_symbol.value = SymbolProcedureGroupValue {
-						group = global.expr,
-					}
-				}
-			}
-		}
+	if result, ok := get_full_symbol_from_selector(ast_context, selector_expr, symbol); ok {
+		full_symbol = result
+	} else if result, ok := get_full_symbol_from_identifier(ast_context, position_context, symbol); ok {
+		full_symbol = result
 	}
 
 	proc_group_value, is_proc_group := full_symbol.value.(SymbolProcedureGroupValue)
@@ -256,3 +230,59 @@ try_resolve_proc_group_overload :: proc(
 
 	return symbol
 }
+
+get_full_symbol_from_selector :: proc(
+	ast_context: ^AstContext,
+	selector_expr: ^ast.Node,
+	symbol: Symbol,
+) -> (
+	full_symbol: Symbol,
+	ok: bool,
+) {
+	if selector_expr == nil do return
+
+	selector := selector_expr.derived.(^ast.Selector_Expr) or_return
+
+	_, is_pkg := symbol.value.(SymbolPackageValue)
+	if !is_pkg && symbol.value != nil do return
+
+	if selector.field == nil do return
+
+	ident := selector.field.derived.(^ast.Ident) or_return
+
+	return lookup(ident.name, symbol.pkg, ast_context.fullpath);
+}
+
+get_full_symbol_from_identifier :: proc(
+	ast_context: ^AstContext,
+	position_context: ^DocumentPositionContext,
+	symbol: Symbol,
+) -> (
+	full_symbol: Symbol,
+	ok: bool,
+) {
+	if position_context.identifier == nil || symbol.value != nil do return
+
+	// For identifiers (non-selector), the symbol from resolve_location_identifier may not have
+	// value set (e.g., for globals). We need to do a lookup to get the full symbol.
+	ident := position_context.identifier.derived.(^ast.Ident) or_return
+
+	pkg := symbol.pkg if symbol.pkg != "" else ast_context.document_package
+
+	if pkg_symbol, ok := lookup(ident.name, pkg, ast_context.fullpath); ok {
+		return pkg_symbol, true
+	}
+
+	// If lookup fails (e.g., in tests without full indexing), try checking if it's a proc group
+
+	global := ast_context.globals[ident.name] or_return
+	if proc_group, is_proc_group := global.expr.derived.(^ast.Proc_Group); is_proc_group {
+		full_symbol = symbol
+		full_symbol.value = SymbolProcedureGroupValue {
+			group = global.expr,
+		}
+		return full_symbol, true
+	}
+
+	return Symbol{}, false
+}
diff --git a/src/server/requests.odin b/src/server/requests.odin
index 3a1a163..0a1d089 100644
--- a/src/server/requests.odin
+++ b/src/server/requests.odin
@@ -453,7 +453,7 @@ read_ols_initialize_options :: proc(config: ^common.Config, ols_config: OlsConfi
 
 	config.enable_fake_method = ols_config.enable_fake_methods.(bool) or_else config.enable_fake_method
 	config.enable_overload_resolution =
-	 ols_config.enable_overload_resolution.(bool) or_else config.enable_overload_resolution
+		ols_config.enable_overload_resolution.(bool) or_else config.enable_overload_resolution
 
 	// Delete overriding collections.
 	for it in ols_config.collections {
@@ -861,7 +861,7 @@ request_definition :: proc(
 		return .InternalError
 	}
 
-	locations, ok2 := get_definition_location(document, definition_params.position)
+	locations, ok2 := get_definition_location(document, definition_params.position, config)
 
 	if !ok2 {
 		log.warn("Failed to get definition location")
diff --git a/src/testing/testing.odin b/src/testing/testing.odin
index 373ef62..2013f79 100644
--- a/src/testing/testing.odin
+++ b/src/testing/testing.odin
@@ -346,7 +346,7 @@ expect_definition_locations :: proc(t: ^testing.T, src: ^Source, expect_location
 	setup(src)
 	defer teardown(src)
 
-	locations, ok := server.get_definition_location(src.document, src.position)
+	locations, ok := server.get_definition_location(src.document, src.position, &src.config)
 
 	if !ok {
 		log.error("Failed get_definition_location")
diff --git a/tests/definition_test.odin b/tests/definition_test.odin
index 62165e2..fe4cd3b 100644
--- a/tests/definition_test.odin
+++ b/tests/definition_test.odin
@@ -742,6 +742,7 @@ ast_goto_proc_group_overload_with_selector :: proc(t: ^testing.T) {
 		}
 	`,
 		packages = packages[:],
+		config = {enable_overload_resolution = true},
 	}
 	// Should go to push_back (line 1, character 3) instead of append (line 3)
 	// because push_back is the overload being used with a single value argument
@@ -765,6 +766,7 @@ ast_goto_proc_group_overload_identifier :: proc(t: ^testing.T) {
 			app{*}end(&arr, 1)
 		}
 	`,
+		config = {enable_overload_resolution = true},
 	}
 	// Should go to push_back (line 1, character 2) instead of append (line 3)
 	// because push_back is the overload being used with a single value argument