Merge pull request #1513 from sinkuu/identical_conversion

Add identity_conversion lint

Author: oli-obk

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<NodeId>,
+}
+
+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, "<expr>").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, "<expr>").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();
+        }
+    }
+}

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,

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"];

clippy_lints/src/vec.rs

@@ -67,15 +67,15 @@ 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;
             }
         },
         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()
         },

tests/ui/identity_conversion.rs

@@ -0,0 +1,40 @@
+#![deny(identity_conversion)]
+
+fn test_generic<T: Copy>(val: T) -> T {
+    let _ = T::from(val);
+    val.into()
+}
+
+fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(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::<i32, i32>(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());
+}

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()`
+