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: broken_intra_doc_links triggers on link to private item #83049

Open
camelid opened this issue Mar 12, 2021 · 11 comments
Open

rustdoc: broken_intra_doc_links triggers on link to private item #83049

camelid opened this issue Mar 12, 2021 · 11 comments
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-low Low priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Mar 12, 2021

⚠️ Warning ⚠️

Discussion needs to take place and agreement needs to be reached before implementing this! See #83049 (comment).


This should succeed, but it does not.

mod foo {
    //! Look at [super::bar::baz].
}

mod bar {
    fn baz() {}
}
$ rustdoc foo.rs --document-private-items
warning: unresolved link to `super::bar::baz`
 --> foo.rs:2:18
  |
2 |     //! Look at [super::bar::baz].
  |                  ^^^^^^^^^^^^^^^ no item named `baz` in module `bar`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default

warning: 1 warning emitted
@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 12, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 12, 2021

Does it work without super? Inner attributes resolve relative to the module they're in.

@camelid
Copy link
Member Author

camelid commented Mar 13, 2021

Does it work without super? Inner attributes resolve relative to the module they're in.

Well, if you look at the label in the error message, the issue is not in finding bar, rather it's in finding baz within bar.

Also, using crate doesn't fix it:

warning: unresolved link to `self::bar::baz`
 --> foo.rs:2:18
  |
2 |     //! Look at [crate::bar::baz].
  |                  ^^^^^^^^^^^^^^^ no item named `baz` in module `bar`
  |
  = note: `#[warn(broken_intra_doc_links)]` on by default

warning: 1 warning emitted

Weirdly, it says self::bar::baz—it should say crate::bar::baz. Probably an unrelated bug.

@jyn514
Copy link
Member

jyn514 commented Mar 14, 2021

Weirdly, it says self::bar::baz—it should say crate::bar::baz. Probably an unrelated bug.

} else if path_str.starts_with("crate::") || is_lone_crate {
use rustc_span::def_id::CRATE_DEF_INDEX;
// HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented.
// But rustdoc wants it to mean the crate this item was originally present in.
// To work around this, remove it and resolve relative to the crate root instead.
// HACK(jynelson)(2): If we just strip `crate::` then suddenly primitives become ambiguous
// (consider `crate::char`). Instead, change it to `self::`. This works because 'self' is now the crate root.
// FIXME(#78696): This doesn't always work.
if is_lone_crate {
path_str = "self";
} else {
resolved_self = format!("self::{}", &path_str["crate::".len()..]);
path_str = &resolved_self;
}
module_id = DefId { krate, index: CRATE_DEF_INDEX };
}

@jyn514
Copy link
Member

jyn514 commented Mar 14, 2021

I think the error is actually correct - baz is not visible to foo because it's in a different module. The diagnostic could be better though.

@camelid
Copy link
Member Author

camelid commented Mar 14, 2021

I think the error is actually correct - baz is not visible to foo because it's in a different module. The diagnostic could be better though.

I thought you're allowed to link to any private item from anywhere else...? I minimized this code from a project I'm working on, so there are definitely use cases.

@jyn514
Copy link
Member

jyn514 commented Mar 14, 2021

I don't think there's ever been docs on what private items you can link to. Notice that this is the "broken" lint and not the "private" lint - rustc_resolve itself is giving this error, not rustdoc, and it would need massive changes to be different (which I doubt @petrochenkov would be happy about).

@camelid camelid added the E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. label Mar 30, 2021
@camelid
Copy link
Member Author

camelid commented Apr 25, 2021

It looks like rustc_resolve is able to give a better error:

mod foo {
    fn error() {
        super::bar::baz()
    }
}

mod bar {
    fn baz() {}
}
error[E0603]: function `baz` is private
 --> src/lib.rs:3:21
  |
3 |         super::bar::baz()
  |                     ^^^ private function
  |
note: the function `baz` is defined here
 --> src/lib.rs:8:5
  |
8 |     fn baz() {}
  |     ^^^^^^^^

I'm not 100% certain, but I think that error might be coming from here:

self.report_privacy_error(error);

// Print the primary message.
let descr = get_descr(binding);
let mut err =
struct_span_err!(self.session, ident.span, E0603, "{} `{}` is private", descr, ident);
err.span_label(ident.span, &format!("private {}", descr));

if !self.is_accessible_from(binding.vis, parent_scope.module) {
self.privacy_errors.push(PrivacyError {
ident,
binding,
dedup_span: path_span,
});
}

Is there some way we can pick up on the fact that the item exists, but is private? Otherwise it's really confusing because the item does exist — so I end up trying to look for typos in the path, when in reality it's just that the item is private.

@jyn514
Copy link
Member

jyn514 commented Apr 25, 2021

@camelid that's #74207

@camelid camelid added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 25, 2021
iTrooz added a commit to iTrooz/efivar-rs that referenced this issue Sep 1, 2023
For the reason the parse module is made pub(crate), see github:rust-lang/rust/issues/83049
iTrooz added a commit to iTrooz/efivar-rs that referenced this issue Sep 1, 2023
For the reason the parse module is made pub(crate), see github:rust-lang/rust/issues/83049
@fmease
Copy link
Member

fmease commented Sep 25, 2023

Simplified:

//! [`priv_mod::priv_fn`]

mod priv_mod { fn priv_fn() {} }

@hikari-no-yume
Copy link

The fact this happens even when using --document-private-items is maddening for my use. My project's docs are always built with that flag set.

@fmease
Copy link
Member

fmease commented Sep 26, 2023

I'm looking into fixing this, can't promise anything though

@fmease fmease self-assigned this Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. P-low Low priority 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

5 participants