]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #40815 - estebank:issue-40006, r=GuillaumeGomez
authorAriel Ben-Yehuda <arielb1@mail.tau.ac.il>
Wed, 5 Apr 2017 23:01:06 +0000 (23:01 +0000)
committerGitHub <noreply@github.com>
Wed, 5 Apr 2017 23:01:06 +0000 (23:01 +0000)
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: missing `fn` for method declaration
 --> file.rs:3:4
  |
3 |     pub hello_method(&self) {
  |        ^ missing `fn`
```

Fix #40006. r? @pnkfelix CC @jonathandturner @GuillaumeGomez

1  2 
src/libsyntax/parse/parser.rs
src/libsyntax_pos/lib.rs

index a27fc070ebec3e59877ae0606661f703f02c6eba,6a577082b526e51564019a6b6d77840e0eca44cc..8595bfc9f79c338fcff59b45315f24ff4aab5418
@@@ -551,33 -551,20 +551,33 @@@ impl<'a> Parser<'a> 
              expected.dedup();
              let expect = tokens_to_string(&expected[..]);
              let actual = self.this_token_to_string();
 -            Err(self.fatal(
 -                &(if expected.len() > 1 {
 -                    (format!("expected one of {}, found `{}`",
 -                             expect,
 -                             actual))
 -                } else if expected.is_empty() {
 -                    (format!("unexpected token: `{}`",
 -                             actual))
 +            let (msg_exp, (label_sp, label_exp)) = if expected.len() > 1 {
 +                let short_expect = if expected.len() > 6 {
 +                    format!("{} possible tokens", expected.len())
                  } else {
 -                    (format!("expected {}, found `{}`",
 -                             expect,
 -                             actual))
 -                })[..]
 -            ))
 +                    expect.clone()
 +                };
 +                (format!("expected one of {}, found `{}`", expect, actual),
 +                 (self.prev_span.next_point(), format!("expected one of {} here", short_expect)))
 +            } else if expected.is_empty() {
 +                (format!("unexpected token: `{}`", actual),
 +                 (self.prev_span, "unexpected token after this".to_string()))
 +            } else {
 +                (format!("expected {}, found `{}`", expect, actual),
 +                 (self.prev_span.next_point(), format!("expected {} here", expect)))
 +            };
 +            let mut err = self.fatal(&msg_exp);
 +            let sp = if self.token == token::Token::Eof {
 +                // This is EOF, don't want to point at the following char, but rather the last token
 +                self.prev_span
 +            } else {
 +                label_sp
 +            };
 +            err.span_label(sp, &label_exp);
 +            if !sp.source_equal(&self.span) {
 +                err.span_label(self.span, &"unexpected token");
 +            }
 +            Err(err)
          }
      }
  
          })
      }
  
-     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)
                  }
              }
          }
                           -> PResult<'a, (Ident, Vec<ast::Attribute>, 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;
              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.
+                     // Create a diagnostic pointing out that `fn` is missing.
+                     //
+                     // x |     pub path(&self) {
+                     //   |        ^ missing `fn` for method declaration
+                     err.cancel();
+                     bang_err.cancel();
+                     //     pub  path(
+                     //        ^^ `sp` below will point to this
+                     let sp = prev_span.between(self.prev_span);
+                     err = self.diagnostic()
+                         .struct_span_err(sp, "missing `fn` for method declaration");
+                     err.span_label(sp, &"missing `fn`");
+                 }
+                 return Err(err);
+             }
  
              // eat a matched-delimiter token tree:
              let (delim, tts) = self.expect_delimited_token_tree()?;
diff --combined src/libsyntax_pos/lib.rs
index 3f09b2009a795541825eb7e0c556c3bf0d49e855,86a33f4103dde7afda90d5876828cbd5c9fa827c..9b88b9f7696fb3a35f68c5165d3f5ccc59ce610f
@@@ -83,13 -83,7 +83,13 @@@ impl Span 
      /// Returns a new span representing just the end-point of this span
      pub fn end_point(self) -> Span {
          let lo = cmp::max(self.hi.0 - 1, self.lo.0);
 -        Span { lo: BytePos(lo), hi: self.hi, ctxt: self.ctxt }
 +        Span { lo: BytePos(lo), ..self }
 +    }
 +
 +    /// Returns a new span representing the next character after the end-point of this span
 +    pub fn next_point(self) -> Span {
 +        let lo = cmp::max(self.hi.0, self.lo.0 + 1);
 +        Span { lo: BytePos(lo), hi: BytePos(lo + 1), ..self }
      }
  
      /// Returns `self` if `self` is not the dummy span, and `other` otherwise.
              Span { hi: end.hi, ..self }
          }
      }
+     pub fn between(self, end: Span) -> Span {
+         Span {
+             lo: self.hi,
+             hi: end.lo,
+             ctxt: if end.ctxt == SyntaxContext::empty() {
+                 end.ctxt
+             } else {
+                 self.ctxt
+             }
+         }
+     }
+     pub fn until(self, end: Span) -> Span {
+         Span {
+             lo: self.lo,
+             hi: end.lo,
+             ctxt: if end.ctxt == SyntaxContext::empty() {
+                 end.ctxt
+             } else {
+                 self.ctxt
+             }
+         }
+     }
  }
  
  #[derive(Clone, Debug)]