Rust-clippy: `clippy_restriction` and `clippy_deprecated` not listed in README

[sanmai-NL]

These categories aren’t listed in the current README. Also, I found that clippy_pedantic does not include clippy_restriction. The README asserts clippy_pedantic covers ‘everything’.

[mikerite]

See #2897 for related discussion.

[mikerite]

You should only enable specific lints in restriction that meet your particular situation or preferences. Enabling the whole group makes no sense as some lints in restriction contradict other lints. Also many of them promote code that is plain unidiomatic. Basically the restriction group is opt-in on per-lint basis IMO.

[sanmai-NL]

@mikerite, I see what you’re trying to say. I do not fully agree with your ideas though. First of all, whatever the reasoning to not include clippy_restriction in clippy_pedantic is, it should be explained in the documentation not in a comment. That’s what I’m asking for and that’s what I’m asking for again in the PR thread you kindly linked to.

As for contradictions, there’s only one mentioned in the PR discussion and that isn’t a contradiction, just one suggestion that could be conflicting with another lint’s. Does clippy_pedantic imply code that is idiomatic?

[mikerite]

I agree with you that those categories should be documented in the README. I was just explaining why the restriction lints are not included in pedantic.

Does clippy_pedantic imply code that is idiomatic?

I'm not sure about that. I know some the lints involving shadowing where moved from pedantic to restriction because some shadowing is considered idiomatic - see #2322.

[sanmai-NL]

Just from the common meaning of the word, ‘pedantic’ signals the highest possible level of strictness, beyond concerns such as idiomaticness. Now of course there’s some historical influence from e.g. gcc with its -Wpedantic and -Weverything.

[ahmedcharles]

Just going to point out that pragmatically, I just wasted 30 minutes trying to figure out why a lint wasn't triggering even though I did #![deny(clippy::all)] and continued to not trigger after #![deny(clippy::pedantic)] was added. That's when I found this issue. Adding one line to the README doesn't seem like an undue burden.

Philosophically, using the word all to mean anything other than all, is beyond incomprehensible. Using clippy::recommended would be significantly more understandable, whether or not clippy::all exists or not. But if it exists, it should do what it says and enable all lints.

[Manishearth]

You should _never_ be turning on all restriction lints, or even some of
them really, they're for very tightly scoped use only. It's easier to
consider these to not be lints in the usual sense, they don't actually tell
you things for improving your code, they're additional checks that may be
useful for specific situations.

The deprecated lints do nothing anyway, they exist so that people don't get
unknown lint warnings.

Nursery lints aren't ready yet.

Pedantic lints _could_ be included, but they're really annoying when taken
as a group and usually should be picked individually. C++ has precedent for
this with -Wall -Wpedantic

clippy::all is "all lints that are on by default".

Changing clippy::all to include nursery and restriction lints is a
non-starter. Making it include pedantic lints is something that could be
discussed, but I'm leaning on the side of not having that, especially for
the sake of non-power-users.

We should definitely document this better.

(We can also deprecate it in favor of clippy::recommended but that has more
discoverability issues)

On Fri, Feb 22, 2019, 11:54 AM Ahmed Charles [e-mail]
wrote:

Just going to point out that pragmatically, I just wasted 30 minutes
trying to figure out why a lint wasn't triggering even though I did

![deny(clippy::all)] and continued to not trigger after

![deny(clippy::pedantic)] was added. That's when I found this issue.

Adding one line to the README doesn't seem like an undue burden.

Philosophically, using the word all to mean anything other than all, is
beyond incomprehensible. Using clippy::recommended would be significantly
more understandable, whether or not clippy::all exists or not. But if it
exists, it should do what it says and enable all lints.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust-clippy/issues/2964#issuecomment-466288174,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABivSOmD_lxMch2gLgNTuLUfP3T_xkIQks5vP40ygaJpZM4VhcUC
.

[Manishearth]

https://github.com/rust-lang/rust-clippy/pull/3796

[ahmedcharles]

Two completely separate/orthogonal things:

1. The scenario under which I want to use all is not quite in production or for 'important' code. It's for when I'm learning about what lints exist and I want to play around with new things and get a feel for how often various lints trigger and whether they are worth it. The argument that it's not intended for production use in widely developed codebases is equally a non-starter (to me).
2. The definition of all is clear in the English language and I'm not sure why I'd have to explain why using it incorrectly is bad. Not using the word all to describe the opposite of all is a perfectly fine thing to do. There are many words that describe the notion of a non-exhaustive list of lints that an average user should turn on, just pick one.

As for the C++ precedent, I reject that notion. MSVC has -Wall and it actually means what normal English speakers would expect. GCC's reluctance to break people who used it incorrectly (and clang's bug-for-bug compatibility with GCC in this regard), shouldn't be taken as reasonable precedent. Many of Rust's design choices deviate from C++, for good reason.

[ahmedcharles]

As somewhat of an aside, in all of my projects, I explicitly enable every lint in rustc (not clippy). There isn't a way to turn on all lints as a group. So, instead, every time I run rustup update I run rustc -W help and manually compare all the lints to see if there are new ones. After I do this, I go through and fix the code to work with the new lints and then bump the version of the compiler in CI. There's no chance of CI breaking due to using a notional all category. And I'm already doing all the work to fix the warnings, but for some reason, I also have to do the work of comparing warnings manually rather than just adding a single line in my crate root.

[Manishearth]

The definition of all is clear in the English language and I'm not sure why I'd have to explain why using it incorrectly is bad

Actually, no, this isn't incorrect, "all" is always "all of _some_ category", if I say "give me all of them" when buying apples, I'm asking for all the apples in front of me, I'm not asking for all the apples in the universe. Language is nuanced.

The mental model we wish to present is: Clippy is a bunch of lints. They are on by default, and you can tweak this with clippy::all. Furthermore, there are some _extra_ lints that you can include if you wish.

You're right that this is a bit confusing, and in retrospect we should have chosen something else that was less ambiguous. It's much harder to change now.

We did have a bit of a discussion about this when picking the name and all was really the best we could think of, we want something that is discoverable (recommended is weird and sounds like yet another lint group rather than a metagroup, all_default is awkward) and useful (including pedantic in all is not useful for most users of clippy)

I want to play around with new things and get a feel for how often various lints trigger and whether they are worth it

This is not a first-class use case of clippy, however. This use case is still supported -- use the lint groups provided -- however I'm not going to support changes that favor this use case at the expense of the main use case. I have already opened a pull request to clarify the documentation that would have solved your initial issue

[Manishearth]

If we can come up with a good, discoverable name for clippy::all we could have a deprecation process moving it over. We'd still have to support it meaning the same thing, however we can stop documenting it and warn when people use it that way.

[sanmai-NL]

A more correct alternative for 'all' would be 'most'. It's very simple.

[Manishearth]

It's not, that's not something very discoverable, nor does it convey useful information. It is more correct, but it isn't more _useful_.

[ahmedcharles]

I'm not sure how much the discoverability of all actually matters. If you're using clippy in a scenario that requires discovering all rather than foo (whatever alias is picked to mean the same thing) suggests that somehow you discovered clippy but not any documentation. Simply running cargo clippy --help should/could say that the primary use cases are the default without an explicit annotation and using #![warn(clippy::recommended)]. The vast majority of clippy users will first use it on an existing project and can therefore see the annotation used. I think the emphasis on discoverability is misplaced when it effects so few people and when correctness about the interpretation of the word all is something that impacts every new user who sees clippy::all anywhere.

Note, the category of all in clippy::all is clippy, so your aside about all apples not containing any oranges or whatever doesn't seem to make sense, especially since the category is explicitly written before the word.

[sanmai-NL]

@Manishearth: whether 'all' is more discoverable than 'most' is of little importance, as @ahmedcharles just explained. Simply put, nobody is going to want to use Clippy without reading any of the very brief documentation in the README. Indeed the README acknowledges this by giving very specific instructions about how to configure Clippy. Clippy isn't an autodiscoverable tool and learning how to get started with it takes just 5 minutes, or at least it should. 'All' conveys as much useful information as 'most', with the difference that the former is plainly wrong and the second isn't. Every programmer needs to read in the README what 'all' means just as he/she would have to for 'most'.

Using wrong names is misleading. Many software projects _do_ want to catch any and all potential issues and add exceptions locally, the value judgments on this issue tracker I've seen about e.g. clippy::correctness aren't based on broad industry consultation for sure.

[Manishearth]

I'm not sure how much the discoverability of all actually matters

I'm using "discoverability" in a broader sense here. The ability to:

• understand what it does when seeing it in the wild
• remember it and be able to reuse it

"most" and "recommended" don't satisfy either. "all" doesn't quite satisfy the first but I'm okay with it for reasons already stated

when correctness about the interpretation of the word all is something that impacts every new user who sees clippy::all anywhere.

You're correct, except it impacts them _in the way we want it to_, The "all" lints _are_ the ones we want people to think of as the main clippy lints; these are all the lints we properly _support_. The rest of the lints are less supported for various reasons; e.g. the pedantic lints are too pedantic to be something we recommend, and similarly the nursery lints are too new and broken. Power users can go and enable them if they want, but this isn't a normal feature.

The number one use case of clippy::all is #![deny(clippy::all)], and in this context we're careful to only bundle in the things we consider make sense to do this with; i.e. the things we actually consider to be good lints.

You're correct in noting that there's a semantic discrepancy here, however it's semi-intentional; and usability trumps semantic correctness for us.

Simply put, nobody is going to want to use Clippy without reading any of the very brief documentation in the README.

Plenty of folks will learn to use it from looking at other code.

And this argument is equally valid for people who want to use the extra lints in clippy, the readme clearly states what's going on there. It didn't before, which is why this issue exists, but I clarified that.

[ahmedcharles]

Turning every instance of #![deny(clippy::foo)] for invalid foo into an error that says to use #![deny(clippy::recommended)] solves almost all of your issues. That just means that someone needs to remember that attempting to use clippy incorrectly will tell them how to fix it.

So, the position that all is better than recommended doesn't seem to hold. Every issue with it can be fixed by an overabundance of reminding people to use recommended, whether it's the README, clippy --help or using clippy::foo.

[Manishearth]

It does not, recommended is not something people will easily remember. (If all is an error/warning to use, you will see it less in the wild so it won't work as a "springboard" into recommended either)

Overall the benefit of semantic correctness here is slight, and all gives off the impression we want it to, even if that impression is a technically incorrect characterization.

[mikerite]

all might make sense for people coming from the C++ but people coming from Javascript/Typescript will expect that group to be called recommended. Both ESLint and TSLint use that naming.

Otherwise, maybe we can call it clippy::default, since it's "all lints that are on by _default_".

[flip1995]

Since I was the one who pushed the all category through, here are my two cents on the topic:

First of here is the short discussion about the naming: https://github.com/rust-lang/rust/issues/44690#issuecomment-398547743

@Manishearth suggested to name it clippy::warn. I decided against that, since this is also misleading, i.e., from #[allow(clippy::warn)] I would expect to only allow lints, that are warn-by-default, but we wanted a lint-group name for all on-by-default (including deny-by-default) lints.

That's why I did go for clippy::all, because I wanted something short, memorable and something that most accurately expresses the behavior of the group. Back then (commit) I defined the group all as the group of all lints, except some specifically excluded lint groups.

The thought behind that was, that new users (that don't really want to read the documentation) can just write #![warn(clippy::all)] and get all the lints, that we're most confident in, that they work without errors and not to bother them with opt-in (pedantic), hardly useful (restriction) or broken (nursery) lints. Users that want to fiddle more with Clippy could read the documentation/README and enable more lints separately.

That being said... In retrospect I would go with @mikerite and agree that clippy::default might have been the better name.

[sanmai-NL]

@flip1995:
So it seems to me your and @Manishearth decision making in this was based on certain ideas about what lints are useful/relevant/working reliably. Note again that if you factor out these views from your decisions, the choice of 'all' is misleading. What you guys may perceive as being 'pedantic' or 'hardly useful' is simply a hard requirement in some organizations. Names being three-letter words and not expecting programmers to read documentation may not be applicable to those organizations at all.

[Manishearth]

What you guys may perceive as being 'pedantic' or 'hardly useful' is simply a hard requirement in some organizations

This isn't a matter of perception, but a matter of what interface we choose to expose first and foremost, and what interface we wish to signal support for. We totally understand that some people want all the pedantic lints too, but that is not the default interface we expose. We still expose everything they need.