Clippy 1.0

[Manishearth]

So I'm thinking of doing a clippy 1.0 soon.

The idea will be to make a users.rlo post asking for feedback that needs to get in before 1.0, after which the semantics of existing lints will be somewhat frozen (can be changed, but needs more discussion). IMO warnings don't qualify as breaking changes anyway, so this isn't a rigid restriction.

However, what I'd like to get figured out before we land:

• Are there any bikesheds that need to be done? (https://github.com/Manishearth/rust-clippy/issues/533)
• Are there any major lints that we must have before 1.0?
• Are there any major bugs that we must fix?

The idea of 1.0 in my mind is that clippy is "ready". Folks have been using it for more than a year now, and this is something we probably should have done ages ago. This just marks it as something we're confident in telling others to use extensively.

This bug is mostly to track progress and to decide if there are any objections to going 1.0. The points listed above will be discussed when I make the internals posts, but can be discussed here if major.

Once we release 1.0 my plan is to try pushing for it to be included as a rustup component so that it can be used on stable.

Thoughts?

cc @llogiq @mcarton @oli-obk @birkenfeld

[mcarton]

I have been asked Sunday why/how the hell we got to 0.0.100 :smile:

Once we release 1.0 my plan is to try pushing for it to be included as a rustup component so that it can be used on stable.

So clippy *1.0.* would be for rustc 1.x, clippy 1.1.* for rustc 1.(x+1)*, etc.?

Are there any bikesheds that need to be done?

There probably is. I would like to merge/rename some lints before we get to stable. I looked at that once, but IIRC, the interface to rename lints is not available to us.

Are there any major lints that we must have before 1.0?

Don't think so.

Are there any major bugs that we must fix?

Most likely. I did want to rewrite loop lints at some points because they have quite a lot of FP, but I haven't had much time.

[Manishearth]

So clippy 1.0. would be for rustc 1.x, clippy 1.1. for rustc 1.(x+1), etc.?

Nah, the stuff shipped with rustc will have arbitrary versions (may not even correspond to clippy published versions due to temporary rustup fixes). I haven't yet settled on how that will work, and this involves discussion with the rustup/rustc folks too.

[oli-obk]

Regarding bikesheds, I say we move all controversial lints to allow by default (goodbye cyclomatic complexity my precious :cry:) and the discuss them back into warn.

Also I think that we need #1295 and #1313 before 1.0, since those seem to be a major blocker for team wide usage.

Only cargo clippy is going into rustup, right? Or will the lints be autoloaded by the distributed rustc?

[mcarton]

I have already started working on #1295, I'll try to finish that soonish.

[Manishearth]

Only cargo clippy is going into rustup, right?

Oui.

I say we move all controversial lints to allow by default

Good idea. I'd prefer to just get them discussed first, and if there are any which haven't been discussed we move them to allow.

[Manishearth]

FWIW I've added a C-1.0-blocking label for things (like lint renames) that must happen before 1.0. It is not an exhaustive list right now. Feel free to apply liberally.

[LegNeato]

A bikeshed on Reddit suggested changing the name from clippy to lint.

Personally I superficially agree and also note that the clippy reference is not in the current zeitgeist and has never been experienced by devs of a certain age. The name and reference will only get less and less relevant as time goes on. Then again, clippy is a fun name for those who know, refers to computing lore/history, and I don't know how much work or fallout there would be changing 😃 .

Some projects using standard "lint" naming:

• Go: https://github.com/golang/lint
• Mozilla's mach: https://firefox-source-docs.mozilla.org/tools/lint/usage.html
• Atom: https://atom.io/packages/linter
• VSCode: https://code.visualstudio.com/docs/languages/javascript#_linters
• Phabricator: https://secure.phabricator.com/book/phabricator/article/arcanist_lint/
• Pylint: https://www.pylint.org/
• JSLint: https://www.jslint.com/

Some projects referring to "linting" but using different names:

• clang-tidy: http://clang.llvm.org/extra/clang-tidy/
• pyflakes: https://pypi.org/project/pyflakes/

[phansch]

I'm personally not opposed to renaming but one problem that comes to mind is: If we want to rename the command to cargo lint is that other Rust projects can also implement their own lints. There's also the lints in Rust itself.

I think calling Clippy by using cargo lint could be confusing in that regard as it's not clear anymore which lints of which project (clippy, rustc or something else) are executed.

[Manishearth]

Yeah, the point of Clippy is specifically to house the more annoying lints (of various levels of annoyingness). I've considered cargo lint in the past and it gets confusing.

I think it's fine if folks don't get the reference, it's still a nice, short, memorable tool name.

[withoutboats]

I think clippy is a great name for this project and wouldn't suggest changing it, but I'm somewhat concerned about the overall Rust + cargo interface implications of what's happened here.

I know this is implemented as a custom subcommand, but if we start shipping this component by default, we've functionally added a new (official) subcommand to cargo. Is that the intention for devtools in general? - any new devtool with a CLI will just be accessed by a cargo <project-name> interface? Where is the step where we think holistically about the interface our tools present to the user?

If we are going to eventually ship clippy by default, I'd be more comfortable with some sort of unified cargo lint interface, possibly with some way to configure sources of lints, of which clippy would be a default source. Or possibly there shouldn't be a subcommand at all once its shipped by default, and you should instead just say you want these lint passes run in addition as part of cargo build.

I don't care about the interface to the clippy-preview optional subcomponent, but once we start shipping components by default I think we need to revisit how they fit into the overall Rust toolchain experience: given that they will be on equal footing with Rust and cargo at that point, there is both a need for further revision and less constraints on what kind of interface you can have (i.e. at that point you can make PRs to cargo and rustc that are necessary to get the interface you want).

[WiSaGaN]

Thanks @LegNeato for investigating conventions from other languages.

As mentioned in the reddit comment quoted below:

It feels a bit unfortunate though to have clippy as the name instead of lint. All previous official cargo commands use straightforward descriptive names: init, build, check, run, test, update, etc. It seems unnecessary to break this convention..

I think the question is whether it is necessary to break the established convention of official cargo subcommand naming by using "clippy".

Following the convention of using straightforward descriptive name also makes it easier for those coming from other languages (e.g. golang), or people not knowing the Microsoft insider jokes to grok what this command does.

[fralalonde]

Following on the bikeshedding path of renaming clippy to something more descriptive of its purpose (and less nostalgic) while reserving lint for a future plugin mechanism (which I support), may I suggest

cargo coach

Coach gives instant feedback before going live (submitting a PR, releasing), and teaches you best practices as you learn Rust. As you get better you need less coaching but it's always there if you need a quick opinion. It's essentially constructive, and shouldn't (I believe) block you from doing whatever, contrarily to lints which could be prescriptive and fail the build.

It's also a subtle play on the cargo "transportation" theme. And it retains the cool consonance that cargo clippy had.

[joshtriplett]

@fralalonde Let's not get into name bikeshedding, please. The question is whether the name needs to become more descriptive, not whether the name should change to some other fanciful/metaphorical name.

I like @withoutboats suggestion here: let's keep cargo clippy, and consider the possibility of having a general "run multiple linters" subcommand like cargo lint.

For that matter, I wonder if we should have a (configurable) way for cargo check and cargo build to run clippy, either via cargo check --lint or some configuration to enable that.

[LegNeato]

At the risk of sounding dense (I am not super familiar with Rust's linting though I worked on a lot of lint stuff at Facebook) why make the distinction of user-defined vs clippy lints vs rustc lints at all? I agree with @withoutboats that these should have a unified user experience. Where the lint came from and what tool is doing the linting is an implementation detail most developers do not care about or need to know.

For example, at Facebook when you run arc lint it may report some issues from pyflakes, others from clang-tidy, others from findbugs, and yet others from regex-based PHP scripts.

Everywhere I have ever worked there has been one interface to run all lints with standard ways to turn them on and off (per-line/file/project/invocation) regardless of where they originated.

So I guess concretely for 1.0...I would suggest shipping clippy by default with rust, default all checks to off, use standard generic linting attributes and config file that do not refer to clippy, etc. Basically decouple the project name from the developer UI.

[llogiq]

Since we were citing prior work, in the Java world there's FindBugs/SpotBugs, CheckStyle,PMD, SonarQube.

[withoutboats]

I like @withoutboats suggestion here: let's keep cargo clippy, and consider the possibility of having a general "run multiple linters" subcommand like cargo lint.

That's not my position. I think this project should be called clippy, but I'm not convinced that cargo clippy is a great interface to this project. I think that aspect of the interface should be reconsidered in light of all of the incredible power that being a first class tool gives clippy.

One possible approach is to have some cargo lint command that can take multiple lint sources, as we've discussed here. Another approach would be to add namespaced custom lint groups, so the interface to clippy is just #![warn(clippy::*)] in your code, no CLI at all. I'm not certain what the best approach is, I just don't think this problem space has been re-explored with the privileges we're now affording clippy, and so it should be.

[oli-obk]

As a first step we can remove cargo-clippy from rustup and only distribute clippy-driver. Then users can install the cargo clippy subcommand themselves until we figure out the details here.

[Manishearth]

I feel like it's better if we let rustup distribute it for now -- there's nothing gained from making the installation have that step imo.

On Tue, Oct 9, 2018, 11:08 PM Oliver S̶c̶h̶n̶e̶i̶d̶e̶r Scherer < notifications@github.com> wrote:

As a first step we can remove cargo-clippy from rustup and only distribute clippy-driver. Then users can install the cargo clippy subcommand themselves until we figure out the details here.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rust-lang-nursery/rust-clippy/issues/1358#issuecomment-428449395, or mute the thread https://github.com/notifications/unsubscribe-auth/ABivSF03LmQxccvVIwVi-5u0SEIPlEJ9ks5ujY7GgaJpZM4K7C6B .

[camsteffen]

I don't think there are any unresolved questions here. Clippy is distributed and versioned with the rust toolchain.