Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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