From cc3c14674947e327215967d8e1f17c7658761e1f Mon Sep 17 00:00:00 2001 From: Alexandros Naskos Date: Sat, 3 Apr 2021 18:54:26 +0300 Subject: [PATCH] Correctly handle skipping self parameters in signature help requests as well as completion requests. --- src/analysis.zig | 42 ++++++++++++++++++- src/main.zig | 64 +++++++++++----------------- src/signature_help.zig | 95 +++++++++++++++++++++++------------------- 3 files changed, 116 insertions(+), 85 deletions(-) diff --git a/src/analysis.zig b/src/analysis.zig index 0700441..6b45547 100644 --- a/src/analysis.zig +++ b/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, }; }, diff --git a/src/main.zig b/src/main.zig index 052f775..ce29140 100644 --- a/src/main.zig +++ b/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 = if (func.ast.params.len > 0) param_check: { - 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; - + const skip_self_param = !(parent_is_type_val orelse true) and + try analysis.hasSelfParam(arena, &document_store, handle, func); 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| { diff --git a/src/signature_help.zig b/src/signature_help.zig index 71529a2..a73ed13 100644 --- a/src/signature_help.zig +++ b/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_end_token: ast.TokenIndex = 0; + var param_label_start: usize = 0; + var param_label_end: usize = 0; if (param.comptime_noalias) |cn| { - param_start_token = cn; - param_end_token = cn; + param_label_start = token_starts[cn]; + param_label_end = param_label_start + tree.tokenSlice(cn).len; } if (param.name_token) |nt| { - if (param_start_token == 0) - param_start_token = nt; - param_end_token = nt; + if (param_label_start == 0) + param_label_start = token_starts[nt]; + param_label_end = token_starts[nt] + tree.tokenSlice(nt).len; } if (param.anytype_ellipsis3) |ae| { - if (param_start_token == 0) - param_start_token = ae; - param_end_token = ae; + if (param_label_start == 0) + param_label_start = token_starts[ae]; + param_label_end = token_starts[ae] + tree.tokenSlice(ae).len; } if (param.type_expr != 0) { - if (param_start_token == 0) - param_start_token = tree.firstToken(param.type_expr); - param_end_token = lastToken(tree, param.type_expr); - } + if (param_label_start == 0) + param_label_start = token_starts[tree.firstToken(param.type_expr)]; - var param_label_buf = std.ArrayListUnmanaged(u8){}; - var first_inserted = false; - 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 last_param_tok = lastToken(tree, param.type_expr); + param_label_end = token_starts[last_param_tok] + tree.tokenSlice(last_param_tok).len; } + 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; - var decl_handle = (try analysis.lookupSymbolContainer( + if (name.len == 0) { + 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, );