Lint enabling the whole restriction group #5750
Conversation
|
r? @flip1995 (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
clippy_lints/src/attrs.rs
Outdated
| /// #![deny(clippy::as_conversions)] | ||
| /// ``` | ||
| pub BLANKET_CLIPPY_RESTRICTION_LINTS, | ||
| correctness, |
There was a problem hiding this comment.
correctness is the wrong group, since enabling those lints doesn't have any influence on code semantics and doesn't change behavior.
I think style is the best group here, since "idomatic Clippy style" is to not enable the complete restriction group.
There was a problem hiding this comment.
Correctness lints are for "code that is just outright wrong or very very useless" so this fits IMO.
There was a problem hiding this comment.
I guess there are different ways to understand this, probably all of them valid.
My take was that this is an incorrect usage of clippy, a footgun that can't be easily prevented due to restriction being a category like the others.
TBH I would be happy just with this being linted, as it's most of the time an error that is not hard to make (just requires not reading the README thoroughly).
There was a problem hiding this comment.
I'm okay with correctness or style. No strong preference. This is an incorrect use of clippy, but erroring about it might be harsh.
There was a problem hiding this comment.
Yeah, I guess this comes down to everyone's interpretation of 'code that is outright wrong or very very useless' from the correctness lint group description in the README.
Going from that description, it would certainly fit correctness. Still, I would tend to agree with @flip1995 here. IMO correctness is meant to warn about actual bugs in the code and nothing further. I don't think an incorrect Clippy usage is a bug in that sense.
There was a problem hiding this comment.
IMO
correctnessis meant to warn about actual bugs in the code and nothing further. I don't think an incorrect Clippy usage is a bug in that sense.
I agree with this
flip1995
added
the
S-needs-discussion
|
I agree with adding this lint |
|
I moved the lint to |
|
@bors r=Manishearth,flip1995,phansch,oli-obk |
|
📌 Commit 3b83040 has been approved by |
…shearth,flip1995,phansch,oli-obk Lint enabling the whole restriction group I've added it to the `correctness` category, but I may be missing some valid use cases. In that case it could be changed to `pedantic`. changelog: Add [`blanket_clippy_restriction_lints`] to check against enabling the whole restriction group.
|
💔 Test failed - checks-action_test |
flip1995
added
S-waiting-on-bors
ebroto
force-pushed
the
blanket_clippy_restriction_lints
branch
from
3b83040
to
c5d8f53
Compare
|
@bors r+ |
|
📌 Commit c5d8f53 has been approved by |
…1995 Lint enabling the whole restriction group I've added it to the `correctness` category, but I may be missing some valid use cases. In that case it could be changed to `pedantic`. changelog: Add [`blanket_clippy_restriction_lints`] to check against enabling the whole restriction group.
|
💥 Test timed out |
|
@bors r=Manishearth,flip1995,phansch,oli-obk |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit c5d8f53 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
I've added it to the
correctnesscategory, but I may be missing some valid use cases. In that case it could be changed topedantic.changelog: Add [
blanket_clippy_restriction_lints] to check against enabling the whole restriction group.