Skip to content

Attribute representation change in rustdoc JSON without version bump #140689

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
obi1kenobi opened this issue May 6, 2025 · 4 comments
Open
Assignees
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@obi1kenobi
Copy link
Member

obi1kenobi commented May 6, 2025

Rustdoc JSON is a public but unstable API, whose contract is that behavior changes are signaled by incrementing its embedded version number.

Unfortunately, rustdoc JSON format v45 appears to have suffered a behavior change without a corresponding version bump. This was caught by the test suites of cargo-semver-checks and its components, which used to pass on the initial nightly version where format v45 was released but no longer pass:
https://github.com/obi1kenobi/trustfall-rustdoc-adapter/actions/runs/14585442486/job/41627682245

Output {
         instantiated_name: "Clone",
         attrs: [
-            "#[automatically_derived]",
+            "#[automatically_derived]\n",
         ],
         canonical_path: [
             "core",
             "clone",

in rustc 1.88.0-nightly (13e879094 2025-05-04)

At a glance, this may have been due to #140606. It may be sufficient to strip the trailing whitespace at the time rustdoc JSON is created to recover the original behavior.

@obi1kenobi obi1kenobi added the C-bug Category: This is a bug. label May 6, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 6, 2025
@obi1kenobi
Copy link
Member Author

@rustbot label +A-rustdoc-json +T-compiler

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 6, 2025
@aDotInTheVoid aDotInTheVoid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 6, 2025
@aDotInTheVoid aDotInTheVoid self-assigned this May 6, 2025
@aDotInTheVoid
Copy link
Member

Short term, we can strip white space inside librustdoc, and get the old behaviour. I’ve got a patch locally that does this, I’ll make a PR once i describe it. It might still be worth bumping the format version, as v45 output might be buggy, but v46 output is good (even if v45 from before #140606 is the same as v45).

Longer term, I’d like to make this less likely to happen again. We should probably add a triage or message on changes to tests/rustdoc-json informing PR authors/reviewers to be careful as this is a public API, and that if they want to change it, the FORMAT_VERSION also needs to be bumped. Not sure exactly what would be most useful to say here. CC @nnethercote @dtolnay, what would have been useful to see when reviewing #140606?

@dtolnay
Copy link
Member

dtolnay commented May 6, 2025

It should get an autoreview like what we do for Cargo.lock: #140535 (comment)

Alona was pinged on the broken PR in #140606 (comment). But this comment is not enough for nnethercote or I to know what the review obligation is. Is there a bigger group to ping that would be more likely to intercept bad changes before landing? Some teams / working groups like clippy, miri, rust-analyzer, gcc, trait solver, and some other projects mention the team like this: #126893 (comment)

I would recommend trying with just a more explanatory autoreview comment and see whether it gets improperly ignored.

@obi1kenobi
Copy link
Member Author

Not a rustdoc reviewer, but I'm happy to be pinged and participate if that's helpful — I've written parts of the rustdoc json test suite and I'm most likely to be downstream of breakage in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants