Skip to content

Commit

Permalink
Add lint for publically constructable Unsafe types`
Browse files Browse the repository at this point in the history
  • Loading branch information
GnomedDev committed Nov 8, 2024
1 parent f712eb5 commit 0743df2
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions clippy_lints/src/constructable_unsafe_type.rs
Original file line number Diff line number Diff line change
@@ -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",
);
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
19 changes: 19 additions & 0 deletions tests/ui/constructable_unsafe_type.rs
Original file line number Diff line number Diff line change
@@ -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() {}
39 changes: 39 additions & 0 deletions tests/ui/constructable_unsafe_type.stderr
Original file line number Diff line number Diff line change
@@ -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

6 changes: 4 additions & 2 deletions tests/ui/new_without_default.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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: () }
}
}

Expand Down
6 changes: 4 additions & 2 deletions tests/ui/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: () }
}
}

Expand Down
10 changes: 5 additions & 5 deletions tests/ui/new_without_default.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 | |
Expand All @@ -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 }
Expand Down

0 comments on commit 0743df2

Please sign in to comment.