From ac73474c7db65e49d3fd17906b7d34d984a88172 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Sun, 16 Aug 2020 18:33:30 -0700
Subject: [PATCH] Add explanation for `&mut self` method call when expecting
 `-> Self`

When a user tries to use a method as if it returned a new value of the
same type as its receiver, we will emit a type error. Try to detect this
and provide extra explanation that the method modifies the receiver
in-place.

This has confused people in the wild, like in
https://users.rust-lang.org/t/newbie-why-the-commented-line-stops-the-snippet-from-compiling/47322
---
 src/librustc_typeck/check/demand.rs           |  1 +
 src/librustc_typeck/check/mod.rs              | 45 +++++++++++++++++++
 .../chain-method-call-mutation-in-place.rs    |  4 ++
 ...chain-method-call-mutation-in-place.stderr | 20 +++++++++
 4 files changed, 70 insertions(+)
 create mode 100644 src/test/ui/suggestions/chain-method-call-mutation-in-place.rs
 create mode 100644 src/test/ui/suggestions/chain-method-call-mutation-in-place.stderr

diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs
index 258c5b77df2..3c367774d68 100644
--- a/src/librustc_typeck/check/demand.rs
+++ b/src/librustc_typeck/check/demand.rs
@@ -35,6 +35,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty);
         self.suggest_missing_await(err, expr, expected, expr_ty);
         self.note_need_for_fn_pointer(err, expected, expr_ty);
+        self.note_internal_mutation_in_method(err, expr, expected, expr_ty);
     }
 
     // Requires that the two types unify, and prints an error message if
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index a40b6860f77..6dd7f0661b8 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -5176,6 +5176,51 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    fn note_internal_mutation_in_method(
+        &self,
+        err: &mut DiagnosticBuilder<'_>,
+        expr: &hir::Expr<'_>,
+        expected: Ty<'tcx>,
+        found: Ty<'tcx>,
+    ) {
+        if found != self.tcx.types.unit {
+            return;
+        }
+        if let ExprKind::MethodCall(path_segment, _, [rcvr, ..], _) = expr.kind {
+            if self
+                .typeck_results
+                .borrow()
+                .expr_ty_adjusted_opt(rcvr)
+                .map_or(true, |ty| expected.peel_refs() != ty.peel_refs())
+            {
+                return;
+            }
+            let mut sp = MultiSpan::from_span(path_segment.ident.span);
+            sp.push_span_label(
+                path_segment.ident.span,
+                format!(
+                    "this call modifies {} in-place",
+                    match rcvr.kind {
+                        ExprKind::Path(QPath::Resolved(
+                            None,
+                            hir::Path { segments: [segment], .. },
+                        )) => format!("`{}`", segment.ident),
+                        _ => "its receiver".to_string(),
+                    }
+                ),
+            );
+            sp.push_span_label(
+                rcvr.span,
+                "you probably want to use this value after calling the method...".to_string(),
+            );
+            err.span_note(
+                sp,
+                &format!("method `{}` modifies its receiver in-place", path_segment.ident),
+            );
+            err.note(&format!("...instead of the `()` output of method `{}`", path_segment.ident));
+        }
+    }
+
     /// When encountering an `impl Future` where `BoxFuture` is expected, suggest `Box::pin`.
     fn suggest_calling_boxed_future_when_appropriate(
         &self,
diff --git a/src/test/ui/suggestions/chain-method-call-mutation-in-place.rs b/src/test/ui/suggestions/chain-method-call-mutation-in-place.rs
new file mode 100644
index 00000000000..cb92ab87a8f
--- /dev/null
+++ b/src/test/ui/suggestions/chain-method-call-mutation-in-place.rs
@@ -0,0 +1,4 @@
+fn main() {}
+fn foo(mut s: String) -> String {
+    s.push_str("asdf") //~ ERROR mismatched types
+}
diff --git a/src/test/ui/suggestions/chain-method-call-mutation-in-place.stderr b/src/test/ui/suggestions/chain-method-call-mutation-in-place.stderr
new file mode 100644
index 00000000000..63e3bb78954
--- /dev/null
+++ b/src/test/ui/suggestions/chain-method-call-mutation-in-place.stderr
@@ -0,0 +1,20 @@
+error[E0308]: mismatched types
+  --> $DIR/chain-method-call-mutation-in-place.rs:3:5
+   |
+LL | fn foo(mut s: String) -> String {
+   |                          ------ expected `std::string::String` because of return type
+LL |     s.push_str("asdf")
+   |     ^^^^^^^^^^^^^^^^^^ expected struct `std::string::String`, found `()`
+   |
+note: method `push_str` modifies its receiver in-place
+  --> $DIR/chain-method-call-mutation-in-place.rs:3:7
+   |
+LL |     s.push_str("asdf")
+   |     - ^^^^^^^^ this call modifies `s` in-place
+   |     |
+   |     you probably want to use this value after calling the method...
+   = note: ...instead of the `()` output of method `push_str`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.