Correctly handle skipping self parameters in signature help requests

as well as completion requests.

src/analysis.zig

@@ -171,6 +171,46 @@ pub fn getFunctionSnippet(
     return buffer.toOwnedSlice();
 }
 
+/// Returns true if a function has a `self` parameter
+pub fn hasSelfParam(
+    arena: *std.heap.ArenaAllocator,
+    document_store: *DocumentStore,
+    handle: *DocumentStore.Handle,
+    func: ast.full.FnProto,
+) !bool {
+    // Non-decl prototypes cannot have a self parameter.
+    if (func.name_token == null) return false;
+    if (func.ast.params.len == 0) return false;
+
+    const tree = handle.tree;
+    var it = func.iterate(tree);
+    const param = it.next().?;
+    if (param.type_expr == 0) return false;
+
+    const token_starts = tree.tokens.items(.start);
+    const token_data = tree.nodes.items(.data);
+    const in_container = innermostContainer(handle, token_starts[func.ast.fn_token]);
+
+    if (try resolveTypeOfNode(document_store, arena, .{
+        .node = param.type_expr,
+        .handle = handle,
+    })) |resolved_type| {
+        if (std.meta.eql(in_container, resolved_type))
+            return true;
+    }
+
+    if (isPtrType(tree, param.type_expr)) {
+        if (try resolveTypeOfNode(document_store, arena, .{
+            .node = token_data[param.type_expr].rhs,
+            .handle = handle,
+        })) |resolved_prefix_op| {
+            if (std.meta.eql(in_container, resolved_prefix_op))
+                return true;
+        }
+    }
+    return false;
+}
+
 /// Gets a function signature (keywords, name, return value)
 pub fn getVariableSignature(tree: ast.Tree, var_decl: ast.full.VarDecl) []const u8 {
     const start = offsets.tokenLocation(tree, var_decl.ast.mut_token).start;
@@ -930,7 +970,7 @@ pub fn resolveTypeOfNodeInternal(
             };
 
             return TypeWithHandle{
-                .type = .{ .data = .{ .pointer = rhs_node }, .is_type_val = false },
+                .type = .{ .data = .{ .pointer = rhs_node }, .is_type_val = rhs_type.type.is_type_val },
                 .handle = rhs_type.handle,
             };
         },

src/main.zig

@@ -329,6 +329,7 @@ fn typeToCompletion(
                 null,
                 orig_handle,
                 type_handle.type.is_type_val,
+                null,
                 config,
             );
         },
@@ -339,6 +340,7 @@ fn typeToCompletion(
             field_access.unwrapped,
             orig_handle,
             type_handle.type.is_type_val,
+            null,
             config,
         ),
         .primitive => {},
@@ -352,13 +354,13 @@ fn nodeToCompletion(
     unwrapped: ?analysis.TypeWithHandle,
     orig_handle: *DocumentStore.Handle,
     is_type_val: bool,
+    parent_is_type_val: ?bool,
     config: Config,
 ) error{OutOfMemory}!void {
     const node = node_handle.node;
     const handle = node_handle.handle;
     const tree = handle.tree;
     const node_tags = tree.nodes.items(.tag);
-    const datas = tree.nodes.items(.data);
     const token_tags = tree.tokens.items(.tag);
 
     const doc_kind: types.MarkupContent.Kind = if (client_capabilities.completion_doc_supports_md)
@@ -385,8 +387,17 @@ fn nodeToCompletion(
             .config = &config,
             .arena = arena,
             .orig_handle = orig_handle,
+            .parent_is_type_val = is_type_val,
         };
-        try analysis.iterateSymbolsContainer(&document_store, arena, node_handle, orig_handle, declToCompletion, context, !is_type_val);
+        try analysis.iterateSymbolsContainer(
+            &document_store,
+            arena,
+            node_handle,
+            orig_handle,
+            declToCompletion,
+            context,
+            !is_type_val,
+        );
     }
 
     if (is_type_val) return;
@@ -402,38 +413,9 @@ fn nodeToCompletion(
             const func = analysis.fnProto(tree, node, &buf).?;
             if (func.name_token) |name_token| {
                 const use_snippets = config.enable_snippets and client_capabilities.supports_snippets;
-
                 const insert_text = if (use_snippets) blk: {
-                    // TODO Also check if we are dot accessing from a type val and dont skip in that case.
+                    const skip_self_param = !(parent_is_type_val orelse true) and
-                    const skip_self_param = if (func.ast.params.len > 0) param_check: {
+                        try analysis.hasSelfParam(arena, &document_store, handle, func);
-                        const in_container = analysis.innermostContainer(handle, tree.tokens.items(.start)[func.ast.fn_token]);
-
-                        var it = func.iterate(tree);
-                        const param = it.next().?;
-
-                        if (param.type_expr == 0) break :param_check false;
-
-                        if (try analysis.resolveTypeOfNode(&document_store, arena, .{
-                            .node = param.type_expr,
-                            .handle = handle,
-                        })) |resolved_type| {
-                            if (std.meta.eql(in_container, resolved_type))
-                                break :param_check true;
-                        }
-
-                        if (analysis.isPtrType(tree, param.type_expr)) {
-                            if (try analysis.resolveTypeOfNode(&document_store, arena, .{
-                                .node = datas[param.type_expr].rhs,
-                                .handle = handle,
-                            })) |resolved_prefix_op| {
-                                if (std.meta.eql(in_container, resolved_prefix_op))
-                                    break :param_check true;
-                            }
-                        }
-
-                        break :param_check false;
-                    } else false;
-
                     break :blk try analysis.getFunctionSnippet(&arena.allocator, tree, func, skip_self_param);
                 } else tree.tokenSlice(func.name_token.?);
 
@@ -952,13 +934,6 @@ fn referencesDefinitionLabel(arena: *std.heap.ArenaAllocator, id: types.RequestI
     });
 }
 
-const DeclToCompletionContext = struct {
-    completions: *std.ArrayList(types.CompletionItem),
-    config: *const Config,
-    arena: *std.heap.ArenaAllocator,
-    orig_handle: *DocumentStore.Handle,
-};
-
 fn hasComment(tree: ast.Tree, start_token: ast.TokenIndex, end_token: ast.TokenIndex) bool {
     const token_starts = tree.tokens.items(.start);
 
@@ -968,6 +943,14 @@ fn hasComment(tree: ast.Tree, start_token: ast.TokenIndex, end_token: ast.TokenI
     return std.mem.indexOf(u8, tree.source[start..end], "//") != null;
 }
 
+const DeclToCompletionContext = struct {
+    completions: *std.ArrayList(types.CompletionItem),
+    config: *const Config,
+    arena: *std.heap.ArenaAllocator,
+    orig_handle: *DocumentStore.Handle,
+    parent_is_type_val: ?bool = null,
+};
+
 fn declToCompletion(context: DeclToCompletionContext, decl_handle: analysis.DeclWithHandle) !void {
     const tree = decl_handle.handle.tree;
     switch (decl_handle.decl.*) {
@@ -978,6 +961,7 @@ fn declToCompletion(context: DeclToCompletionContext, decl_handle: analysis.Decl
             null,
             context.orig_handle,
             false,
+            context.parent_is_type_val,
             context.config.*,
         ),
         .param_decl => |param| {

src/signature_help.zig

@@ -9,18 +9,19 @@ const identifierFromPosition = @import("main.zig").identifierFromPosition;
 usingnamespace @import("ast.zig");
 
 fn fnProtoToSignatureInfo(
+    document_store: *DocumentStore,
     arena: *std.heap.ArenaAllocator,
-    is_self_call: bool,
     commas: u32,
-    tree: ast.Tree,
+    skip_self_param: bool,
+    handle: *DocumentStore.Handle,
     fn_node: ast.Node.Index,
     proto: ast.full.FnProto,
 ) !types.SignatureInformation {
     const ParameterInformation = types.SignatureInformation.ParameterInformation;
 
-    const token_tags = tree.tokens.items(.tag);
+    const tree = handle.tree;
+    const token_starts = tree.tokens.items(.start);
     const alloc = &arena.allocator;
-    const arg_idx = commas + @boolToInt(is_self_call);
     const label = analysis.getFunctionSignature(tree, proto);
     const proto_comments = types.MarkupContent{ .value = if (try analysis.getDocComments(
         alloc,
@@ -32,6 +33,11 @@ fn fnProtoToSignatureInfo(
     else
         "" };
 
+    const arg_idx = if (skip_self_param) blk: {
+        const has_self_param = try analysis.hasSelfParam(arena, document_store, handle, proto);
+        break :blk commas + @boolToInt(has_self_param);
+    } else commas;
+
     var params = std.ArrayListUnmanaged(ParameterInformation){};
     var param_it = proto.iterate(tree);
     while (param_it.next()) |param| {
@@ -45,43 +51,32 @@ fn fnProtoToSignatureInfo(
         else
             null;
 
-        var param_start_token: ast.TokenIndex = 0;
+        var param_label_start: usize = 0;
-        var param_end_token: ast.TokenIndex = 0;
+        var param_label_end: usize = 0;
         if (param.comptime_noalias) |cn| {
-            param_start_token = cn;
+            param_label_start = token_starts[cn];
-            param_end_token = cn;
+            param_label_end = param_label_start + tree.tokenSlice(cn).len;
         }
         if (param.name_token) |nt| {
-            if (param_start_token == 0)
+            if (param_label_start == 0)
-                param_start_token = nt;
+                param_label_start = token_starts[nt];
-            param_end_token = nt;
+            param_label_end = token_starts[nt] + tree.tokenSlice(nt).len;
         }
         if (param.anytype_ellipsis3) |ae| {
-            if (param_start_token == 0)
+            if (param_label_start == 0)
-                param_start_token = ae;
+                param_label_start = token_starts[ae];
-            param_end_token = ae;
+            param_label_end = token_starts[ae] + tree.tokenSlice(ae).len;
         }
         if (param.type_expr != 0) {
-            if (param_start_token == 0)
+            if (param_label_start == 0)
-                param_start_token = tree.firstToken(param.type_expr);
+                param_label_start = token_starts[tree.firstToken(param.type_expr)];
-            param_end_token = lastToken(tree, param.type_expr);
-        }
 
-        var param_label_buf = std.ArrayListUnmanaged(u8){};
+            const last_param_tok = lastToken(tree, param.type_expr);
-        var first_inserted = false;
+            param_label_end = token_starts[last_param_tok] + tree.tokenSlice(last_param_tok).len;
-        var i: ast.TokenIndex = param_start_token;
-        while (i <= param_end_token) : (i += 1) {
-            switch (token_tags[i]) {
-                .doc_comment => continue,
-                else => {},
-            }
-            if (first_inserted) {
-                try param_label_buf.append(alloc, ' ');
-            } else first_inserted = true;
-            try param_label_buf.appendSlice(alloc, tree.tokenSlice(i));
         }
+        const param_label = tree.source[param_label_start..param_label_end];
         try params.append(alloc, .{
-            .label = param_label_buf.items,
+            .label = param_label,
             .documentation = param_comments,
         });
     }
@@ -295,38 +290,49 @@ pub fn getSignatureInfo(
                     const type_handle = result.unwrapped orelse result.original;
                     var node = switch (type_handle.type.data) {
                         .other => |n| n,
-                        else => return null,
+                        else => {
+                            try symbol_stack.append(alloc, .l_paren);
+                            continue;
+                        },
                     };
-                    // TODO Better detection
-                    var is_self_call = expr_last_token > expr_first_token and
-                        token_tags[expr_last_token] == .identifier and
-                        token_tags[expr_last_token - 1] == .period;
 
                     var buf: [1]ast.Node.Index = undefined;
                     if (fnProto(type_handle.handle.tree, node, &buf)) |proto| {
                         return try fnProtoToSignatureInfo(
+                            document_store,
                             arena,
-                            is_self_call,
                             paren_commas,
-                            type_handle.handle.tree,
+                            false,
+                            type_handle.handle,
                             node,
                             proto,
                         );
                     }
 
                     const name = identifierFromPosition(expr_end - 1, handle.*);
-                    if (name.len == 0) return null;
+                    if (name.len == 0) {
-                    var decl_handle = (try analysis.lookupSymbolContainer(
+                        try symbol_stack.append(alloc, .l_paren);
+                        continue;
+                    }
+
+                    const skip_self_param = !type_handle.type.is_type_val;
+                    const decl_handle = (try analysis.lookupSymbolContainer(
                         document_store,
                         arena,
                         .{ .node = node, .handle = type_handle.handle },
                         name,
                         true,
-                    )) orelse return null;
+                    )) orelse {
+                        try symbol_stack.append(alloc, .l_paren);
+                        continue;
+                    };
                     var res_handle = decl_handle.handle;
                     node = switch (decl_handle.decl.*) {
                         .ast_node => |n| n,
-                        else => return null,
+                        else => {
+                            try symbol_stack.append(alloc, .l_paren);
+                            continue;
+                        },
                     };
 
                     if (try analysis.resolveVarDeclAlias(
@@ -345,10 +351,11 @@ pub fn getSignatureInfo(
 
                     if (fnProto(res_handle.tree, node, &buf)) |proto| {
                         return try fnProtoToSignatureInfo(
+                            document_store,
                             arena,
-                            is_self_call,
                             paren_commas,
-                            res_handle.tree,
+                            skip_self_param,
+                            res_handle,
                             node,
                             proto,
                         );