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

native.register_toolchains not recognized (WORKSPACE mode) #248

Closed hauserx closed 4 months ago

hauserx commented 4 months ago

In non-bzlmod solution one can use native.register_toolchain. It is not recognized by starpls. Not sure how much priority is there for fixing non-bzlmod cases, but adding here as noticed it.

https://bazel.build/external/migration#register-toolchains

Adding screenshot (as cannot copy-paste those diagnostics):

image

withered-magic commented 4 months ago

I think this is mainly an issue with the exported native type from the builtin proto missing that member, so it actually doesn't work whether bzlmod is enabled or not I think

The only members available on the native module are the ones listed here as well as all the builtin rules, so I guess register_toolchains isn't part of this set. Should be easy enough to patch the native object though

withered-magic commented 4 months ago

Actually I guess we can just include all the functions in this overrides file on the native object, except for workspace().

hauserx commented 4 months ago

so it actually doesn't work whether bzlmod is enabled or not I think

I maybe worded it incorrectly - register_toolchains cannot be used in MODULE.bazel or extensions, so it will not be a problem if one uses solely bzlmod. But you are right that even if one enables bzlmod AFAIU then WORKSPACE can still be used (for older deps) and use native.register_toolchains.

Actually I guess we can just include all the functions in this overrides file on the native object, except for workspace()

Nice, it should remove the warnings. It will be slightly not precise, as it will sometimes suggest this few methods for non workspace macros/rules, but I guess it will be hard to check whether a given macro is used from WORKSPACE or from BUILD files and use one or another set of methods.

withered-magic commented 4 months ago

Opened https://github.com/withered-magic/starpls/pull/249 if you want to add yourself as reviewer and try it out before I merge!

It will be slightly not precise, as it will sometimes suggest this few methods for non workspace macros/rules, but I guess it will be hard to check whether a given macro is used from WORKSPACE or from BUILD files and use one or another set of methods.

Yeah I think until the issues with the builtins proto are fixed, we should probably optimize for overall helpfulness rather than correctness, even if that means sometimes suggesting things that are technically invalid