Add --no-source option flag to rustdoc

[GuillaumeGomez]

Fixes #75497.

A question though: should we go through an unstable phase or not considering the attribute is already stable?

[rust-highfive]

r? @steveklabnik

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

[GuillaumeGomez]

r? @jyn514

[jyn514]

Looks reasonable to me. I don't have a strong opinion on whether this should be insta-stable, but since it's just exposing an option that already exists through the command line I don't see why not.

[GuillaumeGomez]

Forgot to write it down but if we go stable: I'll also need to add the related documentation too.

[GuillaumeGomez]

Added the documentation.

[jyn514]

r=me after nits are fixed

[GuillaumeGomez]

@bors: r=jyn514

[bors]

📌 Commit a867bd9a5389caa854103fd20221e2de354eaaa8 has been approved by jyn514

[ollie27]

@bors r-

New features require at least an FCP.

An issue with --no-source is that there is no way for rustdoc to know if a dependency has been documented with that flag so [src] links to that depedency would be broken. With the crate level attribute rustdoc can avoid generating broken [src] links to dependencies. Admittedly rustdoc doesn't yet do that but it could and should: #55957.

[GuillaumeGomez]

It's not a new feature, that's why I asked if it required to go through an unstable phase since it's only providing a command-line flag for an already stable attribute. So this is a different issue in my opinion: this PR is about adding a command-line flag for a stable attribute. If the attribute itself has issues, then it should be fixed in another PR. What do you think @ollie27 ?

[crlf0710]

Triage: What's the status here?

[GuillaumeGomez]

ping @ollie27

[GuillaumeGomez]

Fixed conflicts and fixed issues reported by @camelid !

[bors]

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

[ollie27]

To be honest I'm not sure what do here.

As mentioned in #75497 (comment) you can already get the effect of --no-source using feature flags so I'm not convinced that we need a dedicated option for this.

The guidelines for command-line arguments in rustc explicitly recommend not using the no- prefix.

As well as not rendering source files for the current crate, #![doc(html_no_source)] (and --no-source as it's currently proposed) also prevent [src] links from being generated for items defined in external crates. I don't know if that's an intentional effect or not but it's not documented. I would expect rustdoc to only not generate [src] links to an external crate if that crate has #![doc(html_no_source)].

It's not a new feature

As far as I'm concerned, a new command line argument is a new feature no matter how simple it may appear so I would expect to see at least an FCP for it.

If the attribute itself has issues, then it should be fixed in another PR.

The attribute can be fixed but the command line argument can't. rustdoc will have no way of knowing if an external crate has been documented with --no-source or not so will either generate broken [src] links or none when it should. An option might be an argument like --extern-no-source to specify which external crates have been documented with --no-source along with some kind of integration with cargo. Are there plans for something like that?

[bors]

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

[GuillaumeGomez]

As well as not rendering source files for the current crate, #![doc(html_no_source)] (and --no-source as it's currently proposed) also prevent [src] links from being generated for items defined in external crates. I don't know if that's an intentional effect or not but it's not documented. I would expect rustdoc to only not generate [src] links to an external crate if that crate has #![doc(html_no_source)].

I'm pretty sure we never generate [src] links to external crates, and I think we shouldn't. The only thing we should do is link to external items documentation page.

As far as I'm concerned, a new command line argument is a new feature no matter how simple it may appear so I would expect to see at least an FCP for it.

That's fair. Also, we should maybe go through a period of "nightly only" and then propose to stabilize it. So here what I suggest: I make this unstable for the time being, and open another PR to stabilize it with an FCP. What do you think?

The attribute can be fixed but the command line argument can't. rustdoc will have no way of knowing if an external crate has been documented with --no-source or not so will either generate broken [src] links or none when it should. An option might be an argument like --extern-no-source to specify which external crates have been documented with --no-source along with some kind of integration with cargo. Are there plans for something like that?

I'm not sure to understand your concern over this: we can already use an external URL to say where the external crate is supposed to be and we have no idea if it has been generated with source code or not. Also, like said above, we don't generate [src] links to external items' source and I think we should only link to external items' documentation.

[jyn514]

we have no idea if it has been generated with source code or not

I think Ollie's point is that, for the attribute, we can tell when the crate will never have docs and stop generating links to it. If someone passes --extern-html-root-url they're guaranteeing that the URL does in fact have docs at it, it's not rustdoc's responsibility.

I don't have a strong opinion on this either way, but given that there are already a ton of ways to replicate this already (e.g. --crate-attr html_no_source), are you sure there needs to be a dedicated flag? Most attributes aren't duplicated in flags.

[GuillaumeGomez]

we have no idea if it has been generated with source code or not

I think Ollie's point is that, for the attribute, we can tell when the crate will never have docs and stop generating links to it. If someone passes --extern-html-root-url they're guaranteeing that the URL does in fact have docs at it, it's not rustdoc's responsibility.

That argument doesn't work since, like you said, we can reach the same result using -crate-attr html_no_source.

I don't have a strong opinion on this either way, but given that there are already a ton of ways to replicate this already (e.g. --crate-attr html_no_source), are you sure there needs to be a dedicated flag? Most attributes aren't duplicated in flags.

This is mostly for convenience, it's easier to use an explicit argument rather than going through attributes. I'd personally prefer to add this option (obviously) but I'll let it up to the team for the final decision.

[jyn514]

That argument doesn't work since, like you said, we can reach the same result using -crate-attr html_no_source.

No, like Ollie said earlier, rustdoc should stop generating links to source when it knows they won't exist: #55957

I'm not sure who "the team" is (do you want to ping more people?) but given all the discussion here I'd prefer not to add this, rustdoc has a ton of knobs already and this one doesn't seem particularly useful.

[GuillaumeGomez]

By the team I meant ours. 😉

And fine by me! Let's close this then. :)