From 23de8c183d052844f6b23b8ead9d03a33012af27 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Thu, 11 Sep 2025 00:38:40 +0200 Subject: Take expected inlay hints from the source code --- tests/inlay_hints_test.odin | 92 +++++++++------------------------------------ 1 file changed, 18 insertions(+), 74 deletions(-) (limited to 'tests') diff --git a/tests/inlay_hints_test.odin b/tests/inlay_hints_test.odin index 005253a..9d9d074 100644 --- a/tests/inlay_hints_test.odin +++ b/tests/inlay_hints_test.odin @@ -13,7 +13,7 @@ ast_inlay_hints_default_params :: proc(t: ^testing.T) { my_function :: proc(a := false, b := 42) {} main :: proc() { - my_function() + my_function([[a = false]][[, b = 42]]) } `, packages = {}, @@ -22,15 +22,7 @@ ast_inlay_hints_default_params :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 15}, - kind = .Parameter, - label = "a = false", - }, { - position = {5, 15}, - kind = .Parameter, - label = ", b = 42", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -41,7 +33,7 @@ ast_inlay_hints_default_params_after_required :: proc(t: ^testing.T) { my_function :: proc(a: int, b := false, c := 42) {} main :: proc() { - my_function(1) + my_function(1[[, b = false]][[, c = 42]]) } `, packages = {}, @@ -50,15 +42,7 @@ ast_inlay_hints_default_params_after_required :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 16}, - kind = .Parameter, - label = ", b = false", - }, { - position = {5, 16}, - kind = .Parameter, - label = ", c = 42", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -69,7 +53,7 @@ ast_inlay_hints_default_params_after_named :: proc(t: ^testing.T) { my_function :: proc(a: int, b := false, c := 42) {} main :: proc() { - my_function(a=1) + my_function(a=1[[, b = false]][[, c = 42]]) } `, packages = {}, @@ -79,15 +63,7 @@ ast_inlay_hints_default_params_after_named :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 18}, - kind = .Parameter, - label = ", b = false", - }, { - position = {5, 18}, - kind = .Parameter, - label = ", c = 42", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -98,7 +74,7 @@ ast_inlay_hints_default_params_named_ooo :: proc(t: ^testing.T) { my_function :: proc(a: int, b := false, c := 42) {} main :: proc() { - my_function(1, c=42) + my_function(1, c=42[[, b = false]]) } `, packages = {}, @@ -107,11 +83,7 @@ ast_inlay_hints_default_params_named_ooo :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 22}, - kind = .Parameter, - label = ", b = false", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -122,7 +94,7 @@ ast_inlay_hints_params :: proc(t: ^testing.T) { my_function :: proc(param1: int, param2: string) {} main :: proc() { - my_function(123, "hello") + my_function([[param1 = ]]123, [[param2 = ]]"hello") } `, packages = {}, @@ -131,15 +103,7 @@ ast_inlay_hints_params :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 15}, - kind = .Parameter, - label = "param1 = ", - }, { - position = {5, 20}, - kind = .Parameter, - label = "param2 = ", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -150,7 +114,7 @@ ast_inlay_hints_mixed_params :: proc(t: ^testing.T) { my_function :: proc(required: int, optional := false) {} main :: proc() { - my_function(42) + my_function([[required = ]]42[[, optional = false]]) } `, packages = {}, @@ -160,15 +124,7 @@ ast_inlay_hints_mixed_params :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 15}, - kind = .Parameter, - label = "required = ", - }, { - position = {5, 17}, - kind = .Parameter, - label = ", optional = false", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -183,7 +139,7 @@ ast_inlay_hints_selector_call :: proc(t: ^testing.T) { main :: proc() { p: Point - p->move(1.0, 2.0) + p->move([[dx = ]]1.0, [[dy = ]]2.0) } `, packages = {}, @@ -192,15 +148,7 @@ ast_inlay_hints_selector_call :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {9, 11}, - kind = .Parameter, - label = "dx = ", - }, { - position = {9, 16}, - kind = .Parameter, - label = "dy = ", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -222,7 +170,7 @@ ast_inlay_hints_no_hints_same_name :: proc(t: ^testing.T) { } // No hints should be shown when argument name matches parameter name - test.expect_inlay_hints(t, &source, {}) + test.expect_inlay_hints(t, &source) } @(test) @@ -233,7 +181,7 @@ ast_inlay_hints_variadic_params :: proc(t: ^testing.T) { variadic_func :: proc(args: ..int) {} main :: proc() { - variadic_func(1, 2, 3) + variadic_func([[args = ]]1, 2, 3) } `, packages = {}, @@ -242,11 +190,7 @@ ast_inlay_hints_variadic_params :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {{ - position = {5, 17}, - kind = .Parameter, - label = "args = ", - }}) + test.expect_inlay_hints(t, &source) } @(test) @@ -267,5 +211,5 @@ ast_inlay_hints_disabled :: proc(t: ^testing.T) { }, } - test.expect_inlay_hints(t, &source, {}) + test.expect_inlay_hints(t, &source) } -- cgit v1.2.3 From ee09270191cf0d058951d075b74aa4ef439d713b Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Thu, 11 Sep 2025 23:42:40 +0200 Subject: Improve inlay hint algorithm and add more tests (fixes #920) --- src/server/inlay_hints.odin | 192 +++++++++++++++++++++++++------------------- tests/inlay_hints_test.odin | 80 ++++++++---------- 2 files changed, 145 insertions(+), 127 deletions(-) (limited to 'tests') 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 = {}, @@ -66,26 +51,6 @@ ast_inlay_hints_default_params_after_named :: proc(t: ^testing.T) { test.expect_inlay_hints(t, &source) } -@(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 { @@ -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, }, } -- cgit v1.2.3 From ebb81db93dedb49124386f5687e476ec33d703a2 Mon Sep 17 00:00:00 2001 From: Damian Tarnawski Date: Fri, 12 Sep 2025 00:38:59 +0200 Subject: Add additional test case to ast_inlay_hints_multi_return_params --- tests/inlay_hints_test.odin | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/tests/inlay_hints_test.odin b/tests/inlay_hints_test.odin index ab2cd1b..c5600cc 100644 --- a/tests/inlay_hints_test.odin +++ b/tests/inlay_hints_test.odin @@ -165,12 +165,14 @@ 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) {} + takes_three_required :: proc (a, b, c: int) {} + takes_three_optional :: proc (a: int, b: int = 2, c := 3) {} - two :: proc () -> (int, int) {return 1, 2} + returns_two :: proc () -> (int, int) {return 1, 2} - main :: proc() { - func([[a, b = ]]two()[[, c = 3]]) + main :: proc () { + takes_three_required([[a, b = ]]returns_two(), [[c = ]]3) + takes_three_optional([[a, b = ]]returns_two()[[, c = 3]]) } `, packages = {}, -- cgit v1.2.3