withered-magic / starpls

An LSP implementation for Starlark, the configuration language used by Bazel and Buck2.
Apache License 2.0
99 stars 9 forks source link

Support `*.BUILD.bazel` files #262

Closed withered-magic closed 3 months ago

withered-magic commented 3 months ago

Sometimes, we have files named something like foo.BUILD.bazel which are used as templates or similar. We don't necessarily have to resolve targets declared in these files but we should at least offer nicer completions

keith commented 3 months ago

Another case I noticed with this is things like cc_library and glob don't seem to resolve:

image

Also maybe it would be safe to assume that @//whatever could be resolved against the current workspace too?

image
withered-magic commented 3 months ago

Ah, did you run into this for regular BUILD.bazel files? Those should work - if not then there should be logs from the language server around it trying to run bazel info build-language to fetch information about builtin rules and failing. I opened this issue for a pretty niche use case I ran into where files were named something like foo.BUILD.bazel and were meant as templates or otherwise inputs to another rule.

The error logs from the LSP should look something like this for a failed bazel info build-language invocation (in this example, the executable doesn't exist):

server: fetching builtin rules via `bazel info build-language`
server: failed to run `bazel info build-language`: No such file or directory (os error 2

Maybe another follow up question, if this applies to normal BUILD.bazel files, do features like go-to definition for symbols from external repositories also not work in general? If not then something might be misconfigured on the Bazel side since the LSP currently relies on being able to run Bazel commands in the current workspace to provide that functionality.

Here's an example of cc_library and glob resolving as expected in a regular BUILD.bazel file, assuming the bazel info build-language invocation worked correctly:

https://github.com/withered-magic/starpls/assets/153045167/621a849f-e143-47d0-a7b7-e201b66d90c9

As for the @//whatever case, that's separate (and unimplemented) for now, I'll open up a separate issue for that

keith commented 3 months ago

This is for the case you mentioned. I don't think it's that niche when you're pulling in random C++ dependencies and naming them foo.BUILD

withered-magic commented 3 months ago

Ah sounds good, makes sense! Then yeah the behavior you mentioned above is also currently expected, i.e. resolution doesn't work because those globals are only made available in files called BUILD.bazel, but should be easy enough to change that check