diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/identity_conversion.rs new file mode 100644 index 000000000..d64f352d7 --- /dev/null +++ b/clippy_lints/src/identity_conversion.rs @@ -0,0 +1,96 @@ +use rustc::lint::*; +use rustc::hir::*; +use syntax::ast::NodeId; +use utils::{in_macro, match_def_path, match_trait_method, same_tys, snippet, span_lint_and_then}; +use utils::{opt_def_id, paths, resolve_node}; + +/// **What it does:** Checks for always-identical `Into`/`From` conversions. +/// +/// **Why is this bad?** Redundant code. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// // format!() returns a `String` +/// let s: String = format!("hello").into(); +/// ``` +declare_lint! { + pub IDENTITY_CONVERSION, + Warn, + "using always-identical `Into`/`From` conversions" +} + +#[derive(Default)] +pub struct IdentityConversion { + try_desugar_arm: Vec, +} + +impl LintPass for IdentityConversion { + fn get_lints(&self) -> LintArray { + lint_array!(IDENTITY_CONVERSION) + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { + if in_macro(e.span) { + return; + } + + if Some(&e.id) == self.try_desugar_arm.last() { + return; + } + + match e.node { + ExprMatch(_, ref arms, MatchSource::TryDesugar) => { + let e = match arms[0].body.node { + ExprRet(Some(ref e)) | ExprBreak(_, Some(ref e)) => e, + _ => return, + }; + if let ExprCall(_, ref args) = e.node { + self.try_desugar_arm.push(args[0].id); + } else { + return; + } + }, + + ExprMethodCall(ref name, .., ref args) => { + if match_trait_method(cx, e, &paths::INTO[..]) && &*name.name.as_str() == "into" { + let a = cx.tables.expr_ty(e); + let b = cx.tables.expr_ty(&args[0]); + if same_tys(cx, a, b) { + let sugg = snippet(cx, args[0].span, "").into_owned(); + span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |db| { + db.span_suggestion(e.span, "consider removing `.into()`", sugg); + }); + } + } + }, + + ExprCall(ref path, ref args) => if let ExprPath(ref qpath) = path.node { + if let Some(def_id) = opt_def_id(resolve_node(cx, qpath, path.hir_id)) { + if match_def_path(cx.tcx, 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) { + let sugg = snippet(cx, args[0].span, "").into_owned(); + let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from")); + span_lint_and_then(cx, IDENTITY_CONVERSION, e.span, "identical conversion", |db| { + db.span_suggestion(e.span, &sugg_msg, sugg); + }); + } + } + } + }, + + _ => {}, + } + } + + fn check_expr_post(&mut self, _: &LateContext<'a, 'tcx>, e: &'tcx Expr) { + if Some(&e.id) == self.try_desugar_arm.last() { + self.try_desugar_arm.pop(); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 933ec1a8e..d4af88d5f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -93,6 +93,7 @@ pub mod eval_order_dependence; pub mod format; pub mod formatting; pub mod functions; +pub mod identity_conversion; pub mod identity_op; pub mod if_let_redundant_pattern_matching; pub mod if_not_else; @@ -331,6 +332,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box bytecount::ByteCount); reg.register_late_lint_pass(box infinite_iter::Pass); reg.register_late_lint_pass(box invalid_ref::InvalidRef); + reg.register_late_lint_pass(box identity_conversion::IdentityConversion::default()); reg.register_lint_group("clippy_restrictions", vec![ arithmetic::FLOAT_ARITHMETIC, @@ -431,6 +433,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { formatting::SUSPICIOUS_ELSE_FORMATTING, functions::NOT_UNSAFE_PTR_ARG_DEREF, functions::TOO_MANY_ARGUMENTS, + identity_conversion::IDENTITY_CONVERSION, identity_op::IDENTITY_OP, if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING, infinite_iter::INFINITE_ITER, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 2a2eabcca..d517d32b6 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -26,11 +26,13 @@ pub const DOUBLE_ENDED_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", pub const DROP: [&'static str; 3] = ["core", "mem", "drop"]; pub const FMT_ARGUMENTS_NEWV1: [&'static str; 4] = ["core", "fmt", "Arguments", "new_v1"]; pub const FMT_ARGUMENTV1_NEW: [&'static str; 4] = ["core", "fmt", "ArgumentV1", "new"]; +pub const FROM_FROM: [&'static str; 4] = ["core", "convert", "From", "from"]; pub const HASH: [&'static str; 2] = ["hash", "Hash"]; pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "HashMap"]; pub const HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map", "Entry"]; pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "HashSet"]; pub const INIT: [&'static str; 4] = ["core", "intrinsics", "", "init"]; +pub const INTO: [&'static str; 3] = ["core", "convert", "Into"]; pub const INTO_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "IntoIterator"]; pub const IO_PRINT: [&'static str; 4] = ["std", "io", "stdio", "_print"]; pub const IO_READ: [&'static str; 3] = ["std", "io", "Read"]; diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index 367235f7e..6044eaac3 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -67,7 +67,7 @@ fn check_vec_macro<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, vec_args: &higher::VecA .eval(len) .is_ok() { - format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")).into() + format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")) } else { return; } @@ -75,7 +75,7 @@ fn check_vec_macro<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, vec_args: &higher::VecA higher::VecArgs::Vec(args) => if let Some(last) = args.iter().last() { let span = args[0].span.to(last.span); - format!("&[{}]", snippet(cx, span, "..")).into() + format!("&[{}]", snippet(cx, span, "..")) } else { "&[]".into() }, diff --git a/tests/ui/identity_conversion.rs b/tests/ui/identity_conversion.rs new file mode 100644 index 000000000..d254b746d --- /dev/null +++ b/tests/ui/identity_conversion.rs @@ -0,0 +1,40 @@ +#![deny(identity_conversion)] + +fn test_generic(val: T) -> T { + let _ = T::from(val); + val.into() +} + +fn test_generic2 + Into, U: From>(val: T) { + // ok + let _: i32 = val.into(); + let _: U = val.into(); + let _ = U::from(val); +} + +fn test_questionmark() -> Result<(), ()> { + { + let _: i32 = 0i32.into(); + Ok(Ok(())) + }??; + Ok(()) +} + +fn main() { + test_generic(10i32); + test_generic2::(10i32); + test_questionmark().unwrap(); + + let _: String = "foo".into(); + let _: String = From::from("foo"); + let _ = String::from("foo"); + #[allow(identity_conversion)] + { + let _: String = "foo".into(); + let _ = String::from("foo"); + } + + let _: String = "foo".to_string().into(); + let _: String = From::from("foo".to_string()); + let _ = String::from("foo".to_string()); +} diff --git a/tests/ui/identity_conversion.stderr b/tests/ui/identity_conversion.stderr new file mode 100644 index 000000000..152bb8882 --- /dev/null +++ b/tests/ui/identity_conversion.stderr @@ -0,0 +1,42 @@ +error: identical conversion + --> $DIR/identity_conversion.rs:4:13 + | +4 | let _ = T::from(val); + | ^^^^^^^^^^^^ help: consider removing `T::from()`: `val` + | +note: lint level defined here + --> $DIR/identity_conversion.rs:1:9 + | +1 | #![deny(identity_conversion)] + | ^^^^^^^^^^^^^^^^^^^ + +error: identical conversion + --> $DIR/identity_conversion.rs:5:5 + | +5 | val.into() + | ^^^^^^^^^^ help: consider removing `.into()`: `val` + +error: identical conversion + --> $DIR/identity_conversion.rs:17:22 + | +17 | let _: i32 = 0i32.into(); + | ^^^^^^^^^^^ help: consider removing `.into()`: `0i32` + +error: identical conversion + --> $DIR/identity_conversion.rs:37:21 + | +37 | let _: String = "foo".to_string().into(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()` + +error: identical conversion + --> $DIR/identity_conversion.rs:38:21 + | +38 | let _: String = From::from("foo".to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()` + +error: identical conversion + --> $DIR/identity_conversion.rs:39:13 + | +39 | let _ = String::from("foo".to_string()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()` +