Merge pull request #230 from birkenfeld/eq_op_fix

eq_op: cut back to expressions that are guaranteed side effect free

diff --git a/Cargo.toml b/Cargo.toml
index 4b890fc7b065e5b6287d472b47434dbf5bba3cca..28fd03e5d6f0fad5143cf14039eb4a6ba053e8a0 100644 (file)
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "clippy"
-version = "0.0.11"
+version = "0.0.12"
 authors = [
        "Manish Goregaokar <manishsmail@gmail.com>",
        "Andre Bogus <bogusandre@gmail.com>",

diff --git a/src/attrs.rs b/src/attrs.rs
index 3e451ac5edaf655cdda36836f42591b4a4ef7d6d..ad021f28a4d67ef45f43044c76ea6dab12b7ba2c 100644 (file)
--- a/src/attrs.rs
+++ b/src/attrs.rs
@@ -71,14 +71,14 @@ fn is_relevant_block(block: &Block) -> bool {
             _ => ()
         }
     }
-    block.expr.as_ref().map_or(false, |e| is_relevant_expr(&*e))
+    block.expr.as_ref().map_or(false, |e| is_relevant_expr(e))
 }
 
 fn is_relevant_expr(expr: &Expr) -> bool {
     match expr.node {
         ExprBlock(ref block) => is_relevant_block(block),
         ExprRet(Some(ref e)) | ExprParen(ref e) =>
-            is_relevant_expr(&*e),
+            is_relevant_expr(e),
         ExprRet(None) | ExprBreak(_) | ExprMac(_) => false,
         ExprCall(ref path_expr, _) => {
             if let ExprPath(_, ref path) = path_expr.node {

diff --git a/src/collapsible_if.rs b/src/collapsible_if.rs
index 0b6dfc19e6b5bd1d789b17f29f5bdddfabbfbaad..e0b25b7283bf8ab8355cf8b6cf1a717eff137e25 100644 (file)
--- a/src/collapsible_if.rs
+++ b/src/collapsible_if.rs
@@ -48,7 +48,6 @@ fn check_expr_expd(cx: &Context, e: &Expr, info: Option<&ExpnInfo>) {
                 if e.span.expn_id != sp.expn_id {
                     return;
                 }
-                cx.sess().note(&format!("{:?} -- {:?}", e.span, sp));
                 span_help_and_lint(cx, COLLAPSIBLE_IF, e.span,
                     "this if statement can be collapsed",
                     &format!("try\nif {} && {} {}",
@@ -80,7 +79,7 @@ fn single_stmt_of_block(block: &Block) -> Option<&Expr> {
         } else { None }
     } else {
         if block.stmts.is_empty() {
-            if let Some(ref p) = block.expr { Some(&*p) } else { None }
+            if let Some(ref p) = block.expr { Some(p) } else { None }
         } else { None }
     }
 }

diff --git a/src/consts.rs b/src/consts.rs
index e54ac77b599f25f8dfeb27760f4c071e97fb7a5f..1a828317fc2de35a77151d2d93ecff6418c36736 100644 (file)
--- a/src/consts.rs
+++ b/src/consts.rs
@@ -222,7 +222,7 @@ fn neg_float_str(s: String) -> String {
     if s.starts_with('-') {
         s[1..].to_owned()
     } else {
-        format!("-{}", &*s)
+        format!("-{}", s)
     }
 }
 
@@ -299,7 +299,7 @@ fn expr(&mut self, e: &Expr) -> Option<Constant> {
             ExprPath(_, _) => self.fetch_path(e),
             ExprBlock(ref block) => self.block(block),
             ExprIf(ref cond, ref then, ref otherwise) =>
-                self.ifthenelse(&*cond, &*then, &*otherwise),
+                self.ifthenelse(cond, then, otherwise),
             ExprLit(ref lit) => Some(lit_to_constant(&lit.node)),
             ExprVec(ref vec) => self.multi(vec).map(ConstantVec),
             ExprTup(ref tup) => self.multi(tup).map(ConstantTuple),
@@ -362,7 +362,7 @@ fn ifthenelse(&mut self, cond: &Expr, then: &Block, otherwise: &Option<P<Expr>>)
             if b {
                 self.block(then)
             } else {
-                otherwise.as_ref().and_then(|ref expr| self.expr(expr))
+                otherwise.as_ref().and_then(|expr| self.expr(expr))
             }
         } else { None }
     }

diff --git a/src/eta_reduction.rs b/src/eta_reduction.rs
index 25e967b07e54d45df7b50349a2aa3e85ea5d2f36..481512abc6270fd8172afd542cbf82144c9c86f9 100644 (file)
--- a/src/eta_reduction.rs
+++ b/src/eta_reduction.rs
@@ -22,7 +22,7 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
             ExprCall(_, ref args) |
             ExprMethodCall(_, _, ref args) => {
                 for arg in args {
-                    check_closure(cx, &*arg)
+                    check_closure(cx, arg)
                 }
             },
             _ => (),

diff --git a/src/len_zero.rs b/src/len_zero.rs
index 5eaa0256402df186e0d37341cfc55e862ac11a11..ca3ce51bf7cc6deb3c8fafa8b421f299b186b45a 100644 (file)
--- a/src/len_zero.rs
+++ b/src/len_zero.rs
@@ -102,7 +102,7 @@ fn check_len_zero(cx: &Context, span: Span, method: &SpannedIdent,
                   args: &[P<Expr>], lit: &Lit, op: &str) {
     if let Spanned{node: LitInt(0, _), ..} = *lit {
         if method.node.name == "len" && args.len() == 1 &&
-            has_is_empty(cx, &*args[0]) {
+            has_is_empty(cx, &args[0]) {
                 span_lint(cx, LEN_ZERO, span, &format!(
                     "consider replacing the len comparison with `{}{}.is_empty()`",
                     op, snippet(cx, args[0].span, "_")))

diff --git a/src/lifetimes.rs b/src/lifetimes.rs
index 9d07df4a3ed21a5e7822326578907c08603ba585..660d68535bd5a593bb03ef8fd51825ca669844bb 100644 (file)
--- a/src/lifetimes.rs
+++ b/src/lifetimes.rs
@@ -26,14 +26,14 @@ fn check_item(&mut self, cx: &Context, item: &Item) {
 
     fn check_impl_item(&mut self, cx: &Context, item: &ImplItem) {
         if let MethodImplItem(ref sig, _) = item.node {
-            check_fn_inner(cx, &*sig.decl, Some(&sig.explicit_self),
+            check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self),
                            &sig.generics.lifetimes, item.span);
         }
     }
 
     fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) {
         if let MethodTraitItem(ref sig, _) = item.node {
-            check_fn_inner(cx, &*sig.decl, Some(&sig.explicit_self),
+            check_fn_inner(cx, &sig.decl, Some(&sig.explicit_self),
                            &sig.generics.lifetimes, item.span);
         }
     }
@@ -92,7 +92,7 @@ fn could_use_elision(func: &FnDecl, slf: Option<&ExplicitSelf>,
     }
     // extract lifetimes in input argument types
     for arg in &func.inputs {
-        walk_ty(&mut input_visitor, &*arg.ty);
+        walk_ty(&mut input_visitor, &arg.ty);
     }
     // extract lifetimes in output type
     if let Return(ref ty) = func.output {

diff --git a/src/loops.rs b/src/loops.rs
index 5f18439eafe4efbb15e5f4b96f38af8c91bdc350..ca8d3990fc5da424eb0c65297891b4d43baa6682 100644 (file)
--- a/src/loops.rs
+++ b/src/loops.rs
@@ -95,9 +95,9 @@ fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> {
             let PatEnum(_, Some(ref somepats)) = innerarms[0].pats[0].node,
             somepats.len() == 1
         ], {
-            return Some((&*somepats[0],
-                         &*iterargs[0],
-                         &*innerarms[0].body));
+            return Some((&somepats[0],
+                         &iterargs[0],
+                         &innerarms[0].body));
         }
     }
     None

diff --git a/src/matches.rs b/src/matches.rs
index 002da07f50b7180b38eb6e4b815c6c98b5f59604..d1c74daf2cd189c87515ea5cf98e1ec625ddf654 100644 (file)
--- a/src/matches.rs
+++ b/src/matches.rs
@@ -34,7 +34,7 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
                 // when an enum is extended, so we don't consider these cases
                 arms[1].pats[0].node == PatWild(PatWildSingle) &&
                 // finally, we don't want any content in the second arm (unit or empty block)
-                is_unit_expr(&*arms[1].body)
+                is_unit_expr(&arms[1].body)
             {
                 let body_code = snippet_block(cx, arms[0].body.span, "..");
                 let body_code = if let ExprBlock(_) = arms[0].body.node {
@@ -46,10 +46,10 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
                       "you seem to be trying to use match for \
                       destructuring a single pattern. Did you mean to \
                       use `if let`?",
-                      &*format!("try\nif let {} = {} {}",
-                                snippet(cx, arms[0].pats[0].span, ".."),
-                                snippet(cx, ex.span, ".."),
-                                body_code)
+                      &format!("try\nif let {} = {} {}",
+                               snippet(cx, arms[0].pats[0].span, ".."),
+                               snippet(cx, ex.span, ".."),
+                               body_code)
                 );
             }
 

diff --git a/src/methods.rs b/src/methods.rs
index df8e35d98fbcc14c19bf1166e0f60669d88bba7a..07693e11d996589389f58fa8286ac07afcc1c57a 100644 (file)
--- a/src/methods.rs
+++ b/src/methods.rs
@@ -24,7 +24,7 @@ fn get_lints(&self) -> LintArray {
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
         if let ExprMethodCall(ref ident, _, ref args) = expr.node {
-            let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0]));
+            let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&args[0]));
             if ident.node.name == "unwrap" {
                 if match_type(cx, obj_ty, &OPTION_PATH) {
                     span_lint(cx, OPTION_UNWRAP_USED, expr.span,
@@ -41,7 +41,8 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
                 if obj_ty.sty == ty::TyStr {
                     span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster");
                 } else if match_type(cx, obj_ty, &STRING_PATH) {
-                    span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op");
+                    span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op; use \
+                                                                `clone()` to make a copy");
                 }
             }
         }

diff --git a/src/misc.rs b/src/misc.rs
index 81b03db5e143eabf04b214428d515f55aaab65a2..2290af38bb5745621441cb1f5187b54b7bfde417 100644 (file)
--- a/src/misc.rs
+++ b/src/misc.rs
@@ -203,7 +203,7 @@ fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) {
 
 fn is_str_arg(cx: &Context, args: &[P<Expr>]) -> bool {
     args.len() == 1 && if let ty::TyStr =
-        walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty { true } else { false }
+        walk_ptrs_ty(cx.tcx.expr_ty(&args[0])).sty { true } else { false }
 }
 
 declare_lint!(pub MODULO_ONE, Warn, "taking a number modulo 1, which always returns 0");

diff --git a/src/needless_bool.rs b/src/needless_bool.rs
index 7671d63a35d4fde92ed9aaf90de040759c311b70..0fe52c44189cdb584d1e8e993ef9c22a3b43853e 100644 (file)
--- a/src/needless_bool.rs
+++ b/src/needless_bool.rs
@@ -5,7 +5,7 @@
 use rustc::lint::*;
 use syntax::ast::*;
 
-use utils::{de_p, span_lint, snippet};
+use utils::{span_lint, snippet};
 
 declare_lint! {
     pub NEEDLESS_BOOL,
@@ -55,7 +55,7 @@ fn check_expr(&mut self, cx: &Context, e: &Expr) {
 
 fn fetch_bool_block(block: &Block) -> Option<bool> {
     if block.stmts.is_empty() {
-        block.expr.as_ref().map(de_p).and_then(fetch_bool_expr)
+        block.expr.as_ref().and_then(|e| fetch_bool_expr(e))
     } else { None }
 }
 

diff --git a/src/ptr_arg.rs b/src/ptr_arg.rs
index 2d09fcbcca952e4e49bcbfae964756bc0ae998e8..bcbd8dad68a9c74b3f25255c32544f09f9f0bfb5 100644 (file)
--- a/src/ptr_arg.rs
+++ b/src/ptr_arg.rs
@@ -45,7 +45,7 @@ fn check_trait_item(&mut self, cx: &Context, item: &TraitItem) {
 
 fn check_fn(cx: &Context, decl: &FnDecl) {
     for arg in &decl.inputs {
-        if let Some(pat_ty) = cx.tcx.pat_ty_opt(&*arg.pat) {
+        if let Some(pat_ty) = cx.tcx.pat_ty_opt(&arg.pat) {
             if let ty::TyRef(_, ty::TypeAndMut { ty, mutbl: MutImmutable }) = pat_ty.sty {
                 if match_type(cx, ty, &VEC_PATH) {
                     span_lint(cx, PTR_ARG, arg.ty.span,

diff --git a/src/returns.rs b/src/returns.rs
index df0b93f301eae573a2923823a4d8730037229a2d..301072f79123d08598123c2fda3f7f9cef972d69 100644 (file)
--- a/src/returns.rs
+++ b/src/returns.rs
@@ -50,7 +50,7 @@ fn check_final_expr(&mut self, cx: &Context, expr: &Expr) {
             // a match expr, check all arms
             ExprMatch(_, ref arms, _) => {
                 for arm in arms {
-                    self.check_final_expr(cx, &*arm.body);
+                    self.check_final_expr(cx, &arm.body);
                 }
             }
             _ => { }
@@ -76,7 +76,7 @@ fn check_let_return(&mut self, cx: &Context, block: &Block) {
                 let PatIdent(_, Spanned { node: id, .. }, _) = local.pat.node,
                 let Some(ref retexpr) = block.expr,
                 let ExprPath(_, ref path) = retexpr.node,
-                match_path(path, &[&*id.name.as_str()])
+                match_path(path, &[&id.name.as_str()])
             ], {
                 self.emit_let_lint(cx, retexpr.span, initexpr.span);
             }

diff --git a/src/strings.rs b/src/strings.rs
index b24ea345244aeedbb9d13281558393cc0afa2b3e..d03f4d53c606b835a4e0a28f7997522b1ad2f479 100644 (file)
--- a/src/strings.rs
+++ b/src/strings.rs
@@ -70,8 +70,8 @@ fn is_add(cx: &Context, src: &Expr, target: &Expr) -> bool {
             is_exp_equal(cx, target, left),
         ExprBlock(ref block) => block.stmts.is_empty() &&
             block.expr.as_ref().map_or(false,
-                |expr| is_add(cx, &*expr, target)),
-        ExprParen(ref expr) => is_add(cx, &*expr, target),
+                |expr| is_add(cx, expr, target)),
+        ExprParen(ref expr) => is_add(cx, expr, target),
         _ => false
     }
 }

diff --git a/src/types.rs b/src/types.rs
index 4e9dd133ac8e76d57c5417bb6b8cb589b42a183d..7479a65b6ee5d18b9862cf301944d815fe8f8713 100644 (file)
--- a/src/types.rs
+++ b/src/types.rs
@@ -55,7 +55,7 @@ fn check_ty(&mut self, cx: &Context, ast_ty: &ast::Ty) {
 fn check_let_unit(cx: &Context, decl: &Decl, info: Option<&ExpnInfo>) {
     if in_macro(cx, info) { return; }
     if let DeclLocal(ref local) = decl.node {
-        let bindtype = &cx.tcx.pat_ty(&*local.pat).sty;
+        let bindtype = &cx.tcx.pat_ty(&local.pat).sty;
         if *bindtype == ty::TyTuple(vec![]) {
             span_lint(cx, LET_UNIT_VALUE, decl.span, &format!(
                 "this let-binding has unit value. Consider omitting `let {} =`",
@@ -210,7 +210,7 @@ fn get_lints(&self) -> LintArray {
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
         if let ExprCast(ref ex, _) = expr.node {
-            let (cast_from, cast_to) = (cx.tcx.expr_ty(&*ex), cx.tcx.expr_ty(expr));
+            let (cast_from, cast_to) = (cx.tcx.expr_ty(ex), cx.tcx.expr_ty(expr));
             if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx, expr.span) {
                 match (cast_from.is_integral(), cast_to.is_integral()) {
                     (true, false) => {
@@ -263,14 +263,14 @@ fn check_fn(&mut self, cx: &Context, _: FnKind, decl: &FnDecl, _: &Block, _: Spa
     }
 
     fn check_struct_field(&mut self, cx: &Context, field: &StructField) {
-        check_type(cx, &*field.node.ty);
+        check_type(cx, &field.node.ty);
     }
 
     fn check_variant(&mut self, cx: &Context, var: &Variant, _: &Generics) {
         // StructVariant is covered by check_struct_field
         if let TupleVariantKind(ref args) = var.node.kind {
             for arg in args {
-                check_type(cx, &*arg.ty);
+                check_type(cx, &arg.ty);
             }
         }
     }
@@ -312,7 +312,7 @@ fn check_local(&mut self, cx: &Context, local: &Local) {
 
 fn check_fndecl(cx: &Context, decl: &FnDecl) {
     for arg in &decl.inputs {
-        check_type(cx, &*arg.ty);
+        check_type(cx, &arg.ty);
     }
     if let Return(ref ty) = decl.output {
         check_type(cx, ty);

diff --git a/src/utils.rs b/src/utils.rs
index 5e7c63e85d9b820d73c927b48f0b07c42eae1adf..394204bedfc714e11f5ab4089b064523b8d36150 100644 (file)
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -1,7 +1,6 @@
 use rustc::lint::*;
 use syntax::ast::*;
 use syntax::codemap::{ExpnInfo, Span};
-use syntax::ptr::P;
 use rustc::ast_map::Node::NodeExpr;
 use rustc::middle::ty;
 use std::borrow::Cow;
@@ -130,9 +129,6 @@ pub fn get_parent_expr<'c>(cx: &'c Context, e: &Expr) -> Option<&'c Expr> {
         if let NodeExpr(parent) = node { Some(parent) } else { None } )
 }
 
-/// dereference a P<T> and return a ref on the result
-pub fn de_p<T>(p: &P<T>) -> &T { &*p }
-
 #[cfg(not(feature="structured_logging"))]
 pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) {
     cx.span_lint(lint, sp, msg);