add lint infallible_try_from (#14813)

Looks for `TryFrom` implementations with uninhabited error types and
suggests to implement `From` instead.

Fixes #2144

---

changelog: Add [`infallible_try_from`] lint

Author: llogiq

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

clippy_lints/src/declared_lints.rs

@@ -212,6 +212,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,

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<i16> for MyStruct {
+    ///     type Error = Infallible;
+    ///     fn try_from(other: i16) -> Result<Self, Infallible> {
+    ///         Ok(Self(other.into()))
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// struct MyStruct(i16);
+    /// impl From<i16> 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",
+                    );
+                }
+            }
+        }
+    }
+}

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;
@@ -946,5 +947,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`
 }

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;
 

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<i8> for MyStruct {
+    //~^ infallible_try_from
+    type Error = !;
+    fn try_from(other: i8) -> Result<Self, !> {
+        Ok(Self(other.into()))
+    }
+}
+
+impl TryFrom<i16> for MyStruct {
+    //~^ infallible_try_from
+    type Error = Infallible;
+    fn try_from(other: i16) -> Result<Self, Infallible> {
+        Ok(Self(other.into()))
+    }
+}
+
+impl TryFrom<i64> for MyStruct {
+    type Error = i64;
+    fn try_from(other: i64) -> Result<Self, i64> {
+        Ok(Self(i32::try_from(other).map_err(|_| other)?))
+    }
+}
+
+fn main() {
+    // test code goes here
+}

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<i8> 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<i16> for MyStruct {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |
+LL |     type Error = Infallible;
+   |                  ---------- infallible error type
+
+error: aborting due to 2 previous errors
+