Clippy Integration

[smklein]

Hey all, this is more of a feature request than a bug. If discussion would be better served outside a Github issue, lemme know, happy to relocate the conversation.

TL;DR: I have a project depending on rules_rust, and I'd like to be able to use clippy for linting produced libraries, binaries and tests.

It isn't immediately clear what would be the best way to go about doing this integration.

• Given the precedent set by rust_doc, would clippy targets be best implemented as a rust_lint target, listing all crates as dependencies?
• "clippy-driver" appears to be a non-Cargo mechanism for invoking clippy. Is there a similar target already ya'll would recommend taking a look at for "taking a Cargo command, and running it without Cargo"?

Happy to help contribute here, but I wanted to be oriented correctly before trying anything out :P

[mfarrugi]

This is probably most similar to an integration for rust-language-server, discussed in #71 or an aspect implementation of rustdoc (#152). For clippy in particular, I think it could also make sense as an optional action builtin to the crate rules (at the toolchain or crate level).. though I'm not sure if this is mutually exclusive. The last time I looked at aspect business was a while ago, and it was bit complicated.. but the idea is that there is enough information in the existing crate targets to be able to invoke clippy as an aspect without adding a bunch of extra `rust_lint` targets (and ostensibly the same would be possibly for rust_doc etc). @damienmg I think you know the most about this sort of thing.. any thoughts?

…

On Wed, Apr 15, 2020 at 4:40 PM Sean Klein ***@***.***> wrote: Hey all, this is more of a feature request than a bug. If discussion would be better served outside a Github issue, lemme know, happy to relocate the conversation. TL;DR: I have a project depending on rules_rust, and I'd like to be able to use clippy <https://github.com/rust-lang/rust-clippy> for linting produced libraries, binaries and tests. It isn't immediately clear what would be the best way to go about doing this integration. - Given the precedent set by rust_doc, would clippy targets be best implemented as a rust_lint target, listing all crates as dependencies? - "clippy-driver" appears to be a non-Cargo mechanism for invoking clippy. Is there a similar target already ya'll would recommend taking a look at for "taking a Cargo command, and running it without Cargo"? Happy to help contribute here, but I wanted to be oriented correctly before trying anything out :P — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#304>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAROG6C2ZAASG7TYDXTEQALRMYLUXANCNFSM4MI4KGKQ> .

[damienmg]

Implementing it as a rule seems to me like a better approach. The rule itself might use some aspect to collect the sources but this does not seems correct for me to call Bazel with a special command-line for doing those clippy test, although this is more a UI question than an implementation one.

[smklein]

I managed to implement a clang-tidy aspect for C++ code in a private repo, so I think I have a better understanding of how this would work.

I'm finding myself needing to reach into rust/private/rustc.bzl to access CrateInfo, among other things.

@damienmg , would you be okay with me attempting the aspect approach? From a usability perspective on a large codebase, I find aspects to be significantly more convenient.

[damienmg]

@smklein : I don't have an issue with using aspect, I am just saying that the UI itself I find it more logical to be a separate rules (that would then use an aspect to execute the needed action on the build graph). Is that the approach you were considering?

[smklein]

I admittedly don't really need to make much use of the aspect propagation, the real thing I want to be able to do is the following:

bazel build --aspects=@rules_rust//rust:clippy.bzl%clippy_aspect --output_groups=clippy_checks //...

And to be able to dynamically run clippy across portions of a Bazel-oriented Rust workspace

[damienmg]

That's fine for me :)

[smklein]

I have a work-in-progress solution here, which required much more cleanup.

Couple notes so far:

• clippy-driver needs to be tightly coupled with rustc, so this causes some changes in how rustc is downloaded / extracted. As a result, the version of clippy is identical to the version of rust, since they'll be extracted from the same tarball.
• To get a version of clippy-driver which works, I think I'm going to need to force a revision bump past 1.39.0. On that previous version, command line parsing was throwing "unrecognized lint' errors, but on 1.44.0, the driver seems content.

[smklein]

as of #339 , this is done!