Deduplicate completions with branching types

Author: FnControlOption

src/analyser/completions.zig

@@ -4,12 +4,13 @@
 const std = @import("std");
 const InternPool = @import("InternPool.zig");
 const types = @import("lsp").types;
+const Completions = @import("../features/completions.zig").Completions;
 
 /// generates a list of dot completions for the given typed-value in `index`
 /// the given `index` must belong to the given InternPool
 pub fn dotCompletions(
     arena: std.mem.Allocator,
-    completions: *std.ArrayList(types.completion.Item),
+    completions: *Completions,
     ip: *InternPool,
     index: InternPool.Index,
 ) error{OutOfMemory}!void {
@@ -438,9 +439,9 @@ fn testCompletion(
     defer arena_allocator.deinit();
 
     const arena = arena_allocator.allocator();
-    var completions: std.ArrayList(types.completion.Item) = .empty;
+    var completions: Completions = .empty;
 
     try dotCompletions(arena, &completions, ip, index);
 
-    try std.testing.expectEqualDeep(expected, completions.items);
+    try std.testing.expectEqualDeep(expected, completions.items());
 }

src/features/completions.zig

@@ -24,11 +24,82 @@ const Builder = struct {
     arena: std.mem.Allocator,
     orig_handle: *DocumentStore.Handle,
     source_index: usize,
-    completions: std.ArrayList(types.completion.Item),
+    completions: Completions,
     cached_prepare_function_completion_result: ?PrepareFunctionCompletionResult = null,
     use_snippets: bool,
 };
 
+pub const Completions = struct {
+    map: std.StringArrayHashMapUnmanaged(types.completion.Item),
+
+    pub const empty: Completions = .{ .map = .empty };
+
+    pub fn items(completions: Completions) []types.completion.Item {
+        return completions.map.values();
+    }
+
+    pub fn ensureUnusedCapacity(
+        completions: *Completions,
+        allocator: std.mem.Allocator,
+        additional_count: usize,
+    ) !void {
+        try completions.map.ensureUnusedCapacity(allocator, additional_count);
+    }
+
+    pub fn append(
+        completions: *Completions,
+        allocator: std.mem.Allocator,
+        item: types.completion.Item,
+    ) !void {
+        try completions.ensureUnusedCapacity(allocator, 1);
+        completions.appendAssumeCapacity(item);
+    }
+
+    pub fn appendSlice(
+        completions: *Completions,
+        allocator: std.mem.Allocator,
+        slice: []const types.completion.Item,
+    ) !void {
+        try completions.ensureUnusedCapacity(allocator, slice.len);
+        for (slice) |item|
+            completions.appendAssumeCapacity(item);
+    }
+
+    pub fn appendAssumeCapacity(
+        completions: *Completions,
+        item: types.completion.Item,
+    ) void {
+        const gop = completions.map.getOrPutAssumeCapacity(item.label);
+        const ptr = gop.value_ptr;
+
+        if (!gop.found_existing) {
+            ptr.* = item;
+            return;
+        }
+
+        const keep_detail = eqlSlices(u8, ptr.detail, item.detail);
+        const keep_label_details =
+            if (item.labelDetails) |item_details|
+                if (ptr.labelDetails) |ptr_details|
+                    eqlSlices(u8, ptr_details.detail, item_details.detail) and
+                        eqlSlices(u8, ptr_details.description, item_details.description)
+                else
+                    false
+            else
+                false;
+
+        ptr.deprecated = ptr.deprecated orelse item.deprecated;
+        ptr.tags = ptr.tags orelse item.tags;
+        if (!keep_detail) ptr.detail = null;
+        if (!keep_label_details) ptr.labelDetails = null;
+    }
+
+    fn eqlSlices(comptime T: type, a: ?[]const T, b: ?[]const T) bool {
+        return (a == null and b == null) or
+            (a != null and b != null and std.mem.eql(T, a.?, b.?));
+    }
+};
+
 fn typeToCompletion(builder: *Builder, ty: Analyser.Type) Analyser.Error!void {
     const tracy_zone = tracy.trace(@src());
     defer tracy_zone.end();
@@ -175,12 +246,12 @@ fn declToCompletion(builder: *Builder, decl_handle: Analyser.DeclWithHandle) Ana
         }
     }
 
-    const documentation: types.Documentation = .{
+    const documentation: ?types.Documentation = if (doc_comments.items.len > 0) .{
         .markup_content = .{
             .kind = if (builder.server.client_capabilities.completion_doc_supports_md) .markdown else .plaintext,
             .value = try std.mem.join(builder.arena, "\n\n", doc_comments.items),
         },
-    };
+    } else null;
 
     try builder.completions.ensureUnusedCapacity(builder.arena, 1);
 
@@ -906,7 +977,7 @@ fn completeFileSystemStringLiteral(builder: *Builder, pos_context: Analyser.Posi
         const string_content_range = offsets.locToRange(source, .{ .start = insert_loc.start, .end = string_content_loc.end }, builder.server.offset_encoding);
 
         // completions on module replace the entire string literal
-        for (builder.completions.items) |*item| {
+        for (builder.completions.items()) |*item| {
             if (item.kind == .Module and item.textEdit == null) {
                 item.textEdit = createTextEdit(builder, .{ .newText = item.label, .insert = insert_range, .replace = string_content_range });
             }
@@ -1011,7 +1082,7 @@ pub fn completionAtIndex(
 
     if (line_until_index.len == 0 or std.zig.isValidId(line_until_index)) {
         try populateSnippedCompletions(&builder, .top_level);
-        return .{ .isIncomplete = false, .items = builder.completions.items };
+        return .{ .isIncomplete = false, .items = builder.completions.items() };
     }
 
     const pos_context = try Analyser.getPositionContext(arena, &handle.tree, source_index, false);
@@ -1035,7 +1106,7 @@ pub fn completionAtIndex(
         else => return null,
     }
 
-    const completions = builder.completions.items;
+    const completions = builder.completions.items();
     if (completions.len == 0) return null;
 
     const identifier_loc = prepareCompletionLoc(&handle.tree, source_index);

tests/lsp_features/completion.zig

@@ -3475,6 +3475,58 @@ test "either" {
     });
 }
 
+test "either - fields and methods with same name" {
+    try testCompletionWithOptions(
+        \\const Foo = struct {
+        \\    alpha: u32,
+        \\    beta: []const u8,
+        \\    fn gamma(_: @This()) void {}
+        \\    fn delta(_: @This()) f32 {}
+        \\};
+        \\const Bar = struct {
+        \\    alpha: u32,
+        \\    beta: bool,
+        \\    fn gamma(_: @This()) void {}
+        \\    fn delta(_: @This()) f64 {}
+        \\};
+        \\const fizz: if (undefined) Foo else Bar = undefined;
+        \\const buzz = fizz.<cursor>
+    , &.{
+        .{
+            .label = "alpha",
+            .labelDetails = .{
+                .detail = null,
+                .description = "u32",
+            },
+            .kind = .Field,
+            .detail = "u32",
+        },
+        .{
+            .label = "beta",
+            .labelDetails = null,
+            .kind = .Field,
+            .detail = null,
+        },
+        .{
+            .label = "gamma",
+            .labelDetails = .{
+                .detail = "()",
+                .description = "void",
+            },
+            .kind = .Method,
+            .detail = null,
+        },
+        .{
+            .label = "delta",
+            .labelDetails = null,
+            .kind = .Method,
+            .detail = null,
+        },
+    }, .{
+        .check_null_fields = true,
+    });
+}
+
 test "container type inside switch case value" {
     try testCompletion(
         \\test {
@@ -4642,6 +4694,7 @@ fn testCompletionWithOptions(
         enable_snippets: bool = true,
         completion_label_details: bool = true,
         check_order: bool = false,
+        check_null_fields: bool = false,
     },
 ) !void {
     const cursor_idx = std.mem.find(u8, source, "<cursor>").?;
@@ -4742,6 +4795,14 @@ fn testCompletionWithOptions(
                 if (actual_doc) |str| std.zig.fmtString(str) else null,
             });
             return error.InvalidCompletionDoc;
+        } else blk: {
+            if (!options.check_null_fields) break :blk;
+            const actual_doc = actual_completion.documentation orelse break :blk;
+            try error_builder.msgAtIndex("completion item '{s}' has unexpected doc '{f}'", test_uri.raw, cursor_idx, .err, .{
+                label,
+                std.zig.fmtString(actual_doc.markup_content.value),
+            });
+            return error.InvalidCompletionDoc;
         }
 
         try std.testing.expect(actual_completion.insertText == null); // 'insertText' is subject to interpretation on the client so 'textEdit' should be preferred
@@ -4759,6 +4820,14 @@ fn testCompletionWithOptions(
                 actual_completion.detail,
             });
             return error.InvalidCompletionDetail;
+        } else blk: {
+            if (!options.check_null_fields) break :blk;
+            const actual_detail = actual_completion.detail orelse break :blk;
+            try error_builder.msgAtIndex("completion item '{s}' has unexpected detail '{s}'", test_uri.raw, cursor_idx, .err, .{
+                label,
+                actual_detail,
+            });
+            return error.InvalidCompletionDetail;
         }
 
         if (expected_completion.labelDetails) |expected_label_details| {
@@ -4789,6 +4858,13 @@ fn testCompletionWithOptions(
                 });
                 return error.InvalidCompletionLabelDetails;
             }
+        } else blk: {
+            if (!options.check_null_fields) break :blk;
+            if (actual_completion.labelDetails == null) break :blk;
+            try error_builder.msgAtIndex("completion item '{s}' has unexpected label details", test_uri.raw, cursor_idx, .err, .{
+                label,
+            });
+            return error.InvalidCompletionLabelDetails;
         }
 
         blk: {