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

Add a cfail mode for doc comments #24565

Closed
Manishearth opened this issue Apr 18, 2015 · 16 comments
Closed

Add a cfail mode for doc comments #24565

Manishearth opened this issue Apr 18, 2015 · 16 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

Many times the docs mention an error message in non-compiling Rust code to illustrate a concept. For example, the move semantics chapter illustrates how moves can cause errors.

However, it's possible that the error message may update in the future, or that the code (which is marked as rust,ignore and doesn't get fed to the doctests) will fall prey to a different error. Both (especially the latter) can be confusing to a newbie.

I propose we add a "cfail" mode to code blocks, which runs the code as a compile-fail test (via compiletest), and will fail the doctest if the error doesn't match. Something like this:

```rust,cfail
let v = vec![1, 2, 3];

let v2 = v;

println!("v[0] is: {}", v[0]);
//~^ ERROR use of moved value `v`
``‌`

We could choose to make the inline error hidden, though it might actually be helpful and could be kept visible.

@steveklabnik steveklabnik added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. A-an-interesting-project labels Apr 18, 2015
@lambda-fairy
Copy link
Contributor

👍 This would be great for documenting syntax extensions and such.

@bandali
Copy link

bandali commented May 28, 2015

I would like to work on this, if someone could mentor me, since this is my first ever contribution to a language / a project this big :)

@Manishearth
Copy link
Member Author

Happy to see your enthusiasm!

I'm not familiar with this code, but @cmr and @alexcrichton , they should be able to give you some pointers.

This might not be very easy, mind you, and it might require lots of changes to compiletest-rs. Note that compiletest-rs has been taken out of the tree unofficially here, so that implementation might give some pointers.

(compiletest is the test runner for the stuff you find in tests/, and it lets us specify error patterns amongst other things. If you can find a way of feeding little snippets from the docs to compiletest, you're effectively done!)

@bandali
Copy link

bandali commented May 28, 2015

@Manishearth Thank you for your super quick reply!

I see. I guess it's probably a better idea if I start with some Easy issues and work my way up. Although if @cmr or @alexcrichton could guide me, I might be able to do it

I was looking at fn run_cfail_test which I assume is related to what we want to do, but that's all I know so far.

@brson
Copy link
Contributor

brson commented May 28, 2015

Per my conversation with @aminb on IRC this change will be to rustdoc, which is responsible for all doc testing.

@Manishearth
Copy link
Member Author

Yeah; but I bet it will require some ctest changes too to hook them all up.

@alexcrichton
Copy link
Member

Running cfail tests is currently all managed through the in-tree compiletest binary, which we do not distribute out of tree (and probably do not want to). I would personally recommend start out getting standalone test cases to work before rustdoc test cases, but at a high level this may be the most plausible course of action:

  1. Move compiletest to using 100% stable code. This way an out-of-tree copy could be created/kept up to date which can compile against stable Rust. Eventually this could perhaps become a Cargo package and/or dependency.
  2. Move much of compiletest support to a library instead of an executable. This will allow rustdoc to link against it when running doc tests.
  3. Modify rustdoc to detect cfail test blocks and run the compiler, calling into compiletest to parse the compiler output and check everything is A-OK.

Hope that helps!

@carols10cents
Copy link
Member

cc @gankro because of IRL OH

@Gankra
Copy link
Contributor

Gankra commented Jul 30, 2015

So apparently rustdoc eats all leading % lines. We could in principle add rustdoc feature flags like

% my title
% feature(cfail)

blha blah balh ablj

@steveklabnik
Copy link
Member

Triage: no change.

@steveklabnik steveklabnik added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 18, 2017
@QuietMisdreavus
Copy link
Member

Update: Currently, rustdoc can handle doctests tagged with compile_fail if it's a nightly build. You can also add one or more error codes that must result from the compilation for the doctest to "pass".

It's not quite what the issue is asking for (you can't match the error text), but it's somewhat there.

@GuillaumeGomez
Copy link
Member

@QuietMisdreavus: It's working on stable as well too normally. Unless I missed something?

@kennytm
Copy link
Member

kennytm commented Aug 17, 2017

@GuillaumeGomez It should be nightly-only

let mut allow_compile_fail = false;
let mut allow_error_code_check = false;
if UnstableFeatures::from_environment().is_nightly_build() {
allow_compile_fail = true;
allow_error_code_check = true;
}

@GuillaumeGomez
Copy link
Member

Indeed, changing this: #43949.

@GuillaumeGomez
Copy link
Member

It's now in stable. I suppose we can close it. :)

@QuietMisdreavus
Copy link
Member

The basic "this test should not compile" is there, but the OP also asked for something like what compiletest uses, to match error messages to lines where they're triggered. That part is not available in rustdoc. @Manishearth would you consider this issue solved by just the compile_fail flag?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests