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

Specific error message for missplaced doc comments #33922

Merged
merged 1 commit into from
Aug 20, 2016
Merged
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
9 changes: 9 additions & 0 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,15 @@ impl Handler {
self.bump_err_count();
self.panic_if_treat_err_as_bug();
}
pub fn mut_span_err<'a, S: Into<MultiSpan>>(&'a self,
sp: S,
msg: &str)
-> DiagnosticBuilder<'a> {
let mut result = DiagnosticBuilder::new(self, Level::Error, msg);
result.set_span(sp);
self.bump_err_count();
result
}
pub fn span_err_with_code<S: Into<MultiSpan>>(&self, sp: S, msg: &str, code: &str) {
self.emit_with_code(&sp.into(), msg, code, Error);
self.bump_err_count();
Expand Down
52 changes: 36 additions & 16 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,12 +578,21 @@ impl<'a> Parser<'a> {
self.bug("ident interpolation not converted to real token");
}
_ => {
let mut err = self.fatal(&format!("expected identifier, found `{}`",
self.this_token_to_string()));
if self.token == token::Underscore {
err.note("`_` is a wildcard pattern, not an identifier");
}
Err(err)
let last_token = self.last_token.clone().map(|t| *t);
Err(match last_token {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same as self.token which is being matched on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point last_token is the actual doc comment, but the failure is raised by the current self.token not being a valid element at this location, that's why we have to look back to check wether the failure is because the previous element was a doc comment or something else. This is also why we set error to self.last_span instead of self.span.

For example, with the following input file.rs:

fn foo(a: u8, b: u64) {
    /// document
}

The type of self.last_token is Some(DocComment(/// document(61))) while self.token is CloseDelim(Brace) and you get the following output with the current code:

% ./rustc file.rs
file.rs:2:5: 2:17 error: found a documentation comment that doesn't document anything
file.rs:2     /// document
              ^~~~~~~~~~~~
file.rs:2:5: 2:17 help: doc comments must come before what they document, maybe a comment was intended with `//`?

Some(token::DocComment(_)) => self.span_fatal_help(self.last_span,
"found a documentation comment that doesn't document anything",
"doc comments must come before what they document, maybe a comment was \
intended with `//`?"),
_ => {
let mut err = self.fatal(&format!("expected identifier, found `{}`",
self.this_token_to_string()));
if self.token == token::Underscore {
err.note("`_` is a wildcard pattern, not an identifier");
}
err
}
})
}
}
}
Expand Down Expand Up @@ -985,6 +994,7 @@ impl<'a> Parser<'a> {
// Stash token for error recovery (sometimes; clone is not necessarily cheap).
self.last_token = if self.token.is_ident() ||
self.token.is_path() ||
self.token.is_doc_comment() ||
self.token == token::Comma {
Some(Box::new(self.token.clone()))
} else {
Expand Down Expand Up @@ -1076,6 +1086,11 @@ impl<'a> Parser<'a> {
pub fn span_err(&self, sp: Span, m: &str) {
self.sess.span_diagnostic.span_err(sp, m)
}
pub fn span_err_help(&self, sp: Span, m: &str, h: &str) {
let mut err = self.sess.span_diagnostic.mut_span_err(sp, m);
err.help(h);
err.emit();
}
pub fn span_bug(&self, sp: Span, m: &str) -> ! {
self.sess.span_diagnostic.span_bug(sp, m)
}
Expand Down Expand Up @@ -4029,8 +4044,14 @@ impl<'a> Parser<'a> {
None => {
let unused_attrs = |attrs: &[_], s: &mut Self| {
if attrs.len() > 0 {
s.span_err(s.span,
"expected statement after outer attribute");
let last_token = s.last_token.clone().map(|t| *t);
Copy link
Member

Choose a reason for hiding this comment

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

Is last_token needed here? Shouldn't that be what's in self.token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as previous inline comment.

match last_token {
Some(token::DocComment(_)) => s.span_err_help(s.last_span,
"found a documentation comment that doesn't document anything",
"doc comments must come before what they document, maybe a \
comment was intended with `//`?"),
_ => s.span_err(s.span, "expected statement after outer attribute"),
}
}
};

Expand Down Expand Up @@ -5198,14 +5219,13 @@ impl<'a> Parser<'a> {
self.bump();
}
token::CloseDelim(token::Brace) => {}
_ => {
let span = self.span;
let token_str = self.this_token_to_string();
return Err(self.span_fatal_help(span,
&format!("expected `,`, or `}}`, found `{}`",
token_str),
"struct fields should be separated by commas"))
}
token::DocComment(_) => return Err(self.span_fatal_help(self.span,
"found a documentation comment that doesn't document anything",
"doc comments must come before what they document, maybe a comment was \
intended with `//`?")),
_ => return Err(self.span_fatal_help(self.span,
&format!("expected `,`, or `}}`, found `{}`", self.this_token_to_string()),
"struct fields should be separated by commas")),
}
Ok(a_var)
}
Expand Down
10 changes: 9 additions & 1 deletion src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl Token {
pub fn is_lit(&self) -> bool {
match *self {
Literal(_, _) => true,
_ => false,
_ => false,
}
}

Expand All @@ -214,6 +214,14 @@ impl Token {
}
}

/// Returns `true` if the token is a documentation comment.
pub fn is_doc_comment(&self) -> bool {
match *self {
DocComment(..) => true,
_ => false,
}
}

/// Returns `true` if the token is interpolated.
pub fn is_interpolated(&self) -> bool {
match *self {
Expand Down
20 changes: 20 additions & 0 deletions src/test/parse-fail/doc-after-struct-field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
struct X {
a: u8 /** document a */,
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}

fn main() {
let y = X {a = 1};
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/doc-before-extern-rbrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@

extern {
/// hi
//~^ ERROR expected item after doc comment
}
//~^^ ERROR expected item after doc comment
16 changes: 16 additions & 0 deletions src/test/parse-fail/doc-before-fn-rbrace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
fn main() {
/// document
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}
18 changes: 18 additions & 0 deletions src/test/parse-fail/doc-before-identifier.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
fn /// document
foo() {}
//~^^ ERROR expected identifier, found `/// document`
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm, but this test passes for you? Reading the parser it looks like this may fail...

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 added this test documenting the behaviour I was seeing in master on my machine, to change it later with extra info, but I thought it was explicit enough as it was. If you feel there's value in it, we might want to add a note recommending moving the doc to before the fn.

Copy link
Member

Choose a reason for hiding this comment

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

Can you ensure that other constructs like:

use foo as /// bar
bar;

work as well?


fn main() {
foo();
}
15 changes: 15 additions & 0 deletions src/test/parse-fail/doc-before-mod-rbrace.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
mod Foo {
/// document
//~^ ERROR expected item after doc comment
}
3 changes: 2 additions & 1 deletion src/test/parse-fail/doc-before-rbrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@

fn main() {
println!("Hi"); /// hi
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}
//~^ ERROR expected statement
3 changes: 2 additions & 1 deletion src/test/parse-fail/doc-before-semi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

fn main() {
/// hi
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
;
//~^ ERROR expected statement
}
21 changes: 21 additions & 0 deletions src/test/parse-fail/doc-before-struct-rbrace-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
struct X {
a: u8,
/// document
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}

fn main() {
let y = X {a = 1};
}
20 changes: 20 additions & 0 deletions src/test/parse-fail/doc-before-struct-rbrace-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2015 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z continue-parse-after-error
struct X {
a: u8 /// document
//~^ ERROR found a documentation comment that doesn't document anything
//~| HELP maybe a comment was intended
}

fn main() {
let y = X {a = 1};
}