Skip to content

add lint infallible_try_from #14813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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 @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
76 changes: 76 additions & 0 deletions clippy_lints/src/infallible_try_from.rs
Original file line number Diff line number Diff line change
@@ -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,
nursery,
"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",
);
}
}
}
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}
33 changes: 33 additions & 0 deletions tests/ui/infallible_try_from.rs
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 23 additions & 0 deletions tests/ui/infallible_try_from.stderr
Original file line number Diff line number Diff line change
@@ -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