rust
rust copied to clipboard
[rustdoc-json] Partially remove `paths` and introduce `external_index`
From [rustdoc-json] paths is inconsistent and questionably useful
:
Problem: Currently, the items
paths
includes is just the list of paths used in the rustdoc cache. This is implementation-dependent, misses some items that should possibly be included such as re-export source items, and generally imperfect.Solution:
paths
inclusion should be standardized, and separated from the internal cache implementation.
To solve this issue this PR introduce a new external_index
that only contains a ExternalItem
struct from the previous ItemSummary
. This "new" index only contains external item and is here so that no ID are not dangling and to still provide a minimum of informations to those external IDs (crate_id, name, kind).
This PR also rename path
to name
(from ExternalItem
) because it's not clear at all what path
represent (is it one of the public paths but which one or is it a private one but why) so to simplify all this we will now just give the first external name of the item.
Fixes https://github.com/rust-lang/rust/issues/93522 r? @aDotInTheVoid cc @Enselic
rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs
; otherwise, make sure you bump the FORMAT_VERSION
constant.
cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi
While I agree that the current situation with paths
isn't good, I'm not convinced that this is better, as theirs now no way to go from an external ID to that item it the json docs in that crate (you can't just use the id). This is an operation that we should support, untill https://github.com/rust-lang/rust/issues/99513 is resolved. (And even then, you'd need to add not only reexported foreign items, but all foreign items appearing in the public API, and all items exposed via them, and so in.)
An alternative would be to give all publicly accessible paths to an item, but it turns out this is impossible in the general case, as some items can have an infinite number of paths, some of which are infinitely long😱
What about we keep path
at least until https://github.com/rust-lang/rust/issues/99513 is resolved and after it is resolved we see if it's still useful to keep it. Would this address your concern ?
Yes
an infinite number of paths, some of which are infinitely long😱
Yikes! Excellent and terrifying example. This feels like it might be worth a lint in clippy? It wasn't obvious to me that this can happen, and I wouldn't be surprised if many tools (cargo-semver-checks
included) assume the set of paths is finite.
cargo public-api
handles recursive re-exports, and also has tests in place to prevent regressing on that 😎
Two Comments:
-
I Don't think the format churn is worth it, as it's just renames. I'm tracking all the renames I want to do in #100961, and will do in in one big bunch to minimize the amount of new format versions.
-
I'm not sure it's worth removing local items from
paths
/external_index
. What advantage to users of the format does it acheive?
Two Comments:
I Don't think the format churn is worth it, as it's just renames. I'm tracking all the renames I want to do in Rustdoc-Json: More inconsistant names #100961, and will do in in one big bunch to minimize the amount of new format versions.
I'm not sure it's worth removing local items from
paths
/external_index
. What advantage to users of the format does it acheive?
The end goal was to completely remove the path
field but at least for now it's not possible. The rest is inherent to this:
- the renaming is inherent to non inclusion of local items, leaving it as
paths
would be misleading it could be namesexternal_paths
but it does include more that thepath
(crate_id and kind),external_items
makes it very clear that only external items are included in it and that it's similar to an item. - removing the non local items would be the first step towards removing
path
, we cannot yet remove it as it's currently needed for workaround some issues.
Note that @jyn514
proposed on Zulip adding a method in rustdoc-json-types
to get a full path of an Item. This would potential be the replacement for the path
field.