Merge pull request #308 from InterplanetaryEngineer/master

Fix inclusion of toplevel doc comments, remove @async recursion in writeNodeTokens, add a few regression tests

src/analysis.zig

@@ -17,12 +17,7 @@ pub fn deinit() void {
     resolve_trail.deinit();
 }
 
-/// Gets a declaration's doc comments, caller must free memory when a value is returned
+/// Gets a declaration's doc comments. Caller owns returned memory.
-/// Like:
-///```zig
-///var comments = getFunctionDocComments(allocator, tree, func);
-///defer if (comments) |comments_pointer| allocator.free(comments_pointer);
-///```
 pub fn getDocComments(
     allocator: *std.mem.Allocator,
     tree: ast.Tree,
@@ -30,15 +25,32 @@ pub fn getDocComments(
     format: types.MarkupContent.Kind,
 ) !?[]const u8 {
     const base = tree.nodes.items(.main_token)[node];
+    const base_kind = tree.nodes.items(.tag)[node];
     const tokens = tree.tokens.items(.tag);
 
-    if (getDocCommentTokenIndex(tokens, base)) |doc_comment_index| {
+    switch (base_kind) {
-        return try collectDocComments(allocator, tree, doc_comment_index, format);
+        // As far as I know, this does not actually happen yet, but it
+        // may come in useful.
+        .root => return try collectDocComments(allocator, tree, 0, format, true),
+        .fn_proto,
+        .fn_proto_one,
+        .fn_proto_simple,
+        .fn_proto_multi,
+        .fn_decl,
+        .local_var_decl,
+        .global_var_decl,
+        .aligned_var_decl,
+        .simple_var_decl,
+        => {
+            if (getDocCommentTokenIndex(tokens, base)) |doc_comment_index|
+                return try collectDocComments(allocator, tree, doc_comment_index, format, false);
+        },
+        else => {},
     }
     return null;
 }
 
-/// Get a declaration's doc comment token index
+/// Get the first doc comment of a declaration.
 pub fn getDocCommentTokenIndex(tokens: []std.zig.Token.Tag, base_token: ast.TokenIndex) ?ast.TokenIndex {
     var idx = base_token;
     if (idx == 0) return null;
@@ -50,9 +62,9 @@ pub fn getDocCommentTokenIndex(tokens: []std.zig.Token.Tag, base_token: ast.Toke
     if (tokens[idx] == .keyword_pub and idx > 0) idx -= 1;
 
     // Find first doc comment token
-    if (!(tokens[idx] == .doc_comment or tokens[idx] == .container_doc_comment))
+    if (!(tokens[idx] == .doc_comment))
         return null;
-    return while (tokens[idx] == .doc_comment or tokens[idx] == .container_doc_comment) {
+    return while (tokens[idx] == .doc_comment) {
         if (idx == 0) break 0;
         idx -= 1;
     } else idx + 1;
@@ -63,6 +75,7 @@ pub fn collectDocComments(
     tree: ast.Tree,
     doc_comments: ast.TokenIndex,
     format: types.MarkupContent.Kind,
+    container_doc: bool,
 ) ![]const u8 {
     var lines = std.ArrayList([]const u8).init(allocator);
     defer lines.deinit();
@@ -70,28 +83,27 @@ pub fn collectDocComments(
 
     var curr_line_tok = doc_comments;
     while (true) : (curr_line_tok += 1) {
-        switch (tokens[curr_line_tok]) {
+        const comm = tokens[curr_line_tok];
-            .doc_comment, .container_doc_comment => {
+        if ((container_doc and comm == .container_doc_comment) or (!container_doc and comm == .doc_comment)) {
-                try lines.append(std.mem.trim(u8, tree.tokenSlice(curr_line_tok)[3..], &std.ascii.spaces));
+            try lines.append(std.mem.trim(u8, tree.tokenSlice(curr_line_tok)[3..], &std.ascii.spaces));
-            },
+        } else break;
-            else => break,
-        }
     }
 
     return try std.mem.join(allocator, if (format == .Markdown) "  \n" else "\n", lines.items);
 }
 
-/// Gets a function signature (keywords, name, return value)
+/// Gets a function's keyword, name, arguments and return value.
 pub fn getFunctionSignature(tree: ast.Tree, func: ast.full.FnProto) []const u8 {
     const start = offsets.tokenLocation(tree, func.ast.fn_token);
-    // return type can be 0 when user wrote incorrect fn signature
+
-    // to ensure we don't break, just end the signature at end of fn token
+    const end = if (func.ast.return_type != 0)
-    if (func.ast.return_type == 0) return tree.source[start.start..start.end];
+        offsets.tokenLocation(tree, lastToken(tree, func.ast.return_type))
-    const end = offsets.tokenLocation(tree, lastToken(tree, func.ast.return_type)).end;
+    else
-    return tree.source[start.start..end];
+        start;
+    return tree.source[start.start..end.end];
 }
 
-/// Gets a function snippet insert text
+/// Creates snippet insert text for a function. Caller owns returned memory.
 pub fn getFunctionSnippet(
     allocator: *std.mem.Allocator,
     tree: ast.Tree,
@@ -197,7 +209,6 @@ pub fn hasSelfParam(
     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;
     const end = offsets.tokenLocation(tree, lastToken(tree, var_decl.ast.init_node)).end;
@@ -232,14 +243,19 @@ pub fn isGenericFunction(tree: ast.Tree, func: ast.full.FnProto) bool {
     }
     return false;
 }
+
 // STYLE
 
 pub fn isCamelCase(name: []const u8) bool {
-    return !std.ascii.isUpper(name[0]) and std.mem.indexOf(u8, name[0..(name.len - 1)], "_") == null;
+    return !std.ascii.isUpper(name[0]) and !isSnakeCase(name);
 }
 
 pub fn isPascalCase(name: []const u8) bool {
-    return std.ascii.isUpper(name[0]) and std.mem.indexOf(u8, name[0..(name.len - 1)], "_") == null;
+    return std.ascii.isUpper(name[0]) and !isSnakeCase(name);
+}
+
+pub fn isSnakeCase(name: []const u8) bool {
+    return std.mem.indexOf(u8, name, "_") != null;
 }
 
 // ANALYSIS ENGINE
@@ -499,8 +515,7 @@ pub fn resolveReturnType(
             .type = .{ .data = .{ .error_union = child_type_node }, .is_type_val = false },
             .handle = child_type.handle,
         };
-    } else
+    } else return child_type.instanceTypeVal();
-        return child_type.instanceTypeVal();
 }
 
 /// Resolves the child type of an optional type
@@ -698,8 +713,6 @@ pub fn resolveTypeOfNodeInternal(
         if (std.meta.eql(i, node_handle))
             return null;
     }
-    // We use the backing allocator here because the ArrayList expects its
-    // allocated memory to persist while it is empty.
     try resolve_trail.append(node_handle);
     defer _ = resolve_trail.pop();
 
@@ -2271,8 +2284,6 @@ fn resolveUse(
     // it is self-referential and we cannot resolve it.
     if (std.mem.indexOfScalar([*]const u8, using_trail.items, symbol.ptr) != null)
         return null;
-    // We use the backing allocator here because the ArrayList expects its
-    // allocated memory to persist while it is empty.
     try using_trail.append(symbol.ptr);
     defer _ = using_trail.pop();
 
@@ -2636,7 +2647,7 @@ fn makeInnerScope(
                     .insertText = name,
                     .insertTextFormat = .PlainText,
                     .documentation = if (try getDocComments(allocator, tree, decl, .Markdown)) |docs|
-                        .{ .kind = .Markdown, .value = docs }
+                        types.MarkupContent{ .kind = .Markdown, .value = docs }
                     else
                         null,
                 }, {});
@@ -2927,20 +2938,17 @@ fn makeScopeInternal(
                 std.debug.assert(token_tags[name_token] == .identifier);
 
                 const name = tree.tokenSlice(name_token);
-                try scope.decls.putNoClobber(name, if (is_for)
+                try scope.decls.putNoClobber(name, if (is_for) .{
-                    .{
+                    .array_payload = .{
-                        .array_payload = .{
+                        .identifier = name_token,
-                            .identifier = name_token,
+                        .array_expr = while_node.ast.cond_expr,
-                            .array_expr = while_node.ast.cond_expr,
+                    },
-                        },
+                } else .{
-                    }
+                    .pointer_payload = .{
-                else
+                        .name = name_token,
-                    .{
+                        .condition = while_node.ast.cond_expr,
-                        .pointer_payload = .{
+                    },
-                            .name = name_token,
+                });
-                            .condition = while_node.ast.cond_expr,
-                        },
-                    });
 
                 // for loop with index as well
                 if (token_tags[name_token + 1] == .comma) {

src/main.zig

@@ -230,6 +230,7 @@ fn publishDiagnostics(arena: *std.heap.ArenaAllocator, handle: DocumentStore.Han
         });
     }
 
+    // TODO: style warnings for types, values and declarations below root scope
     if (tree.errors.len == 0) {
         for (tree.rootDecls()) |decl_idx| {
             const decl = tree.nodes.items(.tag)[decl_idx];
@@ -643,7 +644,7 @@ fn hoverSymbol(
         },
         .param_decl => |param| def: {
             if (param.first_doc_comment) |doc_comments| {
-                doc_str = try analysis.collectDocComments(&arena.allocator, handle.tree, doc_comments, hover_kind);
+                doc_str = try analysis.collectDocComments(&arena.allocator, handle.tree, doc_comments, hover_kind, false);
             }
 
             const first_token = param.first_doc_comment orelse
@@ -959,7 +960,7 @@ fn declToCompletion(context: DeclToCompletionContext, decl_handle: analysis.Decl
             const doc = if (param.first_doc_comment) |doc_comments|
                 types.MarkupContent{
                     .kind = doc_kind,
-                    .value = try analysis.collectDocComments(&context.arena.allocator, tree, doc_comments, doc_kind),
+                    .value = try analysis.collectDocComments(&context.arena.allocator, tree, doc_comments, doc_kind, false),
                 }
             else
                 null;

src/signature_help.zig

@@ -23,15 +23,7 @@ fn fnProtoToSignatureInfo(
     const token_starts = tree.tokens.items(.start);
     const alloc = &arena.allocator;
     const label = analysis.getFunctionSignature(tree, proto);
-    const proto_comments = types.MarkupContent{ .value = if (try analysis.getDocComments(
+    const proto_comments = (try analysis.getDocComments(alloc, tree, fn_node, .Markdown)) orelse "";
-        alloc,
-        tree,
-        fn_node,
-        .Markdown,
-    )) |dc|
-        dc
-    else
-        "" };
 
     const arg_idx = if (skip_self_param) blk: {
         const has_self_param = try analysis.hasSelfParam(arena, document_store, handle, proto);
@@ -42,14 +34,9 @@ fn fnProtoToSignatureInfo(
     var param_it = proto.iterate(tree);
     while (param_it.next()) |param| {
         const param_comments = if (param.first_doc_comment) |dc|
-            types.MarkupContent{ .value = try analysis.collectDocComments(
+            try analysis.collectDocComments(alloc, tree, dc, .Markdown, false)
-                alloc,
-                tree,
-                dc,
-                .Markdown,
-            ) }
         else
-            null;
+            "";
 
         var param_label_start: usize = 0;
         var param_label_end: usize = 0;
@@ -77,12 +64,12 @@ fn fnProtoToSignatureInfo(
         const param_label = tree.source[param_label_start..param_label_end];
         try params.append(alloc, .{
             .label = param_label,
-            .documentation = param_comments,
+            .documentation = types.MarkupContent{ .value = param_comments },
         });
     }
     return types.SignatureInformation{
         .label = label,
-        .documentation = proto_comments,
+        .documentation = types.MarkupContent{ .value = proto_comments },
         .parameters = params.items,
         .activeParameter = arg_idx,
     };

tests/sessions.zig

@@ -189,9 +189,76 @@ test "Request completion with no trailing whitespace" {
     );
 }
 
+test "Encoded space in file name and usingnamespace on non-existing symbol" {
+    var server = try Server.start(initialize_msg, null);
+    defer server.shutdown();
+
+    try server.request("textDocument/didOpen",
+        \\{"textDocument":{"uri":"file:///%20test.zig","languageId":"zig","version":420,"text":"usingnamespace a.b;\nb."}}
+    , null);
+    try server.request("textDocument/completion",
+        \\{"textDocument":{"uri":"file:///%20test.zig"}, "position":{"line":1,"character":2}}
+    ,
+        \\{"isIncomplete":false,"items":[]}
+    );
+}
+
+test "Self-referential definition" {
+    var server = try Server.start(initialize_msg, null);
+    defer server.shutdown();
+    try server.request("textDocument/didOpen",
+        \\{"textDocument":{"uri":"file:///test.zig","languageId":"zig","version":420,"text":"const h = h(0);\nc"}}
+    , null);
+    try server.request("textDocument/completion",
+        \\{"textDocument":{"uri":"file:///test.zig"}, "position":{"line":1,"character":1}}
+    ,
+        \\{"isIncomplete":false,"items":[{"label":"h","kind":21,"textEdit":null,"filterText":null,"insertText":"h","insertTextFormat":1,"detail":"const h = h(0)","documentation":null}]}
+    );
+}
+test "Missing return type" {
+    var server = try Server.start(initialize_msg, null);
+    defer server.shutdown();
+    try server.request("textDocument/didOpen",
+        \\{"textDocument":{"uri":"file:///test.zig","languageId":"zig","version":420,"text":"fn w() {}\nc"}}
+    , null);
+    try server.request("textDocument/completion",
+        \\{"textDocument":{"uri":"file:///test.zig"}, "position":{"line":1,"character":1}}
+    ,
+        \\{"isIncomplete":false,"items":[{"label":"w","kind":3,"textEdit":null,"filterText":null,"insertText":"w","insertTextFormat":1,"detail":"fn","documentation":null}]}
+    );
+}
+
+test "Pointer and optional deref" {
+    var server = try Server.start(initialize_msg, null);
+    defer server.shutdown();
+    try server.request("textDocument/didOpen",
+        \\{"textDocument":{"uri":"file:///test.zig","languageId":"zig","version":420,"text":"var value: ?struct { data: i32 = 5 } = null;const ptr = &value;\nconst a = ptr.*.?."}}
+    , null);
+    try server.request("textDocument/completion",
+        \\{"textDocument":{"uri":"file:///test.zig"}, "position":{"line":1,"character":18}}
+    ,
+        \\{"isIncomplete":false,"items":[{"label":"data","kind":5,"textEdit":null,"filterText":null,"insertText":"data","insertTextFormat":1,"detail":"data: i32 = 5","documentation":null}]}
+    );
+}
+
 test "Request utf-8 offset encoding" {
     var server = try Server.start(initialize_msg_offs,
         \\{"offsetEncoding":"utf-8","capabilities":{"signatureHelpProvider":{"triggerCharacters":["("],"retriggerCharacters":[","]},"textDocumentSync":1,"renameProvider":true,"completionProvider":{"resolveProvider":false,"triggerCharacters":[".",":","@"]},"documentHighlightProvider":false,"hoverProvider":true,"codeActionProvider":false,"declarationProvider":true,"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":false,"referencesProvider":true,"documentSymbolProvider":true,"colorProvider":false,"documentFormattingProvider":true,"documentRangeFormattingProvider":false,"foldingRangeProvider":false,"selectionRangeProvider":false,"workspaceSymbolProvider":false,"rangeProvider":false,"documentProvider":true,"workspace":{"workspaceFolders":{"supported":false,"changeNotifications":false}},"semanticTokensProvider":{"full":true,"range":false,"legend":{"tokenTypes":["type","parameter","variable","enumMember","field","errorTag","function","keyword","comment","string","number","operator","builtin","label","keywordLiteral"],"tokenModifiers":["namespace","struct","enum","union","opaque","declaration","async","documentation","generic"]}}},"serverInfo":{"name":"zls","version":"0.1.0"}}
     );
     server.shutdown();
 }
+
+// not fixed yet!
+// test "Self-referential import" {
+//     var server = try Server.start(initialize_msg, null);
+//     defer server.shutdown();
+//     try server.request("textDocument/didOpen",
+//         \\{"textDocument":{"uri":"file:///test.zig","languageId":"zig","version":420,"text":"const a = @import(\"test.zig\").a;\nc"}}
+//     , null);
+//     try server.request("textDocument/completion",
+//         \\{"textDocument":{"uri":"file:///test.zig"}, "position":{"line":1,"character":1}}
+//     ,
+//         \\{"isIncomplete":false,"items":[]}
+//     );
+// }
+