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

Empty doc tests should be linted #60319

Closed
phrohdoh opened this issue Apr 26, 2019 · 5 comments · Fixed by #63703
Closed

Empty doc tests should be linted #60319

phrohdoh opened this issue Apr 26, 2019 · 5 comments · Fixed by #63703
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-doctests Area: Documentation tests, run by rustdoc E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@phrohdoh
Copy link

Run tests for https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8ac29ea8121d53906e7a77cd01ccbe70 and you'll get the following diagnostic:

warning: invalid start of a new code block
 --> /playground/src/lib.rs:1:1
  |
1 | //! Hello
  | ^^^^^^^^^

But to be as helpful as possible the diagnostic should, if possible, point to the start of the invalid code block.

@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-doctests Area: Documentation tests, run by rustdoc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 26, 2019
@euclio
Copy link
Contributor

euclio commented Apr 27, 2019

This error was removed in #60140 because I didn't think it could actually be produced. Looks like it was possible to trigger by writing an empty doc test. The error message is misleading, though. It's a valid code block, just empty.

On master, this code produces a passing doc test now. IMO, that is the correct behavior. There should probably be a test for it.

@phrohdoh
Copy link
Author

There should probably be a test for it.

I agree completely.

Would this mean re-introducing this message but improving the message (replacing invalid with empty or something similar)?

I believe that is the best course of action as warning on an empty code block is completely valid and desired IMO.

@varkor varkor added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Apr 27, 2019
@euclio
Copy link
Contributor

euclio commented Apr 27, 2019

The original code only caught this incidentally. I think this check would make more sense as a rustdoc lint instead of a hard warning. Could be added to check-code-block-syntax or a new pass entirely.

@euclio
Copy link
Contributor

euclio commented May 6, 2019

@phrohdoh could you edit this issue to reflect the current status? It should be something like "Empty doc tests should be linted".

@phrohdoh phrohdoh changed the title Invalid code block warning for doc test points to start of doc instead of the code block Empty doc tests should be linted May 6, 2019
@tommilligan
Copy link
Contributor

@euclio @phrohdoh I'd like to take this issue - should have a PR this week for review.

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 A-doctests Area: Documentation tests, run by rustdoc E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants