Skip to content
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

[rustdoc-json] paths is inconsistent and questionably useful #93522

Open
CraftSpider opened this issue Jan 31, 2022 · 5 comments
Open

[rustdoc-json] paths is inconsistent and questionably useful #93522

CraftSpider opened this issue Jan 31, 2022 · 5 comments
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@CraftSpider
Copy link
Contributor

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.

@dsherret
Copy link

dsherret commented Sep 2, 2022

questionably useful

I was using paths to get the fully resolved name of types by their id. Within the past two months it seems this has gotten really unstable. For example, doing something like...

// lib.rs
mod expr;

pub use expr::Expr;

pub enum Main {
    Test,
}

// expr.rs
pub enum Expr {
  Test,
}

...will now only have the Main enum in "paths" and not also Expr, though this previously included both. I guess it has something to do with the cache. I guess I should compute this upfront by traversing everything in index instead (this is very difficult to do).

@Urgau
Copy link
Member

Urgau commented Sep 11, 2022

I would be in favor of removing it completely. Handling re-export is very complicated and even if we were to handle them I still don't see how it could distinguiesh between the "real" location and what the "user" might want.

It could see be re-introduce later but in the mean time I would be I favor of just removing it.

@Urgau
Copy link
Member

Urgau commented Oct 14, 2022

@CraftSpider @Enselic I'm planning on submitting a PR to remove paths, any objection from your part ?

@Enselic
Copy link
Member

Enselic commented Oct 14, 2022

@Urgau Sounds good to me! I don't use it and don't plan to.

@obi1kenobi
Copy link
Contributor

obi1kenobi/cargo-semver-checks#202 ran into an interesting real-world edge case that might be useful for guiding future work in this area.

The affected crate, libp2p-dcutr, includes three different pub enum UpgradeError types in three different modules. One of those modules is public, and the other two are private. The root lib.rs file publicly re-exports and renames the UpgradeError types from the private modules, so all three are accessible under different names.

Here's one of those enums: https://github.com/libp2p/rust-libp2p/blob/1c2712c1bc288dc608aaec5fc3458b0d07181feb/protocols/dcutr/src/protocol/outbound.rs#L118
Here's how it's publicly re-exported and renamed: https://github.com/libp2p/rust-libp2p/blob/1c2712c1bc288dc608aaec5fc3458b0d07181feb/protocols/dcutr/src/lib.rs#L31

Currently, rustdoc JSON v22 and v23 do not include the re-exported items in paths, which I believe is a known issue.

They do however include the renaming re-export:

"0:311": {
    "id": "0:311",
    "crate_id": 0,
    "name": null,
    "span": {
        "filename": "protocols/dcutr/src/lib.rs",
        "begin": [
            31,
            50
        ],
        "end": [
            31,
            96
        ]
    },
    "visibility": "public",
    "docs": null,
    "links": {},
    "attrs": [],
    "deprecation": null,
    "kind": "import",
    "inner": {
        "source": "protocol::outbound::UpgradeError",
        "name": "OutboundUpgradeError",
        "id": "0:377:1572",
        "glob": false
    }
}

The obi1kenobi/cargo-semver-checks#202 is caused by the difficulty in matching up "the same item" across two different versions. Our current approach fails since it doesn't account for renames, and ends up comparing two enums that happen to be defined with the same name but are "not the same enum" across versions. This is certainly a bug in cargo-semver-checks alone and not in rustdoc JSON, and nothing I write here should be construed to imply otherwise. Matching up "the same items" across versions is quite a tricky problem, and I wanted to put it on your radar as a thing to consider while building the future of rustdoc.

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-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

5 participants