Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::fmt::Display;
use std::mem;
use std::ops::Range;

use rustc_ast::attr::AttributeExt;
use rustc_ast::util::comments::may_have_doc_links;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
Expand Down Expand Up @@ -1038,20 +1037,21 @@ fn preprocessed_markdown_links(s: &str) -> Vec<PreprocessedMarkdownLink> {
impl LinkCollector<'_, '_> {
#[instrument(level = "debug", skip_all)]
fn resolve_links(&mut self, item: &Item) {
let tcx = self.cx.tcx;
if !self.cx.document_private()
&& let Some(def_id) = item.item_id.as_def_id()
&& let Some(def_id) = def_id.as_local()
&& !self.cx.tcx.effective_visibilities(()).is_exported(def_id)
&& !tcx.effective_visibilities(()).is_exported(def_id)
&& !has_primitive_or_keyword_or_attribute_docs(&item.attrs.other_attrs)
{
// Skip link resolution for non-exported items.
return;
}

let mut insert_links = |item_id, doc: &str| {
let module_id = match self.cx.tcx.def_kind(item_id) {
DefKind::Mod if item.inner_docs(self.cx.tcx) => item_id,
_ => find_nearest_parent_module(self.cx.tcx, item_id).unwrap(),
let module_id = match tcx.def_kind(item_id) {
DefKind::Mod if item.inner_docs(tcx) => item_id,
_ => find_nearest_parent_module(tcx, item_id).unwrap(),
};
for md_link in preprocessed_markdown_links(&doc) {
let link = self.resolve_link(&doc, item, item_id, module_id, &md_link);
Expand Down Expand Up @@ -1084,15 +1084,33 @@ impl LinkCollector<'_, '_> {

// Also resolve links in the note text of `#[deprecated]`.
for attr in &item.attrs.other_attrs {
let Some(note_sym) = attr.deprecation_note() else { continue };
let rustc_hir::Attribute::Parsed(rustc_hir::attrs::AttributeKind::Deprecation {
span,
deprecation,
}) = attr
else {
continue;
};
let Some(note_sym) = deprecation.note else { continue };
let note = note_sym.as_str();

if !may_have_doc_links(note) {
continue;
}

debug!("deprecated_note={note}");
insert_links(item.item_id.expect_def_id(), note)
// When resolving an intra-doc link inside a deprecation note that is on an inlined
// `use` statement, we need to use the `def_id` of the `use` statement, not the
// inlined item.
// <https://github.com/rust-lang/rust/pull/151120>
let item_id = if let Some(inline_stmt_id) = item.inline_stmt_id
&& item.span(tcx).is_none_or(|item_span| !item_span.inner().contains(*span))
{
inline_stmt_id.to_def_id()
} else {
item.item_id.expect_def_id()
};
insert_links(item_id, note)
Comment on lines 1106 to 1113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let item_id = if let Some(inline_stmt_id) = item.inline_stmt_id
&& !item.span(tcx).is_none_or(|item_span| item_span.inner().contains(*span))
{
inline_stmt_id.to_def_id()
} else {
item.item_id.expect_def_id()
};
insert_links(item_id, note)
// when resolving an intra-doc link inside a deprecation note
// that is on an inlined `use` statement, we need to use the `def_id` of
// the `use` statement, not the inlined item.
// <https://github.com/rust-lang/rust/pull/151120>
let item_id = if let Some(inline_stmt_id) = item.inline_stmt_id
&& item.span(tcx).is_some_and(|item_span| !item_span.inner().contains(*span))
{
inline_stmt_id.to_def_id()
} else {
item.item_id.expect_def_id()
};
insert_links(item_id, note)
  1. I think moving the negation inwards makes the condition easier to reason about
  2. This is a weird edge case so I think it's good to give a comment. yes git blame exists but if a refactor happens between now and when this needs to be re-investigated, it might not be very helpful (it doesn't have to be this exact comment, feel free to write your own if you think you can do better, i left out the macro hazard for brevity).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed for the comment, however if the span is None, or doesn't contain the attribute, then we should use the use span, which means my original condition was wrong too, the ! should have been for the contains, not the whole thing. ^^'

}
}

Expand Down
11 changes: 11 additions & 0 deletions tests/rustdoc-html/intra-doc/ice-deprecated-note-on-reexport.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// This test ensures that the intra-doc link in `deprecated` note is resolved at the correct
// location (ie in the current crate and not in the reexported item's location/crate) and
// therefore doesn't crash.
//
// This is a regression test for <https://github.com/rust-lang/rust/issues/151028>.

#![crate_name = "foo"]

#[deprecated(note = "use [`std::mem::forget`]")]
#[doc(inline)]
pub use std::mem::drop;
Loading