Visibility suggestions for rust_analyzer?

[DavidSouther]

Current docs have rust_analyzer at the root, with the WORKSPACE. This rule needs something like

buildozer "add targets $(bazel query 'kind("rust_*library|rust_binary", //...:all)')" //:rust_analyzer

however, this requires all those targets to effectively have //visibility:public. Is there any workaround or suggestion on how to not have that requirement?

[UebelAndre]

I'm unfortunately not aware of a workaround. One of the stated goals of the rust-analyzer rule (#704) is not to require a list of targets but to have a more 'dynamic' solution for generating rust-project.json. My inital thought is that the @rules_rust//tools/rust_analyzer tool could be updated to find all rust targets using the mentioned query in the docs? There's some consequences to doing that though so not sure if it's a good solution. I'd be happy to try and answer any questions to help find a workaround or collaborate on a PR if you had the time and willingness 😄

[DavidSouther]

Why is rust_analyzer insistent on living in the root of the WORKSPACE? I'd be totally fine with bazel run @rules_rust//tools/rust_analyzer //path/to/project as the workfow, which could handle adding the rust_analyzer per project within the repo. That also "feels" more bazel to me (but that's with a large dose of my personal opinion)

[UebelAndre]

I think the idea is that if you open your workspace in your IDE, language specific support should be enabled for all your targets. Nearly all the Bazel projects I work with are multi language and most of them are mono-repos so what I'd like to be able to do is open my IDE at the root of the workspace and navigate from project to project.

Maybe I misunderstand what you're saying but it sounds like you'd want a rust-project.json file per crate?

[DavidSouther]

My expectation was one rust-project.json per crate, but I'd expect it to live in bazel-out. This is similar to how bazel manages tsconfig.json files in the typescript world. (Or how it used to, I've only used ts_library which is getting phased out.)

Honesty I don't know the right solution! I just know that the docs don't "work" using a common visibility label (private or __subpackages__), and should clearly state "while the rule is experimental, rust_* rules must be //visibility:public". In the future, allowing rules to have limited visibility is an important requirement.

[UebelAndre]

Sorry, I may have been projecting some personal goals vs discussing the actual issue at hand 😅

I do agree that visibility shouldn't prevent you from being able to use rust-analyzer. I do think that there's consensus that having to manually maintain a list of targets for rust-analyzer support is undesirable. Additionally, a personal goal of mine is to not require the user to do recurring actions so that changes to the project are automatically picked up. But I don't have any off hand ideas I'd open a PR for.

Would you be willing to open a pull-request to update the docs? I think that'd be a great start and then hopefully more ideas will surface and we can continue to collaborate on rust-analyzer support 😄

[djmarcin]

The requirements for the rust-project.json location is driven mostly by the mechanism of how the rust-analyzer tool works, documented here. The rust_analyzer rule is currently optimized to work with method 1, auto-discovery, to minimize configuration. This method requires a rust-project.json file at the project root. The other possible configuration would require the user to add a config file with the locations of every rust-project.json file.

I completely agree that the current setup requiring the rust_project rule which requires public visibility needs to be fixed.

[DavidSouther]

Updated the docs in #906, with an explanation in the PR why it must be //visibility:public.

Thanks for listening to my concerns, and glad to hear it's on the roadmap as a thing to solve before moving rust_analyzer out of experimental.

[UebelAndre]

If I understand this correctly, #907 would satisfy the visibility issues brought up here.

[DavidSouther]

Looks like it, yes

…

On Wed, Aug 25, 2021 at 11:21 AM UebelAndre ***@***.***> wrote: If I understand this correctly, #907 <#907> would satisfy the visibility issues brought up here.

[UebelAndre]

Closing this since #907 has been merged