Made Path::name only have item name rather than full name

[AS1100K]

Closes #134853

This PR makes Path::name to only have item name rather than full name, i.e. with the following code

pub mod foo {
    pub struct Bar;
}

pub fn get_bar() -> foo::Bar {
    foo::Bar
}

and running ./rustdoc ./demo.rs -wjson -Zunstable-options gives:

{
    "41": {
        "id": 41,
        "name": "get_bar",
        "inner": {
            "function": {
                "sig": {
                    "inputs": [],
                    "output": {
                        "resolved_path": {
                            "name": "Bar",
                            "id": 0,
                            "args": { "angle_bracketed": { "args": [], "constraints": [] }
                            }
                        }
                    }
                }
            }
        }
    }
}

Information which isn't useful here was trimmed

r? aDotInTheVoid

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aDotInTheVoid (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[aDotInTheVoid]

Sorry for the delay in getting around to reviewing this.

I think this is the right approach (rather than changing the docs to reflect what we currently do), even though the (then incorrect) were only added recently (#127290).

All this information is available in the paths field, and there it's in a more usable form, as it also includes the defining crate.

CC @CraftSpider, @Enselic, @obi1kenobi as this is effectivly a change to rustdoc-types, even though the types themself don't change. How do you feel about this? Also: should we bump FORMAT_VERSION for this? (The de-serialization won't fail, but we've changed the meaning of fields)

I'm planning to merge this in about a week, assuming no-one objects. Also, would you mind squashing your commits, as I don't think there's value in having the test-change be in a separate commit.

Thanks again for finding this and working on the fix.

[Enselic]

Thanks for the ping. This seems like a good change.

I would prefer FORMAT_VERSION to be bumped since the semantics have changed (to the better). But I'm fine with keeping it unchanged if others have reasons to let it remain unchanged.

[obi1kenobi]

I'd also prefer to bump the format version, since this behavior change will affect a handful of cargo-semver-checks lints that I'll have to fix. Thanks for the ping!

[aDotInTheVoid]

r=me with FORMAT_VERSION (defined in src/rustdoc-json-types) bumped.

[rustbot]

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

[AS1100K]

@rustbot ready

[rustbot]

Could not assign reviewer from: aDotInTheVoid.
User(s) aDotInTheVoid are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

[aDotInTheVoid]

@bors r+ rollup

[bors]

📌 Commit 2c4aee9 has been approved by aDotInTheVoid

It is now in the queue for this repository.

[obi1kenobi]

@aDotInTheVoid I'm running into a serious issue making cargo-semver-checks support this format. I'd vote for reverting this change while we look for a better alternative — possibly by reworking what path is in paths.

All this information is available in the paths field, and there it's in a more usable form, as it also includes the defining crate.

This turns out to not be true in practice. The paths field includes the originally-defined path of the item, which is often private and not accessible. For example:

• Where Path::name previously contained Iterator (accessible via the prelude), the paths lookup produces core::iter::traits::iterator::Iterator which is private.
• Where Path::name previously contained std::str::SplitAsciiWhitespace, the paths lookup produces core::str::iter::SplitAsciiWhitespace which is private.
  etc.

This is making cargo-semver-checks unable to properly print types, function signatures, trait bounds, etc. and effectively disables a bunch of lints.

In theory, we could run c-s-c's importable paths analysis to derive a valid, publicly-importable path for the item instead of using the paths field. In practice, this is blocked in two ways:

• It requires cross-crate analysis, since most such items are in std/core/alloc etc. This is a goal, but as we've already chatted, it's many months out and blocked on rustc functionality etc.
• Even if we had cross-crate analysis, it requires the ability to either download or generate rustdoc JSON for std/core/alloc. I don't know of a way to generate it client-side, and AFAIK we can download their rustdoc JSON for nightly Rust only. This would break c-s-c's ability to support multiple stable Rust versions.

[aDotInTheVoid]

Thanks for reporting this. Filed #135600, as it's likely to get lost and forgotten here.

[LukeMathWalker]

Even if we had cross-crate analysis, it requires the ability to either download or generate rustdoc JSON for std/core/alloc. I don't know of a way to generate it client-side, and AFAIK we can download their rustdoc JSON for nightly Rust only. This would break c-s-c's ability to support multiple stable Rust versions.

This is a problem I ran into before, specifically related to paths and std/core/alloc—see this commit.
I think it'd be reasonable to enforce that paths in paths are publicly accessible whenever the item is public.

[obi1kenobi]

I think it'd be reasonable to enforce that paths in paths are publicly accessible whenever the item is public.

That sounds reasonable to me too.

More than anything else, though, I'd like to have either a fix or a revert for this PR before the next Rust beta cutoff, so that we don't end up with a stable version of Rust that cargo-semver-checks can't support.

I'm not sure I can ship a fix, but I'd be happy to submit a revert if that is at all helpful.

[LukeMathWalker]

I finally got around to testing this for Pavex. Unfortunately, I was down sick with a fever for a few days last week, so I'm a bit late.
Most of the test suite passes with no issues, with a distinct exception—this test. We can no longer disambiguate multiple versions of the same crate, since the additional bits in name were part of the workaround for that issue. This would stop being a problem if this MCP gets implemented.