Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to save the json output from rustc to pass to rust-analyzer #1942

Merged
merged 6 commits into from
Dec 5, 2023

Conversation

matts1
Copy link
Contributor

@matts1 matts1 commented Apr 26, 2023

@googleson78 originally wrote #1657 in order to solve this problem. As discussed in that PR, they're no longer working on this, so I offered to pick this up. This is the same PR as that one, but has some bugfixes and refactoring applied.

@matts1 matts1 force-pushed the rustc-output branch 4 times, most recently from 3cb909e to 7fd0456 Compare April 26, 2023 03:46
@matts1 matts1 marked this pull request as ready for review April 26, 2023 03:49
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for working on this!

Would you be able to show how rust-analyzer could consume this? My understanding is that this would allow your IDE to inline errors in the code? Similar to the cargo check behavior. But according to #1633 there isn't any hook in rust-analyzer for this yet or there may not be? Is that correct?

rust/private/rustc.bzl Outdated Show resolved Hide resolved
rust/private/rustc.bzl Outdated Show resolved Hide resolved
@matts1
Copy link
Contributor Author

matts1 commented May 4, 2023

We'd use a custom vscode extension (I've published it here) for bazel + rust (I talked to rust-analyzer folks, and they agreed that all non-cargo build systems should probably write their own rust-analyzer.

I have a prototype with everything working here, but the TLDR is:

  1. A custom flycheck would be registered for rust-analyzer that just cats the output of concatenated.rustc-output (which will be generated by gen-rust-project)
  2. Vscode extension would query a list of open files, and invoke bazel run @rules_rust//tools/gen-rust-project -- --files <currently open files in vscode>
  3. gen-rust-project would convert those files to bazel targets
  4. gen-rust-project would use bazel cquery to calculate the rustc-output files.
  5. gen-rust-project would output a rust-project.json for the bazel targets from step 3
  6. gen-rust-project would watch for changes to any of those files from step 4
  7. When gen-rust-project sees a change, it concatenates all the files from step 4 together to concatenated.rustc-output, then writes a line to stdout, which the vscode extension then receives.
  8. When the vscode extension receives the message, it invokes the vscode command clear flychecks and then the command run flychecks.

@purkhusid
Copy link

@matts1 Have you been using this patch successfully in your project?

@matts1
Copy link
Contributor Author

matts1 commented Jun 19, 2023

Yeah, it's been working fine for me.

@Silcet
Copy link

Silcet commented Jul 10, 2023

I'm crazy interested in this one, but I can see it has stalled a bit. Can I help somehow? I have forked the repo and rebased your changes to solve the conflicts, so I could submit my branch if that helps... 😃

@matts1
Copy link
Contributor Author

matts1 commented Oct 19, 2023

@UebelAndre Could you please take a look again. CI is failing for unrelated reasons (unsupported toolchain 'ubuntu1604-bazel-java8'). It appears someone removed the old version of java from RBE.

You can try it out by the following:

cd examples/bzlmod/hello_world
bazel build //:hello_world --@rules_rust//:output_diagnostics=true --output_groups=+rustc_output,+rustc_rmeta_output

You should then see a bazel-bin/hello_world.rustc_output file in addition to the regular bazel-bin/hello_world

@purkhusid
Copy link

I think this would be awesome to get this change in, any chance that any of the maintainers could give it some time?

@matts1 matts1 force-pushed the rustc-output branch 3 times, most recently from 9d6c94d to f5d3dbc Compare November 13, 2023 02:24
@matts1
Copy link
Contributor Author

matts1 commented Nov 13, 2023

I just fixed the merge conflicts, so it should be good to go as soon as it gets approved.

Copy link
Contributor Author

@matts1 matts1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@UebelAndre Your changes requested doesn't appear to be actual changes, but I can't work out how to resolve that from github's perspective. Should be fine to review.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay here. I think this seems good 😄. Just one question.

rust/private/rustc.bzl Outdated Show resolved Hide resolved
@purkhusid
Copy link

Any chance we can get this on main?

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay here 😅

I think this looks good by my final request would be updating //docs:rust_analyzer.vm to describe the steps mentioned here: #1942 (comment)

@matts1 matts1 force-pushed the rustc-output branch 2 times, most recently from 2149fd4 to d5485a9 Compare December 4, 2023 01:40
@matts1
Copy link
Contributor Author

matts1 commented Dec 4, 2023

@UebelAndre Should be good to go

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

BUILD.bazel Outdated Show resolved Hide resolved
googleson78 and others added 5 commits December 5, 2023 10:41
The error format is checked whenever it's set, not only when
rustc_quit_on_rmeta is set.
When this flag is set, bazel will write diagnostics to the output groups rustc_output and rustc_rmeta_output.

Change-Id: I2690b571a0bc61fa58dc11fb480b725985603bd5
@UebelAndre UebelAndre merged commit 0bded80 into bazelbuild:main Dec 5, 2023
3 checks passed
@matts1 matts1 deleted the rustc-output branch December 5, 2023 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants