Replace match_method_chain() with method_chain_args()

src/methods.rs

@@ -5,9 +5,10 @@ use rustc::middle::subst::{Subst, TypeSpace};
 use std::iter;
 use std::borrow::Cow;
 
-use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, match_method_chain,
+use utils::{snippet, span_lint, span_note_and_lint, match_path, match_type, method_chain_args,
             walk_ptrs_ty_depth, walk_ptrs_ty};
 use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
+use utils::MethodArgs;
 
 use self::SelfKind::*;
 use self::OutType::*;
@@ -158,20 +159,20 @@ impl LintPass for MethodsPass {
 impl LateLintPass for MethodsPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if let ExprMethodCall(_, _, _) = expr.node {
-            if match_method_chain(expr, &["unwrap"]) {
+            if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
-                lint_unwrap(cx, expr);
+                lint_unwrap(cx, expr, arglists[0]);
             }
-            else if match_method_chain(expr, &["to_string"]) {
+            else if let Some(arglists) = method_chain_args(expr, &["to_string"]) {
-                lint_to_string(cx, expr);
+                lint_to_string(cx, expr, arglists[0]);
             }
-            else if match_method_chain(expr, &["ok", "expect"]) {
+            else if let Some(arglists) = method_chain_args(expr, &["ok", "expect"]) {
-                lint_ok_expect(cx, expr);
+                lint_ok_expect(cx, expr, arglists[0]);
             }
-            else if match_method_chain(expr, &["map", "unwrap_or"]) {
+            else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or"]) {
-                lint_map_unwrap_or(cx, expr);
+                lint_map_unwrap_or(cx, expr, arglists[0], arglists[1]);
             }
-            else if match_method_chain(expr, &["map", "unwrap_or_else"]) {
+            else if let Some(arglists) = method_chain_args(expr, &["map", "unwrap_or_else"]) {
-                lint_map_unwrap_or_else(cx, expr);
+                lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
             }
         }
     }
@@ -218,15 +219,10 @@ impl LateLintPass for MethodsPass {
     }
 }
 
+#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
 /// lint use of `unwrap()` for `Option`s and `Result`s
-fn lint_unwrap(cx: &LateContext, expr: &Expr) {
+fn lint_unwrap(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs) {
-    let args = match expr.node {
+    let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&unwrap_args[0]));
-        ExprMethodCall(_, _, ref args) => args,
-        _ => panic!("clippy methods.rs: should not have called `lint_unwrap()` on a non-matching \
-                     expression!"),
-    };
-
-    let (obj_ty, _) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
 
     if match_type(cx, obj_ty, &OPTION_PATH) {
         span_lint(cx, OPTION_UNWRAP_USED, expr.span,
@@ -239,18 +235,13 @@ fn lint_unwrap(cx: &LateContext, expr: &Expr) {
     }
 }
 
+#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
 /// lint use of `to_string()` for `&str`s and `String`s
-fn lint_to_string(cx: &LateContext, expr: &Expr) {
+fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) {
-    let args = match expr.node {
+    let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&to_string_args[0]));
-        ExprMethodCall(_, _, ref args) => args,
-        _ => panic!("clippy methods.rs: should not have called `lint_to_string()` on a \
-                     non-matching expression!"),
-    };
-
-    let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
 
     if obj_ty.sty == ty::TyStr {
-        let mut arg_str = snippet(cx, args[0].span, "_");
+        let mut arg_str = snippet(cx, to_string_args[0].span, "_");
         if ptr_depth > 1 {
             arg_str = Cow::Owned(format!(
                 "({}{})",
@@ -266,19 +257,9 @@ fn lint_to_string(cx: &LateContext, expr: &Expr) {
     }
 }
 
+#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
 /// lint use of `ok().expect()` for `Result`s
-fn lint_ok_expect(cx: &LateContext, expr: &Expr) {
+fn lint_ok_expect(cx: &LateContext, expr: &Expr, ok_args: &MethodArgs) {
-    let expect_args = match expr.node {
-        ExprMethodCall(_, _, ref expect_args) => expect_args,
-        _ => panic!("clippy methods.rs: Should not have called `lint_ok_expect()` on a \
-                     non-matching expression!")
-    };
-    let ok_args = match expect_args[0].node {
-        ExprMethodCall(_, _, ref ok_args) => ok_args,
-        _ => panic!("clippy methods.rs: Should not have called `lint_ok_expect()` on a \
-                     non-matching expression!")
-    };
-
     // lint if the caller of `ok()` is a `Result`
     if match_type(cx, cx.tcx.expr_ty(&ok_args[0]), &RESULT_PATH) {
         let result_type = cx.tcx.expr_ty(&ok_args[0]);
@@ -292,19 +273,10 @@ fn lint_ok_expect(cx: &LateContext, expr: &Expr) {
     }
 }
 
+#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
 /// lint use of `map().unwrap_or()` for `Option`s
-fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr) {
+fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs,
-    let unwrap_args = match expr.node {
+                      map_args: &MethodArgs) {
-        ExprMethodCall(_, _, ref unwrap_args) => unwrap_args,
-        _ => panic!("clippy methods.rs: Should not have called `lint_map_unwrap_or()` on a \
-                     non-matching expression!")
-    };
-    let map_args = match unwrap_args[0].node {
-        ExprMethodCall(_, _, ref map_args) => map_args,
-        _ => panic!("clippy methods.rs: Should not have called `lint_map_unwrap_or()` on a \
-                     non-matching expression!")
-    };
-
     // lint if the caller of `map()` is an `Option`
     if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
         // lint message
@@ -330,19 +302,11 @@ fn lint_map_unwrap_or(cx: &LateContext, expr: &Expr) {
     }
 }
 
+#[allow(ptr_arg)] // Type of MethodArgs is potentially a Vec
 /// lint use of `map().unwrap_or_else()` for `Option`s
-fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr) {
+fn lint_map_unwrap_or_else(cx: &LateContext, expr: &Expr, unwrap_args: &MethodArgs,
-    let unwrap_args = match expr.node {
+                           map_args: &MethodArgs) {
-        ExprMethodCall(_, _, ref unwrap_args) => unwrap_args,
+    // lint if the caller of `map()` is an `Option`
-        _ => panic!("clippy methods.rs: Should not have called `lint_map_unwrap_or_else()` on a \
-                     non-matching expression!")
-    };
-    let map_args = match unwrap_args[0].node {
-        ExprMethodCall(_, _, ref map_args) => map_args,
-        _ => panic!("clippy methods.rs: Should not have called `lint_map_unwrap_or_else()` on a \
-                     non-matching expression!")
-    };
-
     if match_type(cx, cx.tcx.expr_ty(&map_args[0]), &OPTION_PATH) {
         // lint message
         let msg = "called `map(f).unwrap_or_else(g)` on an Option value. This can be done more \

src/utils.rs

@@ -8,10 +8,13 @@ use rustc::middle::ty;
 use std::borrow::Cow;
 use syntax::ast::Lit_::*;
 use syntax::ast;
+use syntax::ptr::P;
 
 use rustc::session::Session;
 use std::str::FromStr;
 
+pub type MethodArgs = HirVec<P<Expr>>;
+
 // module DefPaths for certain structs/enums we check for
 pub const OPTION_PATH: [&'static str; 3] = ["core", "option", "Option"];
 pub const RESULT_PATH: [&'static str; 3] = ["core", "result", "Result"];
@@ -164,24 +167,28 @@ pub fn match_path_ast(path: &ast::Path, segments: &[&str]) -> bool {
         |(a, b)| a.identifier.name.as_str() == *b)
 }
 
-/// match an Expr against a chain of methods. For example, if `expr` represents the `.baz()` in
+/// match an Expr against a chain of methods, and return the matched Exprs. For example, if `expr`
-/// `foo.bar().baz()`, `matched_method_chain(expr, &["bar", "baz"])` will return true.
+/// represents the `.baz()` in `foo.bar().baz()`, `matched_method_chain(expr, &["bar", "baz"])`
-pub fn match_method_chain(expr: &Expr, methods: &[&str]) -> bool {
+/// will return a Vec containing the Exprs for `.bar()` and `.baz()`
-    let mut current = &expr.node ;
+pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option<Vec<&'a MethodArgs>> {
+    let mut current = expr;
+    let mut matched = Vec::with_capacity(methods.len());
     for method_name in methods.iter().rev() { // method chains are stored last -> first
-        if let ExprMethodCall(ref name, _, ref args) = *current {
+        if let ExprMethodCall(ref name, _, ref args) = current.node {
             if name.node.as_str() == *method_name {
-                current = &args[0].node
+                matched.push(args); // build up `matched` backwards
+                current = &args[0] // go to parent expression
             }
             else {
-                return false;
+                return None;
             }
         }
         else {
-            return false;
+            return None;
         }
     }
-    true
+    matched.reverse(); // reverse `matched`, so that it is in the same order as `methods`
+    Some(matched)
 }