diff --git a/CHANGELOG.md b/CHANGELOG.md index 28147dfbea3e..9c06562e18e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5862,6 +5862,7 @@ Released 2018-09-13 [`ineffective_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_open_options [`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match +[`infallible_try_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_try_from [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter [`infinite_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loop [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bb825c7655f8..12dd407711ca 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -211,6 +211,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::indexing_slicing::INDEXING_SLICING_INFO, crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO, crate::ineffective_open_options::INEFFECTIVE_OPEN_OPTIONS_INFO, + crate::infallible_try_from::INFALLIBLE_TRY_FROM_INFO, crate::infinite_iter::INFINITE_ITER_INFO, crate::infinite_iter::MAYBE_INFINITE_ITER_INFO, crate::inherent_impl::MULTIPLE_INHERENT_IMPL_INFO, diff --git a/clippy_lints/src/infallible_try_from.rs b/clippy_lints/src/infallible_try_from.rs new file mode 100644 index 000000000000..b54c289fa7e1 --- /dev/null +++ b/clippy_lints/src/infallible_try_from.rs @@ -0,0 +1,76 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::sym; +use rustc_errors::MultiSpan; +use rustc_hir::{AssocItemKind, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// Finds manual impls of `TryFrom` with infallible error types. + /// + /// ### Why is this bad? + /// + /// Infalliable conversions should be implemented via `From` with the blanket conversion. + /// + /// ### Example + /// ```no_run + /// use std::convert::Infallible; + /// struct MyStruct(i16); + /// impl TryFrom for MyStruct { + /// type Error = Infallible; + /// fn try_from(other: i16) -> Result { + /// Ok(Self(other.into())) + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// struct MyStruct(i16); + /// impl From for MyStruct { + /// fn from(other: i16) -> Self { + /// Self(other) + /// } + /// } + /// ``` + #[clippy::version = "1.88.0"] + pub INFALLIBLE_TRY_FROM, + suspicious, + "TryFrom with infallible Error type" +} +declare_lint_pass!(InfallibleTryFrom => [INFALLIBLE_TRY_FROM]); + +impl<'tcx> LateLintPass<'tcx> for InfallibleTryFrom { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + let ItemKind::Impl(imp) = item.kind else { return }; + let Some(r#trait) = imp.of_trait else { return }; + let Some(trait_def_id) = r#trait.trait_def_id() else { + return; + }; + if !cx.tcx.is_diagnostic_item(sym::TryFrom, trait_def_id) { + return; + } + for ii in imp.items { + if ii.kind == AssocItemKind::Type { + let ii = cx.tcx.hir_impl_item(ii.id); + if ii.ident.name != sym::Error { + continue; + } + let ii_ty = ii.expect_type(); + let ii_ty_span = ii_ty.span; + let ii_ty = clippy_utils::ty::ty_from_hir_ty(cx, ii_ty); + if !ii_ty.is_inhabited_from(cx.tcx, ii.owner_id.to_def_id(), cx.typing_env()) { + let mut span = MultiSpan::from_span(cx.tcx.def_span(item.owner_id.to_def_id())); + span.push_span_label(ii_ty_span, "infallible error type"); + span_lint( + cx, + INFALLIBLE_TRY_FROM, + span, + "infallible TryFrom impl; consider implementing From, instead", + ); + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 006145cc623c..0f60c62ed094 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -169,6 +169,7 @@ mod inconsistent_struct_constructor; mod index_refutable_slice; mod indexing_slicing; mod ineffective_open_options; +mod infallible_try_from; mod infinite_iter; mod inherent_impl; mod inherent_to_string; @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap)); store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix)); store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf))); + store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/crashes/ice-rust-107877.rs b/tests/ui/crashes/ice-rust-107877.rs index 55fe418bed12..dccb0cf8ac15 100644 --- a/tests/ui/crashes/ice-rust-107877.rs +++ b/tests/ui/crashes/ice-rust-107877.rs @@ -4,6 +4,7 @@ struct Foo; +#[allow(clippy::infallible_try_from)] impl<'a> std::convert::TryFrom<&'a String> for Foo { type Error = std::convert::Infallible; diff --git a/tests/ui/infallible_try_from.rs b/tests/ui/infallible_try_from.rs new file mode 100644 index 000000000000..6a1f12f824f5 --- /dev/null +++ b/tests/ui/infallible_try_from.rs @@ -0,0 +1,33 @@ +#![feature(never_type)] +#![warn(clippy::infallible_try_from)] + +use std::convert::Infallible; + +struct MyStruct(i32); + +impl TryFrom for MyStruct { + //~^ infallible_try_from + type Error = !; + fn try_from(other: i8) -> Result { + Ok(Self(other.into())) + } +} + +impl TryFrom for MyStruct { + //~^ infallible_try_from + type Error = Infallible; + fn try_from(other: i16) -> Result { + Ok(Self(other.into())) + } +} + +impl TryFrom for MyStruct { + type Error = i64; + fn try_from(other: i64) -> Result { + Ok(Self(i32::try_from(other).map_err(|_| other)?)) + } +} + +fn main() { + // test code goes here +} diff --git a/tests/ui/infallible_try_from.stderr b/tests/ui/infallible_try_from.stderr new file mode 100644 index 000000000000..705b1188489c --- /dev/null +++ b/tests/ui/infallible_try_from.stderr @@ -0,0 +1,23 @@ +error: infallible TryFrom impl; consider implementing From, instead + --> tests/ui/infallible_try_from.rs:8:1 + | +LL | impl TryFrom for MyStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | type Error = !; + | - infallible error type + | + = note: `-D clippy::infallible-try-from` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::infallible_try_from)]` + +error: infallible TryFrom impl; consider implementing From, instead + --> tests/ui/infallible_try_from.rs:16:1 + | +LL | impl TryFrom for MyStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | type Error = Infallible; + | ---------- infallible error type + +error: aborting due to 2 previous errors +