Add lint to detect `allow` attributes without reason

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b52a6fcd05e979540ef2b559058779a8df04fef..bcdbc73738a97265e98688849aae7d925cbda60d 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3042,6 +3042,7 @@ Released 2018-09-13
 <!-- lint disable no-unused-definitions -->
 <!-- begin autogenerated links to lint list -->
 [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
+[`allow_attributes_without_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes_without_reason
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
 [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions

diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
index a58d12ddd6b4372278e4569126f2233d81183e28..d0b03c0a9c276e644e89c6bfa22499cfdce7e052 100644 (file)
--- a/clippy_lints/src/attrs.rs
+++ b/clippy_lints/src/attrs.rs
@@ -255,7 +255,38 @@
     "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for attributes that allow lints without a reason.
+    ///
+    /// (This requires the `lint_reasons` feature)
+    ///
+    /// ### Why is this bad?
+    /// Allowing a lint should always have a reason. This reason should be documented to
+    /// ensure that others understand the reasoning
+    ///
+    /// ### Example
+    /// Bad:
+    /// ```rust
+    /// #![feature(lint_reasons)]
+    ///
+    /// #![allow(clippy::some_lint)]
+    /// ```
+    ///
+    /// Good:
+    /// ```rust
+    /// #![feature(lint_reasons)]
+    ///
+    /// #![allow(clippy::some_lint, reason = "False positive rust-lang/rust-clippy#1002020")]
+    /// ```
+    #[clippy::version = "1.61.0"]
+    pub ALLOW_ATTRIBUTES_WITHOUT_REASON,
+    restriction,
+    "ensures that all `allow` and `expect` attributes have a reason"
+}
+
 declare_lint_pass!(Attributes => [
+    ALLOW_ATTRIBUTES_WITHOUT_REASON,
     INLINE_ALWAYS,
     DEPRECATED_SEMVER,
     USELESS_ATTRIBUTE,
@@ -269,6 +300,9 @@ fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) {
                 if is_lint_level(ident.name) {
                     check_clippy_lint_names(cx, ident.name, items);
                 }
+                if matches!(ident.name, sym::allow | sym::expect) {
+                    check_lint_reason(cx, ident.name, items, attr);
+                }
                 if items.is_empty() || !attr.has_name(sym::deprecated) {
                     return;
                 }
@@ -404,6 +438,30 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMe
     }
 }
 
+fn check_lint_reason(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem], attr: &'_ Attribute) {
+    // Check for the feature
+    if !cx.tcx.sess.features_untracked().lint_reasons {
+        return;
+    }
+
+    // Check if the reason is present
+    if let Some(item) = items.last().and_then(NestedMetaItem::meta_item)
+        && let MetaItemKind::NameValue(_) = &item.kind
+        && item.path == sym::reason
+    {
+        return;
+    }
+
+    span_lint_and_help(
+        cx,
+        ALLOW_ATTRIBUTES_WITHOUT_REASON,
+        attr.span,
+        &format!("`{}` attribute without specifying a reason", name.as_str()),
+        None,
+        "try adding a reason at the end with `, reason = \"..\"`",
+    );
+}
+
 fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
     if let ItemKind::Fn(_, _, eid) = item.kind {
         is_relevant_expr(cx, cx.tcx.typeck_body(eid), &cx.tcx.hir().body(eid).value)
@@ -659,5 +717,5 @@ fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> {
 }
 
 fn is_lint_level(symbol: Symbol) -> bool {
-    matches!(symbol, sym::allow | sym::warn | sym::deny | sym::forbid)
+    matches!(symbol, sym::allow | sym::expect | sym::warn | sym::deny | sym::forbid)
 }

diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 75ef1b0a9d511125944d69610376da6e0ac5dad1..d4c7eba4cba11ee7bfb23b1554e58fd01caf52fc 100644 (file)
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -42,6 +42,7 @@
     assign_ops::ASSIGN_OP_PATTERN,
     assign_ops::MISREFACTORED_ASSIGN_OP,
     async_yields_async::ASYNC_YIELDS_ASYNC,
+    attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON,
     attrs::BLANKET_CLIPPY_RESTRICTION_LINTS,
     attrs::DEPRECATED_CFG_ATTR,
     attrs::DEPRECATED_SEMVER,

diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs
index f89f35b885c15a377c772e103254c0b0884aeebf..4e30fc381975135ba3c678078173390dcb9e95a9 100644 (file)
--- a/clippy_lints/src/lib.register_restriction.rs
+++ b/clippy_lints/src/lib.register_restriction.rs
@@ -8,6 +8,7 @@
     LintId::of(as_conversions::AS_CONVERSIONS),
     LintId::of(asm_syntax::INLINE_ASM_X86_ATT_SYNTAX),
     LintId::of(asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX),
+    LintId::of(attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON),
     LintId::of(casts::FN_TO_NUMERIC_CAST_ANY),
     LintId::of(create_dir::CREATE_DIR),
     LintId::of(dbg_macro::DBG_MACRO),

diff --git a/tests/ui/allow_attributes_without_reason.rs b/tests/ui/allow_attributes_without_reason.rs
new file mode 100644 (file)
index 0000000..1a0d4e8
--- /dev/null
+++ b/tests/ui/allow_attributes_without_reason.rs
@@ -0,0 +1,14 @@
+#![feature(lint_reasons)]
+#![deny(clippy::allow_attributes_without_reason)]
+
+// These should trigger the lint
+#[allow(dead_code)]
+#[allow(dead_code, deprecated)]
+// These should be fine
+#[allow(dead_code, reason = "This should be allowed")]
+#[warn(dyn_drop, reason = "Warnings can also have reasons")]
+#[warn(deref_nullptr)]
+#[deny(deref_nullptr)]
+#[forbid(deref_nullptr)]
+
+fn main() {}

diff --git a/tests/ui/allow_attributes_without_reason.stderr b/tests/ui/allow_attributes_without_reason.stderr
new file mode 100644 (file)
index 0000000..cd040a1
--- /dev/null
+++ b/tests/ui/allow_attributes_without_reason.stderr
@@ -0,0 +1,23 @@
+error: `allow` attribute without specifying a reason
+  --> $DIR/allow_attributes_without_reason.rs:5:1
+   |
+LL | #[allow(dead_code)]
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/allow_attributes_without_reason.rs:2:9
+   |
+LL | #![deny(clippy::allow_attributes_without_reason)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: try adding a reason at the end with `, reason = ".."`
+
+error: `allow` attribute without specifying a reason
+  --> $DIR/allow_attributes_without_reason.rs:6:1
+   |
+LL | #[allow(dead_code, deprecated)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: try adding a reason at the end with `, reason = ".."`
+
+error: aborting due to 2 previous errors
+