rustdoc fails to properly resolve `Self` in a submodule when the type is not in scope

[de-vri-es]

When a doc comment refers to Self inside an impl block for a type that is not in scope, rustdoc can't properly resolve Self.

It does resolve Self to a more specific name (Foo in the example below), but then it can't find that name.

A minimum example:

struct Foo {
	foo: i32,
}

mod bar {
	impl crate::Foo {
		/// Baz the [`Self::foo`].
		pub fn baz(&self) {
			println!("bazzing the foo");
		}
	}
}

This produces a broken link in the documentation and the following warning:

$ cargo doc
 Documenting foo v0.1.0 (/tmp/2021-05-02-18-45-24/foo)
warning: unresolved link to `Foo::foo`
 --> src/main.rs:7:16
  |
7 |         /// Baz the [`Self::foo`].
  |                      ^^^^^^^^^^^ no item named `Foo` in scope
  |
  = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.66s

Meta

I tested this using:

• rustdoc 1.53.0-nightly (42816d6 2021-04-24)
• rustdoc 1.51.0 (2fd73fa 2021-03-23)

Both show the same problem.

@rustbot modify labels: +A-rustdoc

[de-vri-es]

Also related: if you do import a type with the same name, then rustdoc will think that is Self.

That can be demonstrated with this example:

pub struct Foo {
	pub foo: i32,
}

pub mod bar {
	use crate::other::Foo;

	impl crate::Foo {
		/// Baz the [`Self::foo`].
		pub fn baz(&self) {
			println!("bazzing the foo");
		}
	}
}

pub mod other {
	pub struct Foo;
}

Which produces the following warning:

warning: unresolved link to `Foo::foo`
 --> src/main.rs:9:16
  |
9 |         /// Baz the [`Self::foo`].
  |                      ^^^^^^^^^^^ the struct `Foo` has no field or associated item named `foo`

And if I add a field with the same name, it really links to the wrong Foo.

[jyn514]

This is basically the issue from #83761 (comment), rustdoc should use the DefId instead of strings.

[jyn514]

Relevant code in case someone wants to take a look:

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 837 to 838 in e10cbc3

	// FIXME(jynelson): this shouldn't go through stringification, rustdoc should just use the DefId directly
	let self_name = self_id.and_then(|self_id| {

[jyn514]

Basically the way to do this is to only do the string munging in

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 490 to 518 in e10cbc3

	// Try looking for methods and associated items.
	let mut split = path_str.rsplitn(2, "::");
	// NB: `split`'s first element is always defined, even if the delimiter was not present.
	// NB: `item_str` could be empty when resolving in the root namespace (e.g. `::std`).
	let item_str = split.next().unwrap();
	let item_name = Symbol::intern(item_str);
	let path_root = split
	.next()
	.map(|f| f.to_owned())
	// If there's no `::`, it's not an associated item.
	// So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved.
	.ok_or_else(|| {
	debug!("found no `::`, assumming {} was correctly not in scope", item_name);
	ResolutionFailure::NotResolved {
	module_id,
	partial_res: None,
	unresolved: item_str.into(),
	}
	})?;
	// FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support
	// links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity
	// error instead and special case *only* modules with `#[doc(primitive)]`, not all
	// primitives.
	resolve_primitive(&path_root, TypeNS)
	.or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
	.and_then(|ty_res| {
	let (res, fragment, side_channel) =
	self.resolve_associated_item(ty_res, item_name, ns, module_id)?;

if self_id.is_none(); otherwise use self_id in the place of ty_res.

[jyn514]

Ditto for

rust/src/librustdoc/passes/collect_intra_doc_links.rs

Lines 490 to 518 in e10cbc3

	// Try looking for methods and associated items.
	let mut split = path_str.rsplitn(2, "::");
	// NB: `split`'s first element is always defined, even if the delimiter was not present.
	// NB: `item_str` could be empty when resolving in the root namespace (e.g. `::std`).
	let item_str = split.next().unwrap();
	let item_name = Symbol::intern(item_str);
	let path_root = split
	.next()
	.map(|f| f.to_owned())
	// If there's no `::`, it's not an associated item.
	// So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved.
	.ok_or_else(|| {
	debug!("found no `::`, assumming {} was correctly not in scope", item_name);
	ResolutionFailure::NotResolved {
	module_id,
	partial_res: None,
	unresolved: item_str.into(),
	}
	})?;
	// FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support
	// links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity
	// error instead and special case *only* modules with `#[doc(primitive)]`, not all
	// primitives.
	resolve_primitive(&path_root, TypeNS)
	.or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
	.and_then(|ty_res| {
	let (res, fragment, side_channel) =
	self.resolve_associated_item(ty_res, item_name, ns, module_id)?;

[tdelabro]

@rustbot claim