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

turn errors from loading external docs into a proper lint #46567

Closed

Conversation

QuietMisdreavus
Copy link
Member

Part of #44732

Based on discussion surrounding the original implementation, i wanted (1) the ability to test for errors that can occur during loading the file, and (2) the ability to let people silence them if they don't care about the docs, while also making rustdoc give an error for them.

The current design goes like this:

  • libsyntax transforms a #[doc(include="filename")] into a #[doc(include(file="filename, error="error message"))] if it can't load the file (or if it's not UTF-8).
  • The new external_doc_error lint picks up this format and emits a warning if it finds any.
    • It's not an error because the documentation isn't relevant to the compilation process, but i also wanted to warn about it. I could make it allow-by-default if that makes more sense.
    • The initial implementation currently emits a warning in libsyntax that isn't covered by #[allow(warnings)].
  • Rustdoc picks up this format and emits an error if it finds any.
    • The initial implementation currently ignores these errors, since the attribute is swallowed if libsyntax hits a problem.

I added a test for it, but for some reason it doesn't seem to generate the error i expected. When running compiletest on my own system it didn't seem to generate an error or warning at all. Hence the current WIP status.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 7, 2017
@QuietMisdreavus
Copy link
Member Author

I don't get why the test isn't emitting an error for compiletest. I just copied the file out to a different project and built it with my local rustc, and it gave me exactly the error i expected ("error: couldn't read src/no-docs.md: No such file or directory (os error 2)"), but it looks like compiletest doesn't see an error at all? I don't know what i'm missing.

@QuietMisdreavus QuietMisdreavus changed the title [WIP] turn errors from loading external docs into a proper lint turn errors from loading external docs into a proper lint Dec 7, 2017
@QuietMisdreavus
Copy link
Member Author

Found it! I forgot that compile-fail tests need a main function. Kinda confusing that it didn't emit that error to compiletest, though... >_>

@ollie27
Copy link
Member

ollie27 commented Dec 7, 2017

What are the use cases for this?

@QuietMisdreavus
Copy link
Member Author

Use cases for what? This is a means of better handling errors that can arise from a feature that has already gone through the RFC process.

@ollie27
Copy link
Member

ollie27 commented Dec 7, 2017

The RFC text states:

If a file given to include is missing then this should trigger a compilation error as the given file was supposed to be put into the code but for some reason or other it is not there.

Is there a discussion somewhere about this change?

Perhaps a more specific question would be useful. When would you want rustc to emit a warning instead of an error if it can't load a file?

@QuietMisdreavus
Copy link
Member Author

Ah, I missed that it mentioned making it a total compilation error. In that case i can scrap this change and just make it a full error in libsyntax instead.

@carols10cents
Copy link
Member

In that case i can scrap this change and just make it a full error in libsyntax instead.

@QuietMisdreavus So does this comment mean this PR should be closed or are you going to be modifying this PR?

@QuietMisdreavus
Copy link
Member Author

@carols10cents I'll close this PR and open a new one.

@QuietMisdreavus QuietMisdreavus deleted the external-doc-error branch December 19, 2017 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants