From: Esteban Küber <esteban@kuber.com.ar>
Date: Tue, 30 Apr 2019 02:56:50 +0000 (-0700)
Subject: review comments
X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=693eea5784fc58e9d9a8cade7a5b2afa2f3dbd3e;p=rust.git

review comments
---

diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
index 53d1ac7b024..124afc5491e 100644
--- a/src/librustc_typeck/check/_match.rs
+++ b/src/librustc_typeck/check/_match.rs
@@ -1,8 +1,8 @@
 use crate::check::{FnCtxt, Expectation, Diverges, Needs};
 use crate::check::coercion::CoerceMany;
 use crate::util::nodemap::FxHashMap;
-use errors::Applicability;
-use rustc::hir::{self, PatKind};
+use errors::{Applicability, DiagnosticBuilder};
+use rustc::hir::{self, PatKind, Pat};
 use rustc::hir::def::{Def, CtorKind};
 use rustc::hir::pat_util::EnumerateAndAdjustIterator;
 use rustc::infer;
@@ -377,64 +377,7 @@ pub fn check_pat_walk(
                             // Look for a case like `fn foo(&foo: u32)` and suggest
                             // `fn foo(foo: &u32)`
                             if let Some(mut err) = err {
-                                if let PatKind::Binding(..) = inner.node {
-                                    let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id);
-                                    let parent = tcx.hir().get_by_hir_id(parent_id);
-                                    debug!("inner {:?} pat {:?} parent {:?}", inner, pat, parent);
-                                    match parent {
-                                        hir::Node::Item(hir::Item {
-                                            node: hir::ItemKind::Fn(..), ..
-                                        }) |
-                                        hir::Node::ForeignItem(hir::ForeignItem {
-                                            node: hir::ForeignItemKind::Fn(..), ..
-                                        }) |
-                                        hir::Node::TraitItem(hir::TraitItem {
-                                            node: hir::TraitItemKind::Method(..), ..
-                                        }) |
-                                        hir::Node::ImplItem(hir::ImplItem {
-                                            node: hir::ImplItemKind::Method(..), ..
-                                        }) => { // this pat is likely an argument
-                                            if let Ok(snippet) = tcx.sess.source_map()
-                                                .span_to_snippet(inner.span)
-                                            { // FIXME: turn into structured suggestion, will need
-                                              // a span that also includes the the arg's type.
-                                                err.help(&format!(
-                                                    "did you mean `{}: &{}`?",
-                                                    snippet,
-                                                    expected,
-                                                ));
-                                            }
-                                        }
-                                        hir::Node::Expr(hir::Expr {
-                                            node: hir::ExprKind::Match(..), ..
-                                        }) => { // rely on match ergonomics
-                                            if let Ok(snippet) = tcx.sess.source_map()
-                                                .span_to_snippet(inner.span)
-                                            {
-                                                err.span_suggestion(
-                                                    pat.span,
-                                                    "you can rely on match ergonomics and remove \
-                                                     the explicit borrow",
-                                                    snippet,
-                                                    Applicability::MaybeIncorrect,
-                                                );
-                                            }
-                                        }
-                                        hir::Node::Pat(_) => {  // nested `&&pat`
-                                            if let Ok(snippet) = tcx.sess.source_map()
-                                                .span_to_snippet(inner.span)
-                                            {
-                                                err.span_suggestion(
-                                                    pat.span,
-                                                    "you can probaly remove the explicit borrow",
-                                                    snippet,
-                                                    Applicability::MaybeIncorrect,
-                                                );
-                                            }
-                                        }
-                                        _ => {} // don't provide suggestions in other cases #55175
-                                    }
-                                }
+                                self.borrow_pat_suggestion(&mut err, &pat, &inner, &expected);
                                 err.emit();
                             }
                             (rptr_ty, inner_ty)
@@ -566,6 +509,59 @@ pub fn check_pat_walk(
         // subtyping.
     }
 
+    fn borrow_pat_suggestion(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        pat: &Pat,
+        inner: &Pat,
+        expected: &Ty<'tcx>,
+    ) {
+        let tcx = self.tcx;
+        if let PatKind::Binding(..) = inner.node {
+            let parent_id = tcx.hir().get_parent_node_by_hir_id(pat.hir_id);
+            let parent = tcx.hir().get_by_hir_id(parent_id);
+            debug!("inner {:?} pat {:?} parent {:?}", inner, pat, parent);
+            match parent {
+                hir::Node::Item(hir::Item { node: hir::ItemKind::Fn(..), .. }) |
+                hir::Node::ForeignItem(hir::ForeignItem {
+                    node: hir::ForeignItemKind::Fn(..), ..
+                }) |
+                hir::Node::TraitItem(hir::TraitItem { node: hir::TraitItemKind::Method(..), .. }) |
+                hir::Node::ImplItem(hir::ImplItem { node: hir::ImplItemKind::Method(..), .. }) => {
+                    // this pat is likely an argument
+                    if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) {
+                        // FIXME: turn into structured suggestion, will need a span that also
+                        // includes the the arg's type.
+                        err.help(&format!("did you mean `{}: &{}`?", snippet, expected));
+                    }
+                }
+                hir::Node::Expr(hir::Expr { node: hir::ExprKind::Match(..), .. }) => {
+                    // rely on match ergonomics
+                    if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) {
+                        err.span_suggestion(
+                            pat.span,
+                            "you can rely on match ergonomics and remove the explicit borrow",
+                            snippet,
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
+                }
+                hir::Node::Pat(_) => {
+                    // nested `&&pat`
+                    if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(inner.span) {
+                        err.span_suggestion(
+                            pat.span,
+                            "you can probably remove the explicit borrow",
+                            snippet,
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
+                }
+                _ => {} // don't provide suggestions in other cases #55175
+            }
+        }
+    }
+
     pub fn check_dereferencable(&self, span: Span, expected: Ty<'tcx>, inner: &hir::Pat) -> bool {
         if let PatKind::Binding(..) = inner.node {
             if let Some(mt) = self.shallow_resolve(expected).builtin_deref(true) {
diff --git a/src/test/ui/destructure-trait-ref.stderr b/src/test/ui/destructure-trait-ref.stderr
index 47ecadfd01a..bc3013b78b3 100644
--- a/src/test/ui/destructure-trait-ref.stderr
+++ b/src/test/ui/destructure-trait-ref.stderr
@@ -23,7 +23,7 @@ LL |     let &&x = &1isize as &T;
    |          ^^
    |          |
    |          expected trait T, found reference
-   |          help: you can probaly remove the explicit borrow: `x`
+   |          help: you can probably remove the explicit borrow: `x`
    |
    = note: expected type `dyn T`
               found type `&_`
@@ -35,7 +35,7 @@ LL |     let &&&x = &(&1isize as &T);
    |           ^^
    |           |
    |           expected trait T, found reference
-   |           help: you can probaly remove the explicit borrow: `x`
+   |           help: you can probably remove the explicit borrow: `x`
    |
    = note: expected type `dyn T`
               found type `&_`
diff --git a/src/test/ui/mismatched_types/issue-38371.stderr b/src/test/ui/mismatched_types/issue-38371.stderr
index 833b8998c33..a9347926bda 100644
--- a/src/test/ui/mismatched_types/issue-38371.stderr
+++ b/src/test/ui/mismatched_types/issue-38371.stderr
@@ -15,7 +15,7 @@ LL | fn agh(&&bar: &u32) {
    |         ^^^^
    |         |
    |         expected u32, found reference
-   |         help: you can probaly remove the explicit borrow: `bar`
+   |         help: you can probably remove the explicit borrow: `bar`
    |
    = note: expected type `u32`
               found type `&_`
diff --git a/src/test/ui/suggestions/match-ergonomics.stderr b/src/test/ui/suggestions/match-ergonomics.stderr
index 4239089b7ce..a064e2485ff 100644
--- a/src/test/ui/suggestions/match-ergonomics.stderr
+++ b/src/test/ui/suggestions/match-ergonomics.stderr
@@ -5,7 +5,7 @@ LL |         [&v] => {},
    |          ^^
    |          |
    |          expected i32, found reference
-   |          help: you can probaly remove the explicit borrow: `v`
+   |          help: you can probably remove the explicit borrow: `v`
    |
    = note: expected type `i32`
               found type `&_`