From 918d185deb365651f474ed30f2247cf448d91a5d Mon Sep 17 00:00:00 2001 From: ske Date: Sun, 4 Jan 2026 18:27:49 -0300 Subject: fix in/not_in bit_set complete --- src/server/completion.odin | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/server/completion.odin b/src/server/completion.odin index 31ecf0e..eaa0c9c 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -855,7 +855,7 @@ get_selector_completion :: proc( label = fmt.tprintf(".%s in", name), kind = .EnumMember, detail = in_text, - insertText = in_text[1:], + insertText = in_text, additionalTextEdits = remove_edit, }, }, @@ -868,7 +868,7 @@ get_selector_completion :: proc( label = fmt.tprintf(".%s not_in", name), kind = .EnumMember, detail = not_in_text, - insertText = not_in_text[1:], + insertText = not_in_text, additionalTextEdits = remove_edit, }, }, -- cgit v1.2.3 From 78dafa583beddd1d9b0c3bfcd6024a0e55d7c312 Mon Sep 17 00:00:00 2001 From: ske Date: Sun, 4 Jan 2026 19:14:00 -0300 Subject: oops apply only to Sublime LSP --- src/server/completion.odin | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/server/completion.odin b/src/server/completion.odin index eaa0c9c..9c9ea9c 100644 --- a/src/server/completion.odin +++ b/src/server/completion.odin @@ -835,6 +835,9 @@ get_selector_completion :: proc( remove_edit, rok := create_remove_edit(position_context, true) if !rok {break} + // Sublime Text will remove the original `.` for some reason + is_sublime := config.client_name == "Sublime Text LSP" + for name in enumv.names { append( results, @@ -855,7 +858,7 @@ get_selector_completion :: proc( label = fmt.tprintf(".%s in", name), kind = .EnumMember, detail = in_text, - insertText = in_text, + insertText = is_sublime ? in_text : in_text[1:], additionalTextEdits = remove_edit, }, }, @@ -868,7 +871,7 @@ get_selector_completion :: proc( label = fmt.tprintf(".%s not_in", name), kind = .EnumMember, detail = not_in_text, - insertText = not_in_text, + insertText = is_sublime ? not_in_text : not_in_text[1:], additionalTextEdits = remove_edit, }, }, -- cgit v1.2.3 From c542aa57c87c805f53bba89a1018861194f07127 Mon Sep 17 00:00:00 2001 From: Iyaan Azeez Date: Wed, 14 Jan 2026 15:52:27 +0500 Subject: Feat: Add support for odinfmt custom config path Introduces a new argument to -config which allows a user to pass a json config for the formatter which is not neccesarilly in the path of the files we are formatting. For now the function will use the default_style if any errors occur. This was done due to that being the pattern in the source code. I think it would be useful to know if reading a config has failed or has defaulted to a default style but, I wanted to not include that in this. --- src/odin/format/format.odin | 20 ++++++++++++++++++++ tools/odinfmt/main.odin | 8 +++++++- 2 files changed, 27 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/odin/format/format.odin b/src/odin/format/format.odin index e732d51..2ab6a94 100644 --- a/src/odin/format/format.odin +++ b/src/odin/format/format.odin @@ -49,6 +49,26 @@ find_config_file_or_default :: proc(path: string) -> printer.Config { return config } +// Tries to read the config file from a given path instead +// of searching for it up a directory tree of a path +read_config_file_from_path_or_default :: proc(config_path: string) -> printer.Config { + path := config_path + ok: bool + if path, ok = filepath.abs(config_path); !ok { + return default_style + } + config := default_style + if (os.exists(path)) { + if data, ok := os.read_entire_file(path, context.temp_allocator); ok { + if json.unmarshal(data, &config) == nil { + return config + } + } + } + + return default_style +} + format :: proc( filepath: string, source: string, diff --git a/tools/odinfmt/main.odin b/tools/odinfmt/main.odin index 3470ca5..5096683 100644 --- a/tools/odinfmt/main.odin +++ b/tools/odinfmt/main.odin @@ -17,6 +17,7 @@ Args :: struct { write: bool `args:"name=w" usage:"write the new format to file"`, stdin: bool `usage:"formats code from standard input"`, path: string `args:"pos=0" usage:"set the file or directory to format"`, + config: string `usage:"path to a config file"` } format_file :: proc(filepath: string, config: printer.Config, allocator := context.allocator) -> (string, bool) { @@ -72,7 +73,12 @@ main :: proc() { watermark := 0 - config := format.find_config_file_or_default(args.path) + config: printer.Config + if args.config == "" { + config = format.find_config_file_or_default(args.path) + } else { + config = format.read_config_file_from_path_or_default(args.config) + } if args.stdin { data := make([dynamic]byte, arena_allocator) -- cgit v1.2.3 From 105bc8b8eacd255cc598d251bfe091f7bf169016 Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Thu, 15 Jan 2026 18:42:09 +1100 Subject: Correct hover info for indexed soa pointers --- src/server/analysis.odin | 35 +++++++++++++++++++++++++++++------ tests/hover_test.odin | 18 ++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/server/analysis.odin b/src/server/analysis.odin index 3ced0ad..a4227dc 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -1385,10 +1385,14 @@ resolve_call_directive :: proc(ast_context: ^AstContext, call: ^ast.Call_Expr) - if len(call.args) == 1 { ident := new_type(ast.Ident, call.pos, call.end, ast_context.allocator) ident.name = "u8" - value := SymbolSliceValue{ - expr = ident + value := SymbolSliceValue { + expr = ident, + } + symbol := Symbol { + name = "#load", + pkg = ast_context.current_package, + value = value, } - symbol := Symbol{name = "#load", pkg = ast_context.current_package, value = value} return symbol, true } else if len(call.args) == 2 { return resolve_type_expression(ast_context, call.args[1]) @@ -1407,10 +1411,14 @@ resolve_call_directive :: proc(ast_context: ^AstContext, call: ^ast.Call_Expr) - selector := new_type(ast.Selector_Expr, call.pos, call.end, ast_context.allocator) selector.expr = pkg selector.field = field - value := SymbolSliceValue{ - expr = selector + value := SymbolSliceValue { + expr = selector, + } + symbol := Symbol { + name = "#load_directory", + pkg = ast_context.current_package, + value = value, } - symbol := Symbol{name = "#load_directory", pkg = ast_context.current_package, value = value} return symbol, true } @@ -1429,11 +1437,23 @@ resolve_index_expr :: proc(ast_context: ^AstContext, index_expr: ^ast.Index_Expr #partial switch v in indexed.value { case SymbolDynamicArrayValue: + if .Soa in indexed.flags { + indexed.flags |= { .SoaPointer } + return indexed, true + } ok = internal_resolve_type_expression(ast_context, v.expr, &symbol) case SymbolSliceValue: ok = internal_resolve_type_expression(ast_context, v.expr, &symbol) + if .Soa in indexed.flags { + indexed.flags |= { .SoaPointer } + return indexed, true + } case SymbolFixedArrayValue: ok = internal_resolve_type_expression(ast_context, v.expr, &symbol) + if .Soa in indexed.flags { + indexed.flags |= { .SoaPointer } + return indexed, true + } case SymbolMapValue: ok = internal_resolve_type_expression(ast_context, v.value, &symbol) case SymbolMultiPointerValue: @@ -1471,6 +1491,9 @@ resolve_index_expr :: proc(ast_context: ^AstContext, index_expr: ^ast.Index_Expr } symbol.type = indexed.type + if .Soa in indexed.flags { + symbol.flags |= {.SoaPointer} + } return symbol, ok } diff --git a/tests/hover_test.odin b/tests/hover_test.odin index 8c39b0f..7cd2b24 100644 --- a/tests/hover_test.odin +++ b/tests/hover_test.odin @@ -5986,6 +5986,24 @@ ast_hover_proc_group_bitset :: proc(t: ^testing.T) { } test.expect_hover(t, &source, "test.Foo: .A") } + +@(test) +ast_hover_soa_struct_field_indexed :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + Foo :: struct{} + + Bar :: struct { + foos: #soa[dynamic]Foo, + } + + bazz :: proc(bar: ^Bar, index: int) { + f{*}oo := &bar.foos[index] + } + `, + } + test.expect_hover(t, &source, "test.foo: #soa^#soa[dynamic]Foo") +} /* Waiting for odin fix -- cgit v1.2.3 From 3494339e5b56ca90356dac095918d2bf02832baa Mon Sep 17 00:00:00 2001 From: Louis Dutton Date: Mon, 5 Jan 2026 11:31:27 +0000 Subject: Improve hoverdoc formatting --- src/server/analysis.odin | 8 ++-- src/server/ast.odin | 101 +++++++++++++++++++++++++++--------------- src/server/collector.odin | 4 +- src/server/documentation.odin | 16 +++---- src/server/hover.odin | 6 ++- src/server/signature.odin | 6 +-- src/server/symbol.odin | 12 ++--- tests/completions_test.odin | 6 +-- tests/hover_test.odin | 62 +++++++++++++------------- 9 files changed, 127 insertions(+), 94 deletions(-) (limited to 'src') diff --git a/src/server/analysis.odin b/src/server/analysis.odin index a4227dc..cec34e3 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -1981,8 +1981,8 @@ resolve_local_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, loca return_symbol.flags |= {.Local} return_symbol.value_expr = local.value_expr return_symbol.type_expr = local.type_expr - return_symbol.doc = get_doc(local.docs, ast_context.allocator) - return_symbol.comment = get_comment(local.comment) + return_symbol.doc = get_comment(local.docs, ast_context.allocator) + return_symbol.comment = get_comment(local.comment, ast_context.allocator) return return_symbol, ok } @@ -2071,11 +2071,11 @@ resolve_global_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, glo } if global.docs != nil { - return_symbol.doc = get_doc(global.docs, ast_context.allocator) + return_symbol.doc = get_comment(global.docs, ast_context.allocator) } if global.comment != nil { - return_symbol.comment = get_comment(global.comment) + return_symbol.comment = get_comment(global.comment, ast_context.allocator) } return_symbol.type_expr = global.type_expr diff --git a/src/server/ast.odin b/src/server/ast.odin index d1846f7..0547b83 100644 --- a/src/server/ast.odin +++ b/src/server/ast.odin @@ -7,6 +7,7 @@ import "core:odin/ast" import "core:odin/parser" import path "core:path/slashpath" import "core:strings" +import "core:log" keyword_map: map[string]struct{} = { "typeid" = {}, @@ -563,51 +564,79 @@ get_ast_node_string :: proc(node: ^ast.Node, src: string) -> string { return string(src[node.pos.offset:node.end.offset]) } -get_doc :: proc(comment: ^ast.Comment_Group, allocator: mem.Allocator) -> string { - if comment == nil { - return "" +COMMENT_DELIMITER_LENGTH :: len("//") +#assert(COMMENT_DELIMITER_LENGTH == len("/*")) +#assert(COMMENT_DELIMITER_LENGTH == len("*/")) + +// Returns the minimum indentation across all non-empty lines +get_min_indent :: proc(lines: []string) -> int { + min_indent := max(int) + for line in lines { + if strings.trim_space(line) == "" do continue + for c, i in line { + if !strings.is_space(c) { + min_indent = min(min_indent, i) + break + } + } } + return 0 if min_indent == max(int) else min_indent +} - tmp: string - - for doc in comment.list { - if strings.starts_with(doc.text, "/*") && doc.pos.column != 1 { - lines := strings.split(doc.text, "\n", context.temp_allocator) - for line, i in lines { - if i != 0 && len(line) > 0 { - column := 0 - for column < doc.pos.column - 1 { - if line[column] == '\t' || line[column] == ' ' { - column += 1 - } else { - break - } - } - tmp = strings.concatenate({tmp, "\n", line[column:]}, context.temp_allocator) - } else { - tmp = strings.concatenate({tmp, "\n", line}, context.temp_allocator) - } - } +// Strips min_indent characters from each line and joins with newlines +strip_indent_and_join :: proc(lines: []string, min_indent: int, allocator: mem.Allocator) -> string { + result := make([dynamic]string, context.temp_allocator) + for line in lines { + if len(line) >= min_indent { + append(&result, line[min_indent:]) } else { - tmp = strings.concatenate({tmp, "\n", doc.text}, context.temp_allocator) + append(&result, strings.trim_left_space(line)) } } + return strings.join(result[:], "\n", allocator) +} - if tmp != "" { - no_lines, _ := strings.replace_all(tmp, "//", "", context.temp_allocator) - no_begin_comments, _ := strings.replace_all(no_lines, "/*", "", context.temp_allocator) - no_end_comments, _ := strings.replace_all(no_begin_comments, "*/", "", context.temp_allocator) - return strings.clone(no_end_comments, allocator) - } +// Aggregates the content from the provided comment group, +// omitting extraneous spaces and delimiters. +get_comment :: proc(comment: ^ast.Comment_Group, allocator := context.allocator) -> string { + if comment == nil do return "" - return "" -} + lines := make([dynamic]string, context.temp_allocator) + + for token in comment.list { + if len(token.text) < COMMENT_DELIMITER_LENGTH do continue + delimiter := token.text[:COMMENT_DELIMITER_LENGTH] + + switch delimiter { + case "/*": + if len(token.text) <= COMMENT_DELIMITER_LENGTH * 2 do continue + content := token.text[COMMENT_DELIMITER_LENGTH:len(token.text) - COMMENT_DELIMITER_LENGTH] -get_comment :: proc(comment: ^ast.Comment_Group) -> string { - if comment != nil && len(comment.list) > 0 { - return comment.list[0].text + // Check if this is a single-line block comment (no newlines) + if !strings.contains(content, "\n") { + text := strings.trim_space(content) + if text != "" do append(&lines, text) + } else { + // Multi-line block comment: strip leading/trailing newlines + content = strings.trim(content, "\r\n") + for line in strings.split_lines(content, context.temp_allocator) { + append(&lines, line) + } + } + + case "//": + text := token.text[COMMENT_DELIMITER_LENGTH:] + append(&lines, text) + + case: + log.error("unsupported comment delimiter") + } } - return "" + + if len(lines) == 0 do return "" + + min_indent := get_min_indent(lines[:]) + return strip_indent_and_join(lines[:], min_indent, allocator) } free_ast :: proc { diff --git a/src/server/collector.odin b/src/server/collector.odin index d2b8445..d8a4e65 100644 --- a/src/server/collector.odin +++ b/src/server/collector.odin @@ -705,12 +705,12 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri symbol.range = common.get_token_range(expr.name_expr, file.src) symbol.name = get_index_unique_string(collection, name) symbol.type = token_type - symbol.doc = get_doc(expr.docs, collection.allocator) + symbol.doc = get_comment(expr.docs, collection.allocator) symbol.uri = get_index_unique_string(collection, uri) symbol.type_expr = clone_type(expr.type_expr, collection.allocator, &collection.unique_strings) symbol.value_expr = clone_type(expr.value_expr, collection.allocator, &collection.unique_strings) comment, _ := get_file_comment(file, symbol.range.start.line + 1) - symbol.comment = strings.clone(get_comment(comment), collection.allocator) + symbol.comment = get_comment(comment, collection.allocator) if expr.builtin || strings.contains(uri, "builtin.odin") { symbol.pkg = "$builtin" diff --git a/src/server/documentation.odin b/src/server/documentation.odin index 2fe8842..25e4140 100644 --- a/src/server/documentation.odin +++ b/src/server/documentation.odin @@ -6,6 +6,10 @@ import "core:odin/ast" import path "core:path/slashpath" import "core:strings" +DOC_SECTION_DELIMITER :: "\n---\n" // The string separating each section of documentation +DOC_FMT_ODIN :: "```odin\n%v\n```" // The format for wrapping odin code in a markdown codeblock +DOC_FMT_MARKDOWN :: DOC_FMT_ODIN + DOC_SECTION_DELIMITER + "%v" // The format for presenting documentation on hover + // Adds signature and docs information to the provided symbol // This should only be used for a symbol created with the temp allocator build_documentation :: proc(ast_context: ^AstContext, symbol: ^Symbol, short_signature := true) { @@ -20,21 +24,17 @@ build_documentation :: proc(ast_context: ^AstContext, symbol: ^Symbol, short_sig } } -construct_symbol_docs :: proc(symbol: Symbol, markdown := true, allocator := context.temp_allocator) -> string { +construct_symbol_docs :: proc(symbol: Symbol, allocator := context.temp_allocator) -> string { sb := strings.builder_make(allocator = allocator) if symbol.doc != "" { strings.write_string(&sb, symbol.doc) - if symbol.comment != "" { - strings.write_string(&sb, "\n") - } } if symbol.comment != "" { - if markdown { - fmt.sbprintf(&sb, "\n```odin\n%s\n```", symbol.comment) - } else { - fmt.sbprintf(&sb, "\n%s", symbol.comment) + if symbol.doc != "" { + strings.write_string(&sb, DOC_SECTION_DELIMITER) } + strings.write_string(&sb, symbol.comment) } return strings.to_string(sb) diff --git a/src/server/hover.odin b/src/server/hover.odin index ca791ef..9b42f7e 100644 --- a/src/server/hover.odin +++ b/src/server/hover.odin @@ -16,7 +16,11 @@ write_hover_content :: proc(ast_context: ^AstContext, symbol: Symbol) -> MarkupC if cat != "" { content.kind = "markdown" - content.value = fmt.tprintf("```odin\n%v\n```%v", cat, doc) + if doc != "" { + content.value = fmt.tprintf(DOC_FMT_MARKDOWN, cat, doc) + } else { + content.value = fmt.tprintf(DOC_FMT_ODIN, cat) + } } else { content.kind = "plaintext" } diff --git a/src/server/signature.odin b/src/server/signature.odin index 3148951..f6c79fc 100644 --- a/src/server/signature.odin +++ b/src/server/signature.odin @@ -110,7 +110,7 @@ get_signature_information :: proc( &signature_information, SignatureInformation { label = get_signature(symbol), - documentation = construct_symbol_docs(symbol, markdown = false), + documentation = construct_symbol_docs(symbol), }, ) } @@ -176,7 +176,7 @@ add_proc_signature :: proc( info := SignatureInformation { label = get_signature(call), - documentation = construct_symbol_docs(call, markdown = false), + documentation = construct_symbol_docs(call), parameters = parameters, } append(signature_information, info) @@ -204,7 +204,7 @@ add_proc_signature :: proc( info := SignatureInformation { label = get_signature(symbol), - documentation = construct_symbol_docs(symbol, markdown = false), + documentation = construct_symbol_docs(symbol), parameters = parameters, } diff --git a/src/server/symbol.odin b/src/server/symbol.odin index 956eaa9..96c83ed 100644 --- a/src/server/symbol.odin +++ b/src/server/symbol.odin @@ -922,8 +922,8 @@ construct_struct_field_symbol :: proc(symbol: ^Symbol, parent_name: string, valu symbol.name = value.names[index] symbol.type = .Field symbol.parent_name = parent_name - symbol.doc = get_doc(value.docs[index], context.temp_allocator) - symbol.comment = get_comment(value.comments[index]) + symbol.doc = get_comment(value.docs[index], context.temp_allocator) + symbol.comment = get_comment(value.comments[index], context.temp_allocator) symbol.range = value.ranges[index] } @@ -936,16 +936,16 @@ construct_bit_field_field_symbol :: proc( symbol.name = value.names[index] symbol.parent_name = parent_name symbol.type = .Field - symbol.doc = get_doc(value.docs[index], context.temp_allocator) - symbol.comment = get_comment(value.comments[index]) + symbol.doc = get_comment(value.docs[index], context.temp_allocator) + symbol.comment = get_comment(value.comments[index], context.temp_allocator) symbol.signature = get_bit_field_field_signature(value, index) symbol.range = value.ranges[index] } construct_enum_field_symbol :: proc(symbol: ^Symbol, value: SymbolEnumValue, index: int) { symbol.type = .Field - symbol.doc = get_doc(value.docs[index], context.temp_allocator) - symbol.comment = get_comment(value.comments[index]) + symbol.doc = get_comment(value.docs[index], context.temp_allocator) + symbol.comment = get_comment(value.comments[index], context.temp_allocator) symbol.signature = get_enum_field_signature(value, index) symbol.range = value.ranges[index] } diff --git a/tests/completions_test.odin b/tests/completions_test.odin index 033813b..53c7fe6 100644 --- a/tests/completions_test.odin +++ b/tests/completions_test.odin @@ -28,7 +28,7 @@ ast_simple_struct_completion :: proc(t: ^testing.T) { t, &source, ".", - {"My_Struct.one: int", "My_Struct.two: int\n// test comment", "My_Struct.three: int"}, + {"My_Struct.one: int", "My_Struct.two: int\n---\ntest comment", "My_Struct.three: int"}, ) } @@ -3296,7 +3296,7 @@ ast_completion_struct_documentation :: proc(t: ^testing.T) { packages = packages[:], } - test.expect_completion_docs(t, &source, "", {"Foo.bazz: my_package.My_Struct\n// bazz"}) + test.expect_completion_docs(t, &source, "", {"Foo.bazz: my_package.My_Struct\n---\nbazz"}) } @(test) @@ -3417,7 +3417,7 @@ ast_completion_poly_struct_another_package :: proc(t: ^testing.T) { packages = packages[:], } - test.expect_completion_docs(t, &source, "", {"Runner.state: test.State\n// state"}) + test.expect_completion_docs(t, &source, "", {"Runner.state: test.State\n---\nstate"}) } @(test) diff --git a/tests/hover_test.odin b/tests/hover_test.odin index 7cd2b24..9929ee3 100644 --- a/tests/hover_test.odin +++ b/tests/hover_test.odin @@ -401,7 +401,7 @@ ast_hover_proc_group :: proc(t: ^testing.T) { packages = {}, } - test.expect_hover(t, &source, "test.add :: proc(a, b: int) -> int\n docs\n\n// comment") + test.expect_hover(t, &source, "test.add :: proc(a, b: int) -> int\n---\ndocs\n---\ncomment") } @(test) @@ -672,7 +672,7 @@ ast_hover_struct_field_complex_definition :: proc(t: ^testing.T) { `, } - test.expect_hover(t, &source, "Foo.bar: ^test.Bar\n Docs\n\n// inline docs") + test.expect_hover(t, &source, "Foo.bar: ^test.Bar\n---\nDocs\n---\ninline docs") } @(test) @@ -1084,7 +1084,7 @@ ast_hover_proc_overloading_named_arg_with_selector_expr_with_another_package :: packages = packages[:], } - test.expect_hover(t, &source, "my_package.foo :: proc(x := 1) -> (_: int, _: bool)\n Docs\n\n// comment") + test.expect_hover(t, &source, "my_package.foo :: proc(x := 1) -> (_: int, _: bool)\n---\nDocs\n---\ncomment") } @(test) @@ -1479,7 +1479,7 @@ ast_hover_proc_comments :: proc(t: ^testing.T) { `, } - test.expect_hover(t, &source, "test.foo :: proc()\n doc\n\n// do foo") + test.expect_hover(t, &source, "test.foo :: proc()\n---\ndoc\n---\ndo foo") } @(test) @@ -1507,7 +1507,7 @@ ast_hover_proc_comments_package :: proc(t: ^testing.T) { packages = packages[:], } - test.expect_hover(t, &source, "my_package.foo :: proc()\n// do foo") + test.expect_hover(t, &source, "my_package.foo :: proc()\n---\ndo foo") } @(test) @@ -1525,7 +1525,7 @@ ast_hover_struct_field_distinct :: proc(t: ^testing.T) { `, } - test.expect_hover(t, &source, "S.fb: test.B\n// type: fb") + test.expect_hover(t, &source, "S.fb: test.B\n---\ntype: fb") } @(test) @@ -2135,7 +2135,7 @@ ast_hover_bit_field_field :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "Foo.foo_aa: uint | 6\n// last 6 bits") + test.expect_hover(t, &source, "Foo.foo_aa: uint | 6\n---\nlast 6 bits") } @(test) @@ -2154,7 +2154,7 @@ ast_hover_bit_field_variable_with_docs :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "Foo.foo_a: uint | 2\n doc\n\n// foo a") + test.expect_hover(t, &source, "Foo.foo_a: uint | 2\n---\ndoc\n---\nfoo a") } @(test) @@ -2353,7 +2353,7 @@ ast_hover_struct_field_should_show_docs_and_comments :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "Foo.a: int\n a docs\n\n// a comment") + test.expect_hover(t, &source, "Foo.a: int\n---\na docs\n---\na comment") } @(test) @@ -2367,7 +2367,7 @@ ast_hover_struct_field_should_show_docs_and_comments_field :: proc(t: ^testing.T } `, } - test.expect_hover(t, &source, "Foo.a: int\n a docs\n\n// a comment") + test.expect_hover(t, &source, "Foo.a: int\n---\na docs\n---\na comment") } @(test) @@ -2388,7 +2388,7 @@ ast_hover_struct_field_should_show_docs_and_comments_struct_types :: proc(t: ^te } `, } - test.expect_hover(t, &source, "Foo.bar: test.Bar\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: test.Bar\n---\nbar docs\n---\nbar comment") } @(test) @@ -2407,7 +2407,7 @@ ast_hover_struct_field_should_show_docs_and_comments_procs :: proc(t: ^testing.T } `, } - test.expect_hover(t, &source, "Foo.bar: proc(a: int) -> int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: proc(a: int) -> int\n---\nbar docs\n---\nbar comment") } @(test) @@ -2428,7 +2428,7 @@ ast_hover_struct_field_should_show_docs_and_comments_named_procs :: proc(t: ^tes } `, } - test.expect_hover(t, &source, "Foo.bar: proc(a: int) -> string\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: proc(a: int) -> string\n---\nbar docs\n---\nbar comment") } @(test) @@ -2447,7 +2447,7 @@ ast_hover_struct_field_should_show_docs_and_comments_maps :: proc(t: ^testing.T) } `, } - test.expect_hover(t, &source, "Foo.bar: map[int]int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: map[int]int\n---\nbar docs\n---\nbar comment") } @(test) @@ -2466,7 +2466,7 @@ ast_hover_struct_field_should_show_docs_and_comments_bit_sets :: proc(t: ^testin } `, } - test.expect_hover(t, &source, "Foo.bar: bit_set[0 ..< 10]\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: bit_set[0 ..< 10]\n---\nbar docs\n---\nbar comment") } @(test) @@ -2490,7 +2490,7 @@ ast_hover_struct_field_should_show_docs_and_comments_unions :: proc(t: ^testing. } `, } - test.expect_hover(t, &source, "Foo.bar: test.Bar\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: test.Bar\n---\nbar docs\n---\nbar comment") } @(test) @@ -2509,7 +2509,7 @@ ast_hover_struct_field_should_show_docs_and_comments_multipointers :: proc(t: ^t } `, } - test.expect_hover(t, &source, "Foo.bar: [^]int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: [^]int\n---\nbar docs\n---\nbar comment") } @(test) @@ -2528,7 +2528,7 @@ ast_hover_struct_field_should_show_docs_and_comments_dynamic_arrays :: proc(t: ^ } `, } - test.expect_hover(t, &source, "Foo.bar: [dynamic]int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: [dynamic]int\n---\nbar docs\n---\nbar comment") } @(test) @@ -2547,7 +2547,7 @@ ast_hover_struct_field_should_show_docs_and_comments_fixed_arrays :: proc(t: ^te } `, } - test.expect_hover(t, &source, "Foo.bar: [5]int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: [5]int\n---\nbar docs\n---\nbar comment") } @(test) @@ -2566,7 +2566,7 @@ ast_hover_struct_field_should_show_docs_and_comments_matrix :: proc(t: ^testing. } `, } - test.expect_hover(t, &source, "Foo.bar: matrix[4,5]int\n bar docs\n\n// bar comment") + test.expect_hover(t, &source, "Foo.bar: matrix[4,5]int\n---\nbar docs\n---\nbar comment") } @(test) @@ -3191,7 +3191,7 @@ ast_hover_documentation_reexported :: proc(t: ^testing.T) { `, packages = packages[:], } - test.expect_hover(t, &source, "my_package.Foo :: struct{}\n Documentation for Foo") + test.expect_hover(t, &source, "my_package.Foo :: struct{}\n---\nDocumentation for Foo") } @(test) @@ -3217,7 +3217,7 @@ ast_hover_override_documentation_reexported :: proc(t: ^testing.T) { `, packages = packages[:], } - test.expect_hover(t, &source, "my_package.Foo :: struct{}\n New docs for Foo") + test.expect_hover(t, &source, "my_package.Foo :: struct{}\n---\nNew docs for Foo") } @(test) @@ -3495,7 +3495,7 @@ ast_hover_enum_field_directly :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "test.Foo: .A\n Doc for A and B\n Mulitple lines!\n\n// comment for A and B") + test.expect_hover(t, &source, "test.Foo: .A\n---\nDoc for A and B\nMulitple lines!\n---\ncomment for A and B") } @(test) @@ -3624,7 +3624,7 @@ ast_hover_bit_set_intersection :: proc(t: ^testing.T) { foo_{*}b := foo_bar & {.Foo} // hover for foo_b `, } - test.expect_hover(t, &source, "test.foo_b: distinct bit_set[Flag]\n// hover for foo_b") + test.expect_hover(t, &source, "test.foo_b: distinct bit_set[Flag]\n---\nhover for foo_b") } @(test) @@ -3638,7 +3638,7 @@ ast_hover_bit_set_union :: proc(t: ^testing.T) { foo_{*}b := {.Foo} | foo_bar // hover for foo_b `, } - test.expect_hover(t, &source, "test.foo_b: distinct bit_set[Flag]\n// hover for foo_b") + test.expect_hover(t, &source, "test.foo_b: distinct bit_set[Flag]\n---\nhover for foo_b") } @(test) @@ -5366,7 +5366,7 @@ ast_hover_parapoly_other_package :: proc(t: ^testing.T) { `, packages = packages[:], } - test.expect_hover(t, &source, "my_package.bar :: proc(_: $T)\n Docs!\n\n// Comment!") + test.expect_hover(t, &source, "my_package.bar :: proc(_: $T)\n---\nDocs!\n---\nComment!") } @(test) @@ -5610,7 +5610,7 @@ ast_hover_local_proc_docs :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "test.foo :: proc()\n foo doc") + test.expect_hover(t, &source, "test.foo :: proc()\n---\nfoo doc") } @(test) @@ -5785,7 +5785,7 @@ ast_hover_nested_proc_docs_tabs :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "test.foo :: proc()\n\nDocs!\n\tDocs2\n") + test.expect_hover(t, &source, "test.foo :: proc()\n---\nDocs!\n\tDocs2\n") } @(test) @@ -5802,7 +5802,7 @@ ast_hover_nested_proc_docs_spaces :: proc(t: ^testing.T) { } `, } - test.expect_hover(t, &source, "test.foo :: proc()\n\nDocs!\n Docs2\n") + test.expect_hover(t, &source, "test.foo :: proc()\n---\nDocs!\n Docs2\n") } @(test) @@ -5825,7 +5825,7 @@ ast_hover_propagate_docs_alias_in_package :: proc(t: ^testing.T) { `, packages = packages[:], } - test.expect_hover(t, &source, "my_package.bar :: proc()\n Docs!\n\n// Comment!") + test.expect_hover(t, &source, "my_package.bar :: proc()\n---\nDocs!\n---\nComment!") } @(test) @@ -5849,7 +5849,7 @@ ast_hover_propagate_docs_alias_in_package_override :: proc(t: ^testing.T) { `, packages = packages[:], } - test.expect_hover(t, &source, "my_package.bar :: proc()\n Overridden\n\n// Comment!") + test.expect_hover(t, &source, "my_package.bar :: proc()\n---\nOverridden\n---\nComment!") } @(test) -- cgit v1.2.3 From 00955b4ddf08c2a87a53af72b25c0108f862d95e Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Sat, 24 Jan 2026 09:27:19 +1100 Subject: Correctly resolve hover info for poly types in where clauses for procs --- src/server/analysis.odin | 3 +++ src/server/ast.odin | 2 +- src/server/documentation.odin | 3 +++ src/server/file_resolve.odin | 3 +++ src/server/imports.odin | 2 -- src/server/locals.odin | 10 ++++++++-- src/server/position_context.odin | 7 ++++++- src/server/symbol.odin | 1 + tests/hover_test.odin | 20 ++++++++++++++++++++ 9 files changed, 45 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/server/analysis.odin b/src/server/analysis.odin index cec34e3..8e27d1e 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -1977,6 +1977,9 @@ resolve_local_identifier :: proc(ast_context: ^AstContext, node: ast.Ident, loca if .Variable in local.flags { return_symbol.flags |= {.Variable} } + if .PolyType in local.flags { + return_symbol.flags |= {.PolyType} + } return_symbol.flags |= {.Local} return_symbol.value_expr = local.value_expr diff --git a/src/server/ast.odin b/src/server/ast.odin index 0547b83..42c0ba6 100644 --- a/src/server/ast.odin +++ b/src/server/ast.odin @@ -561,7 +561,7 @@ collect_globals :: proc(file: ast.File) -> []GlobalExpr { } get_ast_node_string :: proc(node: ^ast.Node, src: string) -> string { - return string(src[node.pos.offset:node.end.offset]) + return strings.trim_prefix(string(src[node.pos.offset:node.end.offset]), "$") } COMMENT_DELIMITER_LENGTH :: len("//") diff --git a/src/server/documentation.odin b/src/server/documentation.odin index 25e4140..7c26a37 100644 --- a/src/server/documentation.odin +++ b/src/server/documentation.odin @@ -848,6 +848,9 @@ write_symbol_name :: proc(sb: ^strings.Builder, symbol: Symbol) { } else if pkg != "" && pkg != "$builtin" { fmt.sbprintf(sb, "%v.", pkg) } + if .PolyType in symbol.flags { + strings.write_string(sb, "$") + } strings.write_string(sb, symbol.name) } diff --git a/src/server/file_resolve.odin b/src/server/file_resolve.odin index c526030..8c7de51 100644 --- a/src/server/file_resolve.odin +++ b/src/server/file_resolve.odin @@ -512,6 +512,9 @@ resolve_node :: proc(node: ^ast.Node, data: ^FileResolveData) { data.position_context.struct_type = n resolve_node(n.poly_params, data) resolve_node(n.align, data) + for clause in n.where_clauses { + resolve_node(clause, data) + } resolve_node(n.fields, data) if data.flag != .None { diff --git a/src/server/imports.odin b/src/server/imports.odin index 256295b..16deec5 100644 --- a/src/server/imports.odin +++ b/src/server/imports.odin @@ -1,8 +1,6 @@ package server -import "core:log" import "core:mem" -import "core:odin/ast" import "base:runtime" diff --git a/src/server/locals.odin b/src/server/locals.odin index b02a841..3e3d126 100644 --- a/src/server/locals.odin +++ b/src/server/locals.odin @@ -6,6 +6,7 @@ import "core:odin/ast" LocalFlag :: enum { Mutable, // or constant Variable, // or type + PolyType, } DocumentLocal :: struct { @@ -1066,6 +1067,11 @@ get_locals_proc_param_and_results :: proc( if proc_lit.type != nil && proc_lit.type.params != nil { for arg in proc_lit.type.params.list { for name in arg.names { + flags: bit_set[LocalFlag] = {.Mutable} + if _, ok := name.derived.(^ast.Poly_Type); ok { + flags |= {.PolyType} + } + if arg.type != nil { str := get_ast_node_string(name, file.src) store_local( @@ -1076,7 +1082,7 @@ get_locals_proc_param_and_results :: proc( str, ast_context.non_mutable_only, false, - {.Mutable}, + flags, "", true, ) @@ -1097,7 +1103,7 @@ get_locals_proc_param_and_results :: proc( str, ast_context.non_mutable_only, false, - {.Mutable}, + flags, "", true, ) diff --git a/src/server/position_context.odin b/src/server/position_context.odin index 602d9a1..7670e79 100644 --- a/src/server/position_context.odin +++ b/src/server/position_context.odin @@ -61,7 +61,7 @@ DocumentPositionContext :: struct { import_stmt: ^ast.Import_Decl, type_cast: ^ast.Type_Cast, call_commas: []int, - directive: ^ast.Basic_Directive, + directive: ^ast.Basic_Directive, } @@ -845,6 +845,11 @@ get_document_position_node :: proc(node: ^ast.Node, position_context: ^DocumentP position_context.struct_type = n get_document_position(n.poly_params, position_context) get_document_position(n.align, position_context) + for clause in n.where_clauses { + if position_in_node(clause, position_context.position) { + get_document_position(clause, position_context) + } + } get_document_position(n.fields, position_context) case ^Union_Type: position_context.union_type = n diff --git a/src/server/symbol.odin b/src/server/symbol.odin index 96c83ed..14e231c 100644 --- a/src/server/symbol.odin +++ b/src/server/symbol.odin @@ -212,6 +212,7 @@ SymbolFlag :: enum { SoaPointer, Simd, Parameter, //If the symbol is a procedure argument + PolyType, } SymbolFlags :: bit_set[SymbolFlag] diff --git a/tests/hover_test.odin b/tests/hover_test.odin index 9929ee3..630f4d7 100644 --- a/tests/hover_test.odin +++ b/tests/hover_test.odin @@ -6004,6 +6004,26 @@ ast_hover_soa_struct_field_indexed :: proc(t: ^testing.T) { } test.expect_hover(t, &source, "test.foo: #soa^#soa[dynamic]Foo") } + +@(test) +ast_hover_proc_poly_params :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + foo :: proc($T{*}: int) {} + `, + } + test.expect_hover(t, &source, "test.$T: int") +} + +@(test) +ast_hover_proc_poly_params_where_clause :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + foo :: proc($T: int) where T{*} >= 0 {} + `, + } + test.expect_hover(t, &source, "test.$T: int") +} /* Waiting for odin fix -- cgit v1.2.3 From ce092169ca1d65b0c66fd2d81b0f00b828cf2e8c Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Sat, 24 Jan 2026 09:57:59 +1100 Subject: Correctly resolve child parapoly structs --- src/server/generics.odin | 10 ++++++++++ tests/completions_test.odin | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) (limited to 'src') diff --git a/src/server/generics.odin b/src/server/generics.odin index db858cc..8aa111f 100644 --- a/src/server/generics.odin +++ b/src/server/generics.odin @@ -793,6 +793,14 @@ resolve_poly_struct :: proc(ast_context: ^AstContext, b: ^SymbolStructValueBuild v.elem = expr case ^ast.Pointer_Type: v.elem = expr + case ^ast.Call_Expr: + for arg, i in v.args { + if call_ident, ok := arg.derived.(^ast.Ident); ok { + if ident.name == call_ident.name { + v.args[i] = expr + } + } + } } } else if data.parent_proc == nil { data.symbol_value_builder.types[data.i] = expr @@ -806,6 +814,8 @@ resolve_poly_struct :: proc(ast_context: ^AstContext, b: ^SymbolStructValueBuild data.parent = node case ^ast.Proc_Type: data.parent_proc = v + case ^ast.Call_Expr: + data.parent = v } return visitor diff --git a/tests/completions_test.odin b/tests/completions_test.odin index 53c7fe6..19f5363 100644 --- a/tests/completions_test.odin +++ b/tests/completions_test.odin @@ -5299,3 +5299,31 @@ ast_completion_struct_using_named_vector_types :: proc(t: ^testing.T) { } test.expect_completion_docs(t, &source, "", {"Foo.bar: [3]f32", "r: f32", "x: f32"}) } + +@(test) +ast_completion_parapoly_struct_with_parapoly_child :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + SomeEnum :: enum { + enumVal1, + enumVal2 + } + + ChildStruct:: struct($enumGeneric: typeid){ + Something : string, + GenericParam: enumGeneric + } + + ParentStruct :: struct($enumGeneric: typeid){ + ParentSomething: string, + Child: ChildStruct(enumGeneric) + } + + TestGenericStructs :: proc(){ + parent : ParentStruct(SomeEnum) = {}; + parent.Child.{*} + } + `, + } + test.expect_completion_docs(t, &source, "", {"ChildStruct.GenericParam: test.SomeEnum", "ChildStruct.Something: string"}) +} -- cgit v1.2.3 From 25fb7d45d18abf5a953a5d02ae446747035e0e24 Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Sun, 25 Jan 2026 21:57:42 +1100 Subject: Fix signature labels displaying empty symbol in definitions after commas --- src/server/analysis.odin | 3 +++ tests/signatures_test.odin | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) (limited to 'src') diff --git a/src/server/analysis.odin b/src/server/analysis.odin index 8e27d1e..000080d 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -2299,6 +2299,9 @@ internal_resolve_comp_literal :: proc( set_ast_package_set_scoped(ast_context, symbol.pkg) + if position_context.parent_comp_lit == nil { + return {}, false + } symbol, _ = resolve_type_comp_literal( ast_context, position_context, diff --git a/tests/signatures_test.odin b/tests/signatures_test.odin index d81a4da..4ee9ff3 100644 --- a/tests/signatures_test.odin +++ b/tests/signatures_test.odin @@ -649,6 +649,44 @@ signature_comp_lit_bit_set :: proc(t: ^testing.T) { ) } +@(test) +signature_comp_lit_struct_field_after_comma :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + Foo :: struct { + A,{*} + B, + } + `, + config = { + enable_comp_lit_signature_help = true, + } + } + + test.expect_signature_labels( + t, + &source, + {}, + ) +} + +@(test) +signature_comp_lit_proc_field_after_comma :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + foo :: proc(a, b,{*}: int) {} + `, + config = { + enable_comp_lit_signature_help = true, + } + } + + test.expect_signature_labels( + t, + &source, + {}, + ) +} /* @(test) signature_function_inside_when :: proc(t: ^testing.T) { -- cgit v1.2.3 From ef82ca514e3740b26dee115be9be2a2554a58f58 Mon Sep 17 00:00:00 2001 From: moonz Date: Mon, 26 Jan 2026 19:25:58 +0100 Subject: feat: go to resolved proc from a proc overload group when performing go-to-definition --- src/server/definition.odin | 93 ++++++++++++++++++++++++++++++++++++++++++++-- tests/definition_test.odin | 51 +++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/server/definition.odin b/src/server/definition.odin index c793f32..34d1315 100644 --- a/src/server/definition.odin +++ b/src/server/definition.odin @@ -100,6 +100,12 @@ get_definition_location :: proc(document: ^Document, position: common.Position) } if resolved, ok := resolve_location_selector(&ast_context, position_context.selector_expr); ok { + resolved = try_resolve_proc_group_overload( + &ast_context, + &position_context, + resolved, + position_context.selector_expr, + ) location.range = resolved.range uri = resolved.uri } else { @@ -139,12 +145,10 @@ get_definition_location :: proc(document: ^Document, position: common.Position) &ast_context, position_context.identifier.derived.(^ast.Ident)^, ); ok { + resolved = try_resolve_proc_group_overload(&ast_context, &position_context, resolved) if v, ok := resolved.value.(SymbolAggregateValue); ok { for symbol in v.symbols { - append(&locations, common.Location { - range = symbol.range, - uri = symbol.uri, - }) + append(&locations, common.Location{range = symbol.range, uri = symbol.uri}) } } location.range = resolved.range @@ -167,3 +171,84 @@ get_definition_location :: proc(document: ^Document, position: common.Position) return locations[:], true } + + +try_resolve_proc_group_overload :: proc( + ast_context: ^AstContext, + position_context: ^DocumentPositionContext, + symbol: Symbol, + selector_expr: ^ast.Node = nil, +) -> Symbol { + if position_context.call == nil { + return symbol + } + + call, is_call := position_context.call.derived.(^ast.Call_Expr) + if !is_call { + return symbol + } + + if position_in_exprs(call.args, position_context.position) { + return symbol + } + + // 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, + } + } + } + } + } + + proc_group_value, is_proc_group := full_symbol.value.(SymbolProcedureGroupValue) + if !is_proc_group { + return symbol + } + + old_call := ast_context.call + ast_context.call = call + defer { + ast_context.call = old_call + } + + if resolved, ok := resolve_function_overload(ast_context, proc_group_value.group.derived.(^ast.Proc_Group)); ok { + if resolved.name != "" { + if global, ok := ast_context.globals[resolved.name]; ok { + resolved.range = common.get_token_range(global.name_expr, ast_context.file.src) + resolved.uri = common.create_uri(global.name_expr.pos.file, ast_context.allocator).uri + } else if indexed_symbol, ok := lookup(resolved.name, resolved.pkg, ast_context.fullpath); ok { + resolved.range = indexed_symbol.range + resolved.uri = indexed_symbol.uri + } + } + return resolved + } + + return symbol +} diff --git a/tests/definition_test.odin b/tests/definition_test.odin index 4810361..62165e2 100644 --- a/tests/definition_test.odin +++ b/tests/definition_test.odin @@ -723,3 +723,54 @@ ast_goto_package_declaration_with_alias :: proc(t: ^testing.T) { test.expect_definition_locations(t, &source, locations[:]) } +@(test) +ast_goto_proc_group_overload_with_selector :: proc(t: ^testing.T) { + packages := make([dynamic]test.Package, context.temp_allocator) + + append(&packages, test.Package{pkg = "my_package", source = `package my_package + push_back :: proc(arr: ^[dynamic]int, val: int) {} + push_back_elems :: proc(arr: ^[dynamic]int, vals: ..int) {} + append :: proc{push_back, push_back_elems} + `}) + source := test.Source { + main = `package test + import mp "my_package" + + main :: proc() { + arr: [dynamic]int + mp.app{*}end(&arr, 1) + } + `, + packages = packages[:], + } + // 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 + locations := []common.Location { + {range = {start = {line = 1, character = 3}, end = {line = 1, character = 12}}}, + } + + test.expect_definition_locations(t, &source, locations[:]) +} + +@(test) +ast_goto_proc_group_overload_identifier :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + push_back :: proc(arr: ^[dynamic]int, val: int) {} + push_back_elems :: proc(arr: ^[dynamic]int, vals: ..int) {} + append :: proc{push_back, push_back_elems} + + main :: proc() { + arr: [dynamic]int + app{*}end(&arr, 1) + } + `, + } + // 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 + locations := []common.Location { + {range = {start = {line = 1, character = 2}, end = {line = 1, character = 11}}}, + } + + test.expect_definition_locations(t, &source, locations[:]) +} \ No newline at end of file -- cgit v1.2.3 From df40bd21d618a5e6f6162abf8a1420ee80c33803 Mon Sep 17 00:00:00 2001 From: moonz Date: Tue, 27 Jan 2026 00:16:17 +0100 Subject: feat: added a feature flag to turn overload resoution on and off --- README.md | 2 ++ misc/ols.schema.json | 5 +++++ src/common/config.odin | 1 + src/server/definition.odin | 18 +++++++++++------- src/server/requests.odin | 2 ++ src/server/types.odin | 1 + 6 files changed, 22 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/README.md b/README.md index bc8dfef..e697e66 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,8 @@ Options: - `enable_fake_methods`: Turn on fake methods completion. This is currently highly experimental. +- `enable_overload_resolution`: Enable go-to-definition to resolve overloaded procedures from procedure groups based on call arguments. + - `enable_references`: Turns on finding references for a symbol. _(Enabled by default)_ - `enable_document_highlights`: Turns on highlighting of symbol references in file. _(Enabled by default)_ diff --git a/misc/ols.schema.json b/misc/ols.schema.json index d9fee4e..4c9e447 100644 --- a/misc/ols.schema.json +++ b/misc/ols.schema.json @@ -87,6 +87,11 @@ "description": "Turn on fake methods completion.", "default": false }, + "enable_overload_resolution": { + "type": "boolean", + "description": "Enable go-to-definition to resolve overloaded procedures from procedure groups based on call arguments.", + "default": false + }, "enable_document_links": { "type": "boolean", "description": "Follow links when opening documentation.", diff --git a/src/common/config.odin b/src/common/config.odin index 6890685..a1f2400 100644 --- a/src/common/config.odin +++ b/src/common/config.odin @@ -33,6 +33,7 @@ Config :: struct { enable_std_references: bool, enable_import_fixer: bool, enable_fake_method: bool, + enable_overload_resolution: bool, enable_procedure_snippet: bool, enable_checker_only_saved: bool, enable_auto_import: bool, diff --git a/src/server/definition.odin b/src/server/definition.odin index 34d1315..9994afa 100644 --- a/src/server/definition.odin +++ b/src/server/definition.odin @@ -100,12 +100,14 @@ get_definition_location :: proc(document: ^Document, position: common.Position) } if resolved, ok := resolve_location_selector(&ast_context, position_context.selector_expr); ok { - resolved = try_resolve_proc_group_overload( - &ast_context, - &position_context, - resolved, - position_context.selector_expr, - ) + if common.config.enable_overload_resolution { + resolved = try_resolve_proc_group_overload( + &ast_context, + &position_context, + resolved, + position_context.selector_expr, + ) + } location.range = resolved.range uri = resolved.uri } else { @@ -145,7 +147,9 @@ get_definition_location :: proc(document: ^Document, position: common.Position) &ast_context, position_context.identifier.derived.(^ast.Ident)^, ); ok { - resolved = try_resolve_proc_group_overload(&ast_context, &position_context, resolved) + if common.config.enable_overload_resolution { + resolved = try_resolve_proc_group_overload(&ast_context, &position_context, resolved) + } if v, ok := resolved.value.(SymbolAggregateValue); ok { for symbol in v.symbols { append(&locations, common.Location{range = symbol.range, uri = symbol.uri}) diff --git a/src/server/requests.odin b/src/server/requests.odin index 351c384..3a1a163 100644 --- a/src/server/requests.odin +++ b/src/server/requests.odin @@ -452,6 +452,8 @@ read_ols_initialize_options :: proc(config: ^common.Config, ols_config: OlsConfi ols_config.enable_inlay_hints_implicit_return.(bool) or_else config.enable_inlay_hints_implicit_return 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 // Delete overriding collections. for it in ols_config.collections { diff --git a/src/server/types.odin b/src/server/types.odin index 0377e62..0d57b6f 100644 --- a/src/server/types.odin +++ b/src/server/types.odin @@ -419,6 +419,7 @@ OlsConfig :: struct { enable_hover: Maybe(bool), enable_document_symbols: Maybe(bool), enable_fake_methods: Maybe(bool), + enable_overload_resolution: Maybe(bool), enable_references: Maybe(bool), enable_document_highlights: Maybe(bool), enable_document_links: Maybe(bool), -- cgit v1.2.3 From 396f4b887e6d7fe50957dce2a7c06199a07b670e Mon Sep 17 00:00:00 2001 From: moonz Date: Tue, 27 Jan 2026 00:47:12 +0100 Subject: refactor: cleaning up the methods and config reading --- src/server/definition.odin | 96 ++++++++++++++++++++++++++++++---------------- src/server/requests.odin | 4 +- src/testing/testing.odin | 2 +- tests/definition_test.odin | 2 + 4 files changed, 68 insertions(+), 36 deletions(-) (limited to 'src') 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 -- cgit v1.2.3 From 23b5c69f52b0007a6727bd04a5fe373e97df0739 Mon Sep 17 00:00:00 2001 From: pc Date: Tue, 27 Jan 2026 02:03:56 +0100 Subject: feat: suggest proc group instead of individual procs --- src/server/collector.odin | 285 ++++++++++++++++++++++++++++++++++++-------- src/server/methods.odin | 281 +++++++++++++++++++++++++++++++------------ src/testing/testing.odin | 3 + tests/completions_test.odin | 114 ++++++++++++++++++ 4 files changed, 558 insertions(+), 125 deletions(-) (limited to 'src') diff --git a/src/server/collector.odin b/src/server/collector.odin index d8a4e65..dc7e052 100644 --- a/src/server/collector.odin +++ b/src/server/collector.odin @@ -32,10 +32,11 @@ Method :: struct { } SymbolPackage :: struct { - symbols: map[string]Symbol, - objc_structs: map[string]ObjcStruct, //mapping from struct name to function - methods: map[Method][dynamic]Symbol, - imports: [dynamic]string, //Used for references to figure whether the package is even able to reference the symbol + symbols: map[string]Symbol, + objc_structs: map[string]ObjcStruct, //mapping from struct name to function + methods: map[Method][dynamic]Symbol, + imports: [dynamic]string, //Used for references to figure whether the package is even able to reference the symbol + proc_group_members: map[string]bool, // Tracks procedure names that are part of proc groups (used by fake methods) } get_index_unique_string :: proc { @@ -437,46 +438,187 @@ add_comp_lit_fields :: proc( generic.ranges = ranges[:] } +/* + Records the names of procedures that are part of a proc group. + This is used by the fake methods feature to hide individual procs + when the proc group should be shown instead. +*/ +record_proc_group_members :: proc(collection: ^SymbolCollection, group: ^ast.Proc_Group, pkg_name: string) { + pkg := get_or_create_package(collection, pkg_name) + + for arg in group.args { + name := get_proc_group_member_name(arg) or_continue + pkg.proc_group_members[get_index_unique_string(collection, name)] = true + } +} + +@(private = "file") +get_proc_group_member_name :: proc(expr: ^ast.Expr) -> (name: string, ok: bool) { + #partial switch v in expr.derived { + case ^ast.Ident: + return v.name, true + case ^ast.Selector_Expr: + // For package.proc_name, we only care about the proc name + if field, is_ident := v.field.derived.(^ast.Ident); is_ident { + return field.name, true + } + } + return "", false +} + +@(private = "file") +get_or_create_package :: proc(collection: ^SymbolCollection, pkg_name: string) -> ^SymbolPackage { + pkg := &collection.packages[pkg_name] + if pkg.symbols == nil { + collection.packages[pkg_name] = {} + pkg = &collection.packages[pkg_name] + pkg.symbols = make(map[string]Symbol, 100, collection.allocator) + pkg.methods = make(map[Method][dynamic]Symbol, 100, collection.allocator) + pkg.objc_structs = make(map[string]ObjcStruct, 5, collection.allocator) + pkg.proc_group_members = make(map[string]bool, 10, collection.allocator) + } + return pkg +} + +/* + Collects a procedure as a fake method if it's not part of a proc group. +*/ collect_method :: proc(collection: ^SymbolCollection, symbol: Symbol) { pkg := &collection.packages[symbol.pkg] - if value, ok := symbol.value.(SymbolProcedureValue); ok { - if len(value.arg_types) == 0 { - return + // Skip procedures that are part of proc groups + if symbol.name in pkg.proc_group_members { + return + } + + value, ok := symbol.value.(SymbolProcedureValue) + if !ok { + return + } + if len(value.arg_types) == 0 { + return + } + + method, method_ok := get_method_from_first_arg(collection, value.arg_types[0].type, symbol.pkg) + if !method_ok { + return + } + add_symbol_to_method(collection, pkg, method, symbol) +} + +/* + Collects a proc group as a fake method based on its member procedures' first arguments. + The proc group is registered as a method for each distinct first-argument type + across all its members. +*/ +collect_proc_group_method :: proc(collection: ^SymbolCollection, symbol: Symbol) { + pkg := &collection.packages[symbol.pkg] + + group_value, ok := symbol.value.(SymbolProcedureGroupValue) + if !ok { + return + } + + proc_group, is_proc_group := group_value.group.derived.(^ast.Proc_Group) + if !is_proc_group || len(proc_group.args) == 0 { + return + } + + // Track which method keys we've already registered to avoid duplicates + registered_methods := make(map[Method]bool, len(proc_group.args), context.temp_allocator) + + // Register the proc group as a method for each distinct first-argument type + for member_expr in proc_group.args { + member_name, name_ok := get_proc_group_member_name(member_expr) + if !name_ok { + continue } - expr, _, ok := unwrap_pointer_ident(value.arg_types[0].type) + member_symbol, found := pkg.symbols[member_name] + if !found { + continue + } - if !ok { - return + member_proc, is_proc := member_symbol.value.(SymbolProcedureValue) + if !is_proc || len(member_proc.arg_types) == 0 { + continue } - method: Method + method, method_ok := get_method_from_first_arg(collection, member_proc.arg_types[0].type, symbol.pkg) + if !method_ok { + continue + } - #partial switch v in expr.derived { - case ^ast.Selector_Expr: - if ident, ok := v.expr.derived.(^ast.Ident); ok { - method.pkg = get_index_unique_string(collection, ident.name) - method.name = get_index_unique_string(collection, v.field.name) - } else { - return - } - case ^ast.Ident: - method.pkg = symbol.pkg - method.name = get_index_unique_string(collection, v.name) - case: - return + // Only add once per distinct method key + if method not_in registered_methods { + registered_methods[method] = true + add_symbol_to_method(collection, pkg, method, symbol) } + } +} - symbols := &pkg.methods[method] +@(private = "file") +get_method_from_first_arg :: proc( + collection: ^SymbolCollection, + first_arg_type: ^ast.Expr, + default_pkg: string, +) -> ( + method: Method, + ok: bool, +) { + expr, _, unwrap_ok := unwrap_pointer_ident(first_arg_type) + if !unwrap_ok { + return {}, false + } - if symbols == nil { - pkg.methods[method] = make([dynamic]Symbol, collection.allocator) - symbols = &pkg.methods[method] + #partial switch v in expr.derived { + case ^ast.Selector_Expr: + ident, is_ident := v.expr.derived.(^ast.Ident) + if !is_ident { + return {}, false } + method.pkg = get_index_unique_string(collection, ident.name) + method.name = get_index_unique_string(collection, v.field.name) + case ^ast.Ident: + // Check if this is a builtin type + if is_builtin_type_name(v.name) { + method.pkg = "$builtin" + } else { + method.pkg = default_pkg + } + method.name = get_index_unique_string(collection, v.name) + case: + return {}, false + } - append(symbols, symbol) + return method, true +} + +is_builtin_type_name :: proc(name: string) -> bool { + // Check all builtin type names from untyped_map + for names in untyped_map { + for builtin_name in names { + if name == builtin_name { + return true + } + } + } + // Also check some other builtin types not in untyped_map + switch name { + case "rawptr", "uintptr", "typeid", "any", "rune": + return true + } + return false +} + +@(private = "file") +add_symbol_to_method :: proc(collection: ^SymbolCollection, pkg: ^SymbolPackage, method: Method, symbol: Symbol) { + symbols := &pkg.methods[method] + if symbols == nil { + pkg.methods[method] = make([dynamic]Symbol, collection.allocator) + symbols = &pkg.methods[method] } + append(symbols, symbol) } collect_objc :: proc(collection: ^SymbolCollection, attributes: []^ast.Attribute, symbol: Symbol) { @@ -554,6 +696,20 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri } } + // Compute pkg early so it's available inside the switch + if expr.builtin || strings.contains(uri, "builtin.odin") { + symbol.pkg = "$builtin" + } else if strings.contains(uri, "intrinsics.odin") { + intrinsics_path := filepath.join( + elems = {common.config.collections["base"], "/intrinsics"}, + allocator = context.temp_allocator, + ) + intrinsics_path, _ = filepath.to_slash(intrinsics_path, context.temp_allocator) + symbol.pkg = get_index_unique_string(collection, intrinsics_path) + } else { + symbol.pkg = get_index_unique_string(collection, directory) + } + #partial switch v in col_expr.derived { case ^ast.Matrix_Type: token = v^ @@ -601,6 +757,10 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri symbol.value = SymbolProcedureGroupValue { group = clone_type(col_expr, collection.allocator, &collection.unique_strings), } + // Record proc group members for fake methods feature + if collection.config != nil && collection.config.enable_fake_method { + record_proc_group_members(collection, v, symbol.pkg) + } case ^ast.Struct_Type: token = v^ token_type = .Struct @@ -712,20 +872,7 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri comment, _ := get_file_comment(file, symbol.range.start.line + 1) symbol.comment = get_comment(comment, collection.allocator) - if expr.builtin || strings.contains(uri, "builtin.odin") { - symbol.pkg = "$builtin" - } else if strings.contains(uri, "intrinsics.odin") { - path := filepath.join( - elems = {common.config.collections["base"], "/intrinsics"}, - allocator = context.temp_allocator, - ) - - path, _ = filepath.to_slash(path, context.temp_allocator) - - symbol.pkg = get_index_unique_string(collection, path) - } else { - symbol.pkg = get_index_unique_string(collection, directory) - } + // symbol.pkg was already set earlier before the switch if is_distinct { symbol.flags |= {.Distinct} @@ -764,16 +911,13 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri pkg.symbols = make(map[string]Symbol, 100, collection.allocator) pkg.methods = make(map[Method][dynamic]Symbol, 100, collection.allocator) pkg.objc_structs = make(map[string]ObjcStruct, 5, collection.allocator) + pkg.proc_group_members = make(map[string]bool, 10, collection.allocator) } if .ObjC in symbol.flags { collect_objc(collection, expr.attributes, symbol) } - if symbol.type == .Function && common.config.enable_fake_method { - collect_method(collection, symbol) - } - if v, ok := pkg.symbols[symbol.name]; !ok || v.name == "" { pkg.symbols[symbol.name] = symbol } else { @@ -781,12 +925,59 @@ collect_symbols :: proc(collection: ^SymbolCollection, file: ast.File, uri: stri } } + // Second pass: collect fake methods after all symbols and proc group members are recorded + if collection.config != nil && collection.config.enable_fake_method { + collect_fake_methods(collection, exprs, directory, uri) + } + collect_imports(collection, file, directory) return .None } +/* + Collects fake methods for all procedures and proc groups. + This is done as a second pass after all symbols are collected, + so that we know which procedures are part of proc groups. +*/ +@(private = "file") +collect_fake_methods :: proc(collection: ^SymbolCollection, exprs: []GlobalExpr, directory: string, uri: string) { + for expr in exprs { + // Determine the package name (same logic as in collect_symbols) + pkg_name: string + if expr.builtin || strings.contains(uri, "builtin.odin") { + pkg_name = "$builtin" + } else if strings.contains(uri, "intrinsics.odin") { + intrinsics_path := filepath.join( + elems = {common.config.collections["base"], "/intrinsics"}, + allocator = context.temp_allocator, + ) + intrinsics_path, _ = filepath.to_slash(intrinsics_path, context.temp_allocator) + pkg_name = get_index_unique_string(collection, intrinsics_path) + } else { + pkg_name = get_index_unique_string(collection, directory) + } + + pkg, ok := &collection.packages[pkg_name] + if !ok { + continue + } + + symbol, found := pkg.symbols[expr.name] + if !found { + continue + } + + #partial switch _ in symbol.value { + case SymbolProcedureValue: + collect_method(collection, symbol) + case SymbolProcedureGroupValue: + collect_proc_group_method(collection, symbol) + } + } +} + Reference :: struct { identifiers: [dynamic]common.Location, selectors: map[string][dynamic]common.Range, diff --git a/src/server/methods.odin b/src/server/methods.odin index 757b37d..797a089 100644 --- a/src/server/methods.odin +++ b/src/server/methods.odin @@ -73,7 +73,7 @@ append_method_completion :: proc( for c in cases { method := Method { name = c, - pkg = selector_symbol.pkg, + pkg = "$builtin", // Untyped values are always builtin types } collect_methods( ast_context, @@ -86,9 +86,14 @@ append_method_completion :: proc( ) } } else { + // For typed values, check if it's a builtin type + method_pkg := selector_symbol.pkg + if is_builtin_type_name(selector_symbol.name) { + method_pkg = "$builtin" + } method := Method { name = selector_symbol.name, - pkg = selector_symbol.pkg, + pkg = method_pkg, } collect_methods( ast_context, @@ -114,83 +119,203 @@ collect_methods :: proc( results: ^[dynamic]CompletionResult, ) { for k, v in indexer.index.collection.packages { - if symbols, ok := &v.methods[method]; ok { - for &symbol in symbols { - if should_skip_private_symbol(symbol, ast_context.current_package, ast_context.fullpath) { - continue - } - resolve_unresolved_symbol(ast_context, &symbol) - - range, ok := get_range_from_selection_start_to_dot(position_context) - - if !ok { - return - } - - value: SymbolProcedureValue - value, ok = symbol.value.(SymbolProcedureValue) - - if !ok { - continue - } - - if len(value.arg_types) == 0 || value.arg_types[0].type == nil { - continue - } - - first_arg: Symbol - first_arg, ok = resolve_type_expression(ast_context, value.arg_types[0].type) - - if !ok { - continue - } - - pointers_to_add := first_arg.pointers - pointers - - references := "" - dereferences := "" - - if pointers_to_add > 0 { - for i in 0 ..< pointers_to_add { - references = fmt.tprintf("%v&", references) - } - } else if pointers_to_add < 0 { - for i in pointers_to_add ..< 0 { - dereferences = fmt.tprintf("%v^", dereferences) - } - } - - new_text := "" - - if symbol.pkg != ast_context.document_package { - new_text = fmt.tprintf( - "%v.%v", - path.base(get_symbol_pkg_name(ast_context, &symbol), false, ast_context.allocator), - symbol.name, - ) - } else { - new_text = fmt.tprintf("%v", symbol.name) - } - - if len(symbol.value.(SymbolProcedureValue).arg_types) > 1 { - new_text = fmt.tprintf("%v(%v%v%v$0)", new_text, references, receiver, dereferences) - } else { - new_text = fmt.tprintf("%v(%v%v%v)$0", new_text, references, receiver, dereferences) - } - - item := CompletionItem { - label = symbol.name, - kind = symbol_type_to_completion_kind(symbol.type), - detail = get_short_signature(ast_context, symbol), - additionalTextEdits = remove_edit, - textEdit = TextEdit{newText = new_text, range = {start = range.end, end = range.end}}, - insertTextFormat = .Snippet, - InsertTextMode = .adjustIndentation, - documentation = construct_symbol_docs(symbol), - } - - append(results, CompletionResult{completion_item = item}) + symbols, ok := &v.methods[method] + if !ok { + continue + } + + for &symbol in symbols { + if should_skip_private_symbol(symbol, ast_context.current_package, ast_context.fullpath) { + continue + } + resolve_unresolved_symbol(ast_context, &symbol) + + #partial switch &sym_value in symbol.value { + case SymbolProcedureValue: + add_proc_method_completion( + ast_context, + position_context, + &symbol, + sym_value, + pointers, + receiver, + remove_edit, + results, + ) + case SymbolProcedureGroupValue: + add_proc_group_method_completion( + ast_context, + position_context, + &symbol, + sym_value, + pointers, + receiver, + remove_edit, + results, + ) } } } } + +@(private = "file") +add_proc_method_completion :: proc( + ast_context: ^AstContext, + position_context: ^DocumentPositionContext, + symbol: ^Symbol, + value: SymbolProcedureValue, + pointers: int, + receiver: string, + remove_edit: []TextEdit, + results: ^[dynamic]CompletionResult, +) { + if len(value.arg_types) == 0 || value.arg_types[0].type == nil { + return + } + + range, ok := get_range_from_selection_start_to_dot(position_context) + if !ok { + return + } + + first_arg: Symbol + first_arg, ok = resolve_type_expression(ast_context, value.arg_types[0].type) + if !ok { + return + } + + references, dereferences := compute_pointer_adjustments(first_arg.pointers, pointers) + + new_text := build_method_call_text( + ast_context, + symbol, + receiver, + references, + dereferences, + len(value.arg_types) > 1, + ) + + item := CompletionItem { + label = symbol.name, + kind = symbol_type_to_completion_kind(symbol.type), + detail = get_short_signature(ast_context, symbol^), + additionalTextEdits = remove_edit, + textEdit = TextEdit{newText = new_text, range = {start = range.end, end = range.end}}, + insertTextFormat = .Snippet, + InsertTextMode = .adjustIndentation, + documentation = construct_symbol_docs(symbol^), + } + + append(results, CompletionResult{completion_item = item}) +} + +@(private = "file") +add_proc_group_method_completion :: proc( + ast_context: ^AstContext, + position_context: ^DocumentPositionContext, + symbol: ^Symbol, + value: SymbolProcedureGroupValue, + pointers: int, + receiver: string, + remove_edit: []TextEdit, + results: ^[dynamic]CompletionResult, +) { + proc_group, is_group := value.group.derived.(^ast.Proc_Group) + if !is_group || len(proc_group.args) == 0 { + return + } + + range, ok := get_range_from_selection_start_to_dot(position_context) + if !ok { + return + } + + // Get first member to determine pointer adjustments + first_member: Symbol + first_member, ok = resolve_type_expression(ast_context, proc_group.args[0]) + if !ok { + return + } + + member_proc, is_proc := first_member.value.(SymbolProcedureValue) + if !is_proc || len(member_proc.arg_types) == 0 || member_proc.arg_types[0].type == nil { + return + } + + first_arg: Symbol + first_arg, ok = resolve_type_expression(ast_context, member_proc.arg_types[0].type) + if !ok { + return + } + + references, dereferences := compute_pointer_adjustments(first_arg.pointers, pointers) + + // Proc groups always have multiple args (the receiver plus at least one overload's additional args) + new_text := build_method_call_text(ast_context, symbol, receiver, references, dereferences, true) + + item := CompletionItem { + label = symbol.name, + kind = symbol_type_to_completion_kind(symbol.type), + detail = get_short_signature(ast_context, symbol^), + additionalTextEdits = remove_edit, + textEdit = TextEdit{newText = new_text, range = {start = range.end, end = range.end}}, + insertTextFormat = .Snippet, + InsertTextMode = .adjustIndentation, + documentation = construct_symbol_docs(symbol^), + } + + append(results, CompletionResult{completion_item = item}) +} + +@(private = "file") +compute_pointer_adjustments :: proc( + first_arg_pointers: int, + current_pointers: int, +) -> ( + references: string, + dereferences: string, +) { + pointers_to_add := first_arg_pointers - current_pointers + + if pointers_to_add > 0 { + for _ in 0 ..< pointers_to_add { + references = fmt.tprintf("%v&", references) + } + } else if pointers_to_add < 0 { + for _ in pointers_to_add ..< 0 { + dereferences = fmt.tprintf("%v^", dereferences) + } + } + + return references, dereferences +} + +@(private = "file") +build_method_call_text :: proc( + ast_context: ^AstContext, + symbol: ^Symbol, + receiver: string, + references: string, + dereferences: string, + has_additional_args: bool, +) -> string { + new_text: string + + if symbol.pkg != ast_context.document_package { + new_text = fmt.tprintf( + "%v.%v", + path.base(get_symbol_pkg_name(ast_context, symbol), false, ast_context.allocator), + symbol.name, + ) + } else { + new_text = fmt.tprintf("%v", symbol.name) + } + + if has_additional_args { + new_text = fmt.tprintf("%v(%v%v%v$0)", new_text, references, receiver, dereferences) + } else { + new_text = fmt.tprintf("%v(%v%v%v)$0", new_text, references, receiver, dereferences) + } + + return new_text +} diff --git a/src/testing/testing.odin b/src/testing/testing.odin index 2013f79..20327a7 100644 --- a/src/testing/testing.odin +++ b/src/testing/testing.odin @@ -68,6 +68,9 @@ setup :: proc(src: ^Source) { server.setup_index() + // Set the collection's config to the test's config to enable feature flags like enable_fake_method + server.indexer.index.collection.config = &src.config + server.document_setup(src.document) server.document_refresh(src.document, &src.config, nil) diff --git a/tests/completions_test.odin b/tests/completions_test.odin index 19f5363..8ed9b43 100644 --- a/tests/completions_test.odin +++ b/tests/completions_test.odin @@ -5327,3 +5327,117 @@ ast_completion_parapoly_struct_with_parapoly_child :: proc(t: ^testing.T) { } test.expect_completion_docs(t, &source, "", {"ChildStruct.GenericParam: test.SomeEnum", "ChildStruct.Something: string"}) } + +@(test) +ast_completion_fake_method_simple :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + import "methods" + main :: proc() { + n: int + n.{*} + } + `, + packages = { + { + pkg = "methods", + source = `package methods + double :: proc(x: int) -> int { return x * 2 } + `, + }, + }, + config = {enable_fake_method = true}, + } + // Should show 'double' as a fake method for int + test.expect_completion_labels(t, &source, ".", {"double"}) +} + +@(test) +ast_completion_fake_method_proc_group :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + import "methods" + main :: proc() { + n: int + n.{*} + } + `, + packages = { + { + pkg = "methods", + source = `package methods + add_int :: proc(a, b: int) -> int { return a + b } + add_something :: proc(a: int, b: string) {} + add_float :: proc(a, b: f32) -> f32 { return a + b } + add :: proc { add_float, add_int, add_something } + `, + }, + }, + config = {enable_fake_method = true}, + } + // Should show 'add' (the proc group), not 'add_int' or 'add_something' (individual procs) + test.expect_completion_labels(t, &source, ".", {"add"}, {"add_int", "add_something"}) +} + +@(test) +ast_completion_fake_method_proc_group_only_shows_group :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + import "methods" + main :: proc() { + s: methods.My_Struct + s.{*} + } + `, + packages = { + { + pkg = "methods", + source = `package methods + My_Struct :: struct { x: int } + + do_thing_int :: proc(s: My_Struct, v: int) {} + do_thing_str :: proc(s: My_Struct, v: string) {} + do_thing :: proc { do_thing_int, do_thing_str } + + // standalone proc not in a group + standalone_method :: proc(s: My_Struct) {} + `, + }, + }, + config = {enable_fake_method = true}, + } + // Should show 'do_thing' (group) and 'standalone_method', but NOT 'do_thing_int' or 'do_thing_str' + test.expect_completion_labels(t, &source, ".", {"do_thing", "standalone_method"}, {"do_thing_int", "do_thing_str"}) +} + +@(test) +ast_completion_fake_method_builtin_type_uses_builtin_pkg :: proc(t: ^testing.T) { + // This test verifies that fake methods for builtin types (int, f32, string, etc.) + // are correctly looked up using "$builtin" as the package, not the package where + // the variable is declared. Without this fix, the method lookup would fail because: + // - Storage: method stored with key {pkg = "$builtin", name = "int"} + // - Lookup (wrong): would use {pkg = "test", name = "int"} based on variable's declaring package + // - Lookup (correct): uses {pkg = "$builtin", name = "int"} for builtin types + source := test.Source { + main = `package test + import "math_utils" + main :: proc() { + x: f32 + x.{*} + } + `, + packages = { + { + pkg = "math_utils", + source = `package math_utils + square :: proc(v: f32) -> f32 { return v * v } + cube :: proc(v: f32) -> f32 { return v * v * v } + `, + }, + }, + config = {enable_fake_method = true}, + } + // Both methods should appear as fake methods for f32, proving that + // the lookup correctly uses "$builtin" instead of "test" for the package + test.expect_completion_labels(t, &source, ".", {"square", "cube"}) +} -- cgit v1.2.3 From 219d0157cf409f23751b719080aa212cc1ebc1f5 Mon Sep 17 00:00:00 2001 From: pc Date: Tue, 27 Jan 2026 02:13:42 +0100 Subject: feat: add completion edit text test for proc group with single argument --- src/server/methods.odin | 19 +++++++++++++++++-- src/testing/testing.odin | 44 ++++++++++++++++++++++++++++++++++++++++++++ tests/completions_test.odin | 27 +++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/server/methods.odin b/src/server/methods.odin index 797a089..19d9ff4 100644 --- a/src/server/methods.odin +++ b/src/server/methods.odin @@ -250,8 +250,23 @@ add_proc_group_method_completion :: proc( references, dereferences := compute_pointer_adjustments(first_arg.pointers, pointers) - // Proc groups always have multiple args (the receiver plus at least one overload's additional args) - new_text := build_method_call_text(ast_context, symbol, receiver, references, dereferences, true) + // Check if any member of the proc group has additional arguments beyond the receiver + has_additional_args := false + for member_expr in proc_group.args { + member: Symbol + member, ok = resolve_type_expression(ast_context, member_expr) + if !ok { + continue + } + if proc_val, is_proc_val := member.value.(SymbolProcedureValue); is_proc_val { + if len(proc_val.arg_types) > 1 { + has_additional_args = true + break + } + } + } + + new_text := build_method_call_text(ast_context, symbol, receiver, references, dereferences, has_additional_args) item := CompletionItem { label = symbol.name, diff --git a/src/testing/testing.odin b/src/testing/testing.odin index 20327a7..0ba7b0f 100644 --- a/src/testing/testing.odin +++ b/src/testing/testing.odin @@ -321,6 +321,50 @@ expect_completion_insert_text :: proc( } } +expect_completion_edit_text :: proc( + t: ^testing.T, + src: ^Source, + trigger_character: string, + label: string, + expected_text: string, +) { + setup(src) + defer teardown(src) + + completion_context := server.CompletionContext { + triggerCharacter = trigger_character, + } + + completion_list, ok := server.get_completion_list(src.document, src.position, completion_context, &src.config) + + if !ok { + log.error("Failed get_completion_list") + } + + found := false + for completion in completion_list.items { + if completion.label == label { + found = true + if text_edit, has_edit := completion.textEdit.(server.TextEdit); has_edit { + if text_edit.newText != expected_text { + log.errorf( + "Completion '%v' expected textEdit.newText %q, but received %q", + label, + expected_text, + text_edit.newText, + ) + } + } else { + log.errorf("Completion '%v' has no textEdit", label) + } + break + } + } + if !found { + log.errorf("Expected completion label '%v' not found in %v", label, completion_list.items) + } +} + expect_hover :: proc(t: ^testing.T, src: ^Source, expect_hover_string: string) { setup(src) defer teardown(src) diff --git a/tests/completions_test.odin b/tests/completions_test.odin index 8ed9b43..f0fa8c1 100644 --- a/tests/completions_test.odin +++ b/tests/completions_test.odin @@ -5441,3 +5441,30 @@ ast_completion_fake_method_builtin_type_uses_builtin_pkg :: proc(t: ^testing.T) // the lookup correctly uses "$builtin" instead of "test" for the package test.expect_completion_labels(t, &source, ".", {"square", "cube"}) } + +@(test) +ast_completion_fake_method_proc_group_single_arg_cursor_position :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + import "methods" + main :: proc() { + n: int + n.{*} + } + `, + packages = { + { + pkg = "methods", + source = `package methods + // All members only take a single argument (the receiver) + negate_a :: proc(x: int) -> int { return -x } + negate_b :: proc(x: int) -> int { return 0 - x } + negate :: proc { negate_a, negate_b } + `, + }, + }, + config = {enable_fake_method = true}, + } + // The proc group 'negate' should have cursor AFTER parentheses since no additional args + test.expect_completion_edit_text(t, &source, ".", "negate", "methods.negate(n)$0") +} -- cgit v1.2.3 From 35fd3cef3b94c0495508b229db9ff6da647a6c46 Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:57:34 +1100 Subject: Add option to put the signature help for comp lits in the docs to improve rendering with certain editors --- README.md | 2 ++ misc/ols.schema.json | 5 +++ src/common/config.odin | 77 ++++++++++++++++++++++++----------------------- src/server/requests.odin | 2 ++ src/server/signature.odin | 36 +++++++++++++++------- src/server/types.odin | 65 +++++++++++++++++++-------------------- 6 files changed, 106 insertions(+), 81 deletions(-) (limited to 'src') diff --git a/README.md b/README.md index bc8dfef..75b51ff 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,8 @@ Options: - `enable_comp_lit_signature_help`: Provide signature help for comp lits such as when instantiating structs. Will not display correctly on some editors such as vscode. +- `enable_comp_lit_signature_help_use_docs`: Put signature help for comp lits in the documentation. This will allow it to be rendered nicely using markdown in editors that render the label without colour on one line. + - `odin_command`: Specify the location to your Odin executable, rather than relying on the environment path. - `odin_root_override`: Allows you to specify a custom `ODIN_ROOT` that `ols` will use to look for `odin` core libraries when implementing custom runtimes. diff --git a/misc/ols.schema.json b/misc/ols.schema.json index d9fee4e..c95d8e6 100644 --- a/misc/ols.schema.json +++ b/misc/ols.schema.json @@ -97,6 +97,11 @@ "description": "Provide signature help for comp lits such as when instantiating structs. Will not display correctly on some editors such as vscode.", "default": false }, + "enable_comp_lit_signature_help_use_docs": { + "type": "boolean", + "description": "Put signature help for comp lits in the documentation. This will allow it to be rendered nicely using markdown in editors that render the label without colour on one line.", + "default": false + }, "disable_parser_errors": { "type": "boolean" }, "verbose": { "type": "boolean", diff --git a/src/common/config.odin b/src/common/config.odin index 6890685..441e8a9 100644 --- a/src/common/config.odin +++ b/src/common/config.odin @@ -10,44 +10,45 @@ ConfigProfile :: struct { } Config :: struct { - workspace_folders: [dynamic]WorkspaceFolder, - completion_support_md: bool, - hover_support_md: bool, - signature_offset_support: bool, - collections: map[string]string, - running: bool, - verbose: bool, - enable_format: bool, - enable_hover: bool, - enable_document_symbols: bool, - enable_semantic_tokens: bool, - enable_unused_imports_reporting: bool, - enable_inlay_hints_params: bool, - enable_inlay_hints_default_params: bool, - enable_inlay_hints_implicit_return: bool, - enable_procedure_context: bool, - enable_snippets: bool, - enable_references: bool, - enable_document_highlights: bool, - enable_label_details: bool, - enable_std_references: bool, - enable_import_fixer: bool, - enable_fake_method: bool, - enable_procedure_snippet: bool, - enable_checker_only_saved: bool, - enable_auto_import: bool, - enable_completion_matching: bool, - enable_document_links: bool, - enable_comp_lit_signature_help: bool, - disable_parser_errors: bool, - thread_count: int, - file_log: bool, - odin_command: string, - odin_root_override: string, - checker_args: string, - checker_targets: []string, - client_name: string, - profile: ConfigProfile, + workspace_folders: [dynamic]WorkspaceFolder, + completion_support_md: bool, + hover_support_md: bool, + signature_offset_support: bool, + collections: map[string]string, + running: bool, + verbose: bool, + enable_format: bool, + enable_hover: bool, + enable_document_symbols: bool, + enable_semantic_tokens: bool, + enable_unused_imports_reporting: bool, + enable_inlay_hints_params: bool, + enable_inlay_hints_default_params: bool, + enable_inlay_hints_implicit_return: bool, + enable_procedure_context: bool, + enable_snippets: bool, + enable_references: bool, + enable_document_highlights: bool, + enable_label_details: bool, + enable_std_references: bool, + enable_import_fixer: bool, + enable_fake_method: bool, + enable_procedure_snippet: bool, + enable_checker_only_saved: bool, + enable_auto_import: bool, + enable_completion_matching: bool, + enable_document_links: bool, + enable_comp_lit_signature_help: bool, + enable_comp_lit_signature_help_use_docs: bool, + disable_parser_errors: bool, + thread_count: int, + file_log: bool, + odin_command: string, + odin_root_override: string, + checker_args: string, + checker_targets: []string, + client_name: string, + profile: ConfigProfile, } config: Config diff --git a/src/server/requests.odin b/src/server/requests.odin index 351c384..0d6e025 100644 --- a/src/server/requests.odin +++ b/src/server/requests.odin @@ -376,6 +376,8 @@ read_ols_initialize_options :: proc(config: ^common.Config, ols_config: OlsConfi config.enable_document_links = ols_config.enable_document_links.(bool) or_else config.enable_document_links config.enable_comp_lit_signature_help = ols_config.enable_comp_lit_signature_help.(bool) or_else config.enable_comp_lit_signature_help + config.enable_comp_lit_signature_help_use_docs = + ols_config.enable_comp_lit_signature_help_use_docs.(bool) or_else config.enable_comp_lit_signature_help_use_docs config.verbose = ols_config.verbose.(bool) or_else config.verbose config.file_log = ols_config.file_log.(bool) or_else config.file_log diff --git a/src/server/signature.odin b/src/server/signature.odin index f6c79fc..a53c084 100644 --- a/src/server/signature.odin +++ b/src/server/signature.odin @@ -1,5 +1,6 @@ package server +import "core:fmt" import "core:log" import "core:odin/ast" import "core:odin/tokenizer" @@ -30,7 +31,7 @@ SignatureHelp :: struct { SignatureInformation :: struct { label: string, - documentation: string, + documentation: MarkupContent, parameters: []ParameterInformation, } @@ -105,14 +106,21 @@ get_signature_information :: proc( if config.enable_comp_lit_signature_help { if symbol, ok := resolve_comp_literal(&ast_context, &position_context); ok { - build_documentation(&ast_context, &symbol, short_signature = false) - append( - &signature_information, - SignatureInformation { - label = get_signature(symbol), - documentation = construct_symbol_docs(symbol), - }, - ) + if config.enable_comp_lit_signature_help_use_docs { + build_documentation(&ast_context, &symbol, short_signature = true) + signature := get_signature(symbol) + build_documentation(&ast_context, &symbol, short_signature = false) + append( + &signature_information, + SignatureInformation{label = signature, documentation = write_hover_content(&ast_context, symbol)}, + ) + } else { + build_documentation(&ast_context, &symbol, short_signature = false) + append( + &signature_information, + SignatureInformation{label = get_signature(symbol), documentation = write_markdown_doc(symbol)}, + ) + } } } @@ -176,7 +184,7 @@ add_proc_signature :: proc( info := SignatureInformation { label = get_signature(call), - documentation = construct_symbol_docs(call), + documentation = write_markdown_doc(call), parameters = parameters, } append(signature_information, info) @@ -204,7 +212,7 @@ add_proc_signature :: proc( info := SignatureInformation { label = get_signature(symbol), - documentation = construct_symbol_docs(symbol), + documentation = write_markdown_doc(symbol), parameters = parameters, } @@ -214,3 +222,9 @@ add_proc_signature :: proc( } return active_parameter } + +@(private = "file") +write_markdown_doc :: proc(symbol: Symbol) -> MarkupContent { + doc := construct_symbol_docs(symbol) + return MarkupContent{kind = "markdown", value = fmt.tprintf(DOC_FMT_ODIN, doc)} +} diff --git a/src/server/types.odin b/src/server/types.odin index 0377e62..c0bcf9e 100644 --- a/src/server/types.odin +++ b/src/server/types.odin @@ -277,10 +277,10 @@ DiagnosticSeverity :: enum { Hint = 4, } - DiagnosticTag :: enum int { +DiagnosticTag :: enum int { Unnecessary = 1, Deprecated = 2, - } +} Diagnostic :: struct { range: common.Range, @@ -413,36 +413,37 @@ FileSystemWatcher :: struct { } OlsConfig :: struct { - collections: [dynamic]OlsConfigCollection, - thread_pool_count: Maybe(int), - enable_format: Maybe(bool), - enable_hover: Maybe(bool), - enable_document_symbols: Maybe(bool), - enable_fake_methods: Maybe(bool), - enable_references: Maybe(bool), - enable_document_highlights: Maybe(bool), - enable_document_links: Maybe(bool), - enable_comp_lit_signature_help: Maybe(bool), - enable_completion_matching: Maybe(bool), - enable_inlay_hints_params: Maybe(bool), - enable_inlay_hints_default_params: Maybe(bool), - enable_inlay_hints_implicit_return: Maybe(bool), - enable_semantic_tokens: Maybe(bool), - enable_unused_imports_reporting: Maybe(bool), - enable_procedure_context: Maybe(bool), - enable_snippets: Maybe(bool), - enable_procedure_snippet: Maybe(bool), - enable_checker_only_saved: Maybe(bool), - enable_auto_import: Maybe(bool), - disable_parser_errors: Maybe(bool), - verbose: Maybe(bool), - file_log: Maybe(bool), - odin_command: string, - odin_root_override: string, - checker_args: string, - checker_targets: []string, - profiles: [dynamic]common.ConfigProfile, - profile: string, + collections: [dynamic]OlsConfigCollection, + thread_pool_count: Maybe(int), + enable_format: Maybe(bool), + enable_hover: Maybe(bool), + enable_document_symbols: Maybe(bool), + enable_fake_methods: Maybe(bool), + enable_references: Maybe(bool), + enable_document_highlights: Maybe(bool), + enable_document_links: Maybe(bool), + enable_comp_lit_signature_help: Maybe(bool), + enable_comp_lit_signature_help_use_docs: Maybe(bool), + enable_completion_matching: Maybe(bool), + enable_inlay_hints_params: Maybe(bool), + enable_inlay_hints_default_params: Maybe(bool), + enable_inlay_hints_implicit_return: Maybe(bool), + enable_semantic_tokens: Maybe(bool), + enable_unused_imports_reporting: Maybe(bool), + enable_procedure_context: Maybe(bool), + enable_snippets: Maybe(bool), + enable_procedure_snippet: Maybe(bool), + enable_checker_only_saved: Maybe(bool), + enable_auto_import: Maybe(bool), + disable_parser_errors: Maybe(bool), + verbose: Maybe(bool), + file_log: Maybe(bool), + odin_command: string, + odin_root_override: string, + checker_args: string, + checker_targets: []string, + profiles: [dynamic]common.ConfigProfile, + profile: string, } OlsConfigCollection :: struct { -- cgit v1.2.3 From ee962b842b6d2fc6f73a427301289e2d19b38c40 Mon Sep 17 00:00:00 2001 From: moonz Date: Tue, 27 Jan 2026 11:31:44 +0100 Subject: feat: added one more test to make sure there are no edge cases --- src/server/collector.odin | 4 ---- tests/completions_test.odin | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/server/collector.odin b/src/server/collector.odin index dc7e052..37d3cd8 100644 --- a/src/server/collector.odin +++ b/src/server/collector.odin @@ -486,7 +486,6 @@ get_or_create_package :: proc(collection: ^SymbolCollection, pkg_name: string) - collect_method :: proc(collection: ^SymbolCollection, symbol: Symbol) { pkg := &collection.packages[symbol.pkg] - // Skip procedures that are part of proc groups if symbol.name in pkg.proc_group_members { return } @@ -549,7 +548,6 @@ collect_proc_group_method :: proc(collection: ^SymbolCollection, symbol: Symbol) continue } - // Only add once per distinct method key if method not_in registered_methods { registered_methods[method] = true add_symbol_to_method(collection, pkg, method, symbol) @@ -580,7 +578,6 @@ get_method_from_first_arg :: proc( method.pkg = get_index_unique_string(collection, ident.name) method.name = get_index_unique_string(collection, v.field.name) case ^ast.Ident: - // Check if this is a builtin type if is_builtin_type_name(v.name) { method.pkg = "$builtin" } else { @@ -595,7 +592,6 @@ get_method_from_first_arg :: proc( } is_builtin_type_name :: proc(name: string) -> bool { - // Check all builtin type names from untyped_map for names in untyped_map { for builtin_name in names { if name == builtin_name { diff --git a/tests/completions_test.odin b/tests/completions_test.odin index f0fa8c1..56553c3 100644 --- a/tests/completions_test.odin +++ b/tests/completions_test.odin @@ -5410,6 +5410,38 @@ ast_completion_fake_method_proc_group_only_shows_group :: proc(t: ^testing.T) { test.expect_completion_labels(t, &source, ".", {"do_thing", "standalone_method"}, {"do_thing_int", "do_thing_str"}) } +@(test) +ast_completion_fake_method_proc_group_with_only_one_proc :: proc(t: ^testing.T) { + // This is to verify that even if a proc group has only one member, + // it still shows up as a group and does not show the individual proc. + source := test.Source { + main = `package test + import "methods" + main :: proc() { + s: methods.My_Struct + s.{*} + } + `, + packages = { + { + pkg = "methods", + source = `package methods + My_Struct :: struct { x: int } + + do_thing_int :: proc(s: My_Struct, v: int) {} + do_thing :: proc { do_thing_int } + + // standalone proc not in a group + standalone_method :: proc(s: My_Struct) {} + `, + }, + }, + config = {enable_fake_method = true}, + } + + test.expect_completion_labels(t, &source, ".", {"do_thing", "standalone_method"}, {"do_thing_int" }) +} + @(test) ast_completion_fake_method_builtin_type_uses_builtin_pkg :: proc(t: ^testing.T) { // This test verifies that fake methods for builtin types (int, f32, string, etc.) -- cgit v1.2.3 From c61aa239fb8b15f604dca9cb520f15f3562b427f Mon Sep 17 00:00:00 2001 From: Tasha Companion Date: Fri, 30 Jan 2026 06:24:19 -0800 Subject: #+feature using-stmt `#+feature using-stmt` is required to build successfully now. --- src/odin/printer/visit.odin | 11 ++++++----- src/server/analysis.odin | 1 + src/server/ast.odin | 1 + src/server/clone.odin | 1 + src/server/collector.odin | 1 + src/server/file_resolve.odin | 3 ++- src/server/locals.odin | 1 + src/server/position_context.odin | 1 + src/server/semantic_tokens.odin | 3 ++- src/server/unmarshal.odin | 1 + 10 files changed, 17 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/odin/printer/visit.odin b/src/odin/printer/visit.odin index 40d3f83..d025108 100644 --- a/src/odin/printer/visit.odin +++ b/src/odin/printer/visit.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package odin_printer import "core:fmt" @@ -1007,10 +1008,10 @@ visit_stmt :: proc( } //Special case for when the if statement ends with a call expression - /* + /* if my_function( - - ) { + + ) { } */ if v.init != nil && is_value_decl_statement_ending_with_call(v.init) || @@ -2081,11 +2082,11 @@ visit_struct_field_list :: proc(p: ^Printer, list: ^ast.Field_List, options := L } name_options := List_Options{.Add_Comma} - + if (.Enforce_Newline in options) { if p.config.align_struct_fields { alignment := get_possible_field_alignment(list.list) - + if alignment > 0 { length := 0 for name in field.names { diff --git a/src/server/analysis.odin b/src/server/analysis.odin index 000080d..c9cd863 100644 --- a/src/server/analysis.odin +++ b/src/server/analysis.odin @@ -1,4 +1,5 @@ #+feature dynamic-literals +#+feature using-stmt package server import "core:fmt" diff --git a/src/server/ast.odin b/src/server/ast.odin index 42c0ba6..07f2169 100644 --- a/src/server/ast.odin +++ b/src/server/ast.odin @@ -1,4 +1,5 @@ #+feature dynamic-literals +#+feature using-stmt package server import "core:fmt" diff --git a/src/server/clone.odin b/src/server/clone.odin index 215ab42..5a9a3ba 100644 --- a/src/server/clone.odin +++ b/src/server/clone.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "base:intrinsics" diff --git a/src/server/collector.odin b/src/server/collector.odin index 37d3cd8..c06fbf0 100644 --- a/src/server/collector.odin +++ b/src/server/collector.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "core:mem" diff --git a/src/server/file_resolve.odin b/src/server/file_resolve.odin index 8c7de51..f381aac 100644 --- a/src/server/file_resolve.odin +++ b/src/server/file_resolve.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "core:odin/ast" @@ -58,7 +59,7 @@ resolve_ranged_file :: proc( if range.start.line - margin <= decl.end.line && decl.pos.line <= range.end.line + margin { resolve_decl(&position_context, &ast_context, document, decl, &symbols, .None, allocator) clear(&ast_context.locals) - } + } } return symbols diff --git a/src/server/locals.odin b/src/server/locals.odin index 3e3d126..e24d27b 100644 --- a/src/server/locals.odin +++ b/src/server/locals.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "core:log" diff --git a/src/server/position_context.odin b/src/server/position_context.odin index 7670e79..7687d01 100644 --- a/src/server/position_context.odin +++ b/src/server/position_context.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "core:log" diff --git a/src/server/semantic_tokens.odin b/src/server/semantic_tokens.odin index 885588d..5ff822b 100644 --- a/src/server/semantic_tokens.odin +++ b/src/server/semantic_tokens.odin @@ -5,6 +5,7 @@ https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/spe */ +#+feature using-stmt package server import "core:fmt" @@ -494,7 +495,7 @@ visit_bit_field_fields :: proc(node: ast.Bit_Field_Type, builder: ^SemanticToken visit_import_decl :: proc(decl: ^ast.Import_Decl, builder: ^SemanticTokenBuilder) { /* hightlight the namespace in the import declaration - + import "pkg" ^^^ import "core:fmt" diff --git a/src/server/unmarshal.odin b/src/server/unmarshal.odin index 04e155d..9025428 100644 --- a/src/server/unmarshal.odin +++ b/src/server/unmarshal.odin @@ -1,3 +1,4 @@ +#+feature using-stmt package server import "base:runtime" -- cgit v1.2.3 From 8e5580815af631401dfa46fbf2997e44128681a6 Mon Sep 17 00:00:00 2001 From: pc Date: Tue, 27 Jan 2026 03:50:56 +0100 Subject: feat: add invert if action --- src/server/action.odin | 2 + src/server/action_invert_if_statements.odin | 389 ++++++++++++++++++++++++++++ src/testing/testing.odin | 38 +++ tests/action_test.odin | 380 +++++++++++++++++++++++++++ 4 files changed, 809 insertions(+) create mode 100644 src/server/action_invert_if_statements.odin create mode 100644 tests/action_test.odin (limited to 'src') diff --git a/src/server/action.odin b/src/server/action.odin index d1608b5..cdf06cb 100644 --- a/src/server/action.odin +++ b/src/server/action.odin @@ -71,6 +71,8 @@ get_code_actions :: proc(document: ^Document, range: common.Range, config: ^comm remove_unused_imports(document, strings.clone(document.uri.uri), config, &actions) } + add_invert_if_action(document, position_context.position, strings.clone(document.uri.uri), &actions) + return actions[:], true } diff --git a/src/server/action_invert_if_statements.odin b/src/server/action_invert_if_statements.odin new file mode 100644 index 0000000..ab0c664 --- /dev/null +++ b/src/server/action_invert_if_statements.odin @@ -0,0 +1,389 @@ +#+private file + +package server + +import "core:fmt" +import "core:log" +import "core:odin/ast" +import "core:odin/tokenizer" +import path "core:path/slashpath" +import "core:strings" + +import "src:common" + +/* + * The general idea behind inverting if statements is to allow + * if statements to be inverted without changing their behavior. + * The examples of these changes are provided in the tests. + * We should be careful to only allow this code action when it is safe to do so. + * So for now, we only support only one level of if statements without else-if chains. + */ + +@(private="package") +add_invert_if_action :: proc( + document: ^Document, + position: common.AbsolutePosition, + uri: string, + actions: ^[dynamic]CodeAction, +) { + if_stmt := find_if_stmt_at_position(document.ast.decls[:], position) + if if_stmt == nil { + return + } + + new_text, ok := generate_inverted_if(document, if_stmt) + if !ok { + return + } + + range := common.get_token_range(if_stmt^, document.ast.src) + + textEdits := make([dynamic]TextEdit, context.temp_allocator) + append(&textEdits, TextEdit{range = range, newText = new_text}) + + workspaceEdit: WorkspaceEdit + workspaceEdit.changes = make(map[string][]TextEdit, 0, context.temp_allocator) + workspaceEdit.changes[uri] = textEdits[:] + + append( + actions, + CodeAction { + kind = "refactor.more", + isPreferred = false, + title = "Invert if", + edit = workspaceEdit, + }, + ) +} + +// Find the innermost if statement that contains the given position +// This will NOT return else-if statements, only top-level if statements +// Also will not return an if statement if the position is in its else clause +find_if_stmt_at_position :: proc(stmts: []^ast.Stmt, position: common.AbsolutePosition) -> ^ast.If_Stmt { + for stmt in stmts { + if stmt == nil { + continue + } + if result := find_if_stmt_in_node(stmt, position, false); result != nil { + return result + } + } + return nil +} + +find_if_stmt_in_node :: proc(node: ^ast.Node, position: common.AbsolutePosition, in_else_clause: bool) -> ^ast.If_Stmt { + if node == nil { + return nil + } + + if !(node.pos.offset <= position && position <= node.end.offset) { + return nil + } + + #partial switch n in node.derived { + case ^ast.If_Stmt: + // First check if position is in the else clause + if n.else_stmt != nil && position_in_node(n.else_stmt, position) { + // Position is in the else clause - look for nested ifs inside it + // but mark that we're in an else clause + if nested := find_if_stmt_in_node(n.else_stmt, position, true); nested != nil { + return nested + } + // Position is in else clause but not on a valid nested if + // Don't return the current if statement + return nil + } + + if n.body != nil && position_in_node(n.body, position) { + if nested := find_if_stmt_in_node(n.body, position, false); nested != nil { + return nested + } + } + + // Position is in the condition/init part or we're the closest if + // Only return this if statement if we're NOT in an else clause + // (i.e., this is not an else-if) + if !in_else_clause { + return n + } + return nil + + case ^ast.Block_Stmt: + for stmt in n.stmts { + if result := find_if_stmt_in_node(stmt, position, false); result != nil { + return result + } + } + + case ^ast.Proc_Lit: + if n.body != nil { + return find_if_stmt_in_node(n.body, position, false) + } + + case ^ast.Value_Decl: + for value in n.values { + if result := find_if_stmt_in_node(value, position, false); result != nil { + return result + } + } + + case ^ast.For_Stmt: + if n.body != nil { + return find_if_stmt_in_node(n.body, position, false) + } + + case ^ast.Range_Stmt: + if n.body != nil { + return find_if_stmt_in_node(n.body, position, false) + } + + case ^ast.Switch_Stmt: + if n.body != nil { + return find_if_stmt_in_node(n.body, position, false) + } + + case ^ast.Type_Switch_Stmt: + if n.body != nil { + return find_if_stmt_in_node(n.body, position, false) + } + + case ^ast.Case_Clause: + for stmt in n.body { + if result := find_if_stmt_in_node(stmt, position, false); result != nil { + return result + } + } + + case ^ast.When_Stmt: + if n.body != nil { + if result := find_if_stmt_in_node(n.body, position, false); result != nil { + return result + } + } + if n.else_stmt != nil { + if result := find_if_stmt_in_node(n.else_stmt, position, false); result != nil { + return result + } + } + + case ^ast.Defer_Stmt: + if n.stmt != nil { + return find_if_stmt_in_node(n.stmt, position, false) + } + } + + return nil +} + +// Generate the inverted if statement text +generate_inverted_if :: proc(document: ^Document, if_stmt: ^ast.If_Stmt) -> (string, bool) { + src := document.ast.src + + indent := get_line_indentation(src, if_stmt.pos.offset) + + sb := strings.builder_make(context.temp_allocator) + + if if_stmt.label != nil { + label_text := src[if_stmt.label.pos.offset:if_stmt.label.end.offset] + strings.write_string(&sb, label_text) + strings.write_string(&sb, ": ") + } + + strings.write_string(&sb, "if ") + + if if_stmt.init != nil { + init_text := src[if_stmt.init.pos.offset:if_stmt.init.end.offset] + strings.write_string(&sb, init_text) + strings.write_string(&sb, "; ") + } + + if if_stmt.cond != nil { + inverted_cond, ok := invert_condition(src, if_stmt.cond) + if !ok { + return "", false + } + strings.write_string(&sb, inverted_cond) + } + + strings.write_string(&sb, " ") + + // Now we need to swap the bodies + + if if_stmt.else_stmt != nil { + else_body_text := get_block_body_text(src, if_stmt.else_stmt, indent) + then_body_text := get_block_body_text(src, if_stmt.body, indent) + + strings.write_string(&sb, "{\n") + strings.write_string(&sb, else_body_text) + strings.write_string(&sb, indent) + strings.write_string(&sb, "} else {\n") + strings.write_string(&sb, then_body_text) + strings.write_string(&sb, indent) + strings.write_string(&sb, "}") + } else { + then_body_text := get_block_body_text(src, if_stmt.body, indent) + + strings.write_string(&sb, "{\n") + strings.write_string(&sb, indent) + strings.write_string(&sb, "} else {\n") + strings.write_string(&sb, then_body_text) + strings.write_string(&sb, indent) + strings.write_string(&sb, "}") + } + + return strings.to_string(sb), true +} + +// Get the indentation (leading whitespace) of the line containing the given offset +get_line_indentation :: proc(src: string, offset: int) -> string { + line_start := offset + for line_start > 0 && src[line_start - 1] != '\n' { + line_start -= 1 + } + + indent_end := line_start + for indent_end < len(src) && (src[indent_end] == ' ' || src[indent_end] == '\t') { + indent_end += 1 + } + + return src[line_start:indent_end] +} + +// Extract the body text from a block statement (without the braces) +get_block_body_text :: proc(src: string, stmt: ^ast.Stmt, base_indent: string) -> string { + if stmt == nil { + return "" + } + + #partial switch block in stmt.derived { + case ^ast.Block_Stmt: + if len(block.stmts) == 0 { + return "" + } + + sb := strings.builder_make(context.temp_allocator) + + for s in block.stmts { + if s == nil { + continue + } + stmt_indent := get_line_indentation(src, s.pos.offset) + stmt_text := src[s.pos.offset:s.end.offset] + strings.write_string(&sb, stmt_indent) + strings.write_string(&sb, stmt_text) + strings.write_string(&sb, "\n") + } + + return strings.to_string(sb) + + case ^ast.If_Stmt: + // This is an else-if, need to handle it recursively + if_text, ok := generate_inverted_if_for_else(src, block, base_indent) + if ok { + return if_text + } + } + + // Fallback: just return the statement text + stmt_text := src[stmt.pos.offset:stmt.end.offset] + return fmt.tprintf("%s%s\n", base_indent, stmt_text) +} + +// For else-if chains, we don't invert them, just preserve +generate_inverted_if_for_else :: proc(src: string, if_stmt: ^ast.If_Stmt, base_indent: string) -> (string, bool) { + stmt_indent := get_line_indentation(src, if_stmt.pos.offset) + stmt_text := src[if_stmt.pos.offset:if_stmt.end.offset] + return fmt.tprintf("%s%s\n", stmt_indent, stmt_text), true +} + +// Invert a condition expression +invert_condition :: proc(src: string, cond: ^ast.Expr) -> (string, bool) { + if cond == nil { + return "", false + } + + #partial switch c in cond.derived { + case ^ast.Binary_Expr: + inverted_op, can_invert := get_inverted_operator(c.op.kind) + if can_invert { + left_text := src[c.left.pos.offset:c.left.end.offset] + right_text := src[c.right.pos.offset:c.right.end.offset] + return fmt.tprintf("%s %s %s", left_text, inverted_op, right_text), true + } + + if c.op.kind == .Cmp_And || c.op.kind == .Cmp_Or { + // Just wrap with !() + cond_text := src[cond.pos.offset:cond.end.offset] + return fmt.tprintf("!(%s)", cond_text), true + } + + case ^ast.Unary_Expr: + // If it's already negated with !, remove the negation + if c.op.kind == .Not { + inner_text := src[c.expr.pos.offset:c.expr.end.offset] + return inner_text, true + } + + case ^ast.Paren_Expr: + inner_inverted, ok := invert_condition(src, c.expr) + if ok { + if needs_parentheses(inner_inverted) { + return fmt.tprintf("(%s)", inner_inverted), true + } + return inner_inverted, true + } + } + + // Default: wrap the whole condition with !() + cond_text := src[cond.pos.offset:cond.end.offset] + if is_simple_expr(cond) { + return fmt.tprintf("!%s", cond_text), true + } + return fmt.tprintf("!(%s)", cond_text), true +} + +// Check if an expression is simple (identifier, call, or already parenthesized) +is_simple_expr :: proc(expr: ^ast.Expr) -> bool { + if expr == nil { + return false + } + #partial switch e in expr.derived { + case ^ast.Ident: + return true + case ^ast.Paren_Expr: + return true + case ^ast.Call_Expr: + return true + case ^ast.Selector_Expr: + return true + case ^ast.Index_Expr: + return true + } + return false +} + +// Check if a string needs parentheses (simple heuristic) +needs_parentheses :: proc(s: string) -> bool { + // If it starts with ! and is not wrapped in parens, it might need them + // This is a simple heuristic + return strings.contains(s, " && ") || strings.contains(s, " || ") +} + +// Get the inverted comparison operator +get_inverted_operator :: proc(op: tokenizer.Token_Kind) -> (string, bool) { + #partial switch op { + case .Cmp_Eq: + return "!=", true + case .Not_Eq: + return "==", true + case .Lt: + return ">=", true + case .Lt_Eq: + return ">", true + case .Gt: + return "<=", true + case .Gt_Eq: + return "<", true + } + return "", false +} diff --git a/src/testing/testing.odin b/src/testing/testing.odin index 0ba7b0f..5e927a7 100644 --- a/src/testing/testing.odin +++ b/src/testing/testing.odin @@ -553,6 +553,44 @@ expect_action :: proc(t: ^testing.T, src: ^Source, expect_action_names: []string } } +expect_action_with_edit :: proc(t: ^testing.T, src: ^Source, action_name: string, expected_new_text: string) { + setup(src) + defer teardown(src) + + input_range := common.Range { + start = src.position, + end = src.position, + } + actions, ok := server.get_code_actions(src.document, input_range, &src.config) + if !ok { + log.error("Failed to find actions") + return + } + + for action in actions { + if action.title == action_name { + // Get the text edit for the document + if edits, found := action.edit.changes[src.document.uri.uri]; found { + if len(edits) > 0 { + actual_text := edits[0].newText + testing.expectf( + t, + actual_text == expected_new_text, + "\nExpected edit text:\n%s\n\nGot:\n%s", + expected_new_text, + actual_text, + ) + return + } + } + log.errorf("Action '%s' found but has no edits", action_name) + return + } + } + + log.errorf("Action '%s' not found in actions: %v", action_name, actions) +} + expect_semantic_tokens :: proc(t: ^testing.T, src: ^Source, expected: []server.SemanticToken) { setup(src) defer teardown(src) diff --git a/tests/action_test.odin b/tests/action_test.odin new file mode 100644 index 0000000..f5c57a5 --- /dev/null +++ b/tests/action_test.odin @@ -0,0 +1,380 @@ +package tests + +import "core:testing" + +import test "src:testing" + +@(test) +action_invert_if_simple :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := 5 + if x{*} >= 0 { + foo() + } +} +`, + packages = {}, + } + + test.expect_action(t, &source, {"Invert if"}) +} + +@(test) +action_invert_if_simple_edit :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := 5 + if x{*} >= 0 { + foo() + } +} +`, + packages = {}, + } + + expected := `if x < 0 { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_with_else :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := 5 + if x{*} == 0 { + foo() + } else { + bar() + } +} +`, + packages = {}, + } + + test.expect_action(t, &source, {"Invert if"}) +} + +@(test) +action_invert_if_with_else_edit :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := 5 + if x{*} == 0 { + foo() + } else { + bar() + } +} +`, + packages = {}, + } + + expected := `if x != 0 { + bar() + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_with_init :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} := foo(); x < 0 { + bar() + } +} +`, + packages = {}, + } + + test.expect_action(t, &source, {"Invert if"}) +} + +@(test) +action_invert_if_with_init_edit :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} := foo(); x < 0 { + bar() + } +} +`, + packages = {}, + } + + expected := `if x := foo(); x >= 0 { + } else { + bar() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_not_on_if :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x :={*} 5 +} +`, + packages = {}, + } + + // Should not have the invert action when not on an if statement + test.expect_action(t, &source, {}) +} + +@(test) +action_invert_if_not_eq :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} != 0 { + foo() + } +} +`, + packages = {}, + } + + expected := `if x == 0 { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_lt :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} < 5 { + foo() + } +} +`, + packages = {}, + } + + expected := `if x >= 5 { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_gt :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} > 5 { + foo() + } +} +`, + packages = {}, + } + + expected := `if x <= 5 { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_le :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} <= 5 { + foo() + } +} +`, + packages = {}, + } + + expected := `if x > 5 { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_negated :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if !x{*} { + foo() + } +} +`, + packages = {}, + } + + expected := `if x { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_boolean :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x{*} { + foo() + } +} +`, + packages = {}, + } + + expected := `if !x { + } else { + foo() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_else_if_chain :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := something() + if x{*} > 0 { + statement1() + } else if x < 0 { + statement2() + } else { + statement3() + } +} +`, + packages = {}, + } + + expected := `if x <= 0 { + if x < 0 { + statement2() + } else { + statement3() + } + } else { + statement1() + }` + + test.expect_action_with_edit(t, &source, "Invert if", expected) +} + +@(test) +action_invert_if_not_on_else_if :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := something() + if x > 0 { + statement1() + } else if x{*} < 0 { + statement2() + } else { + statement3() + } +} +`, + packages = {}, + } + + // Should not have the invert action when on an else-if statement + test.expect_action(t, &source, {}) +} + +@(test) +action_invert_if_not_on_else :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := something() + if x > 0 { + statement1() + } else { + statement3(){*} + } +} +`, + packages = {}, + } + + // Should not have the invert action when in the else block (not on an if) + test.expect_action(t, &source, {}) +} + +@(test) +action_invert_if_nested_in_else_if_body :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + x := something() + if x > 0 { + statement1() + } else if x < 0 { + if y{*} > 0 { + statement2() + } + } else { + statement3() + } +} +`, + packages = {}, + } + + // Should have the invert action for an if statement nested inside an else-if body + test.expect_action(t, &source, {"Invert if"}) +} -- cgit v1.2.3 From 80add289c43fa1d71e09779cf2188f228c2f1378 Mon Sep 17 00:00:00 2001 From: moonz Date: Fri, 30 Jan 2026 18:01:09 +0100 Subject: fix: code action is no longer available inside the block of if statement --- src/server/action_invert_if_statements.odin | 13 ++++--------- tests/action_invert_if_test.odin | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/server/action_invert_if_statements.odin b/src/server/action_invert_if_statements.odin index ab0c664..8e904e5 100644 --- a/src/server/action_invert_if_statements.odin +++ b/src/server/action_invert_if_statements.odin @@ -98,6 +98,9 @@ find_if_stmt_in_node :: proc(node: ^ast.Node, position: common.AbsolutePosition, if nested := find_if_stmt_in_node(n.body, position, false); nested != nil { return nested } + // Position is inside the body but no nested if found + // Don't return the current if statement + return nil } // Position is in the condition/init part or we're the closest if @@ -348,15 +351,7 @@ is_simple_expr :: proc(expr: ^ast.Expr) -> bool { return false } #partial switch e in expr.derived { - case ^ast.Ident: - return true - case ^ast.Paren_Expr: - return true - case ^ast.Call_Expr: - return true - case ^ast.Selector_Expr: - return true - case ^ast.Index_Expr: + case ^ast.Ident, ^ast.Paren_Expr, ^ast.Call_Expr, ^ast.Selector_Expr, ^ast.Index_Expr: return true } return false diff --git a/tests/action_invert_if_test.odin b/tests/action_invert_if_test.odin index 0639b69..bb12ad9 100644 --- a/tests/action_invert_if_test.odin +++ b/tests/action_invert_if_test.odin @@ -148,6 +148,24 @@ main :: proc() { test.expect_action(t, &source, {}) } + +@(test) +action_invert_if_inside_of_statement :: proc(t: ^testing.T) { + source := test.Source { + main = `package test + +main :: proc() { + if x != 0 { + foo{*}() + } +} +`, + packages = {}, + } + + test.expect_action(t, &source, {}) +} + @(test) action_invert_if_not_eq :: proc(t: ^testing.T) { source := test.Source { -- cgit v1.2.3 From 4fd8c63387f4af8313051feb5d24efbb9d69537b Mon Sep 17 00:00:00 2001 From: Brad Lewis <22850972+BradLewis@users.noreply.github.com> Date: Sun, 1 Feb 2026 07:08:48 +1100 Subject: Fix null dereference when pkg is nil in collector --- src/server/collector.odin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/server/collector.odin b/src/server/collector.odin index c06fbf0..708c96b 100644 --- a/src/server/collector.odin +++ b/src/server/collector.odin @@ -470,7 +470,7 @@ get_proc_group_member_name :: proc(expr: ^ast.Expr) -> (name: string, ok: bool) @(private = "file") get_or_create_package :: proc(collection: ^SymbolCollection, pkg_name: string) -> ^SymbolPackage { pkg := &collection.packages[pkg_name] - if pkg.symbols == nil { + if pkg == nil || pkg.symbols == nil { collection.packages[pkg_name] = {} pkg = &collection.packages[pkg_name] pkg.symbols = make(map[string]Symbol, 100, collection.allocator) -- cgit v1.2.3