Skip to content

Add document-unstable default-feature #23

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bjoernQ
Copy link

@bjoernQ bjoernQ commented Apr 24, 2025

This adds a default feature to include the cfg(doc) - this makes it possible to opt-out and generate docs including just the public-API-surface (and helps in using e.g. cargo-semver-checks to check the public API surface)

Background

Since #12 the unstable macro expands to this

#[cfg(any(doc, feature = "unstable"))]
#[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
#[doc = " This is unstable"]
#[doc = "# Stability\n\n**This API is marked as unstable** and is only available when the `unstable`\ncrate feature is enabled. This comes with no stability guarantees, and could be changed\nor removed at any time."]
pub fn something_unstable() {
    todo!()
}
#[cfg(not(any(doc, feature = "unstable")))]
#[allow(dead_code)]
#[doc = " This is unstable"]
#[doc = "# Stability\n\n**This API is marked as unstable** and is only available when the `unstable`\ncrate feature is enabled. This comes with no stability guarantees, and could be changed\nor removed at any time."]
pub(crate) fn something_unstable() {
    core::panicking::panic("not yet implemented")
}

This is great and nice since documentation generated from this example will include the something_unstable function without the need to select the unstable feature. It's a great default and probably what most users need or want.

However, since rustdoc unconditionally sets the doc config flag it is not possible to generate documentation without the unstable API included.

While it's probably a niche use-case to generate docs for the stable API-surface only (most likely just for manual inspection) it becomes more important when using tools for semver-checks (e.g. cargo-semver-checks or public-api) - these tools usually rely on generating API-docs in JSON format for further processing and the unstable part of the API shouldn't get checked.

The only way I see to accomplish that is to have a way to emit code from the unstable macro without cfg(doc)

Drawbacks

Features add complexity. If (for some reason) users included instability with default-features set to false they will see a difference in the generated documentation.

Alternatives

Unfortunately, I don't see any real alternatives since rustdoc unconditionally sets the doc config.

The only (quite hacky) alternative I could think of is to pre-process the generated JSON and remove the unstable parts. Unfortunately, the rustdoc JSON format makes this an unpleasant experience especially because we would need to inspect the docs to see if they include a well known pattern. Ideally, I would like to avoid this pre-processing step.

Implementation

I think a default-feature which opt-in to including cfg(doc) is the most straight-forward way to do this.

I am not too confident about the naming of this new feature (i.e. happy to change it) or if you would prefer a non-default feature to opt-out of including cfg(doc)

@joshka
Copy link
Member

joshka commented Apr 24, 2025

Can you please take one step back and describe the problem this solves? I'm missing context here to understand it.

@bjoernQ
Copy link
Author

bjoernQ commented Apr 25, 2025

Can you please take one step back and describe the problem this solves? I'm missing context here to understand it.

Sorry for the way too brief PR-description - I updated it and hopefully it now explains the motivation better. Let me know if the description is still lacking details

@joshka
Copy link
Member

joshka commented Apr 26, 2025

Sorry for the way too brief PR-description - I updated it and hopefully it now explains the motivation better. Let me know if the description is still lacking details

No problem. If I had a dollar for every time I'd jumped into a solution assuming that the other person had the context,...

I'm going to have to think about this one a bit more, but my initial impression of the solution is that adding a feature flag that all downstream consumers of the library for documentation seems a little heavy. I think that will happen here unless I'm mistaken about how that works. Can you confirm that this approach would break downstream uses of the library?

Sometimes speaking in generalities hides the true problem a bit. What actual concrete problems does this cause for you in your daily use. I can see that you're describing the part which you're saying is a problem (inability to turn off generation of docs), but I'd like to see what you're doing that needs this (e.g. what commands are you running, what's the result, and how does this lib cause that). You've sort of alluded to that the json generated is a problem, but I'd like to capture the details a bit more. (This may also surface other alternative solutions).

@bjoernQ
Copy link
Author

bjoernQ commented Apr 28, 2025

This will not break things if

  • the user supplies the unstable feature when generating the documentation (most users probably will do that - they might have more cfg-gates in their code on the feature)
  • the user doesn't opt out of the default features (probably not many will do that - the crate had no default features before - on the other hand some users tend to disable default-features "by default")

But yes - if users opt out of instability's default feature and don't pass the unstable feature when generating docs they will end up with documentation missing the unstable part of the API (which should be easy to spot). Actual code should never be affected by this change.

To illustrate things a bit

cargo doc would currently include the unstable API items.

With this new feature flag (enabled by default) it would be the same.

When adding instability with no-default features enabled (instability = { version = "0.37.0", default-features = false } that above command would generate documentation without the unstable API items but cargo doc --features=unstable would also include the unstable API in that case.

No rush with this: I was able to work around things, but I guess ideally having a way to opt-out of having unstable API items documented always is useful to other's as well

@MabezDev
Copy link

I'm going to have to think about this one a bit more, but my initial impression of the solution is that adding a feature flag that all downstream consumers of the library for documentation seems a little heavy. I think that will happen here unless I'm mistaken about how that works. Can you confirm that this approach would break downstream uses of the library?

What about a --cfg instead of a feature? We only want this for semver checking our API (the stable parts). The workaround @bjoernQ implemented here is really not ideal because we're now at the mercy of rustdoc json format changes :(.

I think given that cargo semver checks is making its way into cargo itself, there should be some way to do this, whether its via feature flag/ cfg or some other mechanism.

@bjoernQ
Copy link
Author

bjoernQ commented Apr 30, 2025

I changed this PR to do what Scott suggested.

The new cfg is instability_exclude_unstable_docs which will not include unstable parts in docs if enabled.

I think it's very unlikely that a user will set this by mistake.

I didn't change the original PR description since it would make the conversation so far look weird

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants