rustdoc: Add an attribute to ignore collecting tests per-item.

[emilio]

This allows to ignore tests in modules we know they have uneffective tests, or
tests not under our control, like for example in:

rust-lang/rust-bindgen#378

r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[GuillaumeGomez]

"inclussive"? ;)

[emilio]

Gah, I don't know how to write, amended, thanks :)

[GuillaumeGomez]

It's awesome! Thanks a lot for this!

[emilio]

Ping?

[GuillaumeGomez]

Since it touches rustdoc source code, I'd prefer to have @rust-lang/tools confirmation first. Let's wait for them to take a look.

[alexcrichton]

@emilio could you expand on the motivation for this? I was under the assumption that ignore annotations or such on markdown blocks would be sufficient?

I'd also want to consider the impact of syntax highlighting here as well. I think rustdoc interprets blocks by default as Rust code, so it seems like an attribute would be inevitably needed anyway to turn that off?

[emilio]

The motivation is that, in bindgen, we don't have control of those markdown blocks, because they belong to third-party libraries.

Mainly, consider a C library that defines the following:

/**
 * This function does something really impressive.
 *
 * Example usage:
 *
 * ```
 * int main() {
 *     do_foo();
 * }
 * ```
 */
void do_foo();

Bindgen tries to preserve documentation, and thus the generate code looks like:

extern "C" {
    /**
     * This function does something really impressive.
     *
     * Example usage:
     *
     * ```
     * int main() {
     *     do_foo();
     * }
     * ```
     */
    pub fn do_foo();
}

Running doctests in this crate obviously fails (because the code in the doc block is not valid rust).

We can solve it downstream processing comments (trying to add ignore annotations) and whatnot, but I think this solution is more elegant.

[alexcrichton]

It seems like this would always be a bug in bindgen? This doesn't really fix the problem because we'll still syntax highlight the foreign blocks copied in as Rust, and possibly error out entirely if they're invalid Rust.

[emilio]

Well, this at least allows running cargo test without failures in a bindgen-generated crate. That has been the only reported error so far. If there's any other, like broken syntax highlighting, probably more checks should be added to prevent this, but I think an attribute that says "please don't make any assumptions about the doc blocks in this module" is helpful.

[alexcrichton]

My point is basically that this is not a complete bug fix. Just because a bug hasn't been reported against bindgen doesn't mean that it shouldn't be fixed. A solution for both of these problems (testing everything and not assuming rust) may look similar. For example bindgen could avoid copying markdown blocks or modify markdown blocks. Similarly rustdoc could have one attribute to control this.

Designing only one solution without considering the other runs a risk of leaving us with a feature in rustdoc we never had in the first place

[alexcrichton]

cc @rust-lang/docs, perhaps y'all've got more thoughts?

[emilio]

Sorry BTW, I haven't forgotten about this, I'm just on exams :) I think at one point I tried to build a crate with bindgen with bogus doc comments, and this was the only problem I found (rustdoc gives up gracefully if it fails to parse the code as Rust, so the syntax highlighting problem doesn't exist as far as I can tell, modulo a warning IIRC). I'd like to get this merged, but if you think I should add checks to other places, or have other suggestions or solutions, I'm happy implementing them when I find the time. If you think this is a unique enough problem this doesn't deserve a solution in rustdoc, and I should just modify the comments in bindgen, I guess I can take that, though adding this built into rustdoc is _way_ easier and more elegant IMO.

…

On Tue, Jan 17, 2017 at 01:17:11PM -0800, Alex Crichton wrote: cc @rust-lang/docs, perhaps y'all've got more thoughts? -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #38825 (comment)

[frewsxcv]

I think I'm -1 on this. It's not obvious to me that the compiler should support this use case. To me, it seems like bindgen is doing the wrong thing here.

Documentation code blocks without a specified programming language are assumed to be Rust and will be syntax highlighted as so. Won't syntax highlighting also be a problem for these code blocks?

[emilio]

Read above, this is not a problem because rustdoc happily skips it if it can't parse it as rust, but I suggest above I could also do the check there if need be.

[alexcrichton]

@emilio I'm not personally convinced by that aspect. If foreign code just happens to lex as Rust code then we still shouldn't highlight it as Rust?

[bors]

☔ The latest upstream changes (presumably #39633) made this pull request unmergeable. Please resolve the merge conflicts.

[emilio]

I guess, though I'd assume that's uncommon enough. Anyway, we're going to need to process those anyway it seems, if only to avoid rust compile errors when others use /*! style comments, so this is no longer saving us any work.

Thanks for all the feedback! :)