Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

better error message for trailing doc comment in traits (issue #56766) #57333

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5940,6 +5940,14 @@ impl<'a> Parser<'a> {
self.expect(&token::OpenDelim(token::Brace))?;
let mut trait_items = vec![];
while !self.eat(&token::CloseDelim(token::Brace)) {
if let token::DocComment(_) = self.token {
if self.look_ahead(1, |tok| tok == &token::Token::CloseDelim(token::Brace)) {
self.span_fatal_err(self.span, Error::UselessDocComment).emit();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if you used struct_span_err(...).emit() instead, and then consume the doc (as you're doing here with self.bump(). That way the parser will "succeed" and continue to type checking in the face of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried doing that, but I was unable to find a way to emit an Error without using span_fatal_err. All other methods of Parsercurrently use a &str instead. I might look deeper into the diagnostics system once I have the time. Might take a while though 😢

Copy link
Contributor

@estebank estebank Jan 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's struct_span_err_with_code which will make you duplicate some of the work. That being said 1) a very small amount of duplication is potentially ok and 2) it might be very easy to unify all uses in a single method.

Also, take your time. We're here to help if you need us.

self.bump();
continue;
}
}

let mut at_end = false;
match self.parse_trait_item(&mut at_end) {
Ok(item) => trait_items.push(item),
Expand Down Expand Up @@ -7676,7 +7684,7 @@ impl<'a> Parser<'a> {
&mut self.token_cursor.stack[prev].last_token
};

// Pull our the toekns that we've collected from the call to `f` above
// Pull our the tokens that we've collected from the call to `f` above
let mut collected_tokens = match *last_token {
LastToken::Collecting(ref mut v) => mem::replace(v, Vec::new()),
LastToken::Was(_) => panic!("our vector went away?"),
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/useless-doc-comment/trait-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//! issue #56766
//!
//! invalid doc comment at the end of a trait declaration

trait Foo {
///
fn foo(&self);
/// I am not documenting anything
//~^ ERROR: found a documentation comment that doesn't document anything [E0585]
}

fn main() {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a let _: usize =""; line to make sure the compiler recovers from the incorrect doc comment.

}
11 changes: 11 additions & 0 deletions src/test/ui/useless-doc-comment/trait-impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0585]: found a documentation comment that doesn't document anything
--> $DIR/trait-impl.rs:8:5
|
LL | /// I am not documenting anything
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: doc comments must come before what they document, maybe a comment was intended with `//`?

error: aborting due to previous error

For more information about this error, try `rustc --explain E0585`.