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

Blank document comment in trait raises error different from with that in function. #56766

Closed
bkda opened this issue Dec 13, 2018 · 8 comments · Fixed by #59041 or #59606
Closed

Blank document comment in trait raises error different from with that in function. #56766

bkda opened this issue Dec 13, 2018 · 8 comments · Fixed by #59041 or #59606
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@bkda
Copy link

bkda commented Dec 13, 2018

If I use document comment that doesn't document anything in function,

fn test(){
    println!("Hello world");
    ///
}

it will raise error:

error[E0585]: found a documentation comment that doesn't document anything.

But if I use this in trait, the error confuses me.

trait User{
    fn test();
    ///
}
fn main() {
    println!("Hello world");
}
error: expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found `}`
 --> src/main.rs:4:1
  |
3 |     ///
  |        - expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe` here
4 | }
  | ^ unexpected token
@bkda bkda changed the title Null document comment in trait raise different error with that in function. Blank document comment in trait raise different error with that in function. Dec 13, 2018
@bkda bkda changed the title Blank document comment in trait raise different error with that in function. Blank document comment in trait raises different error with that in function. Dec 13, 2018
@bkda bkda changed the title Blank document comment in trait raises different error with that in function. Blank document comment in trait raises error different from with that in function. Dec 13, 2018
@bkda
Copy link
Author

bkda commented Dec 13, 2018

From https://doc.rust-lang.org/stable/error-index.html#E0585 , if I use blank document comment but not in the end, it doesn't raise E0585 error.

fn test(){
    ///
    println!("Hello world");
    
}

so should we adjust the error message of E0585 or adjust the parser?

@estebank estebank added the A-diagnostics Area: Messages for errors, warnings, and lints label Dec 13, 2018
@estebank
Copy link
Contributor

When expecting an identifier, the parser will check if the non-identifier found was a documentation comment and emit E0585:

fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, ast::Ident> {
match self.token {
token::Ident(ident, _) => {
if self.token.is_reserved_ident() {
let mut err = self.expected_ident_found();
if recover {
err.emit();
} else {
return Err(err);
}
}
let span = self.span;
self.bump();
Ok(Ident::new(ident.name, span))
}
_ => {
Err(if self.prev_token_kind == PrevTokenKind::DocComment {
self.span_fatal_err(self.prev_span, Error::UselessDocComment)
} else {
self.expected_ident_found()
})
}
}
}

An option would be to modify eat to check for doc comments in the same manner and complain that it isn't documenting anything (which should always be true, as by definition it should only be happening on parts of the parser that do not expect a doc comment):

/// Consume token 'tok' if it exists. Returns true if the given
/// token was present, false otherwise.
pub fn eat(&mut self, tok: &token::Token) -> bool {
let is_present = self.check(tok);
if is_present { self.bump() }
is_present
}

@estebank estebank added C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Dec 13, 2018
@lcnr
Copy link
Contributor

lcnr commented Dec 22, 2018

will do

@lcnr
Copy link
Contributor

lcnr commented Feb 10, 2019

I don't have a lot of time right now, in case someone else wants to fix this you are free to go ahead.

@saleemjaffer
Copy link
Contributor

@estebank Can I take a shot at this? I'm new to rust.

@estebank
Copy link
Contributor

@saleemjaffer feel free to do so! If you look at the linked PR I believe @lcnr was in the right track.

@saleemjaffer
Copy link
Contributor

@est31 I was trying to change the eat function.

I thought of checking if the current token is a DocComment, next token is a CloseDelim and the previous token is not an OpenDelim, then raise a E0585 . But PrevToken does not have an OpenDelim.

So do we proceed this way or proceed in the direction of the linked PR?

@estebank
Copy link
Contributor

@saleemjaffer you can potentially modify the following and PrevTokenKind to track OpenDelim

// Record last token kind for possible error recovery.
self.prev_token_kind = match self.token {
token::DocComment(..) => PrevTokenKind::DocComment,
token::Comma => PrevTokenKind::Comma,
token::BinOp(token::Plus) => PrevTokenKind::Plus,
token::Interpolated(..) => PrevTokenKind::Interpolated,
token::Eof => PrevTokenKind::Eof,
token::Ident(..) => PrevTokenKind::Ident,
_ => PrevTokenKind::Other,
};

but I feel that might be overkill. Why do you need ot modify the eat fn? The code that @lcnr wrote seems correct to me, if the self.bump() after identifying the bad doc comment is removed.

saleemjaffer pushed a commit to saleemjaffer/rust that referenced this issue Mar 9, 2019
Centril added a commit to Centril/rust that referenced this issue Apr 1, 2019
Centril added a commit to Centril/rust that referenced this issue Apr 1, 2019
bors added a commit that referenced this issue Apr 1, 2019
Rollup of 7 pull requests

Successful merges:

 - #58507 (Add a -Z time option which prints only passes which runs once)
 - #58919 (Suggest using anonymous lifetime in `impl Trait` return)
 - #59041 (fixes #56766)
 - #59586 (Fixed URL in cargotest::TEST_REPOS)
 - #59595 (Update rustc-guide submodule)
 - #59601 (Fix small typo)
 - #59603 (stabilize ptr::hash)

Failed merges:

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
4 participants