OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #5750 - ebroto:blanket_clippy_restriction_lints, r=Manishearth,flip1995,phansch,oli-obk

Browse Source 
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.
This commit is contained in:
bors 2020-06-30 21:55:46 +00:00
commit d05d6abf89
6 changed files with 113 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -1352,6 +1352,7 @@ Released 2018-09-13
[`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
[`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
[`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
[`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

95
clippy_lints/src/attrs.rs
View File

@ -2,8 +2,8 @@
use crate::reexport::Name;
use crate::utils::{
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, without_block_comments,
};
use if_chain::if_chain;
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
@ -17,7 +17,7 @@ use rustc_middle::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use rustc_span::symbol::{Symbol, SymbolStr};
use semver::Version;
static UNIX_SYSTEMS: &[&str] = &[
@ -182,6 +182,29 @@ declare_clippy_lint! {
"unknown_lints for scoped Clippy lints"
}
declare_clippy_lint! {
/// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
///
/// **Why is this bad?** Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust.
/// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// #![deny(clippy::restriction)]
/// ```
///
/// Good:
/// ```rust
/// #![deny(clippy::as_conversions)]
/// ```
pub BLANKET_CLIPPY_RESTRICTION_LINTS,
style,
"enabling the complete restriction group"
}
declare_clippy_lint! {
/// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
/// with `#[rustfmt::skip]`.
@ -249,15 +272,17 @@ declare_lint_pass!(Attributes => [
DEPRECATED_SEMVER,
USELESS_ATTRIBUTE,
UNKNOWN_CLIPPY_LINTS,
BLANKET_CLIPPY_RESTRICTION_LINTS,
]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
if let Some(items) = &attr.meta_item_list() {
if let Some(ident) = attr.ident() {
match &*ident.as_str() {
let ident = &*ident.as_str();
match ident {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
check_clippy_lint_names(cx, ident, items);
},
_ => {},
}
@ -363,38 +388,43 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
}
}
#[allow(clippy::single_match_else)]
fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
let lint_store = cx.lints();
for lint in items {
fn check_clippy_lint_names(cx: &LateContext<'_, '_>, ident: &str, items: &[NestedMetaItem]) {
fn extract_name(lint: &NestedMetaItem) -> Option<SymbolStr> {
if_chain! {
if let Some(meta_item) = lint.meta_item();
if meta_item.path.segments.len() > 1;
if let tool_name = meta_item.path.segments[0].ident;
if tool_name.as_str() == "clippy";
let name = meta_item.path.segments.last().unwrap().ident.name;
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
&name.as_str(),
Some(tool_name.name),
);
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
then {
return Some(lint_name.as_str());
}
}
None
}
let lint_store = cx.lints();
for lint in items {
if let Some(lint_name) = extract_name(lint) {
if let CheckLintNameResult::Tool(Err((None, _))) =
lint_store.check_lint_name(&lint_name, Some(sym!(clippy)))
{
span_lint_and_then(
cx,
UNKNOWN_CLIPPY_LINTS,
lint.span(),
&format!("unknown clippy lint: clippy::{}", name),
&format!("unknown clippy lint: clippy::{}", lint_name),
|diag| {
let name_lower = name.as_str().to_lowercase();
let symbols = lint_store.get_lints().iter().map(
|l| Symbol::intern(&l.name_lower())
).collect::<Vec<_>>();
let sugg = find_best_match_for_name(
symbols.iter(),
&format!("clippy::{}", name_lower),
None,
);
if name.as_str().chars().any(char::is_uppercase)
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok() {
let name_lower = lint_name.to_lowercase();
let symbols = lint_store
.get_lints()
.iter()
.map(|l| Symbol::intern(&l.name_lower()))
.collect::<Vec<_>>();
let sugg = find_best_match_for_name(symbols.iter(), &format!("clippy::{}", name_lower), None);
if lint_name.chars().any(char::is_uppercase)
&& lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
{
diag.span_suggestion(
lint.span(),
"lowercase the lint name",
@ -409,10 +439,19 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
Applicability::MachineApplicable,
);
}
}
},
);
} else if lint_name == "restriction" && ident != "allow" {
span_lint_and_help(
cx,
BLANKET_CLIPPY_RESTRICTION_LINTS,
lint.span(),
"restriction lints are not meant to be all enabled",
None,
"try enabling only the lints you really need",
);
}
};
}
}
}

3
clippy_lints/src/lib.rs
View File

@ -474,6 +474,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&assign_ops::ASSIGN_OP_PATTERN,
&assign_ops::MISREFACTORED_ASSIGN_OP,
&atomic_ordering::INVALID_ATOMIC_ORDERING,
&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS,
&attrs::DEPRECATED_CFG_ATTR,
&attrs::DEPRECATED_SEMVER,
&attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
@ -1189,6 +1190,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
LintId::of(&attrs::DEPRECATED_SEMVER),
LintId::of(&attrs::MISMATCHED_TARGET_OS),
@ -1441,6 +1443,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_group(true, "clippy::style", Some("clippy_style"), vec![
LintId::of(&assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
LintId::of(&assign_ops::ASSIGN_OP_PATTERN),
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
LintId::of(&blacklisted_name::BLACKLISTED_NAME),

7
src/lintlist/mod.rs
View File

@ -80,6 +80,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "blacklisted_name",
},
Lint {
name: "blanket_clippy_restriction_lints",
group: "style",
desc: "enabling the complete restriction group",
deprecation: None,
module: "attrs",
},
Lint {
name: "blocks_in_if_conditions",
group: "style",

6
tests/ui/attrs.rs
View File

@ -1,5 +1,11 @@
#![warn(clippy::inline_always, clippy::deprecated_semver)]
#![allow(clippy::assertions_on_constants)]
// Test that the whole restriction group is not enabled
#![warn(clippy::restriction)]
#![deny(clippy::restriction)]
#![forbid(clippy::restriction)]
#![allow(clippy::missing_docs_in_private_items, clippy::panic, clippy::unreachable)]
#[inline(always)]
fn test_attr_lint() {
assert!(true)

33
tests/ui/attrs.stderr
View File

@ -1,5 +1,5 @@
error: you have declared `#[inline(always)]` on `test_attr_lint`. This is usually a bad idea
--> $DIR/attrs.rs:3:1
--> $DIR/attrs.rs:9:1
|
LL | #[inline(always)]
| ^^^^^^^^^^^^^^^^^
@ -7,7 +7,7 @@ LL | #[inline(always)]
= note: `-D clippy::inline-always` implied by `-D warnings`
error: the since field must contain a semver-compliant version
--> $DIR/attrs.rs:23:14
--> $DIR/attrs.rs:29:14
|
LL | #[deprecated(since = "forever")]
| ^^^^^^^^^^^^^^^^^
@ -15,10 +15,35 @@ LL | #[deprecated(since = "forever")]
= note: `-D clippy::deprecated-semver` implied by `-D warnings`
error: the since field must contain a semver-compliant version
--> $DIR/attrs.rs:26:14
--> $DIR/attrs.rs:32:14
|
LL | #[deprecated(since = "1")]
| ^^^^^^^^^^^
error: aborting due to 3 previous errors
error: restriction lints are not meant to be all enabled
--> $DIR/attrs.rs:4:9
|
LL | #![warn(clippy::restriction)]
| ^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::blanket-clippy-restriction-lints` implied by `-D warnings`
= help: try enabling only the lints you really need
error: restriction lints are not meant to be all enabled
--> $DIR/attrs.rs:5:9
|
LL | #![deny(clippy::restriction)]
| ^^^^^^^^^^^^^^^^^^^
|
= help: try enabling only the lints you really need
error: restriction lints are not meant to be all enabled
--> $DIR/attrs.rs:6:11
|
LL | #![forbid(clippy::restriction)]
| ^^^^^^^^^^^^^^^^^^^
|
= help: try enabling only the lints you really need
error: aborting due to 6 previous errors