Skip to content

Add primary label to incorrect doc comments #57382

Closed
@estebank

Description

@estebank
Contributor

Desired output:

error: expected item after doc comment
  --> $DIR/doc-before-extern-rbrace.rs:2:5
   |
LL |     /// hi
   |     ^^^^^^ this doc comment doesn't document anything

Activity

added
E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
A-diagnosticsArea: Messages for errors, warnings, and lints
on Jan 6, 2019
JohnTitor

JohnTitor commented on Jan 8, 2019

@JohnTitor
Member

@estebank I came up with this change (src/libsyntax/parse/parser.rs:4478), but this may be wrong. What do you think?

self.struct_span_err(self.prev_span, message)
    .span_label(self.prev_span, "this doc comment doesn't document anything")
    .emit();
estebank

estebank commented on Jan 8, 2019

@estebank
ContributorAuthor

@JohnTitor that seems correct to me. I believe there are a couple of other places where we check for incorrect doc comments that will also need to have the span_label added.

JohnTitor

JohnTitor commented on Jan 8, 2019

@JohnTitor
Member

@estebank Okay. This is stderr after changing.

error: expected item after doc comment
  --> $DIR/doc-before-extern-rbrace.rs:2:5
   |
LL |     /// hi
   |     ^^^^^^ this doc comment doesn't document anything

error: expected one of `crate`, `fn`, `pub`, `static`, or `type`, found `}`
  --> $DIR/doc-before-extern-rbrace.rs:4:1
   |
LL |     /// hi
   |           - expected one of `crate`, `fn`, `pub`, `static`, or `type` here
LL |     //~^ ERROR expected item after doc comment
LL | }
   | ^ unexpected token

error: aborting due to 2 previous errors

This may be unfavorable since first error doesn't abort processing.

estebank

estebank commented on Jan 8, 2019

@estebank
ContributorAuthor

That's less than ideal. Could you show the diff from your change? It would be unusual the change from span_err to struct_span_err to cause that difference in output.

JohnTitor

JohnTitor commented on Jan 8, 2019

@JohnTitor
Member

here JohnTitor@9ab2320
This change isn't good because doc comments and attributes don't separate. But it wouldn't be matter.

JohnTitor

JohnTitor commented on Jan 15, 2019

@JohnTitor
Member

@estebank I found the cause, so I can add spans. But I cannot find other place to add spans, could you tell me?

estebank

estebank commented on Jan 16, 2019

@estebank
ContributorAuthor

@JohnTitor I'm not sure I understand the problem, could you expand?

JohnTitor

JohnTitor commented on Jan 16, 2019

@JohnTitor
Member

@estebank Sure.
struct_span_err outputs an error using DiagnosticBuilder, but span_err does using emit.
emit checks if continuing after error. This abort_if_errors returns FatalError.raise().

pub fn abort_if_errors(&self) {
if self.err_count() == 0 {
return;
}
FatalError.raise();
}

When I added FatalError.raise() after emit() in src/libsyntax/parse/parser.rs, there is one error(aborted process).

added
T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.
on Jan 16, 2019
JohnTitor

JohnTitor commented on Jan 20, 2019

@JohnTitor
Member

@estebank Is my solution good, or is there better solution?
If it’s not too much trouble, I'd like to make a PR and ask you to review.

estebank

estebank commented on Jan 20, 2019

@estebank
ContributorAuthor

@JohnTitor I would like to avoid having a FatalError.raise() after this. I noticed that the current method expected_item_err doesn't return anything, it should return a PResult with the DiangosticBuilder created, and be called with a ? now. That way when we encounter this error we no longer attempt to parse the item expected after, ending up again with only one error. Other than that, I think you're in the right track.

JohnTitor

JohnTitor commented on Jan 20, 2019

@JohnTitor
Member

@estebank Is this related #52790 ?

estebank

estebank commented on Jan 20, 2019

@estebank
ContributorAuthor

@JohnTitor it is related. Given that you'd start returning the DiagnosticBuilder up the call chain, you could attach a label pointing at the following token's span fairly easily.

JohnTitor

JohnTitor commented on Jan 20, 2019

@JohnTitor
Member

@estebank Okay, I want to solve this issue for now.

added a commit that references this issue on Jan 21, 2019

Rollup merge of rust-lang#57784 - JohnTitor:improve-error-message, r=…

00c60d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-diagnosticsArea: Messages for errors, warnings, and lintsE-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @estebank@GuillaumeGomez@JohnTitor

        Issue actions

          Add primary label to incorrect doc comments · Issue #57382 · rust-lang/rust