From 84e797499b803aed222b027e466113d2947c38ad Mon Sep 17 00:00:00 2001 From: roife Date: Mon, 1 Jan 2024 16:17:22 +0800 Subject: [PATCH 1/9] Add autofix for functions in unnecessary_fallible_conversions --- .../unnecessary_fallible_conversions.rs | 101 ++++++++++++------ 1 file changed, 68 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 89cf20c14fc6..b646c5953e41 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; @@ -29,6 +29,7 @@ fn check<'tcx>( node_args: ty::GenericArgsRef<'tcx>, kind: FunctionKind, primary_span: Span, + qpath: Option<&QPath<'_>>, ) { if let &[self_ty, other_ty] = node_args.as_slice() // useless_conversion already warns `T::try_from(T)`, so ignore it here @@ -45,47 +46,79 @@ 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, - ) + let span = if let Some(unwrap_call) = parent_unwrap_call { + primary_span.with_hi(unwrap_call.hi()) + } else { + primary_span + }; + + let qpath_spans = qpath.and_then(|qpath| match qpath { + QPath::Resolved(_, path) => { + let segments = path.segments.iter().map(|seg| seg.ident).collect::>(); + (segments.len() == 2).then(|| vec![segments[0].span, segments[1].span]) + }, + QPath::TypeRelative(_, seg) => Some(vec![seg.ident.span]), + QPath::LangItem(_, _) => unreachable!("`TryFrom` and `TryInto` are not lang items"), + }); + + let (source_ty, target_ty, sugg, applicability) = match (kind, &qpath_spans, parent_unwrap_call) { + (FunctionKind::TryIntoMethod, _, Some(unwrap_span)) => { + let sugg = vec![(primary_span, String::from("into")), (unwrap_span, String::new())]; + (self_ty, other_ty, sugg, Applicability::MachineApplicable) + }, + (FunctionKind::TryFromFunction, Some(spans), Some(unwrap_span)) => { + let sugg = match spans.len() { + 1 => vec![(spans[0], String::from("from")), (unwrap_span, String::new())], + 2 => vec![ + (spans[0], String::from("From")), + (spans[1], String::from("from")), + (unwrap_span, String::new()), + ], + _ => unreachable!(), + }; + (other_ty, self_ty, sugg, Applicability::MachineApplicable) + }, + (FunctionKind::TryIntoFunction, Some(spans), Some(unwrap_span)) => { + let sugg = match spans.len() { + 1 => vec![(spans[0], String::from("into")), (unwrap_span, String::new())], + 2 => vec![ + (spans[0], String::from("Into")), + (spans[1], String::from("into")), + (unwrap_span, String::new()), + ], + _ => unreachable!(), + }; + (self_ty, other_ty, sugg, Applicability::MachineApplicable) + }, + (FunctionKind::TryFromFunction, _, _) => { + let sugg = vec![(primary_span, String::from("From::from"))]; + (other_ty, self_ty, sugg, Applicability::Unspecified) + }, + (FunctionKind::TryIntoFunction, _, _) => { + let sugg = vec![(primary_span, String::from("Into::into"))]; + (self_ty, other_ty, sugg, Applicability::Unspecified) + }, + (FunctionKind::TryIntoMethod, _, _) => { + let sugg = vec![(primary_span, String::from("into"))]; + (self_ty, other_ty, sugg, Applicability::Unspecified) }, - 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), }; span_lint_and_then( @@ -97,7 +130,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); }, ); } @@ -113,6 +146,7 @@ pub(super) fn check_method(cx: &LateContext<'_>, expr: &Expr<'_>) { cx.typeck_results().node_args(expr.hir_id), FunctionKind::TryIntoMethod, path.ident.span, + None, ); } } @@ -135,6 +169,7 @@ pub(super) fn check_function(cx: &LateContext<'_>, expr: &Expr<'_>, callee: &Exp _ => return, }, callee.span, + Some(qpath), ); } } From 69cc9831553cf40c0ee63bfc614827c438c438bf Mon Sep 17 00:00:00 2001 From: roife Date: Mon, 1 Jan 2024 16:17:46 +0800 Subject: [PATCH 2/9] Add tests for autofix in unnecessary_fallible_conversions --- .../ui/unnecessary_fallible_conversions.fixed | 12 ++ tests/ui/unnecessary_fallible_conversions.rs | 14 ++ .../unnecessary_fallible_conversions.stderr | 124 +++++++++++++++++- 3 files changed, 147 insertions(+), 3 deletions(-) diff --git a/tests/ui/unnecessary_fallible_conversions.fixed b/tests/ui/unnecessary_fallible_conversions.fixed index 9668a6b99bf0..f6027fbfa6a9 100644 --- a/tests/ui/unnecessary_fallible_conversions.fixed +++ b/tests/ui/unnecessary_fallible_conversions.fixed @@ -3,4 +3,16 @@ fn main() { let _: i64 = 0i32.into(); let _: i64 = 0i32.into(); + + let _ = i64::from(0i32); + let _ = i64::from(0i32); + + let _: i64 = i32::into(0); + let _: i64 = i32::into(0i32); + + let _ = >::from(0); + let _ = >::from(0); + + let _: i64 = >::into(0); + let _: i64 = >::into(0); } diff --git a/tests/ui/unnecessary_fallible_conversions.rs b/tests/ui/unnecessary_fallible_conversions.rs index 9fa6c08b1b07..f9df782c4e25 100644 --- a/tests/ui/unnecessary_fallible_conversions.rs +++ b/tests/ui/unnecessary_fallible_conversions.rs @@ -3,4 +3,18 @@ fn main() { let _: i64 = 0i32.try_into().unwrap(); let _: i64 = 0i32.try_into().expect("can't happen"); + + let _ = i64::try_from(0i32).unwrap(); + let _ = i64::try_from(0i32).expect("can't happen"); + + let _: i64 = i32::try_into(0).unwrap(); + let _: i64 = i32::try_into(0i32).expect("can't happen"); + + let _ = >::try_from(0) + .unwrap(); + let _ = >::try_from(0). + expect("can't happen"); + + let _: i64 = >::try_into(0).unwrap(); + let _: i64 = >::try_into(0).expect("can't happen"); } diff --git a/tests/ui/unnecessary_fallible_conversions.stderr b/tests/ui/unnecessary_fallible_conversions.stderr index 26b152515ac7..603c079bc0eb 100644 --- a/tests/ui/unnecessary_fallible_conversions.stderr +++ b/tests/ui/unnecessary_fallible_conversions.stderr @@ -2,19 +2,137 @@ error: use of a fallible conversion when an infallible one could be used --> $DIR/unnecessary_fallible_conversions.rs:4: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 | 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:7: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:8: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:10: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:11: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:13:13 + | +LL | let _ = >::try_from(0) + | _____________^ +LL | | .unwrap(); + | |_________________^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = >::try_from(0) +LL + let _ = >::from(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:15:13 + | +LL | let _ = >::try_from(0). + | _____________^ +LL | | expect("can't happen"); + | |______________________________^ + | + = note: converting `i32` to `i64` cannot fail +help: use + | +LL - let _ = >::try_from(0). +LL + let _ = >::from(0); + | + +error: use of a fallible conversion when an infallible one could be used + --> $DIR/unnecessary_fallible_conversions.rs:18: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:19: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 From 094ce3de3ee98842c9a2ffc74934959973da5d1b Mon Sep 17 00:00:00 2001 From: roife Date: Mon, 1 Jan 2024 16:28:34 +0800 Subject: [PATCH 3/9] Add comments for unnecessary_fallible_conversions --- .../unnecessary_fallible_conversions.rs | 1 + tests/ui/unnecessary_fallible_conversions.rs | 6 ++--- .../unnecessary_fallible_conversions.stderr | 22 ++++++++----------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index b646c5953e41..530693916339 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -63,6 +63,7 @@ fn check<'tcx>( } }); + // 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 { diff --git a/tests/ui/unnecessary_fallible_conversions.rs b/tests/ui/unnecessary_fallible_conversions.rs index f9df782c4e25..9e4fd3b3f903 100644 --- a/tests/ui/unnecessary_fallible_conversions.rs +++ b/tests/ui/unnecessary_fallible_conversions.rs @@ -10,10 +10,8 @@ fn main() { let _: i64 = i32::try_into(0).unwrap(); let _: i64 = i32::try_into(0i32).expect("can't happen"); - let _ = >::try_from(0) - .unwrap(); - let _ = >::try_from(0). - expect("can't happen"); + let _ = >::try_from(0).unwrap(); + let _ = >::try_from(0).expect("can't happen"); let _: i64 = >::try_into(0).unwrap(); let _: i64 = >::try_into(0).expect("can't happen"); diff --git a/tests/ui/unnecessary_fallible_conversions.stderr b/tests/ui/unnecessary_fallible_conversions.stderr index 603c079bc0eb..c16dd239a8b5 100644 --- a/tests/ui/unnecessary_fallible_conversions.stderr +++ b/tests/ui/unnecessary_fallible_conversions.stderr @@ -81,35 +81,31 @@ LL + let _: i64 = i32::into(0i32); error: use of a fallible conversion when an infallible one could be used --> $DIR/unnecessary_fallible_conversions.rs:13:13 | -LL | let _ = >::try_from(0) - | _____________^ -LL | | .unwrap(); - | |_________________^ +LL | let _ = >::try_from(0).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: converting `i32` to `i64` cannot fail help: use | -LL - let _ = >::try_from(0) +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:15:13 + --> $DIR/unnecessary_fallible_conversions.rs:14:13 | -LL | let _ = >::try_from(0). - | _____________^ -LL | | expect("can't happen"); - | |______________________________^ +LL | let _ = >::try_from(0).expect("can't happen"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: converting `i32` to `i64` cannot fail help: use | -LL - let _ = >::try_from(0). +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:18:18 + --> $DIR/unnecessary_fallible_conversions.rs:16:18 | LL | let _: i64 = >::try_into(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -122,7 +118,7 @@ LL + let _: i64 = >::into(0); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:19:18 + --> $DIR/unnecessary_fallible_conversions.rs:17:18 | LL | let _: i64 = >::try_into(0).expect("can't happen"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 1cce757672d9a7ec5fd19628fa4e5f00b31c8f8a Mon Sep 17 00:00:00 2001 From: roife Date: Tue, 2 Jan 2024 01:23:44 +0800 Subject: [PATCH 4/9] Add error-annotations in tests for unnecessary_fallible_conversions --- .../ui/unnecessary_fallible_conversions.fixed | 25 +++++++++++++++++++ tests/ui/unnecessary_fallible_conversions.rs | 25 +++++++++++++++++++ .../unnecessary_fallible_conversions.stderr | 20 +++++++-------- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/tests/ui/unnecessary_fallible_conversions.fixed b/tests/ui/unnecessary_fallible_conversions.fixed index f6027fbfa6a9..b6dd1f267743 100644 --- a/tests/ui/unnecessary_fallible_conversions.fixed +++ b/tests/ui/unnecessary_fallible_conversions.fixed @@ -1,18 +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 9e4fd3b3f903..6f8df7365e89 100644 --- a/tests/ui/unnecessary_fallible_conversions.rs +++ b/tests/ui/unnecessary_fallible_conversions.rs @@ -1,18 +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 c16dd239a8b5..598f4ba91b3c 100644 --- a/tests/ui/unnecessary_fallible_conversions.stderr +++ b/tests/ui/unnecessary_fallible_conversions.stderr @@ -1,5 +1,5 @@ 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(); | ^^^^^^^^^^^^^^^^^^^ @@ -14,7 +14,7 @@ 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"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -27,7 +27,7 @@ LL + let _: i64 = 0i32.into(); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:7:13 + --> $DIR/unnecessary_fallible_conversions.rs:14:13 | LL | let _ = i64::try_from(0i32).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL + let _ = i64::from(0i32); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:8:13 + --> $DIR/unnecessary_fallible_conversions.rs:17:13 | LL | let _ = i64::try_from(0i32).expect("can't happen"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -53,7 +53,7 @@ LL + let _ = i64::from(0i32); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:10:18 + --> $DIR/unnecessary_fallible_conversions.rs:22:18 | LL | let _: i64 = i32::try_into(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -66,7 +66,7 @@ LL + let _: i64 = i32::into(0); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:11:18 + --> $DIR/unnecessary_fallible_conversions.rs:25:18 | LL | let _: i64 = i32::try_into(0i32).expect("can't happen"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -79,7 +79,7 @@ LL + let _: i64 = i32::into(0i32); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:13:13 + --> $DIR/unnecessary_fallible_conversions.rs:30:13 | LL | let _ = >::try_from(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -92,7 +92,7 @@ LL + let _ = >::from(0); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:14:13 + --> $DIR/unnecessary_fallible_conversions.rs:33:13 | LL | let _ = >::try_from(0).expect("can't happen"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -105,7 +105,7 @@ LL + let _ = >::from(0); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:16:18 + --> $DIR/unnecessary_fallible_conversions.rs:38:18 | LL | let _: i64 = >::try_into(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -118,7 +118,7 @@ LL + let _: i64 = >::into(0); | error: use of a fallible conversion when an infallible one could be used - --> $DIR/unnecessary_fallible_conversions.rs:17:18 + --> $DIR/unnecessary_fallible_conversions.rs:41:18 | LL | let _: i64 = >::try_into(0).expect("can't happen"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From eb05b07451b91c7a707adac828841fa1d99725bf Mon Sep 17 00:00:00 2001 From: roife Date: Fri, 2 Feb 2024 15:28:34 +0800 Subject: [PATCH 5/9] Move qpath_spans into FunctionKind --- .../unnecessary_fallible_conversions.rs | 49 +++++++++---------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 530693916339..4ef26cba468d 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -12,15 +12,15 @@ use super::UNNECESSARY_FALLIBLE_CONVERSIONS; /// What function is being called and whether that call is written as a method call or a function /// call -#[derive(Copy, Clone)] +#[derive(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>), } fn check<'tcx>( @@ -29,15 +29,14 @@ fn check<'tcx>( node_args: ty::GenericArgsRef<'tcx>, kind: FunctionKind, primary_span: Span, - qpath: Option<&QPath<'_>>, ) { if let &[self_ty, other_ty] = node_args.as_slice() // useless_conversion already warns `T::try_from(T)`, so ignore it here && 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 @@ -70,21 +69,12 @@ fn check<'tcx>( primary_span }; - let qpath_spans = qpath.and_then(|qpath| match qpath { - QPath::Resolved(_, path) => { - let segments = path.segments.iter().map(|seg| seg.ident).collect::>(); - (segments.len() == 2).then(|| vec![segments[0].span, segments[1].span]) - }, - QPath::TypeRelative(_, seg) => Some(vec![seg.ident.span]), - QPath::LangItem(_, _) => unreachable!("`TryFrom` and `TryInto` are not lang items"), - }); - - let (source_ty, target_ty, sugg, applicability) = match (kind, &qpath_spans, parent_unwrap_call) { - (FunctionKind::TryIntoMethod, _, Some(unwrap_span)) => { + let (source_ty, target_ty, sugg, applicability) = match (kind, parent_unwrap_call) { + (FunctionKind::TryIntoMethod, Some(unwrap_span)) => { let sugg = vec![(primary_span, String::from("into")), (unwrap_span, String::new())]; (self_ty, other_ty, sugg, Applicability::MachineApplicable) }, - (FunctionKind::TryFromFunction, Some(spans), Some(unwrap_span)) => { + (FunctionKind::TryFromFunction(Some(spans)), Some(unwrap_span)) => { let sugg = match spans.len() { 1 => vec![(spans[0], String::from("from")), (unwrap_span, String::new())], 2 => vec![ @@ -96,7 +86,7 @@ fn check<'tcx>( }; (other_ty, self_ty, sugg, Applicability::MachineApplicable) }, - (FunctionKind::TryIntoFunction, Some(spans), Some(unwrap_span)) => { + (FunctionKind::TryIntoFunction(Some(spans)), Some(unwrap_span)) => { let sugg = match spans.len() { 1 => vec![(spans[0], String::from("into")), (unwrap_span, String::new())], 2 => vec![ @@ -108,15 +98,15 @@ fn check<'tcx>( }; (self_ty, other_ty, sugg, Applicability::MachineApplicable) }, - (FunctionKind::TryFromFunction, _, _) => { + (FunctionKind::TryFromFunction(_), _) => { let sugg = vec![(primary_span, String::from("From::from"))]; (other_ty, self_ty, sugg, Applicability::Unspecified) }, - (FunctionKind::TryIntoFunction, _, _) => { + (FunctionKind::TryIntoFunction(_), _) => { let sugg = vec![(primary_span, String::from("Into::into"))]; (self_ty, other_ty, sugg, Applicability::Unspecified) }, - (FunctionKind::TryIntoMethod, _, _) => { + (FunctionKind::TryIntoMethod, _) => { let sugg = vec![(primary_span, String::from("into"))]; (self_ty, other_ty, sugg, Applicability::Unspecified) }, @@ -147,7 +137,6 @@ pub(super) fn check_method(cx: &LateContext<'_>, expr: &Expr<'_>) { cx.typeck_results().node_args(expr.hir_id), FunctionKind::TryIntoMethod, path.ident.span, - None, ); } } @@ -160,17 +149,25 @@ 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) => { + let segments = path.segments.iter().map(|seg| seg.ident).collect::>(); + (segments.len() == 2).then(|| vec![segments[0].span, segments[1].span]) + }, + QPath::TypeRelative(_, seg) => Some(vec![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, - Some(qpath), ); } } From e83c7d49dbc18112bfaa7197a87145a99a9570fc Mon Sep 17 00:00:00 2001 From: roife Date: Fri, 2 Feb 2024 18:59:33 +0800 Subject: [PATCH 6/9] Refactor sugg builder in unnecessary_fallible_conversions --- .../unnecessary_fallible_conversions.rs | 119 +++++++++++------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 4ef26cba468d..42ad700a574a 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -10,17 +10,64 @@ 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(Clone)] +#[derive(Copy, Clone)] #[expect(clippy::enum_variant_names)] enum FunctionKind { /// `T::try_from(U)` - TryFromFunction(Option>), + TryFromFunction(Option), /// `t.try_into()` TryIntoMethod, /// `U::try_into(t)` - TryIntoFunction(Option>), + TryIntoFunction(Option), +} + +impl FunctionKind { + fn applicability(&self, parent_unwrap_call: &Option) -> Applicability { + if parent_unwrap_call.is_none() { + return Applicability::Unspecified; + } + match &self { + FunctionKind::TryFromFunction(None) | FunctionKind::TryIntoFunction(None) => Applicability::Unspecified, + _ => Applicability::MachineApplicable, + } + } + + fn default_sugg(&self, primary_span: Span) -> Vec<(Span, String)> { + match *self { + FunctionKind::TryFromFunction(_) => vec![(primary_span, String::from("From::from"))], + FunctionKind::TryIntoFunction(_) => vec![(primary_span, String::from("Into::into"))], + FunctionKind::TryIntoMethod => vec![(primary_span, String::from("into"))], + } + } + + fn machine_applicable_sugg(&self, primary_span: Span, unwrap_span: Span) -> Vec<(Span, String)> { + let mut sugg = match *self { + FunctionKind::TryFromFunction(Some(spans)) => match spans { + SpansKind::TraitFn { trait_span, fn_span } => { + vec![(trait_span, String::from("From")), (fn_span, String::from("from"))] + }, + SpansKind::Fn { fn_span } => vec![(fn_span, String::from("from"))], + }, + FunctionKind::TryIntoFunction(Some(spans)) => match spans { + SpansKind::TraitFn { trait_span, fn_span } => { + vec![(trait_span, String::from("Into")), (fn_span, String::from("into"))] + }, + SpansKind::Fn { fn_span } => vec![(fn_span, String::from("into"))], + }, + FunctionKind::TryIntoMethod => vec![(primary_span, String::from("into"))], + _ => unreachable!(), + }; + sugg.push((unwrap_span, String::new())); + sugg + } } fn check<'tcx>( @@ -69,47 +116,17 @@ fn check<'tcx>( primary_span }; - let (source_ty, target_ty, sugg, applicability) = match (kind, parent_unwrap_call) { - (FunctionKind::TryIntoMethod, Some(unwrap_span)) => { - let sugg = vec![(primary_span, String::from("into")), (unwrap_span, String::new())]; - (self_ty, other_ty, sugg, Applicability::MachineApplicable) - }, - (FunctionKind::TryFromFunction(Some(spans)), Some(unwrap_span)) => { - let sugg = match spans.len() { - 1 => vec![(spans[0], String::from("from")), (unwrap_span, String::new())], - 2 => vec![ - (spans[0], String::from("From")), - (spans[1], String::from("from")), - (unwrap_span, String::new()), - ], - _ => unreachable!(), - }; - (other_ty, self_ty, sugg, Applicability::MachineApplicable) - }, - (FunctionKind::TryIntoFunction(Some(spans)), Some(unwrap_span)) => { - let sugg = match spans.len() { - 1 => vec![(spans[0], String::from("into")), (unwrap_span, String::new())], - 2 => vec![ - (spans[0], String::from("Into")), - (spans[1], String::from("into")), - (unwrap_span, String::new()), - ], - _ => unreachable!(), - }; - (self_ty, other_ty, sugg, Applicability::MachineApplicable) - }, - (FunctionKind::TryFromFunction(_), _) => { - let sugg = vec![(primary_span, String::from("From::from"))]; - (other_ty, self_ty, sugg, Applicability::Unspecified) - }, - (FunctionKind::TryIntoFunction(_), _) => { - let sugg = vec![(primary_span, String::from("Into::into"))]; - (self_ty, other_ty, sugg, Applicability::Unspecified) - }, - (FunctionKind::TryIntoMethod, _) => { - let sugg = vec![(primary_span, String::from("into"))]; - (self_ty, other_ty, sugg, Applicability::Unspecified) - }, + let (source_ty, target_ty) = match kind { + FunctionKind::TryIntoMethod | FunctionKind::TryIntoFunction(_) => (self_ty, other_ty), + FunctionKind::TryFromFunction(_) => (other_ty, self_ty), + }; + + let applicability = kind.applicability(&parent_unwrap_call); + + let sugg = if applicability == Applicability::MachineApplicable { + kind.machine_applicable_sugg(primary_span, parent_unwrap_call.unwrap()) + } else { + kind.default_sugg(primary_span) }; span_lint_and_then( @@ -151,10 +168,18 @@ pub(super) fn check_function(cx: &LateContext<'_>, expr: &Expr<'_>, callee: &Exp { let qpath_spans = match qpath { QPath::Resolved(_, path) => { - let segments = path.segments.iter().map(|seg| seg.ident).collect::>(); - (segments.len() == 2).then(|| vec![segments[0].span, segments[1].span]) + 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(vec![seg.ident.span]), + QPath::TypeRelative(_, seg) => Some(SpansKind::Fn { + fn_span: seg.ident.span, + }), QPath::LangItem(_, _) => unreachable!("`TryFrom` and `TryInto` are not lang items"), }; From 015ac1095417a61f293ca46b7f320ac935176c83 Mon Sep 17 00:00:00 2001 From: roife Date: Sat, 3 Feb 2024 00:50:20 +0800 Subject: [PATCH 7/9] Refactor machine_applicable_sugg in unnecessary_fallible_conversions --- .../unnecessary_fallible_conversions.rs | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 42ad700a574a..72bd8f7e000c 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -34,6 +34,7 @@ impl FunctionKind { if parent_unwrap_call.is_none() { return Applicability::Unspecified; } + match &self { FunctionKind::TryFromFunction(None) | FunctionKind::TryIntoFunction(None) => Applicability::Unspecified, _ => Applicability::MachineApplicable, @@ -41,30 +42,36 @@ impl FunctionKind { } fn default_sugg(&self, primary_span: Span) -> Vec<(Span, String)> { - match *self { - FunctionKind::TryFromFunction(_) => vec![(primary_span, String::from("From::from"))], - FunctionKind::TryIntoFunction(_) => vec![(primary_span, String::from("Into::into"))], - FunctionKind::TryIntoMethod => vec![(primary_span, String::from("into"))], - } + 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)> { + use FunctionKind::*; + use SpansKind::*; + + let (trait_name, fn_name) = { + let (a, b) = match self { + TryFromFunction(_) => ("From", "from"), + TryIntoFunction(_) | TryIntoMethod => ("Into", "into"), + }; + (a.to_string(), b.to_string()) + }; + let mut sugg = match *self { - FunctionKind::TryFromFunction(Some(spans)) => match spans { - SpansKind::TraitFn { trait_span, fn_span } => { - vec![(trait_span, String::from("From")), (fn_span, String::from("from"))] - }, - SpansKind::Fn { fn_span } => vec![(fn_span, String::from("from"))], + TryFromFunction(Some(spans)) | TryIntoFunction(Some(spans)) => match spans { + TraitFn { trait_span, fn_span } => vec![(trait_span, trait_name), (fn_span, fn_name)], + Fn { fn_span } => vec![(fn_span, fn_name)], }, - FunctionKind::TryIntoFunction(Some(spans)) => match spans { - SpansKind::TraitFn { trait_span, fn_span } => { - vec![(trait_span, String::from("Into")), (fn_span, String::from("into"))] - }, - SpansKind::Fn { fn_span } => vec![(fn_span, String::from("into"))], - }, - FunctionKind::TryIntoMethod => vec![(primary_span, String::from("into"))], + TryIntoMethod => vec![(primary_span, fn_name)], _ => unreachable!(), }; + sugg.push((unwrap_span, String::new())); sugg } From fb9fd513f57ea6939c20829286b2c44398c35d2d Mon Sep 17 00:00:00 2001 From: roife Date: Sat, 3 Feb 2024 13:46:16 +0800 Subject: [PATCH 8/9] Remove wildcard usage for patterns --- .../unnecessary_fallible_conversions.rs | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index 72bd8f7e000c..bb23ae5f8437 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -52,23 +52,18 @@ impl FunctionKind { } fn machine_applicable_sugg(&self, primary_span: Span, unwrap_span: Span) -> Vec<(Span, String)> { - use FunctionKind::*; - use SpansKind::*; - - let (trait_name, fn_name) = { - let (a, b) = match self { - TryFromFunction(_) => ("From", "from"), - TryIntoFunction(_) | TryIntoMethod => ("Into", "into"), - }; - (a.to_string(), b.to_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 { - TryFromFunction(Some(spans)) | TryIntoFunction(Some(spans)) => match spans { - TraitFn { trait_span, fn_span } => vec![(trait_span, trait_name), (fn_span, fn_name)], - Fn { fn_span } => vec![(fn_span, fn_name)], + 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)], }, - TryIntoMethod => vec![(primary_span, fn_name)], + FunctionKind::TryIntoMethod => vec![(primary_span, fn_name)], + // Or the suggestion is not machine-applicable _ => unreachable!(), }; From 3c76b2ceff3526b2d46abb1bbd38180131578808 Mon Sep 17 00:00:00 2001 From: roife Date: Thu, 8 Feb 2024 13:45:57 +0800 Subject: [PATCH 9/9] Merging the calculation of sugg and applicability --- .../unnecessary_fallible_conversions.rs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs index bb23ae5f8437..d46b584f8f44 100644 --- a/clippy_lints/src/methods/unnecessary_fallible_conversions.rs +++ b/clippy_lints/src/methods/unnecessary_fallible_conversions.rs @@ -30,14 +30,19 @@ enum FunctionKind { } impl FunctionKind { - fn applicability(&self, parent_unwrap_call: &Option) -> Applicability { - if parent_unwrap_call.is_none() { - return Applicability::Unspecified; - } + 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, - _ => Applicability::MachineApplicable, + FunctionKind::TryFromFunction(None) | FunctionKind::TryIntoFunction(None) => { + (Applicability::Unspecified, self.default_sugg(primary_span)) + }, + _ => ( + Applicability::MachineApplicable, + self.machine_applicable_sugg(primary_span, unwrap_span), + ), } } @@ -123,13 +128,7 @@ fn check<'tcx>( FunctionKind::TryFromFunction(_) => (other_ty, self_ty), }; - let applicability = kind.applicability(&parent_unwrap_call); - - let sugg = if applicability == Applicability::MachineApplicable { - kind.machine_applicable_sugg(primary_span, parent_unwrap_call.unwrap()) - } else { - kind.default_sugg(primary_span) - }; + let (applicability, sugg) = kind.appl_sugg(parent_unwrap_call, primary_span); span_lint_and_then( cx,