Auto merge of #57291 - euclio:method-call-suggestion, r=estebank

use structured suggestion for method calls

Furthermore, don't suggest calling the method if it is part of a place
expression, as this is invalid syntax.

I'm thinking it might be worth putting a label on the method assignment span like "this is a method" and removing the span from the "methods are immutable" text so it isn't reported twice.

The suggestions in `src/test/ui/did_you_mean/issue-40396.stderr` are suboptimal. I could check if the containing expression is `BinOp`, but I'm not sure if that's general enough. Any ideas?

r? @estebank

Author: bors

src/librustc_typeck/check/method/mod.rs

@@ -11,6 +11,7 @@ pub use self::CandidateSource::*;
 pub use self::suggest::{SelfSource, TraitInfo};
 
 use check::FnCtxt;
+use errors::{Applicability, DiagnosticBuilder};
 use namespace::Namespace;
 use rustc_data_structures::sync::Lrc;
 use rustc::hir;
@@ -123,6 +124,42 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    /// Add a suggestion to call the given method to the provided diagnostic.
+    crate fn suggest_method_call(
+        &self,
+        err: &mut DiagnosticBuilder<'a>,
+        msg: &str,
+        method_name: ast::Ident,
+        self_ty: Ty<'tcx>,
+        call_expr_id: ast::NodeId,
+    ) {
+        let has_params = self
+            .probe_for_name(
+                method_name.span,
+                probe::Mode::MethodCall,
+                method_name,
+                IsSuggestion(false),
+                self_ty,
+                call_expr_id,
+                ProbeScope::TraitsInScope,
+            )
+            .and_then(|pick| {
+                let sig = self.tcx.fn_sig(pick.item.def_id);
+                Ok(sig.inputs().skip_binder().len() > 1)
+            });
+
+        let (suggestion, applicability) = if has_params.unwrap_or_default() {
+            (
+                format!("{}(...)", method_name),
+                Applicability::HasPlaceholders,
+            )
+        } else {
+            (format!("{}()", method_name), Applicability::MaybeIncorrect)
+        };
+
+        err.span_suggestion_with_applicability(method_name.span, msg, suggestion, applicability);
+    }
+
     /// Performs method lookup. If lookup is successful, it will return the callee
     /// and store an appropriate adjustment for the self-expr. In some cases it may
     /// report an error (e.g., invoking the `drop` method).

src/librustc_typeck/check/mod.rs

@@ -3442,19 +3442,37 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                            "field `{}` of struct `{}` is private",
                                            field, struct_path);
             // Also check if an accessible method exists, which is often what is meant.
-            if self.method_exists(field, expr_t, expr.id, false) {
-                err.note(&format!("a method `{}` also exists, perhaps you wish to call it", field));
+            if self.method_exists(field, expr_t, expr.id, false) && !self.expr_in_place(expr.id) {
+                self.suggest_method_call(
+                    &mut err,
+                    &format!("a method `{}` also exists, call it with parentheses", field),
+                    field,
+                    expr_t,
+                    expr.id,
+                );
             }
             err.emit();
             field_ty
         } else if field.name == keywords::Invalid.name() {
             self.tcx().types.err
         } else if self.method_exists(field, expr_t, expr.id, true) {
-            type_error_struct!(self.tcx().sess, field.span, expr_t, E0615,
+            let mut err = type_error_struct!(self.tcx().sess, field.span, expr_t, E0615,
                                "attempted to take value of method `{}` on type `{}`",
-                               field, expr_t)
-                .help("maybe a `()` to call it is missing?")
-                .emit();
+                               field, expr_t);
+
+            if !self.expr_in_place(expr.id) {
+                self.suggest_method_call(
+                    &mut err,
+                    "use parentheses to call the method",
+                    field,
+                    expr_t,
+                    expr.id
+                );
+            } else {
+                err.help("methods are immutable and cannot be assigned to");
+            }
+
+            err.emit();
             self.tcx().types.err
         } else {
             if !expr_t.is_primitive_ty() {
@@ -5507,6 +5525,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             original_values,
             query_result)
     }
+
+    /// Returns whether an expression is contained inside the LHS of an assignment expression.
+    fn expr_in_place(&self, mut expr_id: ast::NodeId) -> bool {
+        let mut contained_in_place = false;
+
+        while let hir::Node::Expr(parent_expr) =
+            self.tcx.hir().get(self.tcx.hir().get_parent_node(expr_id))
+        {
+            match &parent_expr.node {
+                hir::ExprKind::Assign(lhs, ..) | hir::ExprKind::AssignOp(_, lhs, ..) => {
+                    if lhs.id == expr_id {
+                        contained_in_place = true;
+                        break;
+                    }
+                }
+                _ => (),
+            }
+            expr_id = parent_expr.id;
+        }
+
+        contained_in_place
+    }
 }
 
 pub fn check_bounds_are_used<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

src/test/ui/assign-to-method.rs

@@ -18,4 +18,5 @@ fn cat(in_x : usize, in_y : isize) -> Cat {
 fn main() {
   let nyan : Cat = cat(52, 99);
   nyan.speak = || println!("meow"); //~ ERROR attempted to take value of method
+  nyan.speak += || println!("meow"); //~ ERROR attempted to take value of method
 }

src/test/ui/assign-to-method.stderr

@@ -4,8 +4,16 @@ error[E0615]: attempted to take value of method `speak` on type `Cat`
 LL |   nyan.speak = || println!("meow"); //~ ERROR attempted to take value of method
    |        ^^^^^
    |
-   = help: maybe a `()` to call it is missing?
+   = help: methods are immutable and cannot be assigned to
 
-error: aborting due to previous error
+error[E0615]: attempted to take value of method `speak` on type `Cat`
+  --> $DIR/assign-to-method.rs:21:8
+   |
+LL |   nyan.speak += || println!("meow"); //~ ERROR attempted to take value of method
+   |        ^^^^^
+   |
+   = help: methods are immutable and cannot be assigned to
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0615`.

src/test/ui/did_you_mean/issue-40396.stderr

@@ -80,17 +80,13 @@ error[E0615]: attempted to take value of method `collect` on type `std::ops::Ran
   --> $DIR/issue-40396.rs:2:13
    |
 LL |     (0..13).collect<Vec<i32>>();
-   |             ^^^^^^^
-   |
-   = help: maybe a `()` to call it is missing?
+   |             ^^^^^^^ help: use parentheses to call the method: `collect()`
 
 error[E0615]: attempted to take value of method `collect` on type `std::ops::Range<{integer}>`
   --> $DIR/issue-40396.rs:18:13
    |
 LL |     (0..13).collect<Vec<i32>();
-   |             ^^^^^^^
-   |
-   = help: maybe a `()` to call it is missing?
+   |             ^^^^^^^ help: use parentheses to call the method: `collect()`
 
 error[E0308]: mismatched types
   --> $DIR/issue-40396.rs:18:29

src/test/ui/error-codes/E0615.stderr

@@ -2,9 +2,7 @@ error[E0615]: attempted to take value of method `method` on type `Foo`
   --> $DIR/E0615.rs:11:7
    |
 LL |     f.method; //~ ERROR E0615
-   |       ^^^^^^
-   |
-   = help: maybe a `()` to call it is missing?
+   |       ^^^^^^ help: use parentheses to call the method: `method()`
 
 error: aborting due to previous error
 

src/test/ui/implicit-method-bind.stderr

@@ -2,9 +2,7 @@ error[E0615]: attempted to take value of method `abs` on type `i32`
   --> $DIR/implicit-method-bind.rs:2:20
    |
 LL |     let _f = 10i32.abs; //~ ERROR attempted to take value of method
-   |                    ^^^
-   |
-   = help: maybe a `()` to call it is missing?
+   |                    ^^^ help: use parentheses to call the method: `abs()`
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-13853-2.stderr

@@ -2,9 +2,7 @@ error[E0615]: attempted to take value of method `get` on type `std::boxed::Box<(
   --> $DIR/issue-13853-2.rs:5:39
    |
 LL | fn foo(res : Box<ResponseHook>) { res.get } //~ ERROR attempted to take value of method
-   |                                       ^^^
-   |
-   = help: maybe a `()` to call it is missing?
+   |                                       ^^^ help: use parentheses to call the method: `get()`
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-26472.rs

@@ -8,6 +8,6 @@ mod sub {
 
 fn main() {
     let s = sub::S::new();
-    let v = s.len;
-    //~^ ERROR field `len` of struct `sub::S` is private
+    let v = s.len; //~ ERROR field `len` of struct `sub::S` is private
+    s.len = v; //~ ERROR field `len` of struct `sub::S` is private
 }

src/test/ui/issues/issue-26472.stderr

@@ -1,11 +1,17 @@
 error[E0616]: field `len` of struct `sub::S` is private
   --> $DIR/issue-26472.rs:11:13
    |
-LL |     let v = s.len;
-   |             ^^^^^
+LL |     let v = s.len; //~ ERROR field `len` of struct `sub::S` is private
+   |             ^^---
+   |               |
+   |               help: a method `len` also exists, call it with parentheses: `len()`
+
+error[E0616]: field `len` of struct `sub::S` is private
+  --> $DIR/issue-26472.rs:12:5
    |
-   = note: a method `len` also exists, perhaps you wish to call it
+LL |     s.len = v; //~ ERROR field `len` of struct `sub::S` is private
+   |     ^^^^^
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0616`.