Skip to content

Commit

Permalink
Auto merge of #77276 - GuillaumeGomez:reexported-item-lints, r=jyn514…
Browse files Browse the repository at this point in the history
…,ollie27

Warn on broken intra-doc links added to cross-crate re-exports

This emits `broken_intra_doc_links` for docs applied to pub use statements that point to external items and are inlined.
Does not address #77200 - any existing broken links from the original crate will not show warnings.

r? `@jyn514`
  • Loading branch information
bors committed Oct 9, 2020
2 parents 5ddef54 + 7e218bb commit 38d911d
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet<DefId>)
visibility: clean::Public,
stability: None,
deprecation: None,
inner: clean::ImportItem(clean::Import::Simple(
inner: clean::ImportItem(clean::Import::new_simple(
item.ident.to_string(),
clean::ImportSource {
path: clean::Path {
Expand All @@ -514,6 +514,7 @@ fn build_module(cx: &DocContext<'_>, did: DefId, visited: &mut FxHashSet<DefId>)
},
did: None,
},
true,
)),
});
} else if let Some(i) =
Expand Down
22 changes: 18 additions & 4 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2258,8 +2258,7 @@ impl Clean<Vec<Item>> for doctree::Import<'_> {
return items;
}
}

Import::Glob(resolve_use_source(cx, path))
Import::new_glob(resolve_use_source(cx, path), true)
} else {
let name = self.name;
if !please_inline {
Expand All @@ -2273,18 +2272,33 @@ impl Clean<Vec<Item>> for doctree::Import<'_> {
}
if !denied {
let mut visited = FxHashSet::default();
if let Some(items) = inline::try_inline(

if let Some(mut items) = inline::try_inline(
cx,
cx.tcx.parent_module(self.id).to_def_id(),
path.res,
name,
Some(self.attrs),
&mut visited,
) {
items.push(Item {
name: None,
attrs: self.attrs.clean(cx),
source: self.span.clean(cx),
def_id: cx.tcx.hir().local_def_id(self.id).to_def_id(),
visibility: self.vis.clean(cx),
stability: None,
deprecation: None,
inner: ImportItem(Import::new_simple(
self.name.clean(cx),
resolve_use_source(cx, path),
false,
)),
});
return items;
}
}
Import::Simple(name.clean(cx), resolve_use_source(cx, path))
Import::new_simple(name.clean(cx), resolve_use_source(cx, path), true)
};

vec![Item {
Expand Down
24 changes: 21 additions & 3 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ impl Item {
pub fn is_stripped(&self) -> bool {
match self.inner {
StrippedItem(..) => true,
ImportItem(ref i) => !i.should_be_displayed,
_ => false,
}
}
Expand Down Expand Up @@ -1653,11 +1654,28 @@ pub struct Impl {
}

#[derive(Clone, Debug)]
pub enum Import {
pub struct Import {
pub kind: ImportKind,
pub source: ImportSource,
pub should_be_displayed: bool,
}

impl Import {
pub fn new_simple(name: String, source: ImportSource, should_be_displayed: bool) -> Self {
Self { kind: ImportKind::Simple(name), source, should_be_displayed }
}

pub fn new_glob(source: ImportSource, should_be_displayed: bool) -> Self {
Self { kind: ImportKind::Glob, source, should_be_displayed }
}
}

#[derive(Clone, Debug)]
pub enum ImportKind {
// use source as str;
Simple(String, ImportSource),
Simple(String),
// use source::*;
Glob(ImportSource),
Glob,
}

#[derive(Clone, Debug)]
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/doctree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ pub struct ExternCrate<'hir> {
pub span: Span,
}

#[derive(Debug)]
pub struct Import<'hir> {
pub name: Symbol,
pub id: hir::HirId,
Expand Down
16 changes: 8 additions & 8 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,19 +1149,19 @@ impl PrintWithSpace for hir::Mutability {

impl clean::Import {
crate fn print(&self) -> impl fmt::Display + '_ {
display_fn(move |f| match *self {
clean::Import::Simple(ref name, ref src) => {
if *name == src.path.last_name() {
write!(f, "use {};", src.print())
display_fn(move |f| match self.kind {
clean::ImportKind::Simple(ref name) => {
if *name == self.source.path.last_name() {
write!(f, "use {};", self.source.print())
} else {
write!(f, "use {} as {};", src.print(), *name)
write!(f, "use {} as {};", self.source.print(), *name)
}
}
clean::Import::Glob(ref src) => {
if src.path.segments.is_empty() {
clean::ImportKind::Glob => {
if self.source.path.segments.is_empty() {
write!(f, "use *;")
} else {
write!(f, "use {}::*;", src.print())
write!(f, "use {}::*;", self.source.print())
}
}
})
Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4438,8 +4438,9 @@ fn item_ty_to_strs(ty: &ItemType) -> (&'static str, &'static str) {
fn sidebar_module(buf: &mut Buffer, items: &[clean::Item]) {
let mut sidebar = String::new();

if items.iter().any(|it| it.type_() == ItemType::ExternCrate || it.type_() == ItemType::Import)
{
if items.iter().any(|it| {
it.type_() == ItemType::ExternCrate || (it.type_() == ItemType::Import && !it.is_stripped())
}) {
sidebar.push_str(&format!(
"<li><a href=\"#{id}\">{name}</a></li>",
id = "reexports",
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
debug!("ignoring extern crate item {:?}", item.def_id);
return self.fold_item_recur(item);
}
ImportItem(Import::Simple(ref name, ..)) => Some(name.clone()),
ImportItem(Import { kind: ImportKind::Simple(ref name, ..), .. }) => Some(name.clone()),
MacroItem(..) => None,
_ => item.name.clone(),
};
Expand Down
4 changes: 4 additions & 0 deletions src/test/rustdoc-ui/auxiliary/intra-doc-broken.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#![crate_name = "intra_doc_broken"]

/// [not_found]
pub fn foo() {}
8 changes: 8 additions & 0 deletions src/test/rustdoc-ui/intra-doc-broken-reexport.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// aux-build:intra-doc-broken.rs
// check-pass

#![deny(broken_intra_doc_links)]

extern crate intra_doc_broken;

pub use intra_doc_broken::foo;
5 changes: 5 additions & 0 deletions src/test/rustdoc-ui/pub-export-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![deny(broken_intra_doc_links)]

/// [aloha]
//~^ ERROR unresolved link to `aloha`
pub use std::task::RawWakerVTable;
15 changes: 15 additions & 0 deletions src/test/rustdoc-ui/pub-export-lint.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: unresolved link to `aloha`
--> $DIR/pub-export-lint.rs:3:6
|
LL | /// [aloha]
| ^^^^^ no item named `aloha` in scope
|
note: the lint level is defined here
--> $DIR/pub-export-lint.rs:1:9
|
LL | #![deny(broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: aborting due to previous error

9 changes: 9 additions & 0 deletions src/test/rustdoc/reexport-check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![crate_name = "foo"]

// @!has 'foo/index.html' '//code' 'pub use self::i32;'
// @has 'foo/index.html' '//tr[@class="module-item"]' 'i32'
// @has 'foo/i32/index.html'
pub use std::i32;
// @!has 'foo/index.html' '//code' 'pub use self::string::String;'
// @has 'foo/index.html' '//tr[@class="module-item"]' 'String'
pub use std::string::String;

0 comments on commit 38d911d

Please sign in to comment.