Tracking issue: rfc 1990 - add external doc attribute to rustc

[nrc]

RFC PR: rust-lang/rfcs#1990
RFC text: https://github.com/rust-lang/rfcs/blob/master/text/1990-external-doc-attribute.md

Current documentation

---

Summary of how to use this:

1. Add #![feature(external_doc)] to your crate.
2. Add a file "src/some-docs.md" with some docs to your crate.
3. Add an attribute #[doc(include = "some-docs.md")] to something that needs some docs. The file path is relative to lib.rs, so if you want a doc folder to live alongside src, then all your paths inside the doc(include) attributes need to begin with ../doc.

Summary of current status:

• Initial implementation landed in #44781, on 2017-11-22.
• Updates based on conversation in this thread landed in #46858, on 2017-12-21.
• Please use this in your own projects and comment in this thread if something goes wrong! (Or even if it goes well! Knowing it's working as intended is great!)

---

Current tasks:

• Land #44781
• (promote the "failed to load file" warnings to proper lints?) (Make them hard errors instead, per the RFC #46858)
• Ensure line number info is properly preserved in doctest errors

[matklad]

cc @joshtriplett

At today's Cargo meeting, and interesting question was raised how Cargo should check if it should rebuild the docs when an external file with docs changes.

Here's some info on how dependency-tracking works today:

When doing cargo build, Cargo passes an --emit=deb-info flag to rustc, which causes it to emit dep-info files with .d extension, which lists what files were used for compilation (remember, only crate roots are explicitly mentioned in Cargo.toml). These files then used in fingerprint.rs to compute the fingerprint.

Curiously, cargo doc does not emit dep-info at all, and fingerpint.rs does not use depinfo when doing doc. Instead it asks Source for fingerprint which for the local repository boils down to listing all files in a directory with Cargo.toml.

TL;DR: touch Readme.md today causes cargo doc to rebuild docs for the current crate, which might not be the most efficient thing to do, but which should work with external documentation, unless it is fetched from a place outside of Cargo package.

[joshtriplett]

@matklad So, it sounds like the current behavior is correct but could be made more efficient. That's better than being incorrect.

[matklad]

So, it sounds like the current behavior is correct but could be made more efficient.

There's an edge case where it might not do rebuild when it should, if in src/lib.rs you have something like

#[path="../../bar.rs"]
pub mod bar;

Or

include!("../../bar.rs")

That is, if you touch files outside of the Cargo package directory.

Other than that yes, current behavior is correct!.

[luser]

This probably won't cause any issues in the real world, but related to the discussion above about dependency tracking, I noticed that #[doc(include="file")] doesn't wind up in the dependency info output by rustc --emit=dep-info, which doesn't seem right. (In practice, since doc comments presumably don't affect the compiler output, it probably wouldn't cause any actual correctness problems).

Compare vs. include_str! for example:

luser@eye7:/tmp$ rustc +nightly --version
rustc 1.24.0-nightly (560a5da9f 2017-11-27)
luser@eye7:/tmp$ cat > test1.rs <<EOF
> #[allow(unused)]
> const s: &str = include_str!("other-file");
> EOF
luser@eye7:/tmp$ cat > other-file <<EOF
> hello!
> EOF
luser@eye7:/tmp$ rustc +nightly test1.rs --emit=dep-info -o test1.d
luser@eye7:/tmp$ cat test1.d
test1.d: test1.rs other-file

test1.rs:
other-file:
luser@eye7:/tmp$ cat > test2.rs <<EOF
> #![feature(external_doc)]
> #![doc(include="other-file")]
> EOF
luser@eye7:/tmp$ rustc +nightly test2.rs --emit=dep-info -o test2.d
cluser@eye7:/tmp$ cat test2.d
test2.d: test2.rs

test2.rs:

It looks like include_str! et. al. simply call cx.codemap().new_filemap_and_lines(&filename, &contents) to make this happen:

rust/src/libsyntax/ext/source_util.rs

Line 152 in a0db04b

// Add this input file to the code map to make it available as

so presumably doing the same thing inside the implementation of #[doc(include="..")] would fix this (probably here, I'm guessing):

rust/src/libsyntax/ext/expand.rs

Line 1117 in 632ad19

let include_info = vec![

[luser]

I neglected to mention, but it looks like there's even a test to ensure that files used in include_str! and include_bytes! wind up in the dep-info: https://github.com/rust-lang/rust/tree/b1363a73ede57ae595f3a1be2bb75d308ba4f7f6/src/test/run-make/include_bytes_deps

[QuietMisdreavus]

Humorously enough, i asked about that when i was writing it:

<misdreavus> hmm, should i add files loaded by doc(include) into the codemap? include_str does
<jseyfried> I would lean against

Judging from the above discussion in this thread, it may be prudent to add it, but it also may not be necessary. I'd defer to @rust-lang/compiler for that, i guess.

[joshtriplett]

I personally would find it quite unfortunate if changing the included file doesn't lead to rebuilding the code. (One day, incremental compilation should make that a very fast rebuild.) I realize that usually it won't affect the generated code, but I could imagine a syntax extension of some kind processing doc comments (after all documentation has been normalized to the equivalent of a doc attribute containing a string), and it shouldn't be the job of that syntax extension to handle dependencies on included files.

[jonhoo]

The current implementation seems to interact poorly with #![deny_missing]:

$ cat test.md
foo
$ cat test.rs
#![doc(include = "test.md")]
#![feature(external_doc)]
#![deny(missing_docs)]

fn main() {}
$ rustc +nightly test.rs
error: missing documentation for crate
 --> test.rs:1:1
  |
1 | / #![doc(include = "test.md")]
2 | | #![feature(external_doc)]
3 | | #![deny(missing_docs)]
4 | |
5 | | fn main() {}
  | |____________^
  |
note: lint level defined here
 --> test.rs:3:9
  |
3 | #![deny(missing_docs)]
  |         ^^^^^^^^^^^^

error: aborting due to previous error

[thedodd]

I've been running into an issue using this feature.

The particular crate which I would like to use this feature for is https://github.com/thedodd/wither. The crate uses the stable channel. I've been attempting to switch this feature on using the following pattern:

#![cfg_attr(feature="docinclude", feature(external_doc))]
#![cfg_attr(feature="docinclude", doc(include="target-docs.md"))]

NOTE: edited to correct a typo. Accidentally pluralized external_doc, it was singular in the code though. Was only a transcription typo here.

And then I simply invoke cargo +nightly doc --features="docinclude" --open ... seems as though it should work, the compiler doesn't give me any errors, but the docs do not show up. I have the docinclude feature setup properly in my Cargo.toml, so I don't think the issue is there (normally the compiler would complain if there is an error on that front).

It does work as expected if I remove the cfg_attr() wrapping and just enable the feature, but then the crate would require the nightly channel.

Thoughts?

[QuietMisdreavus]

I was just able to get it working with the following, running cargo +nightly doc --features docinclude:

#![cfg_attr(feature="docinclude", feature(external_doc))]

#![cfg_attr(feature="docinclude", doc(include="asdf.md"))]

It's worth noting that the feature gate is external_doc, not external_docs which you have in your sample. One other note is the location of asdf.md here was right next to lib.rs. (However, since you say it works without the cfg_attr, these are probably not that useful...)

[thedodd]

@QuietMisdreavus sorry, I noticed my typo of external_docs earlier when I transcribed it here, but forgot to update. I am using external_doc in the code though.

Well, that's quite strange. I will keep hacking on it and see if I can get it to work. Thanks for the feedback.

[thedodd]

@QuietMisdreavus so, after hacking on this a bit more, it appears as though I can compile the following code on stable (this time, exact copy-paste):

lib.rs

#![cfg_attr(feature="docinclude", feature(external_doc))]
#![cfg_attr(feature="docinclude", doc(include("../../README.md")))]

Cargo.toml:

# ... snip

[features]
docinclude = [] # Used only for activating `doc(include="...")` on stable.

[package.metadata.docs.rs]
features = ["docinclude"] # Activate `docinclude` during docs.rs build.

The command I run: cargo clean && cargo build --features docinclude.

Cargo version:

$ cargo --version
cargo 1.30.0 (a1a4ad372 2018-11-02)

Rustc version:

$ rustc --version
rustc 1.30.1 (1433507eb 2018-11-07)

The end result is that the build goes through, but still no included docs.

Running the same exact thing, but with a nightly compiler, yields the same result. Successful build, but no docs.

[ollie27]

@thedodd it's doc(include = "../../README.md") not doc(include("../../README.md")). It should be a compile error if you don't specify a filename to #[doc(include)] but currently it's just ignored.

[thedodd]

So, another pertinent bit of information is that this repo is a workspace with two crates, wither & wither_derive. When I cd down into the wither directory and then cargo build --features docinclude, the compiler errors out as expected. Invoking with +nightly causes compilation to succeed.

However, once again, the docs do not appear when running from the crate specific directory (as opposed to workspace dir) in any of the cases mentioned above. I've cargo cleaned and such ... doesn't seem to be making a difference.

For reference sake, I have pushed a branch called docinclude in this repo. Let me know if there is any other diagnostic info I can pass along.

[thedodd]

@ollie27 you may have just saved my life ... testing that out now.

[thedodd]

@ollie27 thanks for being the greatest human alive right now. It's always the small things. Thanks for clearing that up for me. I must have edited the attribute at one point while I was setting up the cfg_attr bits and thought that it was working the same way. It appears to be working as expected now.

[JeanMertz]

• Please use this in your own projects and comment in this thread if something goes wrong! (Or even if it goes well! Knowing it's working as intended is great!)

I wanted to quickly note that I too love this feature.

I like to document my libraries as much as is required to make them easy to use by anyone interested. But obviously, I dislike having outdated docs no longer reflect the reality of the code. This works really well when using documentation examples that get automatically added to the test suite.

One downside to this approach is that if you want to make your doctests also remain readable/understandable, you end up duplicating your tests many times, each time commenting out a different part of the test, to only show the part that is relevant to the reader of your documentation.

(side note: it would be nice if you could designate a block as "include this in all other code blocks in this documentation section", but I can see how that can get pretty complicated at some point, and perhaps even reduce readability of the unrendered documentation)

I am glad we have this capability in Rust, but the downside is that inline code documentation can become extremely long (hundreds of lines for a single trait method happens), which breaks my flow when reading the code.

Ideally, I'd want to only see a short description of the method in the code, but have a more robust example and description in the documentation.

This feature lets me do exactly that 👍 ❤️

You can see a small example here: rustic-games/things@ae5280d

I plan on using this a lot from now on. The doc feature flag proposed by others here works to keep the crate on stable, while building the docs with this nightly flag, but it would be nice if this feature gets stabilised sooner rather than later, it's a real boon to improved documentation standards in Rust.

about the bad line number info: yeah, I too noticed this while debugging an example that was failing. It was not a big deal, but it does give you a short pause, before you realise what's going on

[euclio]

I have a branch pending that improves the diagnostics for this feature, but it's based on #56258, so I'm going to let that PR get merged before opening another PR.

[abonander]

It seems like a really underpromoted possibility of this feature (though it obviously hasn't gone unnoticed) is the ability to have your README automatically doctested. I think on that basis alone it should be possible to generate a lot of interest and engineering effort toward getting this working well and stabilized.

[abonander]

I've got a proof-of-concept project for using this feature to test code examples in README.md: https://github.com/abonander/readme-doctest-poc

I chose to apply the attribute to a private typedef since no code gets generated for those. Fixing the file/line numbers for test failures in included files is definitely important though; I can see it being a real pain trying to fix failures if you have a lot of examples (it's not exactly common to add a lot of context to asserts in examples since they're usually self-evident).

[abonander]

Another thought: this would be significantly cooler if the Rustdoc CSS had a class that hides its contents so you could just reuse your README as your crate root docs and just hide stuff that doesn't necessarily need to be in the docs like license information or contributing instructions or CI build status.

That use-case is unfortunately a non-starter for most while this feature is unstable anyway. I don't want to require nightly to build documentation (or just sometimes not have crate root docs).

[jonhoo]

@abonander the way I've usually gone about this is to use cargo-readme to generate the README from the crate-level docs, but that has the issue that relative links won't work.

[Nemo157]

The paths being relative to the root module's file still seems like the wrong choice to me (at least for the use-cases I have). @yoshuawuyts and @SimonSapin appear to agree and @mgattozzi's last comment seems to be ok with changing it (and I haven't seen any other opinions noted since implementation). Is there any chance of re-discussing this now that there's some usage data to base it off?

[mgattozzi]

I'm sure @QuietMisdreavus has a more informed opinion of this now then I would regarding this. I'm of the opinion of the least amount of surprise meaning consistency with what Rust has done before, whether it's relative to the file or the crate root (I'm not sure which it is tbh).

[SimonSapin]

What Rust has done before is that the include!, include_byte!, and include_str! macros take a path relative to that of the source file that contains the macro invocation. This is consistent with how links work in HTML, or URLs on the web in general.

[QuietMisdreavus]

I have the same opinion about source file directories as in my comment from last year:

The reason that lib.rs was chosen was because rustdoc has no concept of Cargo projects, and also because the path behavior of include! and friends is considered a mistake for hygiene reasons. Moving a source file which uses one of these macros changes its behavior. At least, that's the explanation i remember seeing.

I would not be opposed to adding a parameter to rustdoc to set this root arbitrarily, with the expectation that Cargo could use this for cargo doc invocations. The only issue would be that passing this CLI argument would be a pain until Cargo supported it. And i'm not sure how Cargo would be convinced to handle passing the -Z unstable-features flag for such a feature while the flag itself was unstable.