From 57009caabd2a45a6efa4d36149feec39ab0a0658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Mar 2017 23:00:21 -0700 Subject: [PATCH 1/2] Identify missing item category in `impl`s ```rust struct S; impl S { pub hello_method(&self) { println!("Hello"); } } fn main() { S.hello_method(); } ``` ```rust error: can't qualify macro invocation with `pub` --> file.rs:3:4 | 3 | pub hello_method(&self) { | ^^^- - expected `!` here for a macro invocation | | | did you mean to write `fn` here for a method declaration? | = help: try adjusting the macro to put `pub` inside the invocation ``` --- src/libsyntax/parse/parser.rs | 62 ++++++++++++++++----- src/test/ui/did_you_mean/issue-40006.rs | 21 +++++++ src/test/ui/did_you_mean/issue-40006.stderr | 12 ++++ 3 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/did_you_mean/issue-40006.rs create mode 100644 src/test/ui/did_you_mean/issue-40006.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index df4ccc94c04..a19339f8cc1 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4660,25 +4660,30 @@ impl<'a> Parser<'a> { }) } - fn complain_if_pub_macro(&mut self, visa: &Visibility, span: Span) { - match *visa { - Visibility::Inherited => (), + fn complain_if_pub_macro(&mut self, vis: &Visibility, sp: Span) { + if let Err(mut err) = self.complain_if_pub_macro_diag(vis, sp) { + err.emit(); + } + } + + fn complain_if_pub_macro_diag(&mut self, vis: &Visibility, sp: Span) -> PResult<'a, ()> { + match *vis { + Visibility::Inherited => Ok(()), _ => { let is_macro_rules: bool = match self.token { token::Ident(sid) => sid.name == Symbol::intern("macro_rules"), _ => false, }; if is_macro_rules { - self.diagnostic().struct_span_err(span, "can't qualify macro_rules \ - invocation with `pub`") - .help("did you mean #[macro_export]?") - .emit(); + let mut err = self.diagnostic() + .struct_span_err(sp, "can't qualify macro_rules invocation with `pub`"); + err.help("did you mean #[macro_export]?"); + Err(err) } else { - self.diagnostic().struct_span_err(span, "can't qualify macro \ - invocation with `pub`") - .help("try adjusting the macro to put `pub` \ - inside the invocation") - .emit(); + let mut err = self.diagnostic() + .struct_span_err(sp, "can't qualify macro invocation with `pub`"); + err.help("try adjusting the macro to put `pub` inside the invocation"); + Err(err) } } } @@ -4689,14 +4694,41 @@ impl<'a> Parser<'a> { -> PResult<'a, (Ident, Vec, ast::ImplItemKind)> { // code copied from parse_macro_use_or_failure... abstraction! if self.token.is_path_start() { - // method macro. + // Method macro. let prev_span = self.prev_span; - self.complain_if_pub_macro(&vis, prev_span); + // Before complaining about trying to set a macro as `pub`, + // check if `!` comes after the path. + let err = self.complain_if_pub_macro_diag(&vis, prev_span); let lo = self.span.lo; let pth = self.parse_path(PathStyle::Mod)?; - self.expect(&token::Not)?; + let bang_err = self.expect(&token::Not); + if let Err(mut err) = err { + if let Err(mut bang_err) = bang_err { + // Given this code `pub path(`, it seems like this is not setting the + // visibility of a macro invocation, but rather a mistyped method declaration. + // Keep the macro diagnostic, but also provide a hint that `fn` might be + // missing. Don't complain about the missing `!` as a separate diagnostic, add + // label in the appropriate place as part of one unified diagnostic. + // + // x | pub path(&self) { + // | ^^^- - expected `!` here for a macro invocation + // | | + // | did you mean to write `fn` here for a method declaration? + + bang_err.cancel(); + err.span_label(self.span, &"expected `!` here for a macro invocation"); + // pub path( + // ^^ `sp` below will point to this + let sp = mk_sp(prev_span.hi, self.prev_span.lo); + err.span_label(sp, + &"did you mean to write `fn` here for a method declaration?"); + } + return Err(err); + } else if let Err(bang_err) = bang_err { + return Err(bang_err); + } // eat a matched-delimiter token tree: let (delim, tts) = self.expect_delimited_token_tree()?; diff --git a/src/test/ui/did_you_mean/issue-40006.rs b/src/test/ui/did_you_mean/issue-40006.rs new file mode 100644 index 00000000000..cf75929bae2 --- /dev/null +++ b/src/test/ui/did_you_mean/issue-40006.rs @@ -0,0 +1,21 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct S; + +impl S { + pub hello_method(&self) { + println!("Hello"); + } +} + +fn main() { + S.hello_method(); +} diff --git a/src/test/ui/did_you_mean/issue-40006.stderr b/src/test/ui/did_you_mean/issue-40006.stderr new file mode 100644 index 00000000000..93a0c58f91a --- /dev/null +++ b/src/test/ui/did_you_mean/issue-40006.stderr @@ -0,0 +1,12 @@ +error: can't qualify macro invocation with `pub` + --> $DIR/issue-40006.rs:14:5 + | +14 | pub hello_method(&self) { + | ^^^- - expected `!` here for a macro invocation + | | + | did you mean to write `fn` here for a method declaration? + | + = help: try adjusting the macro to put `pub` inside the invocation + +error: aborting due to previous error + From c963d613a2275d5c9b31cd7124dda2f2af61deb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 27 Mar 2017 17:15:16 -0700 Subject: [PATCH 2/2] Simplify error output --- src/libsyntax/parse/parser.rs | 17 ++++++----------- src/test/ui/did_you_mean/issue-40006.stderr | 10 +++------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index a19339f8cc1..2603b3302c6 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4708,26 +4708,21 @@ impl<'a> Parser<'a> { if let Err(mut bang_err) = bang_err { // Given this code `pub path(`, it seems like this is not setting the // visibility of a macro invocation, but rather a mistyped method declaration. - // Keep the macro diagnostic, but also provide a hint that `fn` might be - // missing. Don't complain about the missing `!` as a separate diagnostic, add - // label in the appropriate place as part of one unified diagnostic. + // Create a diagnostic pointing out that `fn` is missing. // // x | pub path(&self) { - // | ^^^- - expected `!` here for a macro invocation - // | | - // | did you mean to write `fn` here for a method declaration? + // | ^ missing `fn` for method declaration + err.cancel(); bang_err.cancel(); - err.span_label(self.span, &"expected `!` here for a macro invocation"); // pub path( // ^^ `sp` below will point to this let sp = mk_sp(prev_span.hi, self.prev_span.lo); - err.span_label(sp, - &"did you mean to write `fn` here for a method declaration?"); + err = self.diagnostic() + .struct_span_err(sp, "missing `fn` for method declaration"); + err.span_label(sp, &"missing `fn`"); } return Err(err); - } else if let Err(bang_err) = bang_err { - return Err(bang_err); } // eat a matched-delimiter token tree: diff --git a/src/test/ui/did_you_mean/issue-40006.stderr b/src/test/ui/did_you_mean/issue-40006.stderr index 93a0c58f91a..460958027ad 100644 --- a/src/test/ui/did_you_mean/issue-40006.stderr +++ b/src/test/ui/did_you_mean/issue-40006.stderr @@ -1,12 +1,8 @@ -error: can't qualify macro invocation with `pub` - --> $DIR/issue-40006.rs:14:5 +error: missing `fn` for method declaration + --> $DIR/issue-40006.rs:14:8 | 14 | pub hello_method(&self) { - | ^^^- - expected `!` here for a macro invocation - | | - | did you mean to write `fn` here for a method declaration? - | - = help: try adjusting the macro to put `pub` inside the invocation + | ^ missing `fn` error: aborting due to previous error