intra-doc links: crate:: considers the traits in the crate scope, not the current scope

[pitaj]

I tried this code:

#![allow(unused_imports, dead_code)]

// Uncomment this and the warning will go away
// use crate::io::Read;

pub mod io {
    pub trait Read {
        fn read(&mut self, buf: &mut [u8]) -> usize;
    }
}

pub mod bufreader {
    use crate::io::Read;

    /// It can be excessively inefficient to work directly with a [`Read`] instance.
    /// For example, every [`Read::read`] call to [`TcpStream::read`] or [`TcpStream::yes`] on [`TcpStream`]
    ///
    /// [`TcpStream::yes`]: crate::net::TcpStream::yes
    /// [`TcpStream::read`]: crate::net::TcpStream::read
    /// [`Read::read`]: Read::read
    /// [`TcpStream`]: crate::net::TcpStream
    pub struct BufReader;
}

pub mod net {
    pub struct TcpStream;

    impl TcpStream {
        fn yes() -> bool {
            true
        }
    }

    impl crate::io::Read for TcpStream {
        fn read(&mut self, buf: &mut [u8]) -> usize {
            buf.len()
        }
    }
}

I expected to see this happen: Rustdoc should complete with no warnings

Instead, this happened:

Only the following warning is output by rustdoc

warning: unresolved link to `self::net::TcpStream::read`
  --> src/main.rs:13:30
   |
13 |     /// [`TcpStream::read`]: crate::net::TcpStream::read
   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the struct `TcpStream` has no field or associated item named `read`
   |
   = note: `#[warn(broken_intra_doc_links)]` on by default

If you uncomment the line as it says, the warning goes away.

Meta

rustc +stage2 --version --verbose:

rustc 1.49.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-unknown-linux-gnu
release: 1.49.0-dev

Commit hash: b202532

Here's a crate that repros this issue: https://github.com/pitaj/rustdoc-78006-repro

@rustbot modify labels: T-doc, A-intra-doc-links

@jyn514 Here's the repro I promised.

First came across this in #78006

[jyn514]

Slightly smaller:

// Uncomment this and the warning will go away
// use crate::io::Read;

pub mod io {
    pub trait Read {
        fn read(&mut self);
    }
}

pub mod bufreader {
    //! [`crate::TcpStream::read`]
    use crate::io::Read;
}

pub struct TcpStream;

impl crate::io::Read for TcpStream {
    fn read(&mut self) {
    }
}

[jyn514]

The issue is that rustdoc thinks the trait isn't in scope for some reason:

2:rustcDEBUG rustdoc::passes::collect_intra_doc_links looking for associated item named read for item DefId(0:8 ~ impl[8787]::TcpStream)
2:rustcDEBUG rustdoc::passes::collect_intra_doc_links considering traits {}
2:rustcDEBUG rustdoc::passes::collect_intra_doc_links the candidates were []
2:rustcDEBUG rustdoc::passes::collect_intra_doc_links got associated item kind None
2:rustcDEBUG rustdoc::passes::collect_intra_doc_links self::TcpStream::read resolved to Err(()) in namespace ValueNS

[jyn514]

Because it's resolving the scope of the crate, because of the hack I did to fix crate:: across crates: #77253. If you use super::TcpStream::read instead, it works fine.

[jyn514]

@pitaj you could get #78006 working by using super instead for now.

[jyn514]

@petrochenkov how hard would it be to allow $crate in resolve_str_path_error? Rustdoc has both the original and current scope available, but right now it can only pass one or the other to resolve.

Using $crate would fix this issue and also some other hacks in rustdoc (see #77253).

[petrochenkov]

resolve_str_path_error should already support $crate internally, but rustdoc passing path as a string (without span) will mean that $crate will always resolve to the current crate. Resolve needs correct spans to perform resolution in cases like this.

[jyn514]

Mentoring instructions: everywhere that currently calls resolve_str_path_error with DUMMY_SP needs to instead pass the span of the item. You can get the span from item.source. fn resolve needs to be changed to take the span as a parameter:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Line 1190 in b385598

match self.resolve(path_str, ns, &current_item, base_node, &extra_fragment) {

There are many different calls to resolve_str_path_error and not all of them go through fn resolve; those will need to be updated as well.

Once you've passed the correct span, change the hack in

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 1014 to 1021 in b385598

	// 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.
	resolved_self = format!("self::{}", &path_str["crate::".len()..]);
	path_str = &resolved_self;
	module_id = DefId { krate, index: CRATE_DEF_INDEX };

to instead replace crate:: with $crate::. Also remove the modification of module_id, which is what will actually fix #78696.

[petrochenkov]

Fixed in #95337.