From b6844489f9dc3ca7aa23c3111a4576099919d65f Mon Sep 17 00:00:00 2001
From: Aaron Hill <aa1ronham@gmail.com>
Date: Sun, 17 May 2020 18:28:12 -0400
Subject: [PATCH] Emit a better diagnostic when function actually has a 'self'
 parameter

Fixes #66898

When we are unable to resolve a reference to `self`, we current assume
that the containing function doesn't have a `self` parameter, and
emit an error message accordingly.

However, if the reference to `self` was created by a macro invocation,
then resolution will correctly fail, due to hygiene. In this case, we
don't want to tell the user that the containing fuction doesn't have a
'self' paramter if it actually has one.

This PR checks for the precense of a 'self' parameter, and adjusts the
error message we emit accordingly.

TODO: The exact error message we emit could probably be improved. Should
we explicitly mention hygiene?
---
 src/librustc_resolve/late.rs                 |  5 +++--
 src/librustc_resolve/late/diagnostics.rs     | 11 ++++++++--
 src/test/ui/hygiene/missing-self-diag.rs     | 23 ++++++++++++++++++++
 src/test/ui/hygiene/missing-self-diag.stderr | 17 +++++++++++++++
 4 files changed, 52 insertions(+), 4 deletions(-)
 create mode 100644 src/test/ui/hygiene/missing-self-diag.rs
 create mode 100644 src/test/ui/hygiene/missing-self-diag.stderr

diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs
index e541920e89ed4..477e3be5cc2f8 100644
--- a/src/librustc_resolve/late.rs
+++ b/src/librustc_resolve/late.rs
@@ -347,7 +347,7 @@ struct DiagnosticMetadata<'ast> {
     currently_processing_generics: bool,
 
     /// The current enclosing function (used for better errors).
-    current_function: Option<Span>,
+    current_function: Option<(FnKind<'ast>, Span)>,
 
     /// A list of labels as of yet unused. Labels will be removed from this map when
     /// they are used (in a `break` or `continue` statement)
@@ -466,7 +466,8 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
             FnKind::Fn(FnCtxt::Free | FnCtxt::Foreign, ..) => FnItemRibKind,
             FnKind::Fn(FnCtxt::Assoc(_), ..) | FnKind::Closure(..) => NormalRibKind,
         };
-        let previous_value = replace(&mut self.diagnostic_metadata.current_function, Some(sp));
+        let previous_value =
+            replace(&mut self.diagnostic_metadata.current_function, Some((fn_kind, sp)));
         debug!("(resolving function) entering function");
         let declaration = fn_kind.decl();
 
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
index dc92b465c2bc1..b1a1f8725a180 100644
--- a/src/librustc_resolve/late/diagnostics.rs
+++ b/src/librustc_resolve/late/diagnostics.rs
@@ -195,8 +195,15 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
                 _ => "`self` value is a keyword only available in methods with a `self` parameter"
                      .to_string(),
             });
-            if let Some(span) = &self.diagnostic_metadata.current_function {
-                err.span_label(*span, "this function doesn't have a `self` parameter");
+            if let Some((fn_kind, span)) = &self.diagnostic_metadata.current_function {
+                // The current function has a `self' parameter, but we were unable to resolve
+                // a reference to `self`. This can only happen if the `self` identifier we
+                // are resolving came from a different hygiene context.
+                if fn_kind.decl().inputs.get(0).map(|p| p.is_self()).unwrap_or(false) {
+                    err.span_label(*span, "this function has a `self` parameter, but a macro invocation can only access identifiers it receives from parameters");
+                } else {
+                    err.span_label(*span, "this function doesn't have a `self` parameter");
+                }
             }
             return (err, Vec::new());
         }
diff --git a/src/test/ui/hygiene/missing-self-diag.rs b/src/test/ui/hygiene/missing-self-diag.rs
new file mode 100644
index 0000000000000..f934f793c7f27
--- /dev/null
+++ b/src/test/ui/hygiene/missing-self-diag.rs
@@ -0,0 +1,23 @@
+// Regression test for issue #66898
+// Tests that we don't emit a nonsensical error message
+// when a macro invocation tries to access `self` from a function
+// that has a 'self' parameter
+
+pub struct Foo;
+
+macro_rules! call_bar {
+    () => {
+        self.bar(); //~ ERROR expected value
+    }
+}
+
+impl Foo {
+    pub fn foo(&self) {
+        call_bar!();
+    }
+
+    pub fn bar(&self) {
+    }
+}
+
+fn main() {}
diff --git a/src/test/ui/hygiene/missing-self-diag.stderr b/src/test/ui/hygiene/missing-self-diag.stderr
new file mode 100644
index 0000000000000..075d6b76bb7b2
--- /dev/null
+++ b/src/test/ui/hygiene/missing-self-diag.stderr
@@ -0,0 +1,17 @@
+error[E0424]: expected value, found module `self`
+  --> $DIR/missing-self-diag.rs:10:9
+   |
+LL |           self.bar();
+   |           ^^^^ `self` value is a keyword only available in methods with a `self` parameter
+...
+LL | /     pub fn foo(&self) {
+LL | |         call_bar!();
+   | |         ------------ in this macro invocation
+LL | |     }
+   | |_____- this function has a `self` parameter, but a macro invocation can only access identifiers it receives from parameters
+   |
+   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0424`.