Fix clippy rule not writing any content to the output file

[ddeville]

I noticed that when running a rust_clippy target, no content was actually written to the output file. This is because Clippy writes its output to stderr but the rule was only setting --stdout-file.

I’ve changed it to write stderr to the output file instead.

Also tweaked the test to check that content was actually written to the file. It fails before the patch and passes after.

[hlopko]

CC @jfgoog

[jfgoog]

When capturing output, the intention is that you should also specify --@rules_rust//:error_format=json, which does write to STDOUT. This is because the captured output is intended to be consumed by another step in the build process, so should be machine-readable.

So, I think this change is not correct (and will actually break us).

But clearly the documentation and tests should be better, for which I apologize.

It would have been nice to make it so that capturing the output implies --error_format=json. The reason we did not do this in #937 was to keep logic related to clippy.bzl from creeping into rustc.bzl, which was already starting to happen until it was addressed in #850.

[ddeville]

Thanks for the review.

I am actually using --@rules_rust//:error_format=json (although not in the test which I should change) but am definitely never seeing the JSON output ever being emitted to the .clippy.out file. Changing it to stderr does end up with the json content being written to the file however it's added as a series of {"message": ... } dictionaries without any top level array so it's not something that is parsable without first breaking down the file into lines unfortunately 🤔.

[jfgoog]

Yes, you do have to parse the JSON 1 line at a time.

But I am surprised you are not seeing anything in STDOUT. What is your platform and rust version?

[ddeville]

I'm on Linux and my current toolchain is stable-x86_64-unknown-linux-gnu with rustc 1.54.0 (a178d0322 2021-07-26).

[jfgoog]

We are using rustc nightly 2021-11-02 on Linux, and rules_rust 0c53d0e from 2021-10-27, and I have re-confirmed that I am seeing the clippy output captured correctly.

[ddeville]

I'm puzzled. I've just pushed a commit that picks the same nightly version you are using when setting up the repository and still only seeing the content being written out when using stderr.

[jfgoog]

I patched in your change, and reproduced what you are seeing with bazel run test/clippy:clippy_failure_test.sh. I agree that with "normal" bazel and rust, the output is going to STDERR, just as you said.

This makes me wonder if there's something weird about blaze vs. bazel, or if we are doing something odd when building our Rust toolchain.

I'm going to keep investigating.

[ddeville]

Thanks for your time, I appreciate it.

[jfgoog]

OK, I think I figured out what's going on. For "reasons", we run clippy via a wrapper script that does "exec 2>&1" to redirect stderr to stdout.

So I think your change is correct.

Would you mind holding off on submitting while I chat with some other folks about how to handle this?

[ddeville]

Absolutely. I'm actually migrating a largish codebase from our own custom rust rules + cargo-clippy to rules_rust so no rush, I can just patch things locally as I'm making progress.

[jfgoog]

Apparently nobody here has any idea why we redirect sterr to stdout. Going to apply your change locally and see if anything breaks.

[jfgoog]

I patched in your change, and things seem to work fine without the redirect. So I think we should review your change and get it submitted. We will just have to remember to remove the exec 2>&1 when we import.

[jfgoog]

Thanks for catching this, and for improving the tests.

[hlopko]

Thank you!