Zls Support

[turmanticant]

Hey! I started using this ruleset recently, and have appreciated how well they work!

I am trying to get IDE feedback from zig_modules. Everything compiles totally fine, but I don't get any symbol resolution from modules.

I am using the zls for IDE feedback (through Zigbrains in a jetbrains IDE).

Are there any tricks to let zls know about these modules?

[aherrmann]

@turmanticant thanks for the kind words and for raising this ticket. I haven't looked into the configuration of ZLS under Bazel, yet. So far I have just used ZLS with a globally installed Zig toolchain that matches the Zig version used under Bazel. Ideally rules_zig ZLS support would capture two features:

1. Configure ZLS to use the rules_zig / Bazel managed Zig toolchain so that the Zig version always matches.
2. Configure ZLS with the correct module search paths so that imports of Bazel managed modules, including generated modules, are found correctly.

Neither of these features is implemented at the time of writing. And, I think those features can be implemented one-by-one in any order.

I had a look at the ZLS configuration schema. I see fields about the Zig installation and standard and builtin library paths. Those would capture point 1 above, but not point 2.

For point 2 ZLS would need to expose some interface to configure module search paths. I don't know if ZLS exposes such an interface, but I didn't find one at a cursory look. A good starting point is probably to look at how ZLS manages module search paths with build.zig based projects.

[turmanticant]

Thanks for the threads to pull on, that is super helpful. I will take a look into point 2 to see what options for supporting this might look like.

[vctrmn]

Very interested here ! I will try to implement the first point : Configure ZLS to use the rules_zig / Bazel managed Zig toolchain so that the Zig version always matches.

And by the way, great work with rules_zig !

[aherrmann]

@vctrmn That's great to hear! Let me know if I can help with any pointers about rules_zig. And thanks for the kind words!

[cerisier]

I have a working version of ZLS for rules_zig and plan to submit a PR shortly.

Before I do that, I would like to validate a few points.

Overview

The way it works currently is as follows:

The first step is to have a way to generate json representation of ZLS BuildConfig structure.

This is done by collecting all transitive zig modules from a set of targets and assign their name to their main.zig file.

We expose this through a runnable completion target.

That completion target is going to be invoked by ZLS itself using a custom zig build runner of our own (Instead of ZLS's own custom build runner that parses std.Build to generate the BuildConfig).

Finally, we expose a bazel runable "ZLS proxy" that will be invoked instead of the real ZLS binary by the various IDE extensions.

That proxy will itself, invoke a hermetically downloaded ZLS binary with a generated config file specificying paths to the Zig toolchain as well as custom build runner that will generate the BuildConfig.

API

I initially had 2 targets, one for the completion and one for the the runner but I think we could get away with just one ?

zls_completion(
    name = "completion",
    visibility = ["//visibility:public"],
    deps = [
        "//path/to/library1",
        "//path/to/library2",
    ],
)

The alternative being:

zls_completion(
    name = "completion",
    visibility = ["//visibility:public"],
    deps = [
        "//path/to/library1",
        "//path/to/library2",
    ],
)

zls_runner(
    name = "runner",
    target = ":completion",
)

In the case of vscode, the configuration then looks like that:

{
  "zig.formattingProvider": "zls",
  "zig.zls.path": "${workspaceFolder}/tools/zls.sh",
}

where zls.sh has to be provided by the workspace as follows (or equivalent):

#!/bin/bash
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(bazel info workspace)"
exec bazel run -- //:completion "${@}"

Would this be acceptable ?

[turmanticant]

I've used a dart ruleset that handled completions similar to this, by generating a special file that the dart tooling knew to look for.

It would be nice if completion could discover all of the zig_* targets. As a user adds new targets, consolidates, or breaks apart existing targets, they are going to need to update the deps of //:completion. With multiple people adding new features this seems like a place where frequent merge conflicts happen. Having the tool automatically discover the targets would be fantastic (gazelle?), but I can appreciate that might be a more complex solution.

Anyway, the ability to have zls to understand bazel is a huge DX improvement in any capacity.

[aherrmann]

This is done by collecting all transitive zig modules from a set of targets and assign their name to their main.zig file. [...]

zls_completion(
    name = "completion",
    visibility = ["//visibility:public"],
    deps = [
        "//path/to/library1",
        "//path/to/library2",
    ],
)

So this would expose something along the lines of library1=path/to/library1/main.zig, library2=path/to/library2/main.zig, depoflib1=path/to/dep/of/lib1/main.zig, ... to ZLS? Makes sense. Out of curiosity, is that implemented through an aspect or is the ZigModuleInfo of the library1, library2 targets even sufficient?

That completion target is going to be invoked by ZLS itself using a custom zig build runner of our own (Instead of ZLS's own custom build runner that parses std.Build to generate the BuildConfig).

Cool, looking forward to seeing this in action.

I initially had 2 targets, one for the completion and one for the the runner but I think we could get away with just one ?

Maybe? Hard to say without looking at the code. Depending on how this is implemented a potential issue could be that it properly generating a runfiles tree typically requires a dedicated target, see also https://github.com/bazelbuild/bazel/issues/15486.

#!/bin/bash
cd "$(dirname "${BASH_SOURCE[0]}")"
cd "$(bazel info workspace)"
exec bazel run -- //:completion "${@}"

Note, bazel run will change directory, so cd $(bazel info workspace) may be redundant. Also note, bazel run holds a lock, depending on this is set up you may want to use bazel run --script_path.

---

It would be nice if completion could discover all of the zig_* targets. As a user adds new targets, consolidates, or breaks apart existing targets, they are going to need to update the deps of //:completion.

I think something like this could be automated with a bazel query to picks up all zig_* targets followed by a buildozer command that writes them all to a zig_completion target. Gazelle could also be used, but may be overkill in this use-case.

In principle that query could even be done dynamically each time, avoiding the need for the zls_completion target. It adds startup overhead though. So, it's a trade-off.

---

Yes, the design sounds good to me. What is needed to update the state of ZLS when the user adds or removes a Zig target? Restart ZLS?

[cerisier]

So this would expose something along the lines of library1=path/to/library1/main.zig, library2=path/to/library2/main.zig, depoflib1=path/to/dep/of/lib1/main.zig, ... to ZLS? Makes sense. Out of curiosity, is that implemented through an aspect or is the ZigModuleInfo of the library1, library2 targets even sufficient?

Exactly. It uses an aspect because we want to discover the dependency graph of zig_binary and the likes so this is done by an aspect. If the dep exposes a ZigModuleInfo, we take it. Otherwise we construct it using the aspect.

I initially had 2 targets, one for the completion and one for the the runner but I think we could get away with just one ?

Maybe? Hard to say without looking at the code. Depending on how this is implemented a potential issue could be that it properly generating a runfiles tree typically requires a dedicated target, see also bazelbuild/bazel#15486.

After a few weeks of testing. This works just fine using a single target !

Note, bazel run will change directory, so cd $(bazel info workspace) may be redundant. Also note, bazel run holds a lock, depending on this is set up you may want to use bazel run --script_path.

Sweet. I'll test that and report.

Yes, the design sounds good to me. What is needed to update the state of ZLS when the user adds or removes a Zig target? Restart ZLS?

For now yes.

[cerisier]

I'll push a Draft PR this week. Where do you think this should live ? //zls or //zig/zls ?