Skip to content

Commit d05d6ab

Browse files
borsbors
committed
Auto merge of rust-lang#5750 - ebroto:blanket_clippy_restriction_lints, r=Manishearth,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.
2 parents ccf7cb3 + c5d8f53 commit d05d6ab

File tree

6 files changed

+113
-32
lines changed

6 files changed

+113
-32
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1352,6 +1352,7 @@ Released 2018-09-13
13521352
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
13531353
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
13541354
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
1355+
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
13551356
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
13561357
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
13571358
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

‎clippy_lints/src/attrs.rs

+67-28
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
33
use crate::reexport::Name;
44
use crate::utils::{
5-
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
6-
span_lint_and_then, without_block_comments,
5+
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
6+
span_lint_and_sugg, span_lint_and_then, without_block_comments,
77
};
88
use if_chain::if_chain;
99
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
@@ -17,7 +17,7 @@ use rustc_middle::lint::in_external_macro;
1717
use rustc_middle::ty;
1818
use rustc_session::{declare_lint_pass, declare_tool_lint};
1919
use rustc_span::source_map::Span;
20-
use rustc_span::symbol::Symbol;
20+
use rustc_span::symbol::{Symbol, SymbolStr};
2121
use semver::Version;
2222

2323
static UNIX_SYSTEMS: &[&str] = &[
@@ -182,6 +182,29 @@ declare_clippy_lint! {
182182
"unknown_lints for scoped Clippy lints"
183183
}
184184

185+
declare_clippy_lint! {
186+
/// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
187+
///
188+
/// **Why is this bad?** Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust.
189+
/// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
190+
///
191+
/// **Known problems:** None.
192+
///
193+
/// **Example:**
194+
/// Bad:
195+
/// ```rust
196+
/// #![deny(clippy::restriction)]
197+
/// ```
198+
///
199+
/// Good:
200+
/// ```rust
201+
/// #![deny(clippy::as_conversions)]
202+
/// ```
203+
pub BLANKET_CLIPPY_RESTRICTION_LINTS,
204+
style,
205+
"enabling the complete restriction group"
206+
}
207+
185208
declare_clippy_lint! {
186209
/// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
187210
/// with `#[rustfmt::skip]`.
@@ -249,15 +272,17 @@ declare_lint_pass!(Attributes => [
249272
DEPRECATED_SEMVER,
250273
USELESS_ATTRIBUTE,
251274
UNKNOWN_CLIPPY_LINTS,
275+
BLANKET_CLIPPY_RESTRICTION_LINTS,
252276
]);
253277

254278
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
255279
fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
256280
if let Some(items) = &attr.meta_item_list() {
257281
if let Some(ident) = attr.ident() {
258-
match &*ident.as_str() {
282+
let ident = &*ident.as_str();
283+
match ident {
259284
"allow" | "warn" | "deny" | "forbid" => {
260-
check_clippy_lint_names(cx, items);
285+
check_clippy_lint_names(cx, ident, items);
261286
},
262287
_ => {},
263288
}
@@ -363,38 +388,43 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
363388
}
364389
}
365390

366-
#[allow(clippy::single_match_else)]
367-
fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
368-
let lint_store = cx.lints();
369-
for lint in items {
391+
fn check_clippy_lint_names(cx: &LateContext<'_, '_>, ident: &str, items: &[NestedMetaItem]) {
392+
fn extract_name(lint: &NestedMetaItem) -> Option<SymbolStr> {
370393
if_chain! {
371394
if let Some(meta_item) = lint.meta_item();
372395
if meta_item.path.segments.len() > 1;
373396
if let tool_name = meta_item.path.segments[0].ident;
374397
if tool_name.as_str() == "clippy";
375-
let name = meta_item.path.segments.last().unwrap().ident.name;
376-
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
377-
&name.as_str(),
378-
Some(tool_name.name),
379-
);
398+
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
380399
then {
400+
return Some(lint_name.as_str());
401+
}
402+
}
403+
None
404+
}
405+
406+
let lint_store = cx.lints();
407+
for lint in items {
408+
if let Some(lint_name) = extract_name(lint) {
409+
if let CheckLintNameResult::Tool(Err((None, _))) =
410+
lint_store.check_lint_name(&lint_name, Some(sym!(clippy)))
411+
{
381412
span_lint_and_then(
382413
cx,
383414
UNKNOWN_CLIPPY_LINTS,
384415
lint.span(),
385-
&format!("unknown clippy lint: clippy::{}", name),
416+
&format!("unknown clippy lint: clippy::{}", lint_name),
386417
|diag| {
387-
let name_lower = name.as_str().to_lowercase();
388-
let symbols = lint_store.get_lints().iter().map(
389-
|l| Symbol::intern(&l.name_lower())
390-
).collect::<Vec<_>>();
391-
let sugg = find_best_match_for_name(
392-
symbols.iter(),
393-
&format!("clippy::{}", name_lower),
394-
None,
395-
);
396-
if name.as_str().chars().any(char::is_uppercase)
397-
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok() {
418+
let name_lower = lint_name.to_lowercase();
419+
let symbols = lint_store
420+
.get_lints()
421+
.iter()
422+
.map(|l| Symbol::intern(&l.name_lower()))
423+
.collect::<Vec<_>>();
424+
let sugg = find_best_match_for_name(symbols.iter(), &format!("clippy::{}", name_lower), None);
425+
if lint_name.chars().any(char::is_uppercase)
426+
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
427+
{
398428
diag.span_suggestion(
399429
lint.span(),
400430
"lowercase the lint name",
@@ -409,10 +439,19 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
409439
Applicability::MachineApplicable,
410440
);
411441
}
412-
}
442+
},
443+
);
444+
} else if lint_name == "restriction" && ident != "allow" {
445+
span_lint_and_help(
446+
cx,
447+
BLANKET_CLIPPY_RESTRICTION_LINTS,
448+
lint.span(),
449+
"restriction lints are not meant to be all enabled",
450+
None,
451+
"try enabling only the lints you really need",
413452
);
414453
}
415-
};
454+
}
416455
}
417456
}
418457

‎clippy_lints/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
474474
&assign_ops::ASSIGN_OP_PATTERN,
475475
&assign_ops::MISREFACTORED_ASSIGN_OP,
476476
&atomic_ordering::INVALID_ATOMIC_ORDERING,
477+
&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS,
477478
&attrs::DEPRECATED_CFG_ATTR,
478479
&attrs::DEPRECATED_SEMVER,
479480
&attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
@@ -1189,6 +1190,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11891190
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
11901191
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
11911192
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
1193+
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
11921194
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
11931195
LintId::of(&attrs::DEPRECATED_SEMVER),
11941196
LintId::of(&attrs::MISMATCHED_TARGET_OS),
@@ -1441,6 +1443,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14411443
store.register_group(true, "clippy::style", Some("clippy_style"), vec![
14421444
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
14431445
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
1446+
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
14441447
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
14451448
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
14461449
LintId::of(&blacklisted_name::BLACKLISTED_NAME),

‎src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
8080
deprecation: None,
8181
module: "blacklisted_name",
8282
},
83+
Lint {
84+
name: "blanket_clippy_restriction_lints",
85+
group: "style",
86+
desc: "enabling the complete restriction group",
87+
deprecation: None,
88+
module: "attrs",
89+
},
8390
Lint {
8491
name: "blocks_in_if_conditions",
8592
group: "style",

‎tests/ui/attrs.rs

+6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
#![warn(clippy::inline_always, clippy::deprecated_semver)]
22
#![allow(clippy::assertions_on_constants)]
3+
// Test that the whole restriction group is not enabled
4+
#![warn(clippy::restriction)]
5+
#![deny(clippy::restriction)]
6+
#![forbid(clippy::restriction)]
7+
#![allow(clippy::missing_docs_in_private_items, clippy::panic, clippy::unreachable)]
8+
39
#[inline(always)]
410
fn test_attr_lint() {
511
assert!(true)

‎tests/ui/attrs.stderr

+29-4
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,49 @@
11
error: you have declared `#[inline(always)]` on `test_attr_lint`. This is usually a bad idea
2-
--> $DIR/attrs.rs:3:1
2+
--> $DIR/attrs.rs:9:1
33
|
44
LL | #[inline(always)]
55
| ^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::inline-always` implied by `-D warnings`
88

99
error: the since field must contain a semver-compliant version
10-
--> $DIR/attrs.rs:23:14
10+
--> $DIR/attrs.rs:29:14
1111
|
1212
LL | #[deprecated(since = "forever")]
1313
| ^^^^^^^^^^^^^^^^^
1414
|
1515
= note: `-D clippy::deprecated-semver` implied by `-D warnings`
1616

1717
error: the since field must contain a semver-compliant version
18-
--> $DIR/attrs.rs:26:14
18+
--> $DIR/attrs.rs:32:14
1919
|
2020
LL | #[deprecated(since = "1")]
2121
| ^^^^^^^^^^^
2222

23-
error: aborting due to 3 previous errors
23+
error: restriction lints are not meant to be all enabled
24+
--> $DIR/attrs.rs:4:9
25+
|
26+
LL | #![warn(clippy::restriction)]
27+
| ^^^^^^^^^^^^^^^^^^^
28+
|
29+
= note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings`
30+
= help: try enabling only the lints you really need
31+
32+
error: restriction lints are not meant to be all enabled
33+
--> $DIR/attrs.rs:5:9
34+
|
35+
LL | #![deny(clippy::restriction)]
36+
| ^^^^^^^^^^^^^^^^^^^
37+
|
38+
= help: try enabling only the lints you really need
39+
40+
error: restriction lints are not meant to be all enabled
41+
--> $DIR/attrs.rs:6:11
42+
|
43+
LL | #![forbid(clippy::restriction)]
44+
| ^^^^^^^^^^^^^^^^^^^
45+
|
46+
= help: try enabling only the lints you really need
47+
48+
error: aborting due to 6 previous errors
2449

0 commit comments

Comments
 (0)