Stream: rustdoc

Topic: json: invalid intra-doc links resolution for method


view this post on Zulip Urgau (Aug 07 2021 at 23:57):

I was exploring the json output of rustdoc with the anyhow crate when I realized that the intra-doc link [Error::chain] in the Chain trait, was pointing to the Error struct and not to the chain method of the Error struct. Is this expected ? If yes, how can I get the method and not the struct ?
Chain item in the json:

"0:598": {
    "id": "0:598",
    "crate_id": 0,
    "name": "Chain",
    [...]
    "docs": "Iterator of a chain of source errors.\n\nThis type is the iterator returned by [`Error::chain`].\n\n# Example\n\n```\nuse anyhow::Error;\nuse std::io;\n\npub fn underlying_io_error_kind(error: &Error) -> Option<io::ErrorKind> {\n    for cause in error.chain() {\n        if let Some(io_error) = cause.downcast_ref::<io::Error>() {\n            return Some(io_error.kind());\n        }\n    }\n    None\n}\n```",
    "links": {
        "`Error::chain`": "0:542"
    },
    [...]
},

Path of Id 0:542:

"0:542": {
    "crate_id": 0,
    "path": [
        "anyhow",
        "Error"
    ],
    "kind": "struct"
},

cc @Joseph Ryan

view this post on Zulip Joseph Ryan (Aug 08 2021 at 00:03):

At first glance that definitely seems like a bug, but the Json backend doesn't do any processing on intra doc links so it might be a problem somewhere else. If you look at the html output does that link actually take you to the method?

view this post on Zulip Urgau (Aug 08 2021 at 00:04):

Yes it take to the method. href is struct.Error.html#method.chain

view this post on Zulip Urgau (Aug 08 2021 at 00:08):

I have also look at the json conversion code and as you stated the json conversion do nothing special for intra-links, just a simple filter_map.

view this post on Zulip Urgau (Aug 08 2021 at 00:10):

If anyone is interested the code for the conversion is here.

view this post on Zulip Joseph Ryan (Aug 08 2021 at 00:10):

Next thing to check would be the html backend code to see if they do anything different to find the actual item. It does seem weird that the id that we get doesn't just point directly at the method (sorry I don't have time to take a look at it at the moment)

view this post on Zulip Urgau (Aug 08 2021 at 00:25):

It seems that an intra-doc link is represented in rustdoc by the ItemLink struct, whitch contain an fragment field describe as The url fragment to append to the link. That must means that did field in ItemLink is not always the DefId of the target.
And because the json conversion only use the did field, this explain why I'm not getting the right "target".

view this post on Zulip Urgau (Aug 09 2021 at 14:15):

@GuillaumeGomez Sorry to bother you. Do you know how this can be solved in rustdoc ? I'm kinda stuck on my side because of these invalid resolutions.

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:18):

@Urgau this is because rustdoc uses the DefId of the page it links to, not the method itself. See https://github.com/rust-lang/rust/blob/eaf6f463599df1f18da94a6965e216ea15795417/src/librustdoc/passes/collect_intra_doc_links.rs#L520; I think it would be medium-hard to fix.

view this post on Zulip GuillaumeGomez (Aug 09 2021 at 14:18):

I'd say that you shouldn't run the intra-doc pass in case you render JSON

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:18):

@GuillaumeGomez that seems definitely wrong, it means you don't know what links resolve and what don't

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:19):

In particular [std::process] and [std::x] are both valid markdown, but one of them is rendered with brackets and the other isn't

view this post on Zulip Urgau (Aug 09 2021 at 14:19):

Yah, that would sucks, if I don't have the intra-doc links resolved.

view this post on Zulip GuillaumeGomez (Aug 09 2021 at 14:19):

I'm not sure of the "scope" of the JSON output: do we want to keep the docs as is or do we want to run some treatment on it? From your comment I assume the latter

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:20):

I don't understand your comment. The JSON backend should see the same docs as the html backend.

view this post on Zulip GuillaumeGomez (Aug 09 2021 at 14:20):

I simply assumed that it "saw" the original markdown

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:21):

Well, so does the HTML backend. But it has knowledge of which links resolve through the DocContext. So the JSON backend should too.

view this post on Zulip Urgau (Aug 09 2021 at 14:21):

The json output has the "original" markdown code + a list of the intra-doc links resolved.

view this post on Zulip GuillaumeGomez (Aug 09 2021 at 14:22):

Oh I see, so it's the same as the HTML then (minus the special cases of the HTML). Then just like @Joshua Nelson said :)

view this post on Zulip Joshua Nelson (Aug 09 2021 at 14:22):

@Urgau I won't have time to work on this, but I think it would be doable as a first issue if you want to take a look at the link I posted above :)

view this post on Zulip CraftSpider (Aug 09 2021 at 14:26):

I also don't have time to do the work right now, but I'd be good to review / help with any issues. Feel free to ping me if you get stuck.

view this post on Zulip Urgau (Aug 09 2021 at 14:30):

Okay, I will try to fix that. Cloning the rust repo, right now.

view this post on Zulip Urgau (Aug 09 2021 at 14:42):

@Joshua Nelson @CraftSpider What modifications do you want me to do ? Return a third argument with the real DefId ?

view this post on Zulip Urgau (Aug 09 2021 at 14:42):

And add it to the ItemLink struct ?

view this post on Zulip Joshua Nelson (Aug 09 2021 at 15:05):

@Urgau remove the hack, so that it always uses the method DefId and never the id of the containing item

view this post on Zulip Joshua Nelson (Aug 09 2021 at 15:05):

there should only be one DefId

view this post on Zulip Urgau (Aug 09 2021 at 17:57):

@Joshua Nelson I've looked more at the code and I don't think I well be able to do it, I don't know enough of the internal, plus their is another hack that need's to be remove as well who use the fragment field for primitive handling. It's seems that this is too hacky for a first-contribution. Sorry, maybe with some mentoring instructions I could retry but not sure.

view this post on Zulip Joshua Nelson (Aug 09 2021 at 19:37):

@Urgau the fragment issue should be fixed by https://github.com/rust-lang/rust/pull/87073

view this post on Zulip Joshua Nelson (Aug 09 2021 at 19:38):

but yeah that's fair, I'll try to dedicate some time to fixing CI for 87073 next weekend and it should be much easier after that

view this post on Zulip Joshua Nelson (Aug 16 2021 at 04:20):

oh look at that I actually got around to it, what are the odds


Last updated: Oct 21 2021 at 22:01 UTC