RFC: Rustdoc configuration via Cargo (includes feature descriptions)

[tgross35]

RFC for rustdoc-cargo-configuration

Rendered

This RFC describes a way for rustdoc to get information from Cargo.toml. Its main use case is the Cargo feature descriptions RFC #3416.

The implementation goal of this RFC is only to get rustdoc working. However, it describes a flexible configuration that could be implemented for other tools (rustfmt, clippy, or any of the cargo-xyz tools) and can be used by build systems other than Cargo (e.g. Make or some WASM extensions of npm)

Related:

• Add a [lints] table to Cargo.toml #3389
• Tracking issue: RFC 2103 - attributes for tools rust#44690

[epage]

Could you add alternatives of doing what clippy and rustfmt do? I'd recommend verifying how they work and calling out differences if they have any

[epage]

One advantage of their approach is that the config files are still usable by third-party build systems like buck2, rather than them having to create their own config source.

[tgross35]

I do need to verify how exactly they locate their files. I do intend to update this proposal to take a --config-search-path or similar and drop Cargo locating any files, which would be easier for buck2 or other systems.

[epage]

Could you add an alternative of using package.metadata / workspace.metadata? What justifies adding a new table for this?

[tgross35]

I will update this, but my loose thought is that package.metadata/workspace.metadata is more for tools that read Cargo.toml themselves, whereas tools.x would be for tools that Cargo invokes. The other "soft" reason is that package.metadata.rustdoc seems like fairly deep configuration nesting for an official rust tool: tools.clippy or tools.rustdoc seem more fitting for something with first class support that Cargo is aware of.

(I will express the above in this RFC, but don't personally have a problem with package.metadata if the justification isn't sufficient).

[tgross35]

This does of course relate to [lints] and some of the decisions made there. I believe that as rustdoc configuration isn't a lint that putting this information in the [lints] table doesn't make sense... but it does very much run into the same questions as in #3389 (comment)

[tgross35]

On relation to the lints RFC, the schema loosely proposed in this section could potentially also specify usage of the [lints.x] table for a generic tool. Hm...

[epage]

If a table is completely opaque to cargo then I'm less convinced of the value of baking in support for it (something that came up several times in the [lints] RFC)

[epage]

Please dedicate a section for providing reference-level documentation of the tools table. What is the purpose and scope? Is only the rustdoc key allowed for now?

[calebcartwright]

I won't speak for clippy, but can say authoritatively that this is not accurate for rustfmt. For reference many of the issues were articulated back in #3389

[tgross35]

I wrote this hastily and didn't articulate the goal very well. To be clear: are you saying that rustfmt has no interest in being configurable via Cargo.toml, even if it doesn't need to parse the file itself? I did see #3389 (comment) but I didn't see any specifics there.

It is a good point that the clippy discussion is moot here, since that manifested into [lints]

[calebcartwright]

I did see #3389 (comment) but I didn't see any specifics there.

#3389 (comment)

To be clear: are you saying that rustfmt has no interest in being configurable via Cargo.toml

I'm saying that it's not accurate to state that rustfmt is "currently in the process of figuring out how to also allow specification via Cargo.toml". We're absolutely not working towards this, nor are we even contemplating it.

even if it doesn't need to parse the file itself?

I don't know how this can be avoided. I'm not sure about rustdoc, but I know that rustfmt (and rustc) are regularly utilized independent of cargo. In those cases, rustfmt would only have two options: either deal directly with the cargo manifest file or completely ignore it. Both of those have significant drawbacks, a couple of which I spoke to a bit in the comment linked above.

[tgross35]

I'm saying that it's not accurate to state that rustfmt is "currently in the process of figuring out how to also allow specification via Cargo.toml". We're absolutely not working towards this, nor are we even contemplating it.

That makes sense; I have seen feature requests and users.rust-lang questions, but I never saw the official follow up.

I don't know how this can be avoided. I'm not sure about rustdoc, but I know that rustfmt (and rustc) are regularly utilized independent of cargo. In those cases, rustfmt would only have two options: either deal directly with the cargo manifest file or completely ignore it. Both of those have significant drawbacks, a couple of which I spoke to a bit in the comment linked above.

If the design in this RFC were to be followed, rustfmt would just accept an argument like this:

rustfmt . --config-json '{"array_width": 100, "wrap_comments": true, ...}'

The config argument would be created by Cargo (or other build systems) and the data would follow the exact schema of rustfmt.toml so the same deserializers can be used. So rustfmt doesn't need to be aware of Cargo.toml or parse it itself.

I haven't been able to view your comment, if you already discussed something like this there. Github navigates to the PR but not the comment, maybe it's on a rebased commit (there's nothing on this page either, which is where it navigates when I click your name in the "Reviewers" list, and it's nowhere in the entire chain of that PR for me). Would you mind pasting the contents here?

[calebcartwright]

Indeed the direct link seems to be inoperable, thanks GitHub 😄 Given the scope and objective of this thread, I'm going to put the text of that comment into a collapsed section, as I don't want things to get over-indexed on rustfmt when rustfmt isn't really a focus topic of this proposal; I just didn't want there to be any misrepresentation about rustfmt happenings in the text.

In a similar vein, I'd rather not try to debate or solution rustfmt specifics here. Suffice to say the rustfmt position is "no", we're not actively pursuing changing that, and there's several challenges that would have to be discussed and resolved before it'd be remotely feasible for rustfmt go that route (setting aside whether or not we should). Such discussions would be better suited to either t-rustfmt on zulip or in the r-l/rustfmt repo, though full transparency, I don't feel we've got the adequate bandwidth right now, even just to dive more deeply into the challenges and design topics.

Comment text from PR 3389#discussion_r1110364556

I will also speak into the rustfmt specifics of this, but I first want to underscore my view that it's important to consider this holistically around the experience and expectations developers may have for configuration across the various official tools if we move ahead with this proposal.

I do not have the receipts handy, but I'm sure many have seen/heard comments to the effect of "I have edition set to 2018 in Cargo.toml, but when I run `rustc ...` it's using edition 2015 instead of edition 2018".

Essentially, it's a relatively common occurrence for some developers to (incorrectly) assume that info defined in Cargo.toml is honored by all tools even outside a cargo context. While this isn't necessarily a problem for tools typically, or always, invoked as part of some cargo ... subcommand flow, it's a persistent topic that pops up for tools that are utilized from both cargo and non-cargo contexts (rustc, rustdoc, rustfmt).

Fundamentally, my concern is that supporting more configuration values in Cargo.toml could exacerbate this issue. If this proceeds, then I'd love to find a way to help ameliorate this by somehow increasing awareness of when values in Cargo.toml can be used/will be honored, and crucially, when they won't. (though unfortunately I've no prescriptions).

As for rustfmt, quick overview for anyone unfamiliar:

rustfmt can and often is invoked in a cargo context via `cargo fmt`, but it's also commonly invoked directly both by humans and machines (e.g. editor/ide use cases to format single file, subset of a single file, etc.). In the `cargo fmt` scenario obviously relevant Cargo.toml info is utilized, but `rustfmt ...` itself doesn't, so when running `rustfmt` directly, one needs to explicitly specify the edition to use a non-2015 edition. rustfmt also supports various [config options](https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=) that can be specified in a rustfmt.toml file.

We've had requests to support allowing rustfmt config options to also be defined in Cargo.toml, but it's something we've always pushed back on for a number of reasons, one of which being that rustfmt would either have to ignore the edition value in Cargo.toml while honoring other aspects of Cargo.toml, or essentially introduce breaking behavior where the default/implicit edition rustfmt would utilize would change in some contexts (since the ecosystem default is still 2015 and rustfmt does not currently utilize Cargo.toml defined edition, like rustc and rustdoc)

I understand this proposal is focused on lints, and it's not my intent to scope creep. I would, however, feel more comfortable if we could either set an upper bound on how far this could extend to other tools (explicitly, even if only notionally, to not expand beyond lints), or determine if this could/should expand, and if so then try to address the potential challenges this could present for other tools

another comment from that same thread that highlights some of the issues

Shared as a screen grab:

[tgross35]

I'm going to mark this RFC as a draft again, just since I'd like to focus on #3416 first, and also have some coordination to do with the tools teams. I do intend to keep the overall idea the same, so feedback is still very much welcome.

[obi1kenobi]

As maintainer of cargo-semver-checks, having data on crate features (including default features, which other features they require, and what features each requires in dependencies) would be very helpful. I believe all those data points may be the cause of accidental SemVer breakage, and it would both massively simplify my life and offer better ergonomics for cargo-semver-checks users if that data were part of rustdoc JSON.

Happy to go into details if anyone is curious.

[epage]

As maintainer of cargo-semver-checks, having data on crate features (including default features, which other features they require, and what features each requires in dependencies) would be very helpful. I believe all those data points may be the cause of accidental SemVer breakage, and it would both massively simplify my life and offer better ergonomics for cargo-semver-checks users if that data were part of rustdoc JSON.

If rustdoc is like rustc, then it will have little to no cargo knowledge and extending it with this knowledge in a non-abstract way would be violating these separation of concerns.

I also see this as a shortcut. It might help you in the short term to have rustdoc expose it but there is other semver-relevant content in manifests that will need checking, like public-private dependencies which are just a couple bugs in rustc away from being stabilized.

[obi1kenobi]

If rustdoc is like rustc, then it will have little to no cargo knowledge and extending it with this knowledge in a non-abstract way would be violating these separation of concerns.

I also see this as a shortcut.

I don't dispute this. But if rustdoc is already including info on features and descriptions, then all your concerns still apply regardless of cargo-semver-checks, no?

If there's already going to be an officially-supported of getting the information we need into rustdoc, I don't want to unnecessarily maintain another one. It's better for the entire community if we don't needlessly duplicate work.

Even if we eventually need more information than it makes sense for rustdoc to include, and cargo-semver-checks ends up parsing manifests anyway, getting more functionality available sooner and at lesser development and maintenance cost is worth it in itself IMHO.

[epage]

I don't dispute this. But if rustdoc is already including info on features and descriptions, then all your concerns still apply regardless of cargo-semver-checks, no?

If there's already going to be an officially-supported of getting the information we need into rustdoc, I don't want to unnecessarily maintain another one.

This depends on what rustdoc / cargo find appropriate to pass to rustdoc. It could be as simple as cfgs and documentation for cfgs and their values. That would lose high level information you need like requires

It's better for the entire community if we don't needlessly duplicate work.

Even if we eventually need more information than it makes sense for rustdoc to include, and cargo-semver-checks ends up parsing manifests anyway, getting more functionality available sooner and at lesser development and maintenance cost is worth it in itself IMHO.

This feels like an overly broad generalization. Just because something exists and is of interest for cargo-semver-checks it, doesn't mean it is needed for rendering documentation, will be presented / modeled in a way that is relevant for cargo-semver-checks, or be implemented in a timely manner as any cargo information requires new CLI contracts between rustdoc and cargo.