diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 89cf20c14..d46b584f8 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::get_parent_expr; use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; +use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::LateContext; use rustc_middle::ty; use rustc_middle::ty::print::with_forced_trimmed_paths; @@ -10,17 +10,71 @@ use rustc_span::{sym, Span}; use super::UNNECESSARY_FALLIBLE_CONVERSIONS; +#[derive(Copy, Clone)] +enum SpansKind { + TraitFn { trait_span: Span, fn_span: Span }, + Fn { fn_span: Span }, +} + /// What function is being called and whether that call is written as a method call or a function /// call #[derive(Copy, Clone)] #[expect(clippy::enum_variant_names)] enum FunctionKind { /// `T::try_from(U)` - TryFromFunction, + TryFromFunction(Option), /// `t.try_into()` TryIntoMethod, /// `U::try_into(t)` - TryIntoFunction, + TryIntoFunction(Option), +} + +impl FunctionKind { + fn appl_sugg(&self, parent_unwrap_call: Option, primary_span: Span) -> (Applicability, Vec<(Span, String)>) { + let Some(unwrap_span) = parent_unwrap_call else { + return (Applicability::Unspecified, self.default_sugg(primary_span)); + }; + + match &self { + FunctionKind::TryFromFunction(None) | FunctionKind::TryIntoFunction(None) => { + (Applicability::Unspecified, self.default_sugg(primary_span)) + }, + _ => ( + Applicability::MachineApplicable, + self.machine_applicable_sugg(primary_span, unwrap_span), + ), + } + } + + fn default_sugg(&self, primary_span: Span) -> Vec<(Span, String)> { + let replacement = match *self { + FunctionKind::TryFromFunction(_) => "From::from", + FunctionKind::TryIntoFunction(_) => "Into::into", + FunctionKind::TryIntoMethod => "into", + }; + + vec![(primary_span, String::from(replacement))] + } + + fn machine_applicable_sugg(&self, primary_span: Span, unwrap_span: Span) -> Vec<(Span, String)> { + let (trait_name, fn_name) = match self { + FunctionKind::TryFromFunction(_) => ("From".to_owned(), "from".to_owned()), + FunctionKind::TryIntoFunction(_) | FunctionKind::TryIntoMethod => ("Into".to_owned(), "into".to_owned()), + }; + + let mut sugg = match *self { + FunctionKind::TryFromFunction(Some(spans)) | FunctionKind::TryIntoFunction(Some(spans)) => match spans { + SpansKind::TraitFn { trait_span, fn_span } => vec![(trait_span, trait_name), (fn_span, fn_name)], + SpansKind::Fn { fn_span } => vec![(fn_span, fn_name)], + }, + FunctionKind::TryIntoMethod => vec![(primary_span, fn_name)], + // Or the suggestion is not machine-applicable + _ => unreachable!(), + }; + + sugg.push((unwrap_span, String::new())); + sugg + } } fn check<'tcx>( @@ -35,8 +89,8 @@ fn check<'tcx>( && self_ty != other_ty && let Some(self_ty) = self_ty.as_type() && let Some(from_into_trait) = cx.tcx.get_diagnostic_item(match kind { - FunctionKind::TryFromFunction => sym::From, - FunctionKind::TryIntoMethod | FunctionKind::TryIntoFunction => sym::Into, + FunctionKind::TryFromFunction(_) => sym::From, + FunctionKind::TryIntoMethod | FunctionKind::TryIntoFunction(_) => sym::Into, }) // If `T: TryFrom` and `T: From` both exist, then that means that the `TryFrom` // _must_ be from the blanket impl and cannot have been manually implemented @@ -45,49 +99,37 @@ fn check<'tcx>( && implements_trait(cx, self_ty, from_into_trait, &[other_ty]) && let Some(other_ty) = other_ty.as_type() { + // Extend the span to include the unwrap/expect call: + // `foo.try_into().expect("..")` + // ^^^^^^^^^^^^^^^^^^^^^^^ + // + // `try_into().unwrap()` specifically can be trivially replaced with just `into()`, + // so that can be machine-applicable let parent_unwrap_call = get_parent_expr(cx, expr).and_then(|parent| { if let ExprKind::MethodCall(path, .., span) = parent.kind && let sym::unwrap | sym::expect = path.ident.name { - Some(span) + // include `.` before `unwrap`/`expect` + Some(span.with_lo(expr.span.hi())) } else { None } }); - let (source_ty, target_ty, sugg, span, applicability) = match kind { - FunctionKind::TryIntoMethod if let Some(unwrap_span) = parent_unwrap_call => { - // Extend the span to include the unwrap/expect call: - // `foo.try_into().expect("..")` - // ^^^^^^^^^^^^^^^^^^^^^^^ - // - // `try_into().unwrap()` specifically can be trivially replaced with just `into()`, - // so that can be machine-applicable - ( - self_ty, - other_ty, - "into()", - primary_span.with_hi(unwrap_span.hi()), - Applicability::MachineApplicable, - ) - }, - FunctionKind::TryFromFunction => ( - other_ty, - self_ty, - "From::from", - primary_span, - Applicability::Unspecified, - ), - FunctionKind::TryIntoFunction => ( - self_ty, - other_ty, - "Into::into", - primary_span, - Applicability::Unspecified, - ), - FunctionKind::TryIntoMethod => (self_ty, other_ty, "into", primary_span, Applicability::Unspecified), + // If there is an unwrap/expect call, extend the span to include the call + let span = if let Some(unwrap_call) = parent_unwrap_call { + primary_span.with_hi(unwrap_call.hi()) + } else { + primary_span }; + let (source_ty, target_ty) = match kind { + FunctionKind::TryIntoMethod | FunctionKind::TryIntoFunction(_) => (self_ty, other_ty), + FunctionKind::TryFromFunction(_) => (other_ty, self_ty), + }; + + let (applicability, sugg) = kind.appl_sugg(parent_unwrap_call, primary_span); + span_lint_and_then( cx, UNNECESSARY_FALLIBLE_CONVERSIONS, @@ -97,7 +139,7 @@ fn check<'tcx>( with_forced_trimmed_paths!({ diag.note(format!("converting `{source_ty}` to `{target_ty}` cannot fail")); }); - diag.span_suggestion(span, "use", sugg, applicability); + diag.multipart_suggestion("use", sugg, applicability); }, ); } @@ -125,13 +167,30 @@ pub(super) fn check_function(cx: &LateContext<'_>, expr: &Expr<'_>, callee: &Exp && let Some(item_def_id) = cx.qpath_res(qpath, callee.hir_id).opt_def_id() && let Some(trait_def_id) = cx.tcx.trait_of_item(item_def_id) { + let qpath_spans = match qpath { + QPath::Resolved(_, path) => { + if let [trait_seg, fn_seg] = path.segments { + Some(SpansKind::TraitFn { + trait_span: trait_seg.ident.span, + fn_span: fn_seg.ident.span, + }) + } else { + None + } + }, + QPath::TypeRelative(_, seg) => Some(SpansKind::Fn { + fn_span: seg.ident.span, + }), + QPath::LangItem(_, _) => unreachable!("`TryFrom` and `TryInto` are not lang items"), + }; + check( cx, expr, cx.typeck_results().node_args(callee.hir_id), match cx.tcx.get_diagnostic_name(trait_def_id) { - Some(sym::TryFrom) => FunctionKind::TryFromFunction, - Some(sym::TryInto) => FunctionKind::TryIntoFunction, + Some(sym::TryFrom) => FunctionKind::TryFromFunction(qpath_spans), + Some(sym::TryInto) => FunctionKind::TryIntoFunction(qpath_spans), _ => return, }, callee.span, diff --git a/tests/ui/unnecessary_fallible_conversions.fixed b/tests/ui/unnecessary_fallible_conversions.fixed index 9668a6b99..b6dd1f267 100644 --- a/tests/ui/unnecessary_fallible_conversions.fixed +++ b/tests/ui/unnecessary_fallible_conversions.fixed @@ -1,6 +1,43 @@ #![warn(clippy::unnecessary_fallible_conversions)] fn main() { + // --- TryFromMethod `T::try_from(u)` --- + let _: i64 = 0i32.into(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + let _: i64 = 0i32.into(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryFromFunction `T::try_from(U)` --- + + let _ = i64::from(0i32); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _ = i64::from(0i32); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryIntoFunction `U::try_into(t)` --- + + let _: i64 = i32::into(0); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _: i64 = i32::into(0i32); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryFromFunction `>::try_from(U)` --- + + let _ = >::from(0); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _ = >::from(0); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryIntoFunction `>::try_into(U)` --- + + let _: i64 = >::into(0); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _: i64 = >::into(0); + //~^ ERROR: use of a fallible conversion when an infallible one could be used } diff --git a/tests/ui/unnecessary_fallible_conversions.rs b/tests/ui/unnecessary_fallible_conversions.rs index 9fa6c08b1..6f8df7365 100644 --- a/tests/ui/unnecessary_fallible_conversions.rs +++ b/tests/ui/unnecessary_fallible_conversions.rs @@ -1,6 +1,43 @@ #![warn(clippy::unnecessary_fallible_conversions)] fn main() { + // --- TryFromMethod `T::try_from(u)` --- + let _: i64 = 0i32.try_into().unwrap(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + let _: i64 = 0i32.try_into().expect("can't happen"); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryFromFunction `T::try_from(U)` --- + + let _ = i64::try_from(0i32).unwrap(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _ = i64::try_from(0i32).expect("can't happen"); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryIntoFunction `U::try_into(t)` --- + + let _: i64 = i32::try_into(0).unwrap(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _: i64 = i32::try_into(0i32).expect("can't happen"); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryFromFunction `>::try_from(U)` --- + + let _ = >::try_from(0).unwrap(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _ = >::try_from(0).expect("can't happen"); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + // --- TryIntoFunction `>::try_into(U)` --- + + let _: i64 = >::try_into(0).unwrap(); + //~^ ERROR: use of a fallible conversion when an infallible one could be used + + let _: i64 = >::try_into(0).expect("can't happen"); + //~^ ERROR: use of a fallible conversion when an infallible one could be used } diff --git a/tests/ui/unnecessary_fallible_conversions.stderr b/tests/ui/unnecessary_fallible_conversions.stderr index 26b152515..598f4ba91 100644 --- a/tests/ui/unnecessary_fallible_conversions.stderr +++ b/tests/ui/unnecessary_fallible_conversions.stderr @@ -1,20 +1,134 @@ error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:4:23 + --> $DIR/unnecessary_fallible_conversions.rs:6:23 | LL | let _: i64 = 0i32.try_into().unwrap(); - | ^^^^^^^^^^^^^^^^^^^ help: use: `into()` + | ^^^^^^^^^^^^^^^^^^^ | = note: converting `i32` to `i64` cannot fail = note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]` +help: use + | +LL - let _: i64 = 0i32.try_into().unwrap(); +LL + let _: i64 = 0i32.into(); + | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:5:23 + --> $DIR/unnecessary_fallible_conversions.rs:9:23 | LL | let _: i64 = 0i32.try_into().expect("can't happen"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `into()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _: i64 = 0i32.try_into().expect("can't happen"); +LL + let _: i64 = 0i32.into(); + | -error: aborting due to 2 previous errors +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:14:13 + | +LL | let _ = i64::try_from(0i32).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = i64::try_from(0i32).unwrap(); +LL + let _ = i64::from(0i32); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:17:13 + | +LL | let _ = i64::try_from(0i32).expect("can't happen"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = i64::try_from(0i32).expect("can't happen"); +LL + let _ = i64::from(0i32); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:22:18 + | +LL | let _: i64 = i32::try_into(0).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _: i64 = i32::try_into(0).unwrap(); +LL + let _: i64 = i32::into(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:25:18 + | +LL | let _: i64 = i32::try_into(0i32).expect("can't happen"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _: i64 = i32::try_into(0i32).expect("can't happen"); +LL + let _: i64 = i32::into(0i32); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:30:13 + | +LL | let _ = >::try_from(0).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = >::try_from(0).unwrap(); +LL + let _ = >::from(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:33:13 + | +LL | let _ = >::try_from(0).expect("can't happen"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = >::try_from(0).expect("can't happen"); +LL + let _ = >::from(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:38:18 + | +LL | let _: i64 = >::try_into(0).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _: i64 = >::try_into(0).unwrap(); +LL + let _: i64 = >::into(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:41:18 + | +LL | let _: i64 = >::try_into(0).expect("can't happen"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _: i64 = >::try_into(0).expect("can't happen"); +LL + let _: i64 = >::into(0); + | + +error: aborting due to 10 previous errors