Nuke the clippy plugin interface

[oli-obk]

It's been a month or so 10 days since the plugin interface emits unsilencable warnings. I have not seen any complaints. I guess noone was using the plugin interface anymore.

[phansch]

Hmm, this GitHub search reveals quite a few results where the plugin interface is used.

Some popular users include:

• diesel (lib)
• https://github.com/cretonne/cretonne (bin/lib)
• https://github.com/racer-rust/racer (lib)
• https://github.com/habitat-sh/habitat (bin)
• https://github.com/citybound/citybound (bin)
• https://github.com/Wilfred/remacs (bin)
• https://github.com/pingcap/tikv (bin)

Perhaps we should communicate this better (through the dev-tools news, maybe) before removing it?

[oli-obk]

Indeed. Let's wait a while longer.

Not actively using it:

• racer (0.0.103) https://github.com/racer-rust/racer/blob/d465e38e0893850790a3f6d7c99933552064babf/Cargo.toml#L33
• cretonne (just has the plugin attributes, but uses cargo clippy) https://github.com/cretonne/cretonne/blob/122249ad2dd8dbb6c6e3c24162366e7a9bafdf45/clippy-all.sh#L7

PRs open to remove it

• habitat: Use cargo clippy instead of the clippy plugin habitat-sh/habitat#5102

Issues open for removing it

• citybound Remove the clippy plugin usage citybound/citybound#264
• remacs Use cargo clippy instead of the clippy plugin remacs/remacs#757
• tikv Use cargo clippy instead of the clippy plugin tikv/tikv#3090
• diesel Use cargo clippy instead of the clippy plugin diesel-rs/diesel#1709

[oli-obk]

Getting the word out via twir: https://twitter.com/elegmit/status/998178638234341377

[ssokolow]

Drat. I was looking forward to when clippy was available on stable channel so I could switch to the plugin API.

I suppose I could do the same sort of thing I was hoping to avoid with my contributions to the request for a post-build script and set up a system where cargo build and cargo run error out if called directly, rather than via a wrapper.

(The wrapper would first call cargo clippy and I'd then tie essentially all compiler output to #![cfg_attr(feature="cargo-clippy", ...)] to avoid duplication.)

Of course, then I'd also have to fork setuptools-rust to keep things working in some of my projects, which would be annoying.

[oli-obk]

Well... even without this change we would not be stabilizing the plugin interface.

What you can always do is just set the RUSTC_WRAPPER env var to clippy-driver and CLIPPY_TESTS to true to unconditionally replace rustc with the driver.

[ssokolow]

Ahh. That'd help.

I haven't needed to use build scripts before so I'm uncertain how powerful they are.

Are they capable of forcing the value of RUSTC_WRAPPER and CLIPPY_TESTS within the appropriate execution context when cargo build or cargo run are run, or do I have to resort to using env! or something in the vein of #[cfg(not(cargo-clippy))] to abort the build with a "Use the wrapper" message?

[oli-obk]

You want to force users of your library to use clippy?

[ssokolow]

Only in the case of certain projects and, in all such cases, it being a library is an implementation detail of a single-repository, multi-language binary.

For example, I have a project which is a Python-Rust hybrid creation where I'd do it purely in Rust as a binary, but there are no acceptably mature QWidget bindings for Rust, so I'm forced to resort to putting as much as possible in a Rust backend via rust-cpython, which makes no attempt to be reusable outside the Python code which glues it to Qt's Python bindings.

(Naturally, I will eventually split the reusable bits out into their own non-clippy-forcing crates when I find time to work on my "GUI for divvying up a git repository's contents while verifying correct assignment of hunks in previous revisions" project.)

[oli-obk]

Yea in that case I'd just resolve to the #[cfg(not(cargo-clippy))] trick. Seems like an edge case

[flip1995]

Do we want to do this before releasing Clippy 1.0?

[oli-obk]

It seems all the linked issues have been resolved.

The only thing left is to figure out how to make cargo not rebuild everything when going from cargo build to cargo clippy. I believe we need cargo support for this.

[flip1995]

Ping @oli-obk. Should we S-inactive-closed this and revisit it (in a new PR) once #3837 is resolved?

[oli-obk]

yea, that makes sense

[tylerhawkes]

Well... even without this change we would not be stabilizing the plugin interface.

What you can always do is just set the RUSTC_WRAPPER env var to clippy-driver and CLIPPY_TESTS to true to unconditionally replace rustc with the driver.

I just tried this on a non-trivial project and it doubled my build time from 20 minutes to 40 minutes. I've been wanting a way to use clippy without having to also separately build but nothing seems to be working. It's nice to have clippy automatically run whenever I do a build so I don't have to remember it and fix clippy issues later. Any ideas on how we can keep build times reasonable without having to do multiple clippy steps?

[mati865]

@tylerhawkes on nightly you can try cargo clippy-preview -Z unstable-options.

[jonboh]

@rustbot label -S-inactive-closed