Improve inlay hint algorithm and add more tests (fixes #920)

2 files changed, 145 insertions, 127 deletions

diff --git a/src/server/inlay_hints.odin b/src/server/inlay_hints.odin
index 1f21163..307b19b 100644
--- a/src/server/inlay_hints.odin
+++ b/src/server/inlay_hints.odin
@@ -1,5 +1,6 @@
 package server
 
+import "core:slice"
 import "core:fmt"
 import "core:log"
 import "core:odin/ast"
@@ -47,120 +48,147 @@ get_inlay_hints :: proc(
 		ast.walk(&visitor, decl)
 	}
 
+	expr_name :: proc (node: ^ast.Node) -> (name: string, ok: bool) {
+		#partial switch v in node.derived {
+		case ^ast.Ident: return v.name, true
+		case ^ast.Poly_Type: return expr_name(v.type)
+		case: return
+		}
+	}
+
 	visit_call :: proc (
 		call: ^ast.Call_Expr,
 		data: ^Visitor_Data,
 	) -> (ok: bool) {
 
-		is_ellipsis := false
-		has_added_default := false
-
 		selector, is_selector_call := call.expr.derived.(^ast.Selector_Expr)
 		is_selector_call &&= selector.op.kind == .Arrow_Right
 
-		end_pos := common.token_pos_to_position(call.close, string(data.document.text))
+		src := string(data.document.text)
+		end_pos := common.token_pos_to_position(call.close, src)
+
+		symbol_and_node := data.symbols[uintptr(call.expr)] or_return
+		proc_symbol := symbol_and_node.symbol.value.(SymbolProcedureValue) or_return
 
-		symbol_and_node := data.symbols[cast(uintptr)call.expr] or_return
-		symbol_call := symbol_and_node.symbol.value.(SymbolProcedureValue) or_return
+		param_idx := 1 if is_selector_call else 0
+		label_idx := 0
+		arg_idx   := 0
 
-		positional_arg_idx := 0
+		// Positional arguments
+		for ; arg_idx < len(call.args); arg_idx += 1 {
+			arg := call.args[arg_idx]
 
-		expr_name :: proc (node: ^ast.Node) -> (name: string, ok: bool) {
-			#partial switch v in node.derived {
-			case ^ast.Ident: return v.name, true
-			case ^ast.Poly_Type: return expr_name(v.type)
-			case: return
+			// provided as named
+			if _, is_field := arg.derived.(^ast.Field_Value); is_field do break
+
+			param := slice.get(proc_symbol.arg_types, param_idx) or_return
+
+			// param is variadic
+			if param.type != nil {
+				if _, is_variadic := param.type.derived.(^ast.Ellipsis); is_variadic do break
+			}
+
+			label := slice.get(param.names, label_idx) or_return
+
+			label_name := expr_name(label) or_return
+			arg_name, arg_has_name := expr_name(arg)
+
+			// Add param name hint (skip idents with same name as param)
+			if data.config.enable_inlay_hints_params && (!arg_has_name || arg_name != label_name) {
+				range := common.get_token_range(arg, string(data.document.text))
+				hint_label := fmt.tprintf("%v = ", label_name)
+				append(&data.hints, InlayHint{range.start, .Parameter, hint_label})
+			}
+
+			label_idx += 1
+			if label_idx >= len(param.names) {
+				param_idx += 1
+				label_idx = 0
+				if param_idx >= len(proc_symbol.arg_types) {
+					return // end of parameters
+				}
 			}
 		}
 
-		for arg, arg_type_idx in symbol_call.arg_types {
-			if arg_type_idx == 0 && is_selector_call {
-				continue
+		// Variadic arguments
+		variadic: {
+			param := slice.get(proc_symbol.arg_types, param_idx) or_return
+
+			// param is variadic
+			if param.type == nil do break variadic
+			_ = param.type.derived.(^ast.Ellipsis) or_break variadic
+
+			label := slice.get(param.names, 0) or_return
+			label_name := expr_name(label) or_return
+
+			init_arg_idx := arg_idx
+			for arg_idx < len(call.args) {
+
+				// provided as named
+				if _, is_field := call.args[arg_idx].derived.(^ast.Field_Value); is_field do break
+
+				arg_idx += 1
 			}
 
-			for name, name_idx in arg.names {
+			// Add param name hint
+			if arg_idx > init_arg_idx && data.config.enable_inlay_hints_params {
+				// get range from first variadic arg
+				range := common.get_token_range(call.args[init_arg_idx], string(data.document.text))
+				hint_label := fmt.tprintf("%v = ", label_name)
+				append(&data.hints, InlayHint{range.start, .Parameter, hint_label})
+			}
 
-				arg_call_idx := arg_type_idx + name_idx
-				if is_selector_call do arg_call_idx -= 1
+			param_idx += 1
+			label_idx = 0
+			if param_idx >= len(proc_symbol.arg_types) {
+				return // end of parameters
+			}
+		}
 
-				label := expr_name(name) or_return
+		// Named arguments
+		named: if data.config.enable_inlay_hints_default_params {
 
-				is_provided_named, is_provided_positional: bool
-				call_arg: ^ast.Expr
+			init_arg_idx := arg_idx
+			added_default_hint := false
 
-				for a, a_i in call.args[positional_arg_idx:] {
-					call_arg_idx := a_i + positional_arg_idx
-					// provided as named
-					if field_value, ok := a.derived.(^ast.Field_Value); ok {
-						ident := field_value.field.derived.(^ast.Ident) or_break
-						if ident.name == label {
-							is_provided_named = true
-							call_arg = a
-						}
-						break
-					} // provided as positional
-					else if arg_call_idx == call_arg_idx {
-						is_provided_positional = true
-						positional_arg_idx += 1
-						call_arg = a
-						break
-					}
-				}
+			for ; param_idx < len(proc_symbol.arg_types); param_idx, label_idx = param_idx + 1, 0 {
+				param := slice.get(proc_symbol.arg_types, param_idx) or_return
 
-				if is_ellipsis || (!is_provided_named && !is_provided_positional) {
-					// This parameter is not provided, so it should use default value
-					if arg.default_value == nil {
-						return
-					}
+				label_loop: for ; label_idx < len(param.names); label_idx += 1 {
+					label := slice.get(param.names, label_idx) or_return
+					label_name := expr_name(label) or_return
 
-					if !data.config.enable_inlay_hints_default_params {
-						return
-					}
+					if param.default_value == nil do continue
+
+					// check if was already provided
+					for arg in call.args[init_arg_idx:] {
 
-					value := node_to_string(arg.default_value)
+						field_value := arg.derived.(^ast.Field_Value) or_break named
+						ident := field_value.field.derived.(^ast.Ident) or_break named
 
-					needs_leading_comma := arg_call_idx > 0
+						if ident.name == label_name {
+							continue label_loop
+						}
+					}
 
-					if !has_added_default && needs_leading_comma {
-						till_end := string(data.document.text[:call.close.offset])
-						#reverse for ch in till_end {
+					needs_leading_comma := added_default_hint || param_idx > 0 || label_idx > 0
+					if needs_leading_comma && !added_default_hint {
+						// check for existing trailing comma
+						#reverse for ch in string(data.document.text[:call.close.offset]) {
 							switch ch {
-							case ' ', '\t', '\n':
-								continue
-							case ',':
-								needs_leading_comma = false
+							case ' ', '\t', '\n': continue
+							case ',': needs_leading_comma = false
 							}
 							break
 						}
 					}
 
-					hint := InlayHint {
-						kind     = .Parameter,
-						label    = fmt.tprintf("%s%v = %v", needs_leading_comma ? ", " : "", label, value),
-						position = end_pos,
-					}
-					append(&data.hints, hint)
-
-					has_added_default = true
-				} else if data.config.enable_inlay_hints_params && is_provided_positional && !is_provided_named {
-					// This parameter is provided via positional argument, show parameter hint
-
-					// if the arg name and param name are the same, don't add it.
-					call_arg_name, _ := expr_name(call_arg)
-					if call_arg_name != label {
-						range := common.get_token_range(call_arg, string(data.document.text))
-						hint := InlayHint {
-							kind     = .Parameter,
-							label    = fmt.tprintf("%v = ", label),
-							position = range.start,
-						}
-						append(&data.hints, hint)
-					}
-				}
+					// Add default param hint
+					value := node_to_string(param.default_value)
+					hint_label := fmt.tprintf("%s%v = %v", needs_leading_comma ? ", " : "", label_name, value)
+					append(&data.hints, InlayHint{end_pos, .Parameter, hint_label})
 
-				if arg.type != nil {
-					_, is_current_ellipsis := arg.type.derived.(^ast.Ellipsis)
-					is_ellipsis ||= is_current_ellipsis
+					added_default_hint = true
 				}
 			}
 		}
diff --git a/tests/inlay_hints_test.odin b/tests/inlay_hints_test.odin
index 9d9d074..ab2cd1b 100644
--- a/tests/inlay_hints_test.odin
+++ b/tests/inlay_hints_test.odin
@@ -10,30 +10,12 @@ ast_inlay_hints_default_params :: proc(t: ^testing.T) {
 	source := test.Source {
 		main     = `package test
 
-		my_function :: proc(a := false, b := 42) {}
+		foo :: proc(a := false, b := 42) {}
+		bar :: proc(a: int, b := false, c := 42) {}
 
 		main :: proc() {
-			my_function([[a = false]][[, b = 42]])
-		}
-		`,
-		packages = {},
-		config = {
-			enable_inlay_hints_default_params = true,
-		},
-	}
-
-	test.expect_inlay_hints(t, &source)
-}
-
-@(test)
-ast_inlay_hints_default_params_after_required :: proc(t: ^testing.T) {
-	source := test.Source {
-		main     = `package test
-
-		my_function :: proc(a: int, b := false, c := 42) {}
-
-		main :: proc() {
-			my_function(1[[, b = false]][[, c = 42]])
+			foo([[a = false]][[, b = 42]])
+			bar(1[[, b = false]][[, c = 42]])
 		}
 		`,
 		packages = {},
@@ -50,10 +32,13 @@ ast_inlay_hints_default_params_after_named :: proc(t: ^testing.T) {
 	source := test.Source {
 		main     = `package test
 
-		my_function :: proc(a: int, b := false, c := 42) {}
+		my_function :: proc(a: int = 1, b := false, c := 42) {}
 
 		main :: proc() {
 			my_function(a=1[[, b = false]][[, c = 42]])
+			my_function([[a = ]]1, c=42[[, b = false]])
+			my_function(c=42, a=1[[, b = false]])
+			my_function(b=true, a=1[[, c = 42]])
 		}
 		`,
 		packages = {},
@@ -67,26 +52,6 @@ ast_inlay_hints_default_params_after_named :: proc(t: ^testing.T) {
 }
 
 @(test)
-ast_inlay_hints_default_params_named_ooo :: proc(t: ^testing.T) {
-	source := test.Source {
-		main     = `package test
-
-		my_function :: proc(a: int, b := false, c := 42) {}
-
-		main :: proc() {
-			my_function(1, c=42[[, b = false]])
-		}
-		`,
-		packages = {},
-		config = {
-			enable_inlay_hints_default_params = true,
-		},
-	}
-
-	test.expect_inlay_hints(t, &source)
-}
-
-@(test)
 ast_inlay_hints_params :: proc(t: ^testing.T) {
 	source := test.Source {
 		main     = `package test
@@ -178,15 +143,40 @@ ast_inlay_hints_variadic_params :: proc(t: ^testing.T) {
 	source := test.Source {
 		main     = `package test
 
-		variadic_func :: proc(args: ..int) {}
+		variadic_func :: proc(args: ..int, default := 2) {}
 
 		main :: proc() {
-			variadic_func([[args = ]]1, 2, 3)
+			variadic_func([[args = ]]1, 2, 3[[, default = 2]])
+			variadic_func([[args = ]]1, 2, default=3)
 		}
 		`,
 		packages = {},
 		config = {
 			enable_inlay_hints_params = true,
+			enable_inlay_hints_default_params = true,
+		},
+	}
+
+	test.expect_inlay_hints(t, &source)
+}
+
+@(test)
+ast_inlay_hints_multi_return_params :: proc(t: ^testing.T) {
+	source := test.Source {
+		main     = `package test
+
+		func :: proc (a: int, b: int = 2, c := 3) {}
+
+		two :: proc () -> (int, int) {return 1, 2}
+
+		main :: proc() {
+			func([[a, b = ]]two()[[, c = 3]])
+		}
+		`,
+		packages = {},
+		config = {
+			enable_inlay_hints_params = true,
+			enable_inlay_hints_default_params = true,
 		},
 	}