Skip to content

fix intra-doc links on nested use and extern crate items #113958

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

Merged
merged 5 commits into from
Jul 25, 2023
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
9 changes: 4 additions & 5 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2244,13 +2244,12 @@ pub fn provide(providers: &mut Providers) {
tcx.resolutions(())
.doc_link_resolutions
.get(&def_id)
.expect("no resolutions for a doc link")
.unwrap_or_else(|| span_bug!(tcx.def_span(def_id), "no resolutions for a doc link"))
},
doc_link_traits_in_scope: |tcx, def_id| {
tcx.resolutions(())
.doc_link_traits_in_scope
.get(&def_id)
.expect("no traits in scope for a doc link")
tcx.resolutions(()).doc_link_traits_in_scope.get(&def_id).unwrap_or_else(|| {
span_bug!(tcx.def_span(def_id), "no traits in scope for a doc link")
})
},
traits: |tcx, LocalCrate| {
let mut traits = Vec::new();
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,17 @@ passes_doc_keyword_not_mod =
passes_doc_keyword_only_impl =
`#[doc(keyword = "...")]` should be used on impl blocks

passes_doc_masked_not_extern_crate_self =
this attribute cannot be applied to an `extern crate self` item
.label = not applicable on `extern crate self` items
.extern_crate_self_label = `extern crate self` defined here

passes_doc_masked_only_extern_crate =
this attribute can only be applied to an `extern crate` item
.label = only applicable on `extern crate` items
.not_an_extern_crate_label = not an `extern crate` item
.note = read <https://doc.rust-lang.org/unstable-book/language-features/doc-masked.html> for more information

passes_doc_test_literal = `#![doc(test(...)]` does not take a literal

passes_doc_test_takes_list =
Expand Down
49 changes: 49 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,44 @@ impl CheckAttrVisitor<'_> {
}
}

fn check_doc_masked(
&self,
attr: &Attribute,
meta: &NestedMetaItem,
hir_id: HirId,
target: Target,
) -> bool {
if target != Target::ExternCrate {
self.tcx.emit_spanned_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
errors::DocMaskedOnlyExternCrate {
attr_span: meta.span(),
item_span: (attr.style == AttrStyle::Outer)
.then(|| self.tcx.hir().span(hir_id)),
},
);
return false;
}

if self.tcx.extern_mod_stmt_cnum(hir_id.owner).is_none() {
self.tcx.emit_spanned_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
errors::DocMaskedNotExternCrateSelf {
attr_span: meta.span(),
item_span: (attr.style == AttrStyle::Outer)
.then(|| self.tcx.hir().span(hir_id)),
},
);
return false;
}

true
}

/// Checks that an attribute is *not* used at the crate level. Returns `true` if valid.
fn check_attr_not_crate_level(
&self,
Expand Down Expand Up @@ -1048,6 +1086,17 @@ impl CheckAttrVisitor<'_> {
is_valid = false;
}

sym::masked
if !self.check_doc_masked(
attr,
meta,
hir_id,
target,
) =>
{
is_valid = false;
}

// no_default_passes: deprecated
// passes: deprecated
// plugins: removed, but rustdoc warns about it itself
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,25 @@ pub struct DocInlineOnlyUse {
pub item_span: Option<Span>,
}

#[derive(LintDiagnostic)]
#[diag(passes_doc_masked_only_extern_crate)]
#[note]
pub struct DocMaskedOnlyExternCrate {
#[label]
pub attr_span: Span,
#[label(passes_not_an_extern_crate_label)]
pub item_span: Option<Span>,
}

#[derive(LintDiagnostic)]
#[diag(passes_doc_masked_not_extern_crate_self)]
pub struct DocMaskedNotExternCrateSelf {
#[label]
pub attr_span: Span,
#[label(passes_extern_crate_self_label)]
pub item_span: Option<Span>,
}

#[derive(Diagnostic)]
#[diag(passes_doc_attr_not_crate_level)]
pub struct DocAttrNotCrateLevel<'a> {
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ enum MaybeExported<'a> {
Ok(NodeId),
Impl(Option<DefId>),
ImplItem(Result<DefId, &'a Visibility>),
NestedUse(&'a Visibility),
}

impl MaybeExported<'_> {
Expand All @@ -559,7 +560,9 @@ impl MaybeExported<'_> {
trait_def_id.as_local()
}
MaybeExported::Impl(None) => return true,
MaybeExported::ImplItem(Err(vis)) => return vis.kind.is_pub(),
MaybeExported::ImplItem(Err(vis)) | MaybeExported::NestedUse(vis) => {
return vis.kind.is_pub();
}
};
def_id.map_or(true, |def_id| r.effective_visibilities.is_exported(def_id))
}
Expand Down Expand Up @@ -2284,7 +2287,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
fn resolve_item(&mut self, item: &'ast Item) {
let mod_inner_docs =
matches!(item.kind, ItemKind::Mod(..)) && rustdoc::inner_docs(&item.attrs);
if !mod_inner_docs && !matches!(item.kind, ItemKind::Impl(..)) {
if !mod_inner_docs && !matches!(item.kind, ItemKind::Impl(..) | ItemKind::Use(..)) {
self.resolve_doc_links(&item.attrs, MaybeExported::Ok(item.id));
}

Expand Down Expand Up @@ -2428,6 +2431,12 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

ItemKind::Use(ref use_tree) => {
let maybe_exported = match use_tree.kind {
UseTreeKind::Simple(_) | UseTreeKind::Glob => MaybeExported::Ok(item.id),
UseTreeKind::Nested(_) => MaybeExported::NestedUse(&item.vis),
};
self.resolve_doc_links(&item.attrs, maybe_exported);

self.future_proof_import(use_tree);
}

Expand Down
15 changes: 6 additions & 9 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2643,15 +2643,12 @@ fn clean_extern_crate<'tcx>(
}
}

// FIXME: using `from_def_id_and_kind` breaks `rustdoc/masked` for some reason
vec![Item {
name: Some(name),
attrs: Box::new(Attributes::from_ast(attrs)),
item_id: crate_def_id.into(),
kind: Box::new(ExternCrateItem { src: orig_name }),
cfg: attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
inline_stmt_id: Some(krate_owner_def_id),
}]
vec![Item::from_def_id_and_parts(
krate_owner_def_id,
Some(name),
ExternCrateItem { src: orig_name },
cx,
)]
}

fn clean_use_statement<'tcx>(
Expand Down
12 changes: 8 additions & 4 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
for it in &module.items {
// `compiler_builtins` should be masked too, but we can't apply
// `#[doc(masked)]` to the injected `extern crate` because it's unstable.
if it.is_extern_crate()
&& (it.attrs.has_doc_flag(sym::masked)
|| cx.tcx.is_compiler_builtins(it.item_id.krate()))
{
if cx.tcx.is_compiler_builtins(it.item_id.krate()) {
cx.cache.masked_crates.insert(it.item_id.krate());
} else if it.is_extern_crate()
&& it.attrs.has_doc_flag(sym::masked)
&& let Some(def_id) = it.item_id.as_def_id()
&& let Some(local_def_id) = def_id.as_local()
&& let Some(cnum) = cx.tcx.extern_mod_stmt_cnum(local_def_id)
{
cx.cache.masked_crates.insert(cnum);
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_metadata::creader::{CStore, LoadedMacro};
use rustc_middle::ty;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -662,6 +662,14 @@ pub(crate) fn href_with_root_path(
// documented on their parent's page
tcx.parent(did)
}
DefKind::ExternCrate => {
// Link to the crate itself, not the `extern crate` item.
if let Some(local_did) = did.as_local() {
tcx.extern_mod_stmt_cnum(local_did).unwrap_or(LOCAL_CRATE).as_def_id()
} else {
did
}
}
_ => did,
};
let cache = cx.cache();
Expand Down
10 changes: 8 additions & 2 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_hir::def::{DefKind, Namespace, PerNS};
use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::Mutability;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_middle::{bug, ty};
use rustc_middle::{bug, span_bug, ty};
use rustc_resolve::rustdoc::{has_primitive_or_keyword_docs, prepare_to_doc_link_resolution};
use rustc_resolve::rustdoc::{strip_generics_from_path, MalformedGenerics};
use rustc_session::lint::Lint;
Expand Down Expand Up @@ -402,7 +402,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
// `doc_link_resolutions` is missing a `path_str`, that means that there are valid links
// that are being missed. To fix the ICE, change
// `rustc_resolve::rustdoc::attrs_to_preprocessed_links` to cache the link.
.unwrap_or_else(|| panic!("no resolution for {:?} {:?} {:?}", path_str, ns, module_id))
.unwrap_or_else(|| {
span_bug!(
self.cx.tcx.def_span(item_id),
"no resolution for {path_str:?} {ns:?} {module_id:?}",
)
})
.and_then(|res| res.try_into().ok())
.or_else(|| resolve_primitive(path_str, ns));
debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns);
Expand Down Expand Up @@ -963,6 +968,7 @@ fn preprocessed_markdown_links(s: &str) -> Vec<PreprocessedMarkdownLink> {
}

impl LinkCollector<'_, '_> {
#[instrument(level = "debug", skip_all)]
fn resolve_links(&mut self, item: &Item) {
if !self.cx.render_options.document_private
&& let Some(def_id) = item.item_id.as_def_id()
Expand Down
16 changes: 16 additions & 0 deletions tests/rustdoc-ui/intra-doc/broken-link-in-unused-doc-string.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Test that we don't ICE with broken links that don't show up in the docs.

// check-pass
// edition: 2021

/// [1]
//~^ WARN unresolved link to `1`
//~| WARN unresolved link to `1`
pub use {std, core};

/// [2]
pub use {};

/// [3]
//~^ WARN unresolved link to `3`
pub extern crate alloc;
27 changes: 27 additions & 0 deletions tests/rustdoc-ui/intra-doc/broken-link-in-unused-doc-string.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
warning: unresolved link to `3`
--> $DIR/broken-link-in-unused-doc-string.rs:14:6
|
LL | /// [3]
| ^ no item named `3` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
= note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

warning: unresolved link to `1`
--> $DIR/broken-link-in-unused-doc-string.rs:6:6
|
LL | /// [1]
| ^ no item named `1` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

warning: unresolved link to `1`
--> $DIR/broken-link-in-unused-doc-string.rs:6:6
|
LL | /// [1]
| ^ no item named `1` in scope
|
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

warning: 3 warnings emitted

15 changes: 15 additions & 0 deletions tests/rustdoc-ui/lints/invalid-doc-attr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#![crate_type = "lib"]
#![deny(warnings)]
#![feature(doc_masked)]

#![doc(masked)]
//~^ ERROR this attribute can only be applied to an `extern crate` item
//~| WARN is being phased out

#[doc(test(no_crate_inject))]
//~^ ERROR can only be applied at the crate level
Expand Down Expand Up @@ -30,3 +35,13 @@ pub mod bar {
//~^^ ERROR conflicting doc inlining attributes
//~| HELP remove one of the conflicting attributes
pub use bar::baz;

#[doc(masked)]
//~^ ERROR this attribute can only be applied to an `extern crate` item
//~| WARN is being phased out
pub struct Masked;

#[doc(masked)]
//~^ ERROR this attribute cannot be applied to an `extern crate self` item
//~| WARN is being phased out
pub extern crate self as reexport;
Loading