make enable_snippets == false never add parentheses

The old behavior was slightly inconsistent. It was still adding parentheses if the function accepted no arguments.

fixes #1939
closes #2037

src/features/completions.zig

@@ -347,6 +347,8 @@ fn functionTypeCompletion(
                 })});
             }
 
+            if (!use_snippets) break :blk func_name;
+
             switch (func.ast.params.len) {
                 // No arguments, leave cursor at the end
                 0 => break :blk try std.fmt.allocPrint(builder.arena, "{s}()", .{func_name}),
@@ -357,12 +359,10 @@ fn functionTypeCompletion(
                     }
 
                     // Non-self parameter, leave the cursor in the parentheses
-                    if (!use_snippets) break :blk func_name;
                     break :blk try std.fmt.allocPrint(builder.arena, "{s}(${{1:}})", .{func_name});
                 },
                 else => {
                     // At least one non-self parameter, leave the cursor in the parentheses
-                    if (!use_snippets) break :blk func_name;
                     break :blk try std.fmt.allocPrint(builder.arena, "{s}(${{1:}})", .{func_name});
                 },
             }
@@ -478,6 +478,7 @@ fn populateSnippedCompletions(builder: *Builder, snippets: []const snipped_data.
 const FunctionCompletionFormat = enum { snippet, only_name };
 
 fn prepareFunctionCompletion(builder: *Builder) struct { types.Range, types.Range, FunctionCompletionFormat } {
+    const use_snippets = builder.server.config.enable_snippets and builder.server.client_capabilities.supports_snippets;
     const source = builder.orig_handle.tree.source;
 
     var start_index = builder.source_index;
@@ -495,16 +496,16 @@ fn prepareFunctionCompletion(builder: *Builder) struct { types.Range, types.Rang
 
     var format: FunctionCompletionFormat = .only_name;
 
-    const insert_can_be_snippet = std.mem.startsWith(u8, source[insert_loc.end..], "()");
-    const replace_can_be_snippet = std.mem.startsWith(u8, source[replace_loc.end..], "()");
+    const insert_can_be_snippet = use_snippets and std.mem.startsWith(u8, source[insert_loc.end..], "()");
+    const replace_can_be_snippet = use_snippets and std.mem.startsWith(u8, source[replace_loc.end..], "()");
 
     if (insert_can_be_snippet and replace_can_be_snippet) {
         insert_loc.end += 2;
         replace_loc.end += 2;
         format = .snippet;
     } else if (insert_can_be_snippet or replace_can_be_snippet) {
         // snippet completions would be possible but insert and replace would need different `newText`
-    } else if (!std.mem.startsWith(u8, source[end_index..], "(")) {
+    } else if (use_snippets and !std.mem.startsWith(u8, source[end_index..], "(")) {
         format = .snippet;
     }
 
@@ -525,13 +526,13 @@ fn completeBuiltin(builder: *Builder) error{OutOfMemory}!void {
 
     try builder.completions.ensureUnusedCapacity(builder.arena, data.builtins.kvs.len);
     for (data.builtins.keys(), data.builtins.values()) |name, builtin| {
-        const new_text = switch (new_text_format) {
-            .only_name => name,
+        const new_text, const insertTextFormat: types.InsertTextFormat = switch (new_text_format) {
+            .only_name => .{ name, .PlainText },
             .snippet => blk: {
-                if (builtin.arguments.len == 0) break :blk try std.fmt.allocPrint(builder.arena, "{s}()", .{name});
-                if (use_snippets and use_placeholders) break :blk builtin.snippet;
-                if (use_snippets) break :blk try std.fmt.allocPrint(builder.arena, "{s}(${{1:}})", .{name});
-                break :blk name;
+                std.debug.assert(use_snippets);
+                if (builtin.arguments.len == 0) break :blk .{ try std.fmt.allocPrint(builder.arena, "{s}()", .{name}), .PlainText };
+                if (use_placeholders) break :blk .{ builtin.snippet, .Snippet };
+                break :blk .{ try std.fmt.allocPrint(builder.arena, "{s}(${{1:}})", .{name}), .Snippet };
             },
         };
 
@@ -540,7 +541,7 @@ fn completeBuiltin(builder: *Builder) error{OutOfMemory}!void {
             .kind = .Function,
             .filterText = name[1..],
             .detail = builtin.signature,
-            .insertTextFormat = if (use_snippets) .Snippet else .PlainText,
+            .insertTextFormat = insertTextFormat,
             .textEdit = if (builder.server.client_capabilities.supports_completion_insert_replace_support)
                 .{ .InsertReplaceEdit = .{ .newText = new_text[1..], .insert = insert_range, .replace = replace_range } }
             else
@@ -1307,7 +1308,7 @@ fn collectContainerFields(
             .label = name,
             .kind = if (field.ast.tuple_like) .EnumMember else .Field,
             .detail = Analyser.getContainerFieldSignature(handle.tree, field),
-            .insertText = if (field.ast.tuple_like or likely == .enum_comparison or likely == .switch_case)
+            .insertText = if (!use_snippets or field.ast.tuple_like or likely == .enum_comparison or likely == .switch_case)
                 name
             else
                 try std.fmt.allocPrint(builder.arena, "{s} = ", .{name}),

tests/lsp_features/completion.zig

@@ -3056,8 +3056,8 @@ test "insert replace behaviour - builtin with no parameters" {
     try testCompletionTextEdit(.{
         .source = "const foo = @<cursor>;",
         .label = "@src",
-        .expected_insert_line = "const foo = @src();",
-        .expected_replace_line = "const foo = @src();",
+        .expected_insert_line = "const foo = @src;",
+        .expected_replace_line = "const foo = @src;",
     });
     try testCompletionTextEdit(.{
         .source = "const foo = @<cursor>();",
@@ -3208,6 +3208,7 @@ test "insert replace behaviour - function 'self parameter' detection" {
         .label = "f",
         .expected_insert_line = "s.f()",
         .expected_replace_line = "s.f()",
+        .enable_snippets = true,
     });
     try testCompletionTextEdit(.{
         .source =
@@ -3218,34 +3219,11 @@ test "insert replace behaviour - function 'self parameter' detection" {
         \\S.<cursor>
         ,
         .label = "f",
-        .expected_insert_line = "S.f",
-        .expected_replace_line = "S.f",
-    });
-    try testCompletionTextEdit(.{
-        .source =
-        \\const S = struct {
-        \\    alpha: u32,
-        \\    fn f() void {}
-        \\};
-        \\S.<cursor>
-        ,
-        .label = "f",
-        .expected_insert_line = "S.f()",
-        .expected_replace_line = "S.f()",
-    });
-    try testCompletionTextEdit(.{
-        .source =
-        \\const S = struct {
-        \\    alpha: u32,
-        \\    fn f(self: S) void {}
-        \\};
-        \\const s = S{};
-        \\s.<cursor>
-        ,
-        .label = "f",
-        .expected_insert_line = "s.f()",
-        .expected_replace_line = "s.f()",
+        .expected_insert_line = "S.f(${1:})",
+        .expected_replace_line = "S.f(${1:})",
+        .enable_snippets = true,
     });
+
     try testCompletionTextEdit(.{
         .source =
         \\const S = struct {
@@ -3258,6 +3236,7 @@ test "insert replace behaviour - function 'self parameter' detection" {
         .label = "f",
         .expected_insert_line = "s.f()",
         .expected_replace_line = "s.f()",
+        .enable_snippets = true,
     });
     try testCompletionTextEdit(.{
         .source =
@@ -3271,19 +3250,7 @@ test "insert replace behaviour - function 'self parameter' detection" {
         .label = "f",
         .expected_insert_line = "s.f()",
         .expected_replace_line = "s.f()",
-    });
-    try testCompletionTextEdit(.{
-        .source =
-        \\const S = struct {
-        \\    alpha: u32,
-        \\    fn f(self: S) void {}
-        \\};
-        \\const s = S{};
-        \\s.<cursor>
-        ,
-        .label = "f",
-        .expected_insert_line = "s.f()",
-        .expected_replace_line = "s.f()",
+        .enable_snippets = true,
     });
     try testCompletionTextEdit(.{
         .source =
@@ -3295,8 +3262,9 @@ test "insert replace behaviour - function 'self parameter' detection" {
         \\s.<cursor>
         ,
         .label = "f",
-        .expected_insert_line = "s.f",
-        .expected_replace_line = "s.f",
+        .expected_insert_line = "s.f(${1:})",
+        .expected_replace_line = "s.f(${1:})",
+        .enable_snippets = true,
     });
 }