Auto merge of #4191 - g-bartoszek:redundant-closure-deref, r=flip1995

redundant_closure_for_method_calls fixes

lint does not trigger when there is a difference in mutability
lint does not trigger when the method belongs to a trait which is not implemebted directly (Deref)

<!--
Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only updates to the latest nightly, you can leave the
`changelog` entry as `none`. Otherwise, please write a short comment
explaining your change.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [ ] `cargo test` passes locally
- [ ] Executed `util/dev update_lints`
- [ ] Added lint documentation
- [ ] Run `cargo fmt`

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR -->

changelog: none

Author: bors

clippy_lints/src/eta_reduction.rs

@@ -6,7 +6,9 @@ use rustc::ty::{self, Ty};
 use rustc::{declare_lint_pass, declare_tool_lint};
 use rustc_errors::Applicability;
 
-use crate::utils::{is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then, type_is_unsafe_function};
+use crate::utils::{
+    implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_then, type_is_unsafe_function,
+};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for closures which just call another function where
@@ -152,7 +154,9 @@ fn get_ufcs_type_name(
     let actual_type_of_self = &cx.tables.node_type(self_arg.hir_id);
 
     if let Some(trait_id) = cx.tcx.trait_of_item(method_def_id) {
-        if match_borrow_depth(expected_type_of_self, &actual_type_of_self) {
+        if match_borrow_depth(expected_type_of_self, &actual_type_of_self)
+            && implements_trait(cx, actual_type_of_self, trait_id, &[])
+        {
             return Some(cx.tcx.def_path_str(trait_id));
         }
     }
@@ -168,7 +172,7 @@ fn get_ufcs_type_name(
 
 fn match_borrow_depth(lhs: Ty<'_>, rhs: Ty<'_>) -> bool {
     match (&lhs.sty, &rhs.sty) {
-        (ty::Ref(_, t1, _), ty::Ref(_, t2, _)) => match_borrow_depth(&t1, &t2),
+        (ty::Ref(_, t1, mut1), ty::Ref(_, t2, mut2)) => mut1 == mut2 && match_borrow_depth(&t1, &t2),
         (l, r) => match (l, r) {
             (ty::Ref(_, _, _), _) | (_, ty::Ref(_, _, _)) => false,
             (_, _) => true,
@@ -183,9 +187,8 @@ fn match_types(lhs: Ty<'_>, rhs: Ty<'_>) -> bool {
         | (ty::Int(_), ty::Int(_))
         | (ty::Uint(_), ty::Uint(_))
         | (ty::Str, ty::Str) => true,
-        (ty::Ref(_, t1, _), ty::Ref(_, t2, _))
-        | (ty::Array(t1, _), ty::Array(t2, _))
-        | (ty::Slice(t1), ty::Slice(t2)) => match_types(t1, t2),
+        (ty::Ref(_, t1, mut1), ty::Ref(_, t2, mut2)) => mut1 == mut2 && match_types(t1, t2),
+        (ty::Array(t1, _), ty::Array(t2, _)) | (ty::Slice(t1), ty::Slice(t2)) => match_types(t1, t2),
         (ty::Adt(def1, _), ty::Adt(def2, _)) => def1 == def2,
         (_, _) => false,
     }

tests/ui/eta.fixed

@@ -184,3 +184,22 @@ fn make_lazy(f: impl Fn() -> fn(u8) -> u8) -> impl Fn(u8) -> u8 {
     // called. This changes semantics, so the closure must stay.
     Box::new(move |x| f()(x))
 }
+
+fn call<F: FnOnce(&mut String) -> String>(f: F) -> String {
+    f(&mut "Hello".to_owned())
+}
+fn test_difference_in_mutability() {
+    call(|s| s.clone());
+}
+
+struct Bar;
+impl std::ops::Deref for Bar {
+    type Target = str;
+    fn deref(&self) -> &str {
+        "hi"
+    }
+}
+
+fn test_deref_with_trait_method() {
+    let _ = [Bar].iter().map(|s| s.to_string()).collect::<Vec<_>>();
+}

tests/ui/eta.rs

@@ -184,3 +184,22 @@ fn make_lazy(f: impl Fn() -> fn(u8) -> u8) -> impl Fn(u8) -> u8 {
     // called. This changes semantics, so the closure must stay.
     Box::new(move |x| f()(x))
 }
+
+fn call<F: FnOnce(&mut String) -> String>(f: F) -> String {
+    f(&mut "Hello".to_owned())
+}
+fn test_difference_in_mutability() {
+    call(|s| s.clone());
+}
+
+struct Bar;
+impl std::ops::Deref for Bar {
+    type Target = str;
+    fn deref(&self) -> &str {
+        "hi"
+    }
+}
+
+fn test_deref_with_trait_method() {
+    let _ = [Bar].iter().map(|s| s.to_string()).collect::<Vec<_>>();
+}