]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/misc.rs
Update for rustc 1.19.0-nightly (4bf5c99af 2017-06-10).
[rust.git] / clippy_lints / src / misc.rs
index 885d10d2fd98d3e8f2d66206e2a4b7a362667163..fda048110fe4641e460adfb108677fc458a20477 100644 (file)
@@ -6,11 +6,12 @@
 use rustc::ty;
 use rustc_const_eval::ConstContext;
 use rustc_const_math::ConstFloat;
-use syntax::codemap::{Span, Spanned, ExpnFormat};
+use syntax::codemap::{Span, ExpnFormat};
 use utils::{get_item_name, get_parent_expr, implements_trait, in_macro, is_integer_literal, match_path, snippet,
-            span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats, in_constant};
+            span_lint, span_lint_and_then, walk_ptrs_ty, last_path_segment, iter_input_pats, in_constant,
+            match_trait_method, paths};
 use utils::sugg::Sugg;
-use syntax::ast::LitKind;
+use syntax::ast::{LitKind, CRATE_NODE_ID};
 
 /// **What it does:** Checks for function arguments and let bindings denoted as `ref`.
 ///
@@ -297,17 +298,17 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                     if let ExprPath(QPath::Resolved(_, ref path)) = right.node {
                         check_nan(cx, path, expr);
                     }
-                    check_to_owned(cx, left, right, true, cmp.span);
-                    check_to_owned(cx, right, left, false, cmp.span)
+                    check_to_owned(cx, left, right);
+                    check_to_owned(cx, right, left);
                 }
                 if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) {
                     if is_allowed(cx, left) || is_allowed(cx, right) {
                         return;
                     }
                     if let Some(name) = get_item_name(cx, expr) {
-                        let name = &*name.as_str();
+                        let name = name.as_str();
                         if name == "eq" || name == "ne" || name == "is_nan" || name.starts_with("eq_") ||
-                        name.ends_with("_eq") {
+                           name.ends_with("_eq") {
                             return;
                         }
                     }
@@ -316,17 +317,17 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                         let rhs = Sugg::hir(cx, right, "..");
 
                         db.span_suggestion(expr.span,
-                                        "consider comparing them within some error",
-                                        format!("({}).abs() < error", lhs - rhs));
+                                           "consider comparing them within some error",
+                                           format!("({}).abs() < error", lhs - rhs));
                         db.span_note(expr.span, "std::f32::EPSILON and std::f64::EPSILON are available.");
                     });
                 } else if op == BiRem && is_integer_literal(right, 1) {
                     span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
                 }
             },
-            _ => {}
+            _ => {},
         }
-        if in_attributes_expansion(cx, expr) {
+        if in_attributes_expansion(expr) {
             // Don't lint things expanded by #[derive(...)], etc
             return;
         }
@@ -335,7 +336,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                 let binding = last_path_segment(qpath).name.as_str();
                 if binding.starts_with('_') &&
                     !binding.starts_with("__") &&
-                    &*binding != "_result" && // FIXME: #944
+                    binding != "_result" && // FIXME: #944
                     is_used(cx, expr) &&
                     // don't lint if the declaration is in a macro
                     non_macro_local(cx, &cx.tables.qpath_def(qpath, expr.id)) {
@@ -378,11 +379,11 @@ fn check_pat(&mut self, cx: &LateContext<'a, 'tcx>, pat: &'tcx Pat) {
 
 fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) {
     if !in_constant(cx, expr.id) {
-        path.segments.last().map(|seg| if &*seg.name.as_str() == "NAN" {
+        path.segments.last().map(|seg| if seg.name == "NAN" {
             span_lint(cx,
-                    CMP_NAN,
-                    expr.span,
-                    "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
+                      CMP_NAN,
+                      expr.span,
+                      "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead");
         });
     }
 }
@@ -422,12 +423,11 @@ fn is_float(cx: &LateContext, expr: &Expr) -> bool {
     matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).sty, ty::TyFloat(_))
 }
 
-fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: Span) {
+fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr) {
     let (arg_ty, snip) = match expr.node {
-        ExprMethodCall(Spanned { node: ref name, .. }, _, ref args) if args.len() == 1 => {
-            let name = &*name.as_str();
-            if name == "to_string" || name == "to_owned" && is_str_arg(cx, args) {
-                (cx.tables.expr_ty(&args[0]), snippet(cx, args[0].span, ".."))
+        ExprMethodCall(.., ref args) if args.len() == 1 => {
+            if match_trait_method(cx, expr, &paths::TO_STRING) || match_trait_method(cx, expr, &paths::TO_OWNED) {
+                (cx.tables.expr_ty_adjusted(&args[0]), snippet(cx, args[0].span, ".."))
             } else {
                 return;
             }
@@ -435,7 +435,7 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: S
         ExprCall(ref path, ref v) if v.len() == 1 => {
             if let ExprPath(ref path) = path.node {
                 if match_path(path, &["String", "from_str"]) || match_path(path, &["String", "from"]) {
-                    (cx.tables.expr_ty(&v[0]), snippet(cx, v[0].span, ".."))
+                    (cx.tables.expr_ty_adjusted(&v[0]), snippet(cx, v[0].span, ".."))
                 } else {
                     return;
                 }
@@ -446,40 +446,49 @@ fn check_to_owned(cx: &LateContext, expr: &Expr, other: &Expr, left: bool, op: S
         _ => return,
     };
 
-    let other_ty = cx.tables.expr_ty(other);
+    let other_ty = cx.tables.expr_ty_adjusted(other);
     let partial_eq_trait_id = match cx.tcx.lang_items.eq_trait() {
         Some(id) => id,
         None => return,
     };
 
-    if !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty], None) {
+    // *arg impls PartialEq<other>
+    if !arg_ty
+        .builtin_deref(true, ty::LvaluePreference::NoPreference)
+        .map_or(false, |tam| implements_trait(cx, tam.ty, partial_eq_trait_id, &[other_ty]))
+        // arg impls PartialEq<*other>
+        && !other_ty
+        .builtin_deref(true, ty::LvaluePreference::NoPreference)
+        .map_or(false, |tam| implements_trait(cx, arg_ty, partial_eq_trait_id, &[tam.ty]))
+        // arg impls PartialEq<other>
+        && !implements_trait(cx, arg_ty, partial_eq_trait_id, &[other_ty]) {
         return;
     }
 
-    if left {
-        span_lint(cx,
-                  CMP_OWNED,
-                  expr.span,
-                  &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \
-                            compare without allocation",
-                           snip,
-                           snippet(cx, op, "=="),
-                           snippet(cx, other.span, "..")));
-    } else {
-        span_lint(cx,
-                  CMP_OWNED,
-                  expr.span,
-                  &format!("this creates an owned instance just for comparison. Consider using `{} {} {}` to \
-                            compare without allocation",
-                           snippet(cx, other.span, ".."),
-                           snippet(cx, op, "=="),
-                           snip));
-    }
-
-}
-
-fn is_str_arg(cx: &LateContext, args: &[Expr]) -> bool {
-    args.len() == 1 && matches!(walk_ptrs_ty(cx.tables.expr_ty(&args[0])).sty, ty::TyStr)
+    span_lint_and_then(cx,
+                       CMP_OWNED,
+                       expr.span,
+                       "this creates an owned instance just for comparison",
+                       |db| {
+        // this is as good as our recursion check can get, we can't prove that the current function is
+        // called by
+        // PartialEq::eq, but we can at least ensure that this code is not part of it
+        let parent_fn = cx.tcx.hir.get_parent(expr.id);
+        let parent_impl = cx.tcx.hir.get_parent(parent_fn);
+        if parent_impl != CRATE_NODE_ID {
+            if let map::NodeItem(item) = cx.tcx.hir.get(parent_impl) {
+                if let ItemImpl(.., Some(ref trait_ref), _, _) = item.node {
+                    if trait_ref.path.def.def_id() == partial_eq_trait_id {
+                        // we are implementing PartialEq, don't suggest not doing `to_owned`, otherwise we go into
+                        // recursion
+                        db.span_label(expr.span, "try calling implementing the comparison without allocating");
+                        return;
+                    }
+                }
+            }
+        }
+        db.span_suggestion(expr.span, "try", snip.to_string());
+    });
 }
 
 /// Heuristic to see if an expression is used. Should be compatible with `unused_variables`'s idea
@@ -498,10 +507,8 @@ fn is_used(cx: &LateContext, expr: &Expr) -> bool {
 
 /// Test whether an expression is in a macro expansion (e.g. something generated by
 /// `#[derive(...)`] or the like).
-fn in_attributes_expansion(cx: &LateContext, expr: &Expr) -> bool {
-    cx.sess().codemap().with_expn_info(expr.span.expn_id, |info_opt| {
-        info_opt.map_or(false, |info| matches!(info.callee.format, ExpnFormat::MacroAttribute(_)))
-    })
+fn in_attributes_expansion(expr: &Expr) -> bool {
+    expr.span.ctxt.outer().expn_info().map_or(false, |info| matches!(info.callee.format, ExpnFormat::MacroAttribute(_)))
 }
 
 /// Test whether `def` is a variable defined outside a macro.
@@ -510,7 +517,7 @@ fn non_macro_local(cx: &LateContext, def: &def::Def) -> bool {
         def::Def::Local(id) |
         def::Def::Upvar(id, _, _) => {
             if let Some(span) = cx.tcx.hir.span_if_local(id) {
-                !in_macro(cx, span)
+                !in_macro(span)
             } else {
                 true
             }