Completion for errors

[psznm]

This adds completion for errors.

There are issues demonstrated by tests

Issue test 11

"switch on error set - 11" is catch |err| switch (err) construct.

I suspect that scope detection might be wrong which is why "switch on error set - 11" does not work. innermostScopeAtIndex in lookupSymbolGlobal returns scope that looks garbage when printed out.

This:

    var current_scope = innermostScopeAtIndex(document_scope, source_index);
    const node = document_scope.getScopeAstNode(current_scope);
    if (node) |nod| {
        const src = handle.tree.getNodeSource(nod);
        std.debug.print("\nSRC=---\n{s}\n---\n", .{src});
    }

Prints out

       SRC=---
       {
         idk

Trying to tackle the issue is intimidating to me as I don't even know where I would start. Maybe someone more experienced could give some leads how to solve the issue?

---

Issue test 13

"switch on error set - 13" is completion in a function in a structure.

This also feels like issue with scopes. I started digging into how scopes are created. I found DocumentScope where it walks the document. It seems like structure declarations are completely ignored, seemingly on the std side, because the walkContainerDecl just walks the const std = @import("std") and then just stops. When I print out container_decl.ast.members.len , it gives 1. When I delete the std import, there is 0. Nothing gets walked.

---

After some more digging, it seems both issues are basically the same one. The 13 is structure member that is not given to zls from std. The 11 seems like a block statement that is not given to zls from std (the walker sees test_decl and its block, but nothing else). Is there any chance of fixing this from zls side, or does this need to be fixed on std side?

EDIT: Okay, I found out that the Ast parser is not error-prone enough for zls. Both issues are caused by that the parser refuses to give us these specific members/blocks that contain invalid syntax. So this is to be improved on the stdlib side. But here we can at least add completions for "empty" context to auto-complete errors. Those should work with both of the problematic cases, and honestly, even be the most ergonomic completions. I'll see if I can do that.

[Techatrix]

Instead of enumerating in test names, each test name should have a descriptive name based on what it is trying to test. You may also consider group tests together if they are similar enough.

Here is the summary of the suggested code changes (plus some minor refactors):

for (containers) |container| {
    const ip = builder.analyser.ip;
    const key = switch (container.data) {
        .ip_index => |info| ip.indexToKey(info.type),
        else => return,
    };
    if (key != .error_set_type) return;
    const names = try key.error_set_type.names.dupe(builder.arena, ip);
    try builder.completions.ensureUnusedCapacity(builder.arena, names.len);
    for (names) |interned_name| {
        const name = try std.fmt.allocPrint(builder.arena, "{f}", .{interned_name.fmt(ip.io, &ip.string_pool)});
        if (used_members_set.contains(name)) continue;
        builder.completions.appendAssumeCapacity(.{
            .label = try std.fmt.allocPrint(builder.arena, "error.{s}", .{name}),
            .kind = .Constant,
            .insertText = try std.fmt.allocPrint(builder.arena, "{s} => ", .{name}),
        });
    }
}

This is small enough that the collectErrorSetNames function can just be inlined manually.

[psznm]

Instead of enumerating in test names, each test name should have a descriptive name based on what it is trying to test. You may also consider group tests together if they are similar enough.

Here is the summary of the suggested code changes (plus some minor refactors):

for (containers) |container| {
    const ip = builder.analyser.ip;
    const key = switch (container.data) {
        .ip_index => |info| ip.indexToKey(info.type),
        else => return,
    };
    if (key != .error_set_type) return;
    const names = try key.error_set_type.names.dupe(builder.arena, ip);
    try builder.completions.ensureUnusedCapacity(builder.arena, names.len);
    for (names) |interned_name| {
        const name = try std.fmt.allocPrint(builder.arena, "{f}", .{interned_name.fmt(ip.io, &ip.string_pool)});
        if (used_members_set.contains(name)) continue;
        builder.completions.appendAssumeCapacity(.{
            .label = try std.fmt.allocPrint(builder.arena, "error.{s}", .{name}),
            .kind = .Constant,
            .insertText = try std.fmt.allocPrint(builder.arena, "{s} => ", .{name}),
        });
    }
}

This is small enough that the collectErrorSetNames function can just be inlined manually.

Thanks, I had the code similiar enough after solving other comments. I still took heavy inspiration from it, but not exactly as you wrote it. I also kept it as a function, because I felt the scopes were kinda awkward with continues at different levels when inlined. So i feel function is much nicer here.