Skip to content

Commit

Permalink
remove parent fn call check, make .try_into().unwrap()
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Oct 22, 2023
1 parent f6b64d1 commit f7df011
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 85 deletions.
47 changes: 32 additions & 15 deletions clippy_lints/src/methods/unnecessary_fallible_conversions.rs
Expand Up @@ -18,7 +18,7 @@ enum FunctionKind {
TryFromFunction,
/// `t.try_into()`
TryIntoMethod,
/// `U::try_into()`
/// `U::try_into(t)`
TryIntoFunction,
}

Expand All @@ -40,31 +40,48 @@ fn check<'tcx>(
// If `T: TryFrom<U>` and `T: From<U>` both exist, then that means that the `TryFrom`
// _must_ be from the blanket impl and cannot have been manually implemented
// (else there would be conflicting impls, even with #![feature(spec)]), so we don't even need to check
// what `<T as TryFrom<U>>::Error` is.
// what `<T as TryFrom<U>>::Error` is: it's always `Infallible`
&& implements_trait(cx, self_ty, from_into_trait, &[other_ty])
// Avoid linting if this is an argument to a function: `f(i32.try_into())`.
// The function expects a `Result` either way, so users would need to wrap it back in `Ok(..)`
// and save on nothing
&& get_parent_expr(cx, expr).map_or(true, |parent| !matches!(parent.kind, ExprKind::Call(..)))
{
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)
} else {
None
}
});

let (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

("into()", primary_span.with_hi(unwrap_span.hi()), Applicability::MachineApplicable)
}
FunctionKind::TryFromFunction => ("From::from", primary_span, Applicability::Unspecified),
FunctionKind::TryIntoFunction => ("Into::into", primary_span, Applicability::Unspecified),
FunctionKind::TryIntoMethod => ("into", primary_span, Applicability::Unspecified),
};

span_lint_and_sugg(
cx,
UNNECESSARY_FALLIBLE_CONVERSIONS,
primary_span,
span,
match kind {
FunctionKind::TryFromFunction => "calling `TryFrom::try_from` when `From` could be used",
FunctionKind::TryIntoMethod
| FunctionKind::TryIntoFunction => "calling `TryInto::try_into` when `Into` could be used",
},
"use",
match kind {
FunctionKind::TryFromFunction => "From::from".into(),
FunctionKind::TryIntoFunction => "Into::into".into(),
FunctionKind::TryIntoMethod => "into".into(),
},
// Suggestion likely results in compile errors, so it needs to be adjusted a bit by the user
// (e.g. removing `unwrap()`s as a result, changing type annotations, ...)
Applicability::Unspecified
sugg.into(),
applicability
);
}
}
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/unnecessary_fallible_conversions.fixed
@@ -0,0 +1,6 @@
#![warn(clippy::unnecessary_fallible_conversions)]

fn main() {
let _: i64 = 0i32.into();
let _: i64 = 0i32.into();
}
41 changes: 2 additions & 39 deletions tests/ui/unnecessary_fallible_conversions.rs
@@ -1,43 +1,6 @@
//@aux-build:proc_macros.rs
//@no-rustfix
#![warn(clippy::unnecessary_fallible_conversions)]

extern crate proc_macros;

struct Foo;
impl TryFrom<i32> for Foo {
type Error = ();
fn try_from(_: i32) -> Result<Self, Self::Error> {
Ok(Foo)
}
}
impl From<i64> for Foo {
fn from(_: i64) -> Self {
Foo
}
}

fn main() {
// `Foo` only implements `TryFrom<i32>` and not `From<i32>`, so don't lint
let _: Result<Foo, _> = 0i32.try_into();
let _: Result<Foo, _> = i32::try_into(0i32);
let _: Result<Foo, _> = Foo::try_from(0i32);

// ... it does impl From<i64> however
let _: Result<Foo, _> = 0i64.try_into();
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<Foo, _> = i64::try_into(0i64);
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<Foo, _> = Foo::try_from(0i64);
//~^ ERROR: calling `TryFrom::try_from` when `From` could be used

let _: Result<i64, _> = 0i32.try_into();
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<i64, _> = i32::try_into(0i32);
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<i64, _> = <_>::try_from(0i32);
//~^ ERROR: calling `TryFrom::try_from` when `From` could be used

// From a macro
let _: Result<i64, _> = proc_macros::external!(0i32).try_into();
let _: i64 = 0i32.try_into().unwrap();
let _: i64 = 0i32.try_into().expect("can't happen");
}
38 changes: 7 additions & 31 deletions tests/ui/unnecessary_fallible_conversions.stderr
@@ -1,41 +1,17 @@
error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions.rs:27:34
--> $DIR/unnecessary_fallible_conversions.rs:4:23
|
LL | let _: Result<Foo, _> = 0i64.try_into();
| ^^^^^^^^ help: use: `into`
LL | let _: i64 = 0i32.try_into().unwrap();
| ^^^^^^^^^^^^^^^^^^^ help: use: `into()`
|
= note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions.rs:29:29
--> $DIR/unnecessary_fallible_conversions.rs:5:23
|
LL | let _: Result<Foo, _> = i64::try_into(0i64);
| ^^^^^^^^^^^^^ help: use: `Into::into`
LL | let _: i64 = 0i32.try_into().expect("can't happen");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `into()`

error: calling `TryFrom::try_from` when `From` could be used
--> $DIR/unnecessary_fallible_conversions.rs:31:29
|
LL | let _: Result<Foo, _> = Foo::try_from(0i64);
| ^^^^^^^^^^^^^ help: use: `From::from`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions.rs:34:34
|
LL | let _: Result<i64, _> = 0i32.try_into();
| ^^^^^^^^ help: use: `into`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions.rs:36:29
|
LL | let _: Result<i64, _> = i32::try_into(0i32);
| ^^^^^^^^^^^^^ help: use: `Into::into`

error: calling `TryFrom::try_from` when `From` could be used
--> $DIR/unnecessary_fallible_conversions.rs:38:29
|
LL | let _: Result<i64, _> = <_>::try_from(0i32);
| ^^^^^^^^^^^^^ help: use: `From::from`

error: aborting due to 6 previous errors
error: aborting due to 2 previous errors

43 changes: 43 additions & 0 deletions tests/ui/unnecessary_fallible_conversions_unfixable.rs
@@ -0,0 +1,43 @@
//@aux-build:proc_macros.rs
//@no-rustfix
#![warn(clippy::unnecessary_fallible_conversions)]

extern crate proc_macros;

struct Foo;
impl TryFrom<i32> for Foo {
type Error = ();
fn try_from(_: i32) -> Result<Self, Self::Error> {
Ok(Foo)
}
}
impl From<i64> for Foo {
fn from(_: i64) -> Self {
Foo
}
}

fn main() {
// `Foo` only implements `TryFrom<i32>` and not `From<i32>`, so don't lint
let _: Result<Foo, _> = 0i32.try_into();
let _: Result<Foo, _> = i32::try_into(0i32);
let _: Result<Foo, _> = Foo::try_from(0i32);

// ... it does impl From<i64> however
let _: Result<Foo, _> = 0i64.try_into();
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<Foo, _> = i64::try_into(0i64);
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<Foo, _> = Foo::try_from(0i64);
//~^ ERROR: calling `TryFrom::try_from` when `From` could be used

let _: Result<i64, _> = 0i32.try_into();
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<i64, _> = i32::try_into(0i32);
//~^ ERROR: calling `TryInto::try_into` when `Into` could be used
let _: Result<i64, _> = <_>::try_from(0i32);
//~^ ERROR: calling `TryFrom::try_from` when `From` could be used

// From a macro
let _: Result<i64, _> = proc_macros::external!(0i32).try_into();
}
41 changes: 41 additions & 0 deletions tests/ui/unnecessary_fallible_conversions_unfixable.stderr
@@ -0,0 +1,41 @@
error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:27:34
|
LL | let _: Result<Foo, _> = 0i64.try_into();
| ^^^^^^^^ help: use: `into`
|
= note: `-D clippy::unnecessary-fallible-conversions` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_fallible_conversions)]`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:29:29
|
LL | let _: Result<Foo, _> = i64::try_into(0i64);
| ^^^^^^^^^^^^^ help: use: `Into::into`

error: calling `TryFrom::try_from` when `From` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:31:29
|
LL | let _: Result<Foo, _> = Foo::try_from(0i64);
| ^^^^^^^^^^^^^ help: use: `From::from`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:34:34
|
LL | let _: Result<i64, _> = 0i32.try_into();
| ^^^^^^^^ help: use: `into`

error: calling `TryInto::try_into` when `Into` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:36:29
|
LL | let _: Result<i64, _> = i32::try_into(0i32);
| ^^^^^^^^^^^^^ help: use: `Into::into`

error: calling `TryFrom::try_from` when `From` could be used
--> $DIR/unnecessary_fallible_conversions_unfixable.rs:38:29
|
LL | let _: Result<i64, _> = <_>::try_from(0i32);
| ^^^^^^^^^^^^^ help: use: `From::from`

error: aborting due to 6 previous errors

0 comments on commit f7df011

Please sign in to comment.