mirror of
https://github.com/rust-lang/rust.git
synced 2024-12-15 10:07:30 +00:00
Auto merge of #5631 - ThibsG:ExtendUselessConversion, r=matthiaskrgr
Extend useless conversion This PR extends `useless_conversion` lint with `TryFrom` and `TryInto` fixes: #5344 changelog: Extend `useless_conversion` with `TryFrom` and `TryInto`
This commit is contained in:
commit
ee3088f27b
@ -1,14 +1,17 @@
|
||||
use crate::utils::{
|
||||
match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg,
|
||||
is_type_diagnostic_item, match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite,
|
||||
span_lint_and_help, span_lint_and_sugg,
|
||||
};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for `Into`/`From`/`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.
|
||||
///
|
||||
@ -26,7 +29,7 @@ declare_clippy_lint! {
|
||||
/// ```
|
||||
pub USELESS_CONVERSION,
|
||||
complexity,
|
||||
"calls to `Into`/`From`/`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)]
|
||||
@ -36,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() {
|
||||
@ -63,12 +67,11 @@ 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,
|
||||
e.span,
|
||||
"useless conversion",
|
||||
"useless conversion to the same type",
|
||||
"consider removing `.into()`",
|
||||
sugg,
|
||||
Applicability::MachineApplicable, // snippet
|
||||
@ -84,22 +87,70 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
|
||||
cx,
|
||||
USELESS_CONVERSION,
|
||||
e.span,
|
||||
"useless conversion",
|
||||
"useless conversion to the same type",
|
||||
"consider removing `.into_iter()`",
|
||||
sugg,
|
||||
Applicability::MachineApplicable, // snippet
|
||||
);
|
||||
}
|
||||
}
|
||||
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) => {
|
||||
if let ExprKind::Path(ref qpath) = path.kind {
|
||||
if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id() {
|
||||
if match_def_path(cx, def_id, &paths::FROM_FROM) {
|
||||
let a = cx.tables.expr_ty(e);
|
||||
let b = cx.tables.expr_ty(&args[0]);
|
||||
if same_tys(cx, a, b) {
|
||||
if_chain! {
|
||||
if args.len() == 1;
|
||||
if let ExprKind::Path(ref qpath) = path.kind;
|
||||
if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id();
|
||||
let a = cx.tables.expr_ty(e);
|
||||
let b = cx.tables.expr_ty(&args[0]);
|
||||
|
||||
then {
|
||||
if_chain! {
|
||||
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().next();
|
||||
if same_tys(cx, a_type, b);
|
||||
|
||||
then {
|
||||
let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from"));
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
USELESS_CONVERSION,
|
||||
e.span,
|
||||
"useless conversion to the same type",
|
||||
None,
|
||||
&hint,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
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"));
|
||||
@ -107,7 +158,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion {
|
||||
cx,
|
||||
USELESS_CONVERSION,
|
||||
e.span,
|
||||
"useless conversion",
|
||||
"useless conversion to the same type",
|
||||
&sugg_msg,
|
||||
sugg,
|
||||
Applicability::MachineApplicable, // snippet
|
||||
|
@ -128,8 +128,10 @@ pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"
|
||||
pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
|
||||
pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
|
||||
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"];
|
||||
|
@ -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",
|
||||
},
|
||||
|
@ -1,4 +1,4 @@
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:6:13
|
||||
|
|
||||
LL | let _ = T::from(val);
|
||||
@ -10,55 +10,55 @@ note: the lint level is defined here
|
||||
LL | #![deny(clippy::useless_conversion)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:7:5
|
||||
|
|
||||
LL | val.into()
|
||||
| ^^^^^^^^^^ help: consider removing `.into()`: `val`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:19:22
|
||||
|
|
||||
LL | let _: i32 = 0i32.into();
|
||||
| ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:51:21
|
||||
|
|
||||
LL | let _: String = "foo".to_string().into();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:52:21
|
||||
|
|
||||
LL | let _: String = From::from("foo".to_string());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:53:13
|
||||
|
|
||||
LL | let _ = String::from("foo".to_string());
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:54:13
|
||||
|
|
||||
LL | let _ = String::from(format!("A: {:04}", 123));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:55:13
|
||||
|
|
||||
LL | let _ = "".lines().into_iter();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:56:13
|
||||
|
|
||||
LL | let _ = vec![1, 2, 3].into_iter().into_iter();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()`
|
||||
|
||||
error: useless conversion
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion.rs:57:21
|
||||
|
|
||||
LL | let _: String = format!("Hello {}", "world").into();
|
||||
|
42
tests/ui/useless_conversion_try.rs
Normal file
42
tests/ui/useless_conversion_try.rs
Normal file
@ -0,0 +1,42 @@
|
||||
#![deny(clippy::useless_conversion)]
|
||||
|
||||
use std::convert::{TryFrom, TryInto};
|
||||
|
||||
fn test_generic<T: Copy>(val: T) -> T {
|
||||
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 = "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();
|
||||
let _: String = "".to_owned().try_into().unwrap();
|
||||
let _: String = match String::from("_").try_into() {
|
||||
Ok(a) => a,
|
||||
Err(_) => "".into(),
|
||||
};
|
||||
// FIXME this is a false negative
|
||||
#[allow(clippy::cmp_owned)]
|
||||
if String::from("a") == TryInto::<String>::try_into(String::from("a")).unwrap() {}
|
||||
}
|
79
tests/ui/useless_conversion_try.stderr
Normal file
79
tests/ui/useless_conversion_try.stderr
Normal file
@ -0,0 +1,79 @@
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion_try.rs:6:13
|
||||
|
|
||||
LL | let _ = T::try_from(val).unwrap();
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: the lint level is defined here
|
||||
--> $DIR/useless_conversion_try.rs:1:9
|
||||
|
|
||||
LL | #![deny(clippy::useless_conversion)]
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
= help: consider removing `T::try_from()`
|
||||
|
||||
error: useless conversion to the same type
|
||||
--> $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: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:32:13
|
||||
|
|
||||
LL | let _ = String::try_from(format!("A: {:04}", 123)).unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing `String::try_from()`
|
||||
|
||||
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: useless conversion to the same type
|
||||
--> $DIR/useless_conversion_try.rs:34:21
|
||||
|
|
||||
LL | let _: String = "".to_owned().try_into().unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing `.try_into()`
|
||||
|
||||
error: useless conversion to the same type
|
||||
--> $DIR/useless_conversion_try.rs:35:27
|
||||
|
|
||||
LL | let _: String = match String::from("_").try_into() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider removing `.try_into()`
|
||||
|
||||
error: aborting due to 9 previous errors
|
||||
|
Loading…
Reference in New Issue
Block a user