Skip to content

Commit

Permalink
Extend useless_conversion lint with TryInto
Browse files Browse the repository at this point in the history
  • Loading branch information
ThibsG committed May 25, 2020
1 parent 4f8909f commit 705bfdc
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 19 deletions.
38 changes: 31 additions & 7 deletions clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// **What it does:** Checks for `Into`, `From`, `TryFrom`,`IntoIter` calls that useless converts
/// to the same type as caller.
/// **What it does:** Checks for `Into`, `TryInto`, `From`, `TryFrom`,`IntoIter` calls
/// that useless converts to the same type as caller.
///
/// **Why is this bad?** Redundant code.
///
Expand All @@ -29,7 +29,7 @@ declare_clippy_lint! {
/// ```
pub USELESS_CONVERSION,
complexity,
"calls to `Into`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type"
"calls to `Into`, `TryInto`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type"
}

#[derive(Default)]
Expand All @@ -39,6 +39,7 @@ pub struct UselessConversion {

impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);

#[allow(clippy::too_many_lines)]
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
if e.span.from_expansion() {
Expand Down Expand Up @@ -66,7 +67,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
let b = cx.tables.expr_ty(&args[0]);
if same_tys(cx, a, b) {
let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();

span_lint_and_sugg(
cx,
USELESS_CONVERSION,
Expand Down Expand Up @@ -94,6 +94,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
);
}
}
if match_trait_method(cx, e, &paths::TRY_INTO_TRAIT) && &*name.ident.as_str() == "try_into" {
if_chain! {
let a = cx.tables.expr_ty(e);
let b = cx.tables.expr_ty(&args[0]);
if is_type_diagnostic_item(cx, a, sym!(result_type));
if let ty::Adt(_, substs) = a.kind;
if let Some(a_type) = substs.types().next();
if same_tys(cx, a_type, b);

then {
span_lint_and_help(
cx,
USELESS_CONVERSION,
e.span,
"Useless conversion to the same type",
None,
"consider removing `.try_into()`",
);
}
}
}
},

ExprKind::Call(ref path, ref args) => {
Expand All @@ -109,7 +130,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
if match_def_path(cx, def_id, &paths::TRY_FROM);
if is_type_diagnostic_item(cx, a, sym!(result_type));
if let ty::Adt(_, substs) = a.kind;
if let Some(a_type) = substs.types().nth(0);
if let Some(a_type) = substs.types().next();
if same_tys(cx, a_type, b);

then {
Expand All @@ -125,8 +146,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
}
}

if match_def_path(cx, def_id, &paths::FROM_FROM) {
if same_tys(cx, a, b) {
if_chain! {
if match_def_path(cx, def_id, &paths::FROM_FROM);
if same_tys(cx, a, b);

then {
let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
let sugg_msg =
format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
pub const TRY_FROM: [&str; 4] = ["core", "convert", "TryFrom", "try_from"];
pub const TRY_FROM_ERROR: [&str; 4] = ["std", "ops", "Try", "from_error"];
pub const TRY_INTO_RESULT: [&str; 4] = ["std", "ops", "Try", "into_result"];
pub const TRY_INTO_TRAIT: [&str; 3] = ["core", "convert", "TryInto"];
pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"];
pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2421,7 +2421,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
Lint {
name: "useless_conversion",
group: "complexity",
desc: "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type",
desc: "calls to `Into`, `TryInto`, `From`, `TryFrom`, `IntoIter` that performs useless conversions to the same type",
deprecation: None,
module: "useless_conversion",
},
Expand Down
17 changes: 13 additions & 4 deletions tests/ui/useless_conversion_try.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,34 @@
#![deny(clippy::useless_conversion)]

use std::convert::TryFrom;
use std::convert::{TryFrom, TryInto};

fn test_generic<T: Copy>(val: T) -> T {
T::try_from(val).unwrap()
let _ = T::try_from(val).unwrap();
val.try_into().unwrap()
}

fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
// ok
let _: i32 = val.try_into().unwrap();
let _: U = val.try_into().unwrap();
let _ = U::try_from(val).unwrap();
}

fn main() {
test_generic(10i32);
test_generic2::<i32, i32>(10i32);

let _: String = "foo".try_into().unwrap();
let _: String = TryFrom::try_from("foo").unwrap();
let _ = String::try_from("foo").unwrap();
#[allow(clippy::useless_conversion)]
let _ = String::try_from("foo").unwrap();

{
let _ = String::try_from("foo").unwrap();
let _: String = "foo".try_into().unwrap();
}
let _: String = "foo".to_string().try_into().unwrap();
let _: String = TryFrom::try_from("foo".to_string()).unwrap();
let _ = String::try_from("foo".to_string()).unwrap();
let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
let _: String = format!("Hello {}", "world").try_into().unwrap();
}
38 changes: 31 additions & 7 deletions tests/ui/useless_conversion_try.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:6:5
--> $DIR/useless_conversion_try.rs:6:13
|
LL | T::try_from(val).unwrap()
| ^^^^^^^^^^^^^^^^
LL | let _ = T::try_from(val).unwrap();
| ^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/useless_conversion_try.rs:1:9
Expand All @@ -12,28 +12,52 @@ LL | #![deny(clippy::useless_conversion)]
= help: consider removing `T::try_from()`

error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:22:21
--> $DIR/useless_conversion_try.rs:7:5
|
LL | val.try_into().unwrap()
| ^^^^^^^^^^^^^^
|
= help: consider removing `.try_into()`

error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:29:21
|
LL | let _: String = "foo".to_string().try_into().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing `.try_into()`

error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:30:21
|
LL | let _: String = TryFrom::try_from("foo".to_string()).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing `TryFrom::try_from()`

error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:23:13
--> $DIR/useless_conversion_try.rs:31:13
|
LL | let _ = String::try_from("foo".to_string()).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing `String::try_from()`

error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:24:13
--> $DIR/useless_conversion_try.rs:32:13
|
LL | let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing `String::try_from()`

error: aborting due to 4 previous errors
error: Useless conversion to the same type
--> $DIR/useless_conversion_try.rs:33:21
|
LL | let _: String = format!("Hello {}", "world").try_into().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider removing `.try_into()`

error: aborting due to 7 previous errors

0 comments on commit 705bfdc

Please sign in to comment.