Auto merge of #4649 - Lythenas:use_match_function_call, r=phansch

Use match_function_call wherever possible Move `match_function_call` to `utils` and use it wherever possible as discussed in #4635. changelog: none
rust-lang · Oct 12, 2019 · a865d0a · a865d0a
2 parents 4d0e897 + 15b433a
commit a865d0a
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 62 deletions.
diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs
@@ -1,6 +1,6 @@
 use crate::consts::{constant, Constant};
 use crate::utils::paths;
-use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, snippet_opt, span_help_and_lint};
+use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, snippet_opt, span_help_and_lint};
 use if_chain::if_chain;
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -145,23 +145,3 @@ fn match_assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx E
     }
     None
 }
-
-/// Matches a function call with the given path and returns the arguments.
-///
-/// Usage:
-///
-/// ```rust,ignore
-/// if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
-/// ```
-fn match_function_call<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, path: &[&str]) -> Option<&'a [Expr]> {
-    if_chain! {
-        if let ExprKind::Call(ref fun, ref args) = expr.kind;
-        if let ExprKind::Path(ref qpath) = fun.kind;
-        if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
-        if match_def_path(cx, fun_def_id, path);
-        then {
-            return Some(&args)
-        }
-    };
-    None
-}
diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs
@@ -1,4 +1,4 @@
-use crate::utils::{is_expn_of, match_def_path, paths, resolve_node, span_lint, span_lint_and_sugg};
+use crate::utils::{is_expn_of, match_function_call, paths, span_lint, span_lint_and_sugg};
 use if_chain::if_chain;
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -41,12 +41,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExplicitWrite {
             if write_fun.ident.name == sym!(write_fmt);
             // match calls to std::io::stdout() / std::io::stderr ()
             if write_args.len() > 0;
-            if let ExprKind::Call(ref dest_fun, _) = write_args[0].kind;
-            if let ExprKind::Path(ref qpath) = dest_fun.kind;
-            if let Some(dest_fun_id) = resolve_node(cx, qpath, dest_fun.hir_id).opt_def_id();
-            if let Some(dest_name) = if match_def_path(cx, dest_fun_id, &paths::STDOUT) {
+            if let Some(dest_name) = if match_function_call(cx, &write_args[0], &paths::STDOUT).is_some() {
                 Some("stdout")
-            } else if match_def_path(cx, dest_fun_id, &paths::STDERR) {
+            } else if match_function_call(cx, &write_args[0], &paths::STDERR).is_some() {
                 Some("stderr")
             } else {
                 None

diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs
@@ -1,6 +1,7 @@
 use crate::utils::paths;
 use crate::utils::{
-    is_expn_of, last_path_segment, match_def_path, match_type, resolve_node, snippet, span_lint_and_then, walk_ptrs_ty,
+    is_expn_of, last_path_segment, match_def_path, match_function_call, match_type, snippet, span_lint_and_then,
+    walk_ptrs_ty,
 };
 use if_chain::if_chain;
 use rustc::hir::*;
@@ -70,19 +71,16 @@ fn span_useless_format<T: LintContext>(cx: &T, span: Span, help: &str, mut sugg:
     });
 }
 
-fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arms: &'a [Arm]) -> Option<String> {
+fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arms: &'tcx [Arm]) -> Option<String> {
     if_chain! {
         if let ExprKind::AddrOf(_, ref format_args) = expr.kind;
         if let ExprKind::Array(ref elems) = arms[0].body.kind;
         if elems.len() == 1;
-        if let ExprKind::Call(ref fun, ref args) = elems[0].kind;
-        if let ExprKind::Path(ref qpath) = fun.kind;
-        if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id();
-        if match_def_path(cx, did, &paths::FMT_ARGUMENTV1_NEW);
+        if let Some(args) = match_function_call(cx, &elems[0], &paths::FMT_ARGUMENTV1_NEW);
         // matches `core::fmt::Display::fmt`
         if args.len() == 2;
         if let ExprKind::Path(ref qpath) = args[1].kind;
-        if let Some(did) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id();
+        if let Some(did) = cx.tables.qpath_res(qpath, args[1].hir_id).opt_def_id();
         if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD);
         // check `(arg0,)` in match block
         if let PatKind::Tuple(ref pats, None) = arms[0].pat.kind;
@@ -114,11 +112,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm
 
 fn on_new_v1<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<String> {
     if_chain! {
-        if let ExprKind::Call(ref fun, ref args) = expr.kind;
+        if let Some(args) = match_function_call(cx, expr, &paths::FMT_ARGUMENTS_NEW_V1);
         if args.len() == 2;
-        if let ExprKind::Path(ref qpath) = fun.kind;
-        if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id();
-        if match_def_path(cx, did, &paths::FMT_ARGUMENTS_NEW_V1);
         // Argument 1 in `new_v1()`
         if let ExprKind::AddrOf(_, ref arr) = args[0].kind;
         if let ExprKind::Array(ref pieces) = arr.kind;
@@ -144,11 +139,8 @@ fn on_new_v1<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<S
 
 fn on_new_v1_fmt<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<String> {
     if_chain! {
-        if let ExprKind::Call(ref fun, ref args) = expr.kind;
+        if let Some(args) = match_function_call(cx, expr, &paths::FMT_ARGUMENTS_NEW_V1_FORMATTED);
         if args.len() == 3;
-        if let ExprKind::Path(ref qpath) = fun.kind;
-        if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id();
-        if match_def_path(cx, did, &paths::FMT_ARGUMENTS_NEW_V1_FORMATTED);
         if check_unformatted(&args[2]);
         // Argument 1 in `new_v1_formatted()`
         if let ExprKind::AddrOf(_, ref arr) = args[0].kind;

diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/identity_conversion.rs
@@ -1,7 +1,6 @@
 use crate::utils::{
-    match_def_path, match_trait_method, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then,
+    match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then,
 };
-use crate::utils::{paths, resolve_node};
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_tool_lint, impl_lint_pass};
@@ -88,7 +87,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
 
             ExprKind::Call(ref path, ref args) => {
                 if let ExprKind::Path(ref qpath) = path.kind {
-                    if let Some(def_id) = resolve_node(cx, qpath, path.hir_id).opt_def_id() {
+                    if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id() {
                         if match_def_path(cx, def_id, &paths::FROM_FROM) {
                             let a = cx.tables.expr_ty(e);
                             let b = cx.tables.expr_ty(&args[0]);

diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs
@@ -1,7 +1,7 @@
 use crate::utils::{
     match_def_path,
     paths::{BEGIN_PANIC, BEGIN_PANIC_FMT},
-    resolve_node, snippet_opt, span_lint_and_then,
+    snippet_opt, span_lint_and_then,
 };
 use if_chain::if_chain;
 use rustc::{
@@ -109,7 +109,7 @@ fn expr_match(cx: &LateContext<'_, '_>, expr: &Expr) {
         ExprKind::Call(expr, ..) => {
             if_chain! {
                 if let ExprKind::Path(qpath) = &expr.kind;
-                if let Some(path_def_id) = resolve_node(cx, qpath, expr.hir_id).opt_def_id();
+                if let Some(path_def_id) = cx.tables.qpath_res(qpath, expr.hir_id).opt_def_id();
                 if match_def_path(cx, path_def_id, &BEGIN_PANIC) ||
                     match_def_path(cx, path_def_id, &BEGIN_PANIC_FMT);
                 then { }

diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs
@@ -1,6 +1,5 @@
-use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, paths, resolve_node, span_lint};
+use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, paths, span_lint};
 use if_chain::if_chain;
-use rustc::hir::ptr::P;
 use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
@@ -49,10 +48,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PanicUnimplemented {
         if_chain! {
             if let ExprKind::Block(ref block, _) = expr.kind;
             if let Some(ref ex) = block.expr;
-            if let ExprKind::Call(ref fun, ref params) = ex.kind;
-            if let ExprKind::Path(ref qpath) = fun.kind;
-            if let Some(fun_def_id) = resolve_node(cx, qpath, fun.hir_id).opt_def_id();
-            if match_def_path(cx, fun_def_id, &paths::BEGIN_PANIC);
+            if let Some(params) = match_function_call(cx, ex, &paths::BEGIN_PANIC);
             if params.len() == 2;
             then {
                 if is_expn_of(expr.span, "unimplemented").is_some() {
@@ -81,7 +77,7 @@ fn get_outer_span(expr: &Expr) -> Span {
     }
 }
 
-fn match_panic(params: &P<[Expr]>, expr: &Expr, cx: &LateContext<'_, '_>) {
+fn match_panic(params: &[Expr], expr: &Expr, cx: &LateContext<'_, '_>) {
     if_chain! {
         if let ExprKind::Lit(ref lit) = params[0].kind;
         if is_direct_expn_of(expr.span, "panic").is_some();

diff --git a/clippy_lints/src/utils/higher.rs b/clippy_lints/src/utils/higher.rs
@@ -3,7 +3,7 @@
 
 #![deny(clippy::missing_docs_in_private_items)]
 
-use crate::utils::{is_expn_of, match_def_path, match_qpath, paths, resolve_node};
+use crate::utils::{is_expn_of, match_def_path, match_qpath, paths};
 use if_chain::if_chain;
 use rustc::lint::LateContext;
 use rustc::{hir, ty};
@@ -250,9 +250,9 @@ pub enum VecArgs<'a> {
 pub fn vec_macro<'e>(cx: &LateContext<'_, '_>, expr: &'e hir::Expr) -> Option<VecArgs<'e>> {
     if_chain! {
         if let hir::ExprKind::Call(ref fun, ref args) = expr.kind;
-        if let hir::ExprKind::Path(ref path) = fun.kind;
+        if let hir::ExprKind::Path(ref qpath) = fun.kind;
         if is_expn_of(fun.span, "vec").is_some();
-        if let Some(fun_def_id) = resolve_node(cx, path, fun.hir_id).opt_def_id();
+        if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
         then {
             return if match_def_path(cx, fun_def_id, &paths::VEC_FROM_ELEM) && args.len() == 2 {
                 // `vec![elem; size]` case

diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
@@ -358,11 +358,6 @@ pub fn has_drop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool {
     }
 }
 
-/// Resolves the definition of a node from its `HirId`.
-pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> Res {
-    cx.tables.qpath_res(qpath, id)
-}
-
 /// Returns the method names and argument list of nested method call expressions that make up
 /// `expr`. method/span lists are sorted with the most recent call first.
 pub fn method_calls(expr: &Expr, max_depth: usize) -> (Vec<Symbol>, Vec<&[Expr]>, Vec<Span>) {
@@ -1085,6 +1080,30 @@ pub fn has_iter_method(cx: &LateContext<'_, '_>, probably_ref_ty: Ty<'_>) -> Opt
     None
 }
 
+/// Matches a function call with the given path and returns the arguments.
+///
+/// Usage:
+///
+/// ```rust,ignore
+/// if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
+/// ```
+pub fn match_function_call<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    expr: &'tcx Expr,
+    path: &[&str],
+) -> Option<&'tcx [Expr]> {
+    if_chain! {
+        if let ExprKind::Call(ref fun, ref args) = expr.kind;
+        if let ExprKind::Path(ref qpath) = fun.kind;
+        if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
+        if match_def_path(cx, fun_def_id, path);
+        then {
+            return Some(&args)
+        }
+    };
+    None
+}
+
 #[cfg(test)]
 mod test {
     use super::{trim_multiline, without_block_comments};