diff --git a/CHANGELOG.md b/CHANGELOG.md index 161fa630ed47..27412ebfc960 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/constructable_unsafe_type.rs b/clippy_lints/src/constructable_unsafe_type.rs new file mode 100644 index 000000000000..dae303fb49ab --- /dev/null +++ b/clippy_lints/src/constructable_unsafe_type.rs @@ -0,0 +1,60 @@ +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 + && { + // If the type contains `Unsafe`, but is not exactly. + let name = item.ident.as_str(); + name.contains("Unsafe") && name.len() != "Unsafe".len() + } + && 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", + ); + } + } +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index edb52851e0cb..aae9c64d6bc5 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c9064df25ac8..77fa480909c6 100644 --- a/clippy_lints/src/lib.rs +++ b/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` } diff --git a/tests/ui/constructable_unsafe_type.rs b/tests/ui/constructable_unsafe_type.rs new file mode 100644 index 000000000000..5807646f4479 --- /dev/null +++ b/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() {} diff --git a/tests/ui/constructable_unsafe_type.stderr b/tests/ui/constructable_unsafe_type.stderr new file mode 100644 index 000000000000..742f3e636091 --- /dev/null +++ b/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 + diff --git a/tests/ui/new_without_default.fixed b/tests/ui/new_without_default.fixed index 5a6a92394a7f..390677726114 100644 --- a/tests/ui/new_without_default.fixed +++ b/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: () } } } diff --git a/tests/ui/new_without_default.rs b/tests/ui/new_without_default.rs index 12ea729253ac..675abfd47f1c 100644 --- a/tests/ui/new_without_default.rs +++ b/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: () } } } diff --git a/tests/ui/new_without_default.stderr b/tests/ui/new_without_default.stderr index 57bf4bd847cc..3569fa6ea018 100644 --- a/tests/ui/new_without_default.stderr +++ b/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` - --> 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` - --> 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` - --> 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 Foo { | error: you should consider adding a `Default` implementation for `MyStruct` - --> 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 }