From 9652ef501f0085fd5e335427de95482625b2d1ea Mon Sep 17 00:00:00 2001 From: DanielGavin Date: Sat, 6 Apr 2024 14:25:02 +0200 Subject: [PATCH 1/4] Fix issues with objc completion and hover --- src/odin/printer/visit.odin | 5 ++ src/server/analysis.odin | 30 ++++++++---- src/server/completion.odin | 4 ++ src/server/hover.odin | 20 +++++++- tests/objc_test.odin | 98 +++++++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 11 deletions(-) diff --git a/src/odin/printer/visit.odin b/src/odin/printer/visit.odin index 92ebd928..1dac9994 100644 --- a/src/odin/printer/visit.odin +++ b/src/odin/printer/visit.odin @@ -577,6 +577,11 @@ is_values_nestable_if_break_assign :: proc(list: []^ast.Expr) -> bool { #partial switch v in expr.derived { case ^ast.Call_Expr, ^ast.Comp_Lit, ^ast.Or_Return_Expr: return true + case ^ast.Unary_Expr: + #partial switch v2 in v.expr.derived { + case ^ast.Call_Expr: + return true + } } } return false diff --git a/src/server/analysis.odin b/src/server/analysis.odin index 7ffc1771..7f77958d 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -961,7 +961,12 @@ internal_resolve_type_expression :: proc( v.expr, ); ok { ast_context.use_locals = false - ast_context.current_package = selector.pkg + + if selector.pkg != "" { + ast_context.current_package = selector.pkg + } else { + ast_context.current_package = ast_context.document_package + } #partial switch s in selector.value { case SymbolProcedureValue: @@ -982,6 +987,12 @@ internal_resolve_type_expression :: proc( ); ok { ast_context.use_locals = false + if selector.pkg != "" { + ast_context.current_package = selector.pkg + } else { + ast_context.current_package = ast_context.document_package + } + #partial switch s in selector.value { case SymbolFixedArrayValue: components_count := 0 @@ -1039,18 +1050,13 @@ internal_resolve_type_expression :: proc( ) selector_expr.expr = s.return_types[0].type selector_expr.field = v.field + return internal_resolve_type_expression( ast_context, selector_expr, ) } case SymbolStructValue: - if selector.pkg != "" { - ast_context.current_package = selector.pkg - } else { - ast_context.current_package = ast_context.document_package - } - for name, i in s.names { if v.field != nil && name == v.field.name { ast_context.field_name = v.field^ @@ -1063,8 +1069,6 @@ internal_resolve_type_expression :: proc( } } case SymbolPackageValue: - ast_context.current_package = selector.pkg - try_build_package(ast_context.current_package) if v.field != nil { @@ -4865,10 +4869,16 @@ get_document_position_node :: proc( case ^Selector_Call_Expr: if position_context.hint == .Definition || position_context.hint == .Hover || - position_context.hint == .SignatureHelp { + position_context.hint == .SignatureHelp || + position_context.hint == .Completion { position_context.selector = n.expr position_context.field = n.call position_context.selector_expr = cast(^Selector_Expr)node + + if _, ok := n.call.derived.(^ast.Call_Expr); ok { + position_context.call = n.call + } + get_document_position(n.expr, position_context) get_document_position(n.call, position_context) diff --git a/src/server/completion.odin b/src/server/completion.odin index c10a48ba..e09aa0a4 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -1099,6 +1099,10 @@ get_implicit_completion :: proc( ok && parameter_ok { ast_context.current_package = symbol.pkg + if .ObjC in symbol.flags { + parameter_index += 1 + } + if proc_value, ok := symbol.value.(SymbolProcedureValue); ok { if len(proc_value.arg_types) <= parameter_index { return diff --git a/src/server/hover.odin b/src/server/hover.odin index be6adadb..c3dccbc9 100644 --- a/src/server/hover.odin +++ b/src/server/hover.odin @@ -170,7 +170,9 @@ get_hover_information :: proc( } } - if position_context.selector != nil && position_context.identifier != nil { + if position_context.selector != nil && + position_context.identifier != nil && + position_context.field == position_context.identifier { hover.range = common.get_token_range( position_context.identifier^, ast_context.file.src, @@ -211,6 +213,7 @@ get_hover_information :: proc( } selector: Symbol + selector, ok = resolve_type_expression( &ast_context, position_context.selector, @@ -229,6 +232,21 @@ get_hover_information :: proc( } } + if v, is_proc := selector.value.(SymbolProcedureValue); is_proc { + if len(v.return_types) == 0 || v.return_types[0].type == nil { + return {}, false, false + } + + ast_context.current_package = selector.pkg + + if selector, ok = resolve_type_expression( + &ast_context, + v.return_types[0].type, + ); !ok { + return {}, false, false + } + } + ast_context.current_package = selector.pkg #partial switch v in selector.value { diff --git a/tests/objc_test.odin b/tests/objc_test.odin index c1b38bc1..af17c0ac 100644 --- a/tests/objc_test.odin +++ b/tests/objc_test.odin @@ -46,3 +46,101 @@ cobj_return_type_with_selector_expression :: proc(t: ^testing.T) { {"Window.initWithContentRect: my_package.Window_initWithContentRect"}, ) } + +@(test) +cobj_return_type_with_selector_expression_2 :: proc(t: ^testing.T) { + packages := make([dynamic]test.Package) + + append( + &packages, + test.Package { + pkg = "my_package", + source = `package my_package + @(objc_class="NSWindow") + Window :: struct { dummy: int} + + @(objc_type=Window, objc_name="alloc", objc_is_class_method=true) + Window_alloc :: proc "c" () -> ^Window { + } + @(objc_type=Window, objc_name="initWithContentRect") + Window_initWithContentRect :: proc (self: ^Window, contentRect: Rect, styleMask: WindowStyleMask, backing: BackingStoreType, doDefer: BOOL) -> ^Window { + } + `, + }, + ) + + source := test.Source { + main = `package test + import "my_package" + + main :: proc() { + window := my_package.Window.alloc()->initWithContentRect( + {{0, 0}, {500, 400}}, + {.Titled, .Closable, .Resizable}, + .Buffered, + false, + ) + + window->{*} + } + `, + packages = packages[:], + } + + test.expect_completion_details( + t, + &source, + "->", + {"Window.initWithContentRect: my_package.Window_initWithContentRect"}, + ) +} + + +@(test) +cobj_hover_chained_selector :: proc(t: ^testing.T) { + packages := make([dynamic]test.Package) + + append( + &packages, + test.Package { + pkg = "my_package", + source = `package my_package + @(objc_class="NSWindow") + Window :: struct { dummy: int} + + @(objc_type=Window, objc_name="alloc", objc_is_class_method=true) + Window_alloc :: proc "c" () -> ^Window { + } + @(objc_type=Window, objc_name="initWithContentRect") + Window_initWithContentRect :: proc (self: ^Window, contentRect: Rect, styleMask: WindowStyleMask, backing: BackingStoreType, doDefer: BOOL) -> ^Window { + } + + My_Struct :: struct { + dummy: int, + } + `, + }, + ) + + source := test.Source { + main = `package test + import "my_package" + + main :: proc() { + window := my_package.Window.alloc()->initWithConte{*}ntRect( + {{0, 0}, {500, 400}}, + {.Titled, .Closable, .Resizable}, + .Buffered, + false, + ) + } + `, + packages = packages[:], + } + + test.expect_hover( + t, + &source, + "Window.initWithContentRect: proc(self: ^Window, contentRect: Rect, styleMask: WindowStyleMask, backing: BackingStoreType, doDefer: BOOL)", + ) +} From 7ca20296e16ab36c196140e34a47afea0a64b805 Mon Sep 17 00:00:00 2001 From: DanielGavin Date: Sat, 6 Apr 2024 14:36:07 +0200 Subject: [PATCH 2/4] Call expression should always increment the current argument counter, since argument 1 is always the struct type. --- src/server/completion.odin | 10 ++++++-- tests/objc_test.odin | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/server/completion.odin b/src/server/completion.odin index e09aa0a4..eabac77b 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -1099,8 +1099,14 @@ get_implicit_completion :: proc( ok && parameter_ok { ast_context.current_package = symbol.pkg - if .ObjC in symbol.flags { - parameter_index += 1 + //Selector call expression always set the first argument to be the type of struct called, so increment it. + if position_context.selector_expr != nil { + if selector_call, ok := position_context.selector_expr.derived.(^ast.Selector_Call_Expr); + ok { + if selector_call.call == position_context.call { + parameter_index += 1 + } + } } if proc_value, ok := symbol.value.(SymbolProcedureValue); ok { diff --git a/tests/objc_test.odin b/tests/objc_test.odin index af17c0ac..f86f331b 100644 --- a/tests/objc_test.odin +++ b/tests/objc_test.odin @@ -144,3 +144,51 @@ cobj_hover_chained_selector :: proc(t: ^testing.T) { "Window.initWithContentRect: proc(self: ^Window, contentRect: Rect, styleMask: WindowStyleMask, backing: BackingStoreType, doDefer: BOOL)", ) } + +@(test) +cobj_hover_chained_selector :: proc(t: ^testing.T) { + packages := make([dynamic]test.Package) + + append( + &packages, + test.Package { + pkg = "my_package", + source = `package my_package + My_Enum :: enum { + Regular = 0, + Accessory = 1, + Prohibited = 2, + } + + @(objc_class="NSWindow") + Window :: struct { dummy: int} + + @(objc_type=Window, objc_name="alloc", objc_is_class_method=true) + Window_alloc :: proc "c" () -> ^Window { + } + @(objc_type=Window, objc_name="initWithContentRect") + Window_initWithContentRect :: proc (self: ^Window, my_enum: My_Enum) -> ^Window { + } + + My_Struct :: struct { + dummy: int, + } + `, + }, + ) + + source := test.Source { + main = `package test + import "my_package" + + main :: proc() { + window := my_package.Window.alloc()->initWithContentRect( + .{*} + ) + } + `, + packages = packages[:], + } + + test.expect_completion_labels(t, &source, ".", {"Accessory"}) +} From 1bd1e7eda452e738e98cc6281513d55a9bb77140 Mon Sep 17 00:00:00 2001 From: DanielGavin Date: Sat, 6 Apr 2024 14:47:41 +0200 Subject: [PATCH 3/4] Typo --- tests/objc_test.odin | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/objc_test.odin b/tests/objc_test.odin index f86f331b..760d1937 100644 --- a/tests/objc_test.odin +++ b/tests/objc_test.odin @@ -7,7 +7,7 @@ import test "src:testing" @(test) -cobj_return_type_with_selector_expression :: proc(t: ^testing.T) { +objc_return_type_with_selector_expression :: proc(t: ^testing.T) { packages := make([dynamic]test.Package) append( @@ -48,7 +48,7 @@ cobj_return_type_with_selector_expression :: proc(t: ^testing.T) { } @(test) -cobj_return_type_with_selector_expression_2 :: proc(t: ^testing.T) { +objc_return_type_with_selector_expression_2 :: proc(t: ^testing.T) { packages := make([dynamic]test.Package) append( @@ -97,7 +97,7 @@ cobj_return_type_with_selector_expression_2 :: proc(t: ^testing.T) { @(test) -cobj_hover_chained_selector :: proc(t: ^testing.T) { +objc_hover_chained_selector :: proc(t: ^testing.T) { packages := make([dynamic]test.Package) append( @@ -146,7 +146,7 @@ cobj_hover_chained_selector :: proc(t: ^testing.T) { } @(test) -cobj_hover_chained_selector :: proc(t: ^testing.T) { +objc_implicit_enum_completion :: proc(t: ^testing.T) { packages := make([dynamic]test.Package) append( From c5a7a39ee1e7b74d512d50da44e21c15aa5e65d6 Mon Sep 17 00:00:00 2001 From: DanielGavin Date: Tue, 9 Apr 2024 00:52:36 +0200 Subject: [PATCH 4/4] Check for len being null in array. --- src/server/completion.odin | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/server/completion.odin b/src/server/completion.odin index eabac77b..caba5bf8 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -341,9 +341,11 @@ get_selector_completion :: proc( expr_len := 0 - if basic, ok := v.len.derived.(^ast.Basic_Lit); ok { - if expr_len, ok = strconv.parse_int(basic.tok.text); !ok { - expr_len = 0 + if v.len != nil { + if basic, ok := v.len.derived.(^ast.Basic_Lit); ok { + if expr_len, ok = strconv.parse_int(basic.tok.text); !ok { + expr_len = 0 + } } }