Add lint for publically constructable Unsafe types`

Author: GnomedDev

CHANGELOG.md

@@ -5409,6 +5409,7 @@ Released 2018-09-13
 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
 [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
 [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime
+[`constructable_unsafe_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#constructable_unsafe_type
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def
 [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir

clippy_lints/src/constructable_unsafe_type.rs

@@ -0,0 +1,56 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use rustc_hir::{Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::declare_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Detects types with `Unsafe` in the name that are publically constructable.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// `Unsafe` in the name of a type implies that there is some kind of safety invariant
+    /// being held by constructing said type, however, this invariant may not be checked
+    /// if a user can safely publically construct it.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// pub struct UnsafeToken {}
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// pub struct UnsafeToken {
+    ///     _private: ()
+    /// }
+    /// ```
+    #[clippy::version = "1.84.0"]
+    pub CONSTRUCTABLE_UNSAFE_TYPE,
+    suspicious,
+    "`Unsafe` types that are publically constructable"
+}
+
+declare_lint_pass!(ConstructableUnsafeType => [CONSTRUCTABLE_UNSAFE_TYPE]);
+
+impl LateLintPass<'_> for ConstructableUnsafeType {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        if let ItemKind::Struct(variant, generics) = item.kind
+            && item.ident.as_str().contains("Unsafe")
+            && generics.params.is_empty()
+            && cx.effective_visibilities.is_reachable(item.owner_id.def_id)
+            && variant
+                .fields()
+                .iter()
+                .all(|f| cx.effective_visibilities.is_exported(f.def_id))
+        {
+            span_lint_and_help(
+                cx,
+                CONSTRUCTABLE_UNSAFE_TYPE,
+                item.span,
+                "`Unsafe` type is publically constructable",
+                None,
+                "give this type a private field, or make it private",
+            );
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -106,6 +106,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::collapsible_if::COLLAPSIBLE_IF_INFO,
     crate::collection_is_never_read::COLLECTION_IS_NEVER_READ_INFO,
     crate::comparison_chain::COMPARISON_CHAIN_INFO,
+    crate::constructable_unsafe_type::CONSTRUCTABLE_UNSAFE_TYPE_INFO,
     crate::copies::BRANCHES_SHARING_CODE_INFO,
     crate::copies::IFS_SAME_COND_INFO,
     crate::copies::IF_SAME_THEN_ELSE_INFO,

clippy_lints/src/lib.rs

@@ -98,6 +98,7 @@ mod cognitive_complexity;
 mod collapsible_if;
 mod collection_is_never_read;
 mod comparison_chain;
+mod constructable_unsafe_type;
 mod copies;
 mod copy_iterator;
 mod crate_in_macro_def;
@@ -963,5 +964,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
     store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
+    store.register_late_pass(|_| Box::new(constructable_unsafe_type::ConstructableUnsafeType));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui/constructable_unsafe_type.rs

@@ -0,0 +1,19 @@
+#![warn(clippy::constructable_unsafe_type)]
+
+struct PrivateUnsafeToken;
+pub struct GoodUnsafeToken {
+    _private: (),
+}
+
+pub struct DangerousUnsafeToken1;
+//~^ error: `Unsafe` type is publically constructable
+pub struct DangerousUnsafeToken2();
+//~^ error: `Unsafe` type is publically constructable
+pub struct DangerousUnsafeToken3 {}
+//~^ error: `Unsafe` type is publically constructable
+pub struct DangerousUnsafeToken4 {
+    //~^ error: `Unsafe` type is publically constructable
+    pub public: (),
+}
+
+fn main() {}

tests/ui/constructable_unsafe_type.stderr

@@ -0,0 +1,39 @@
+error: `Unsafe` type is publically constructable
+  --> tests/ui/constructable_unsafe_type.rs:8:1
+   |
+LL | pub struct DangerousUnsafeToken1;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: give this type a private field, or make it private
+   = note: `-D clippy::constructable-unsafe-type` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::constructable_unsafe_type)]`
+
+error: `Unsafe` type is publically constructable
+  --> tests/ui/constructable_unsafe_type.rs:10:1
+   |
+LL | pub struct DangerousUnsafeToken2();
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: give this type a private field, or make it private
+
+error: `Unsafe` type is publically constructable
+  --> tests/ui/constructable_unsafe_type.rs:12:1
+   |
+LL | pub struct DangerousUnsafeToken3 {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: give this type a private field, or make it private
+
+error: `Unsafe` type is publically constructable
+  --> tests/ui/constructable_unsafe_type.rs:14:1
+   |
+LL | / pub struct DangerousUnsafeToken4 {
+LL | |
+LL | |     pub public: (),
+LL | | }
+   | |_^
+   |
+   = help: give this type a private field, or make it private
+
+error: aborting due to 4 previous errors
+

tests/ui/new_without_default.fixed

@@ -161,11 +161,13 @@ pub trait TraitWithNew: Sized {
     }
 }
 
-pub struct IgnoreUnsafeNew;
+pub struct IgnoreUnsafeNew {
+    _private: (),
+}
 
 impl IgnoreUnsafeNew {
     pub unsafe fn new() -> Self {
-        IgnoreUnsafeNew
+        IgnoreUnsafeNew { _private: () }
     }
 }
 

tests/ui/new_without_default.rs

@@ -137,11 +137,13 @@ pub trait TraitWithNew: Sized {
     }
 }
 
-pub struct IgnoreUnsafeNew;
+pub struct IgnoreUnsafeNew {
+    _private: (),
+}
 
 impl IgnoreUnsafeNew {
     pub unsafe fn new() -> Self {
-        IgnoreUnsafeNew
+        IgnoreUnsafeNew { _private: () }
     }
 }
 

tests/ui/new_without_default.stderr

@@ -73,7 +73,7 @@ LL + }
    |
 
 error: you should consider adding a `Default` implementation for `NewNotEqualToDerive`
-  --> tests/ui/new_without_default.rs:181:5
+  --> tests/ui/new_without_default.rs:183:5
    |
 LL | /     pub fn new() -> Self {
 LL | |
@@ -91,7 +91,7 @@ LL + }
    |
 
 error: you should consider adding a `Default` implementation for `FooGenerics<T>`
-  --> tests/ui/new_without_default.rs:190:5
+  --> tests/ui/new_without_default.rs:192:5
    |
 LL | /     pub fn new() -> Self {
 LL | |
@@ -109,7 +109,7 @@ LL + }
    |
 
 error: you should consider adding a `Default` implementation for `BarGenerics<T>`
-  --> tests/ui/new_without_default.rs:198:5
+  --> tests/ui/new_without_default.rs:200:5
    |
 LL | /     pub fn new() -> Self {
 LL | |
@@ -127,7 +127,7 @@ LL + }
    |
 
 error: you should consider adding a `Default` implementation for `Foo<T>`
-  --> tests/ui/new_without_default.rs:210:9
+  --> tests/ui/new_without_default.rs:212:9
    |
 LL | /         pub fn new() -> Self {
 LL | |
@@ -147,7 +147,7 @@ LL ~     impl<T> Foo<T> {
    |
 
 error: you should consider adding a `Default` implementation for `MyStruct<K, V>`
-  --> tests/ui/new_without_default.rs:256:5
+  --> tests/ui/new_without_default.rs:258:5
    |
 LL | /     pub fn new() -> Self {
 LL | |         Self { _kv: None }