From d3f4c48b49929b9d5de7d65c608a244a1b5f9e81 Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 11:48:12 -0800 Subject: [PATCH 01/14] rustdoc: Render visibilities succinctly --- src/librustdoc/clean/mod.rs | 4 +-- src/librustdoc/html/format.rs | 13 ++++++++-- src/librustdoc/html/render/mod.rs | 43 ++++++++++++++++++------------- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index de53ce8d95c1..4fca6416ce29 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2299,14 +2299,14 @@ impl Clean for (&hir::MacroDef<'_>, Option) { if matchers.len() <= 1 { format!( "{}macro {}{} {{\n ...\n}}", - vis.print_with_space(cx.tcx), + vis.print_with_space(cx.tcx, item.hir_id.owner), name, matchers.iter().map(|span| span.to_src(cx)).collect::(), ) } else { format!( "{}macro {} {{\n{}}}", - vis.print_with_space(cx.tcx), + vis.print_with_space(cx.tcx, item.hir_id.owner), name, matchers .iter() diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 7b0b219570b9..bd5f5a3c6cc8 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -12,7 +12,7 @@ use std::fmt; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::{DefId, CRATE_DEF_INDEX}; +use rustc_span::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX}; use rustc_target::spec::abi::Abi; use crate::clean::{self, PrimitiveType}; @@ -1085,12 +1085,21 @@ impl Function<'_> { } impl clean::Visibility { - crate fn print_with_space<'tcx>(self, tcx: TyCtxt<'tcx>) -> impl fmt::Display + 'tcx { + crate fn print_with_space<'tcx>( + self, + tcx: TyCtxt<'tcx>, + item_did: LocalDefId, + ) -> impl fmt::Display + 'tcx { use rustc_span::symbol::kw; display_fn(move |f| match self { clean::Public => f.write_str("pub "), clean::Inherited => Ok(()), + clean::Visibility::Restricted(did) + if did.index == tcx.parent_module_from_def_id(item_did).local_def_index => + { + Ok(()) + } clean::Visibility::Restricted(did) if did.index == CRATE_DEF_INDEX => { write!(f, "pub(crate) ") } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 97e7c38ecb8c..2d87b0c104f2 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2157,14 +2157,14 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl Some(ref src) => write!( w, "{}extern crate {} as {};", - myitem.visibility.print_with_space(cx.tcx()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), anchor(myitem.def_id, &*src.as_str()), name ), None => write!( w, "{}extern crate {};", - myitem.visibility.print_with_space(cx.tcx()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), anchor(myitem.def_id, &*name.as_str()) ), } @@ -2175,7 +2175,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl write!( w, "{}{}", - myitem.visibility.print_with_space(cx.tcx()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), import.print() ); } @@ -2392,7 +2392,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean:: write!( w, "{vis}const {name}: {typ}", - vis = it.visibility.print_with_space(cx.tcx()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), name = it.name.as_ref().unwrap(), typ = c.type_.print(), ); @@ -2426,7 +2426,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St write!( w, "{vis}static {mutability}{name}: {typ}", - vis = it.visibility.print_with_space(cx.tcx()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), mutability = s.mutability.print_with_space(), name = it.name.as_ref().unwrap(), typ = s.type_.print() @@ -2437,7 +2437,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) { let header_len = format!( "{}{}{}{}{:#}fn {}{:#}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), f.header.constness.print_with_space(), f.header.asyncness.print_with_space(), f.header.unsafety.print_with_space(), @@ -2452,7 +2452,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean:: w, "{vis}{constness}{asyncness}{unsafety}{abi}fn \ {name}{generics}{decl}{spotlight}{where_clause}", - vis = it.visibility.print_with_space(cx.tcx()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), constness = f.header.constness.print_with_space(), asyncness = f.header.asyncness.print_with_space(), unsafety = f.header.unsafety.print_with_space(), @@ -2578,7 +2578,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra write!( w, "{}{}{}trait {}{}{}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), t.unsafety.print_with_space(), if t.is_auto { "auto " } else { "" }, it.name.as_ref().unwrap(), @@ -2896,7 +2896,7 @@ fn assoc_const( w, "{}{}const {}: {}", extra, - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), naive_assoc_href(it, link), it.name.as_ref().unwrap(), ty.print() @@ -3015,7 +3015,7 @@ fn render_assoc_item( }; let mut header_len = format!( "{}{}{}{}{}{:#}fn {}{:#}", - meth.visibility.print_with_space(cx.tcx()), + meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()), header.constness.print_with_space(), header.asyncness.print_with_space(), header.unsafety.print_with_space(), @@ -3037,7 +3037,7 @@ fn render_assoc_item( "{}{}{}{}{}{}{}fn {name}\ {generics}{decl}{spotlight}{where_clause}", if parent == ItemType::Trait { " " } else { "" }, - meth.visibility.print_with_space(cx.tcx()), + meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()), header.constness.print_with_space(), header.asyncness.print_with_space(), header.unsafety.print_with_space(), @@ -3189,7 +3189,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum write!( w, "{}enum {}{}{}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), it.name.as_ref().unwrap(), e.generics.print(), WhereClause { gens: &e.generics, indent: 0, end_newline: true } @@ -3364,7 +3364,7 @@ fn render_struct( write!( w, "{}{}{}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), if structhead { "struct " } else { "" }, it.name.as_ref().unwrap() ); @@ -3384,7 +3384,7 @@ fn render_struct( w, "\n{} {}{}: {},", tab, - field.visibility.print_with_space(cx.tcx()), + field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()), field.name.as_ref().unwrap(), ty.print() ); @@ -3413,7 +3413,14 @@ fn render_struct( match field.kind { clean::StrippedItem(box clean::StructFieldItem(..)) => write!(w, "_"), clean::StructFieldItem(ref ty) => { - write!(w, "{}{}", field.visibility.print_with_space(cx.tcx()), ty.print()) + write!( + w, + "{}{}", + field + .visibility + .print_with_space(cx.tcx(), field.def_id.expect_local()), + ty.print() + ) } _ => unreachable!(), } @@ -3446,7 +3453,7 @@ fn render_union( write!( w, "{}{}{}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), if structhead { "union " } else { "" }, it.name.as_ref().unwrap() ); @@ -3461,7 +3468,7 @@ fn render_union( write!( w, " {}{}: {},\n{}", - field.visibility.print_with_space(cx.tcx()), + field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()), field.name.as_ref().unwrap(), ty.print(), tab @@ -4100,7 +4107,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, cache: write!( w, " {}type {};\n}}", - it.visibility.print_with_space(cx.tcx()), + it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), it.name.as_ref().unwrap(), ); From 50c1c27fa6adbb20ce61fce7caaea489e3984265 Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 15:38:46 -0800 Subject: [PATCH 02/14] Fix bugs; fix and add tests --- src/librustdoc/clean/mod.rs | 10 +++- src/librustdoc/clean/utils.rs | 24 +++++++- src/librustdoc/html/format.rs | 56 ++++++++++--------- src/librustdoc/html/render/mod.rs | 38 ++++++------- .../passes/collect_intra_doc_links.rs | 22 +------- src/test/rustdoc/decl_macro_priv.rs | 2 +- src/test/rustdoc/pub-restricted.rs | 32 +++++------ src/test/rustdoc/visibility.rs | 13 +++++ 8 files changed, 110 insertions(+), 87 deletions(-) create mode 100644 src/test/rustdoc/visibility.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 4fca6416ce29..548cca0761e7 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2299,14 +2299,20 @@ impl Clean for (&hir::MacroDef<'_>, Option) { if matchers.len() <= 1 { format!( "{}macro {}{} {{\n ...\n}}", - vis.print_with_space(cx.tcx, item.hir_id.owner), + vis.print_with_space( + cx.tcx, + cx.tcx.hir().local_def_id(item.hir_id).to_def_id() + ), name, matchers.iter().map(|span| span.to_src(cx)).collect::(), ) } else { format!( "{}macro {} {{\n{}}}", - vis.print_with_space(cx.tcx, item.hir_id.owner), + vis.print_with_space( + cx.tcx, + cx.tcx.hir().local_def_id(item.hir_id).to_def_id() + ), name, matchers .iter() diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 270321aaa118..fe1b58164474 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -15,7 +15,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::mir::interpret::ConstValue; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; -use rustc_middle::ty::{self, DefIdTree, Ty}; +use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt}; use rustc_span::symbol::{kw, sym, Symbol}; use std::mem; @@ -624,3 +624,25 @@ where *cx.impl_trait_bounds.borrow_mut() = old_bounds; r } + +crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option { + let mut current = def_id; + // The immediate parent might not always be a module. + // Find the first parent which is. + loop { + if let Some(parent) = tcx.parent(current) { + if tcx.def_kind(parent) == DefKind::Mod { + break Some(parent); + } + current = parent; + } else { + debug!( + "{:?} has no parent (kind={:?}, original was {:?})", + current, + tcx.def_kind(current), + def_id + ); + break None; + } + } +} diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index bd5f5a3c6cc8..6a611ccf58ec 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -12,10 +12,10 @@ use std::fmt; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::{DefId, LocalDefId, CRATE_DEF_INDEX}; +use rustc_span::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_target::spec::abi::Abi; -use crate::clean::{self, PrimitiveType}; +use crate::clean::{self, utils::find_closest_parent_module, PrimitiveType}; use crate::formats::cache::cache; use crate::formats::item_type::ItemType; use crate::html::escape::Escape; @@ -1088,38 +1088,40 @@ impl clean::Visibility { crate fn print_with_space<'tcx>( self, tcx: TyCtxt<'tcx>, - item_did: LocalDefId, + item_did: DefId, ) -> impl fmt::Display + 'tcx { use rustc_span::symbol::kw; display_fn(move |f| match self { clean::Public => f.write_str("pub "), clean::Inherited => Ok(()), - clean::Visibility::Restricted(did) - if did.index == tcx.parent_module_from_def_id(item_did).local_def_index => - { - Ok(()) - } - clean::Visibility::Restricted(did) if did.index == CRATE_DEF_INDEX => { - write!(f, "pub(crate) ") - } - clean::Visibility::Restricted(did) => { - f.write_str("pub(")?; - let path = tcx.def_path(did); - debug!("path={:?}", path); - let first_name = - path.data[0].data.get_opt_name().expect("modules are always named"); - if path.data.len() != 1 || (first_name != kw::SelfLower && first_name != kw::Super) - { - f.write_str("in ")?; - } - // modified from `resolved_path()` to work with `DefPathData` - let last_name = path.data.last().unwrap().data.get_opt_name().unwrap(); - for seg in &path.data[..path.data.len() - 1] { - write!(f, "{}::", seg.data.get_opt_name().unwrap())?; + + clean::Visibility::Restricted(vis_did) => { + if find_closest_parent_module(tcx, item_did) == Some(vis_did) { + // `pub(in foo)` where `foo` is the parent module + // is the same as no visibility modifier + Ok(()) + } else if vis_did.index == CRATE_DEF_INDEX { + write!(f, "pub(crate) ") + } else { + f.write_str("pub(")?; + let path = tcx.def_path(vis_did); + debug!("path={:?}", path); + let first_name = + path.data[0].data.get_opt_name().expect("modules are always named"); + if path.data.len() != 1 + || (first_name != kw::SelfLower && first_name != kw::Super) + { + f.write_str("in ")?; + } + // modified from `resolved_path()` to work with `DefPathData` + let last_name = path.data.last().unwrap().data.get_opt_name().unwrap(); + for seg in &path.data[..path.data.len() - 1] { + write!(f, "{}::", seg.data.get_opt_name().unwrap())?; + } + let path = anchor(vis_did, &last_name.as_str()).to_string(); + write!(f, "{}) ", path) } - let path = anchor(did, &last_name.as_str()).to_string(); - write!(f, "{}) ", path) } }) } diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 2d87b0c104f2..190a09d12400 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2157,14 +2157,14 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl Some(ref src) => write!( w, "{}extern crate {} as {};", - myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id), anchor(myitem.def_id, &*src.as_str()), name ), None => write!( w, "{}extern crate {};", - myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id), anchor(myitem.def_id, &*name.as_str()) ), } @@ -2175,7 +2175,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl write!( w, "{}{}", - myitem.visibility.print_with_space(cx.tcx(), myitem.def_id.expect_local()), + myitem.visibility.print_with_space(cx.tcx(), myitem.def_id), import.print() ); } @@ -2392,7 +2392,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean:: write!( w, "{vis}const {name}: {typ}", - vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id), name = it.name.as_ref().unwrap(), typ = c.type_.print(), ); @@ -2426,7 +2426,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St write!( w, "{vis}static {mutability}{name}: {typ}", - vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id), mutability = s.mutability.print_with_space(), name = it.name.as_ref().unwrap(), typ = s.type_.print() @@ -2437,7 +2437,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) { let header_len = format!( "{}{}{}{}{:#}fn {}{:#}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), f.header.constness.print_with_space(), f.header.asyncness.print_with_space(), f.header.unsafety.print_with_space(), @@ -2452,7 +2452,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean:: w, "{vis}{constness}{asyncness}{unsafety}{abi}fn \ {name}{generics}{decl}{spotlight}{where_clause}", - vis = it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + vis = it.visibility.print_with_space(cx.tcx(), it.def_id), constness = f.header.constness.print_with_space(), asyncness = f.header.asyncness.print_with_space(), unsafety = f.header.unsafety.print_with_space(), @@ -2578,7 +2578,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra write!( w, "{}{}{}trait {}{}{}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), t.unsafety.print_with_space(), if t.is_auto { "auto " } else { "" }, it.name.as_ref().unwrap(), @@ -2896,7 +2896,7 @@ fn assoc_const( w, "{}{}const {}: {}", extra, - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), naive_assoc_href(it, link), it.name.as_ref().unwrap(), ty.print() @@ -3015,7 +3015,7 @@ fn render_assoc_item( }; let mut header_len = format!( "{}{}{}{}{}{:#}fn {}{:#}", - meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()), + meth.visibility.print_with_space(cx.tcx(), meth.def_id), header.constness.print_with_space(), header.asyncness.print_with_space(), header.unsafety.print_with_space(), @@ -3037,7 +3037,7 @@ fn render_assoc_item( "{}{}{}{}{}{}{}fn {name}\ {generics}{decl}{spotlight}{where_clause}", if parent == ItemType::Trait { " " } else { "" }, - meth.visibility.print_with_space(cx.tcx(), meth.def_id.expect_local()), + meth.visibility.print_with_space(cx.tcx(), meth.def_id), header.constness.print_with_space(), header.asyncness.print_with_space(), header.unsafety.print_with_space(), @@ -3189,7 +3189,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum write!( w, "{}enum {}{}{}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), it.name.as_ref().unwrap(), e.generics.print(), WhereClause { gens: &e.generics, indent: 0, end_newline: true } @@ -3364,7 +3364,7 @@ fn render_struct( write!( w, "{}{}{}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), if structhead { "struct " } else { "" }, it.name.as_ref().unwrap() ); @@ -3384,7 +3384,7 @@ fn render_struct( w, "\n{} {}{}: {},", tab, - field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()), + field.visibility.print_with_space(cx.tcx(), field.def_id), field.name.as_ref().unwrap(), ty.print() ); @@ -3416,9 +3416,7 @@ fn render_struct( write!( w, "{}{}", - field - .visibility - .print_with_space(cx.tcx(), field.def_id.expect_local()), + field.visibility.print_with_space(cx.tcx(), field.def_id), ty.print() ) } @@ -3453,7 +3451,7 @@ fn render_union( write!( w, "{}{}{}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), if structhead { "union " } else { "" }, it.name.as_ref().unwrap() ); @@ -3468,7 +3466,7 @@ fn render_union( write!( w, " {}{}: {},\n{}", - field.visibility.print_with_space(cx.tcx(), field.def_id.expect_local()), + field.visibility.print_with_space(cx.tcx(), field.def_id), field.name.as_ref().unwrap(), ty.print(), tab @@ -4107,7 +4105,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, cache: write!( w, " {}type {};\n}}", - it.visibility.print_with_space(cx.tcx(), it.def_id.expect_local()), + it.visibility.print_with_space(cx.tcx(), it.def_id), it.name.as_ref().unwrap(), ); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a8adfe08b256..f392f321fbfc 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -31,7 +31,7 @@ use std::cell::Cell; use std::mem; use std::ops::Range; -use crate::clean::{self, Crate, Item, ItemLink, PrimitiveType}; +use crate::clean::{self, utils::find_closest_parent_module, Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::markdown_links; @@ -774,25 +774,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } else if item.def_id.is_top_level_module() { Some(item.def_id) } else { - let mut current = item.def_id; - // The immediate parent might not always be a module. - // Find the first parent which is. - loop { - if let Some(parent) = self.cx.tcx.parent(current) { - if self.cx.tcx.def_kind(parent) == DefKind::Mod { - break Some(parent); - } - current = parent; - } else { - debug!( - "{:?} has no parent (kind={:?}, original was {:?})", - current, - self.cx.tcx.def_kind(current), - item.def_id - ); - break None; - } - } + find_closest_parent_module(self.cx.tcx, item.def_id) }; if parent_node.is_some() { diff --git a/src/test/rustdoc/decl_macro_priv.rs b/src/test/rustdoc/decl_macro_priv.rs index 4e1279e34d93..cb71bca9cf20 100644 --- a/src/test/rustdoc/decl_macro_priv.rs +++ b/src/test/rustdoc/decl_macro_priv.rs @@ -2,7 +2,7 @@ #![feature(decl_macro)] -// @has decl_macro_priv/macro.crate_macro.html //pre 'pub(crate) macro crate_macro() {' +// @has decl_macro_priv/macro.crate_macro.html //pre 'macro crate_macro() {' // @has - //pre '...' // @has - //pre '}' pub(crate) macro crate_macro() {} diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs index 6720d848ac3b..9ff3cad070d8 100644 --- a/src/test/rustdoc/pub-restricted.rs +++ b/src/test/rustdoc/pub-restricted.rs @@ -6,27 +6,27 @@ // @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic' pub struct FooPublic; -// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate' +// @has 'foo/struct.FooJustCrate.html' '//pre' 'struct FooJustCrate' crate struct FooJustCrate; -// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate' +// @has 'foo/struct.FooPubCrate.html' '//pre' 'struct FooPubCrate' pub(crate) struct FooPubCrate; -// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf' +// @has 'foo/struct.FooSelf.html' '//pre' 'struct FooSelf' pub(self) struct FooSelf; -// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf' +// @has 'foo/struct.FooInSelf.html' '//pre' 'struct FooInSelf' pub(in self) struct FooInSelf; mod a { - // @has 'foo/a/struct.FooSuper.html' '//pre' 'pub(crate) struct FooSuper' - pub(super) struct FooSuper; - // @has 'foo/a/struct.FooInSuper.html' '//pre' 'pub(crate) struct FooInSuper' - pub(in super) struct FooInSuper; - // @has 'foo/a/struct.FooInA.html' '//pre' 'pub(in a) struct FooInA' - pub(in a) struct FooInA; + // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper' + pub(super) struct FooASuper; + // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper' + pub(in super) struct FooAInSuper; + // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA' + pub(in a) struct FooAInA; mod b { - // @has 'foo/a/b/struct.FooInSelfSuperB.html' '//pre' 'pub(in a::b) struct FooInSelfSuperB' - pub(in a::b) struct FooInSelfSuperB; - // @has 'foo/a/b/struct.FooInSuperSuper.html' '//pre' 'pub(crate) struct FooInSuperSuper' - pub(in super::super) struct FooInSuperSuper; - // @has 'foo/a/b/struct.FooInAB.html' '//pre' 'pub(in a::b) struct FooInAB' - pub(in a::b) struct FooInAB; + // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper' + pub(super) struct FooBSuper; + // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper' + pub(in super::super) struct FooBInSuperSuper; + // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB' + pub(in a::b) struct FooBInAB; } } diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs new file mode 100644 index 000000000000..9dd0b68b1d95 --- /dev/null +++ b/src/test/rustdoc/visibility.rs @@ -0,0 +1,13 @@ +// compile-flags: --document-private-items + +#![crate_name = "foo"] + +// @has 'foo/fn.foo.html' '//pre' 'fn foo' +// !@has 'foo/fn.foo.html' '//pre' 'pub' +fn foo() {} + +mod bar { + // @has 'foo/bar/fn.baz.html' '//pre' 'fn baz' + // !@has 'foo/bar/fn.baz.html' '//pre' 'pub' + fn baz() {} +} From 00652e429aaa2250f2561a457e46387bb5b16737 Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 15:54:04 -0800 Subject: [PATCH 03/14] Handle `pub(super)` --- src/librustdoc/html/format.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 6a611ccf58ec..a8eae52fc565 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1097,12 +1097,20 @@ impl clean::Visibility { clean::Inherited => Ok(()), clean::Visibility::Restricted(vis_did) => { - if find_closest_parent_module(tcx, item_did) == Some(vis_did) { + let parent_module = find_closest_parent_module(tcx, item_did); + + if parent_module == Some(vis_did) { // `pub(in foo)` where `foo` is the parent module // is the same as no visibility modifier Ok(()) } else if vis_did.index == CRATE_DEF_INDEX { write!(f, "pub(crate) ") + } else if parent_module + .map(|parent| find_closest_parent_module(tcx, parent)) + .flatten() + == Some(vis_did) + { + write!(f, "pub(super) ") } else { f.write_str("pub(")?; let path = tcx.def_path(vis_did); From b959c75b3135189a90a1d3f630d702266270763f Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 16:16:40 -0800 Subject: [PATCH 04/14] Prefer `pub(crate)` over no modifier --- src/librustdoc/html/format.rs | 6 +++--- src/test/rustdoc/decl_macro_priv.rs | 2 +- src/test/rustdoc/pub-restricted.rs | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index a8eae52fc565..90f4aaefc9bc 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1099,12 +1099,12 @@ impl clean::Visibility { clean::Visibility::Restricted(vis_did) => { let parent_module = find_closest_parent_module(tcx, item_did); - if parent_module == Some(vis_did) { + if vis_did.index == CRATE_DEF_INDEX { + write!(f, "pub(crate) ") + } else if parent_module == Some(vis_did) { // `pub(in foo)` where `foo` is the parent module // is the same as no visibility modifier Ok(()) - } else if vis_did.index == CRATE_DEF_INDEX { - write!(f, "pub(crate) ") } else if parent_module .map(|parent| find_closest_parent_module(tcx, parent)) .flatten() diff --git a/src/test/rustdoc/decl_macro_priv.rs b/src/test/rustdoc/decl_macro_priv.rs index cb71bca9cf20..4e1279e34d93 100644 --- a/src/test/rustdoc/decl_macro_priv.rs +++ b/src/test/rustdoc/decl_macro_priv.rs @@ -2,7 +2,7 @@ #![feature(decl_macro)] -// @has decl_macro_priv/macro.crate_macro.html //pre 'macro crate_macro() {' +// @has decl_macro_priv/macro.crate_macro.html //pre 'pub(crate) macro crate_macro() {' // @has - //pre '...' // @has - //pre '}' pub(crate) macro crate_macro() {} diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs index 9ff3cad070d8..f828e642abdc 100644 --- a/src/test/rustdoc/pub-restricted.rs +++ b/src/test/rustdoc/pub-restricted.rs @@ -6,13 +6,13 @@ // @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic' pub struct FooPublic; -// @has 'foo/struct.FooJustCrate.html' '//pre' 'struct FooJustCrate' +// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate' crate struct FooJustCrate; -// @has 'foo/struct.FooPubCrate.html' '//pre' 'struct FooPubCrate' +// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate' pub(crate) struct FooPubCrate; -// @has 'foo/struct.FooSelf.html' '//pre' 'struct FooSelf' +// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf' pub(self) struct FooSelf; -// @has 'foo/struct.FooInSelf.html' '//pre' 'struct FooInSelf' +// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf' pub(in self) struct FooInSelf; mod a { // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper' From bb4761d1ebd47ec966a4774c087c645dc3a0227d Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 16:27:56 -0800 Subject: [PATCH 05/14] Merge `pub-restricted` and `visibility` test --- src/test/rustdoc/pub-restricted.rs | 32 ----------------------- src/test/rustdoc/visibility.rs | 41 +++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 39 deletions(-) delete mode 100644 src/test/rustdoc/pub-restricted.rs diff --git a/src/test/rustdoc/pub-restricted.rs b/src/test/rustdoc/pub-restricted.rs deleted file mode 100644 index f828e642abdc..000000000000 --- a/src/test/rustdoc/pub-restricted.rs +++ /dev/null @@ -1,32 +0,0 @@ -// compile-flags: --document-private-items - -#![feature(crate_visibility_modifier)] - -#![crate_name = "foo"] - -// @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic' -pub struct FooPublic; -// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate' -crate struct FooJustCrate; -// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate' -pub(crate) struct FooPubCrate; -// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf' -pub(self) struct FooSelf; -// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf' -pub(in self) struct FooInSelf; -mod a { - // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper' - pub(super) struct FooASuper; - // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper' - pub(in super) struct FooAInSuper; - // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA' - pub(in a) struct FooAInA; - mod b { - // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper' - pub(super) struct FooBSuper; - // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper' - pub(in super::super) struct FooBInSuperSuper; - // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB' - pub(in a::b) struct FooBInAB; - } -} diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs index 9dd0b68b1d95..ebb314a7941b 100644 --- a/src/test/rustdoc/visibility.rs +++ b/src/test/rustdoc/visibility.rs @@ -1,13 +1,40 @@ // compile-flags: --document-private-items +#![feature(crate_visibility_modifier)] + #![crate_name = "foo"] -// @has 'foo/fn.foo.html' '//pre' 'fn foo' -// !@has 'foo/fn.foo.html' '//pre' 'pub' -fn foo() {} +// @has 'foo/struct.FooPublic.html' '//pre' 'pub struct FooPublic' +pub struct FooPublic; +// @has 'foo/struct.FooJustCrate.html' '//pre' 'pub(crate) struct FooJustCrate' +crate struct FooJustCrate; +// @has 'foo/struct.FooPubCrate.html' '//pre' 'pub(crate) struct FooPubCrate' +pub(crate) struct FooPubCrate; +// @has 'foo/struct.FooSelf.html' '//pre' 'pub(crate) struct FooSelf' +pub(self) struct FooSelf; +// @has 'foo/struct.FooInSelf.html' '//pre' 'pub(crate) struct FooInSelf' +pub(in self) struct FooInSelf; +// @has 'foo/struct.FooPriv.html' '//pre' 'pub(crate) struct FooPriv' +struct FooPriv; + +mod a { + // @has 'foo/a/struct.FooASuper.html' '//pre' 'pub(crate) struct FooASuper' + pub(super) struct FooASuper; + // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper' + pub(in super) struct FooAInSuper; + // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA' + pub(in a) struct FooAInA; + // @has 'foo/a/struct.FooAPriv.html' '//pre' 'struct FooAPriv' + struct FooAPriv; -mod bar { - // @has 'foo/bar/fn.baz.html' '//pre' 'fn baz' - // !@has 'foo/bar/fn.baz.html' '//pre' 'pub' - fn baz() {} + mod b { + // @has 'foo/a/b/struct.FooBSuper.html' '//pre' 'pub(super) struct FooBSuper' + pub(super) struct FooBSuper; + // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper' + pub(in super::super) struct FooBInSuperSuper; + // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB' + pub(in a::b) struct FooBInAB; + // @has 'foo/a/b/struct.FooBPriv.html' '//pre' 'struct FooBPriv' + struct FooBPriv; + } } From 4b1b277cf90886466e2e0dc5017ea535ef5e00ce Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 16:33:15 -0800 Subject: [PATCH 06/14] Add missing code to `find_closest_parent_module` --- src/librustdoc/clean/utils.rs | 40 +++++++++++-------- .../passes/collect_intra_doc_links.rs | 10 +---- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index fe1b58164474..4b9541b7e142 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -626,23 +626,31 @@ where } crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option { - let mut current = def_id; - // The immediate parent might not always be a module. - // Find the first parent which is. - loop { - if let Some(parent) = tcx.parent(current) { - if tcx.def_kind(parent) == DefKind::Mod { - break Some(parent); + if item.is_fake() { + // FIXME: is this correct? + None + // If we're documenting the crate root itself, it has no parent. Use the root instead. + } else if item.def_id.is_top_level_module() { + Some(item.def_id) + } else { + let mut current = def_id; + // The immediate parent might not always be a module. + // Find the first parent which is. + loop { + if let Some(parent) = tcx.parent(current) { + if tcx.def_kind(parent) == DefKind::Mod { + break Some(parent); + } + current = parent; + } else { + debug!( + "{:?} has no parent (kind={:?}, original was {:?})", + current, + tcx.def_kind(current), + def_id + ); + break None; } - current = parent; - } else { - debug!( - "{:?} has no parent (kind={:?}, original was {:?})", - current, - tcx.def_kind(current), - def_id - ); - break None; } } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index f392f321fbfc..4e261c3fd193 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -767,15 +767,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { fn fold_item(&mut self, mut item: Item) -> Option { use rustc_middle::ty::DefIdTree; - let parent_node = if item.is_fake() { - // FIXME: is this correct? - None - // If we're documenting the crate root itself, it has no parent. Use the root instead. - } else if item.def_id.is_top_level_module() { - Some(item.def_id) - } else { - find_closest_parent_module(self.cx.tcx, item.def_id) - }; + let parent_node = find_closest_parent_module(self.cx.tcx, item.def_id); if parent_node.is_some() { trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id); From eaf851288ed31090afd924eeb22f5e0f202151d4 Mon Sep 17 00:00:00 2001 From: Camelid Date: Fri, 25 Dec 2020 16:35:28 -0800 Subject: [PATCH 07/14] Simplify loop and remove old debugging code Co-authored-by: Joshua Nelson --- src/librustdoc/clean/utils.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 4b9541b7e142..b49ed07f8e83 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -636,21 +636,12 @@ crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option Date: Fri, 25 Dec 2020 16:37:09 -0800 Subject: [PATCH 08/14] Extract local variable --- src/librustdoc/clean/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 548cca0761e7..666d418cc0e2 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2295,24 +2295,20 @@ impl Clean for (&hir::MacroDef<'_>, Option) { ) } else { let vis = item.vis.clean(cx); + let vis_printed_with_space = + vis.print_with_space(cx.tcx, cx.tcx.hir().local_def_id(item.hir_id).to_def_id()); if matchers.len() <= 1 { format!( "{}macro {}{} {{\n ...\n}}", - vis.print_with_space( - cx.tcx, - cx.tcx.hir().local_def_id(item.hir_id).to_def_id() - ), + vis_printed_with_space, name, matchers.iter().map(|span| span.to_src(cx)).collect::(), ) } else { format!( "{}macro {} {{\n{}}}", - vis.print_with_space( - cx.tcx, - cx.tcx.hir().local_def_id(item.hir_id).to_def_id() - ), + vis_printed_with_space, name, matchers .iter() From 75705ab3a92bdc5bd5de1aba93013ecd852be6f7 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 30 Dec 2020 16:38:25 -0800 Subject: [PATCH 09/14] Update `find_nearest_parent_module` --- src/librustdoc/clean/utils.rs | 12 +++++------- src/librustdoc/html/format.rs | 6 +++--- src/librustdoc/passes/collect_intra_doc_links.rs | 9 +++++++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index b49ed07f8e83..4009a42955f8 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -625,13 +625,11 @@ where r } -crate fn find_closest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option { - if item.is_fake() { - // FIXME: is this correct? - None - // If we're documenting the crate root itself, it has no parent. Use the root instead. - } else if item.def_id.is_top_level_module() { - Some(item.def_id) +/// Find the nearest parent module of a [`DefId`]. +crate fn find_nearest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option { + if def_id.is_top_level_module() { + // The crate root has no parent. Use it as the root instead. + Some(def_id) } else { let mut current = def_id; // The immediate parent might not always be a module. diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 90f4aaefc9bc..9fca005c34ee 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -15,7 +15,7 @@ use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{DefId, CRATE_DEF_INDEX}; use rustc_target::spec::abi::Abi; -use crate::clean::{self, utils::find_closest_parent_module, PrimitiveType}; +use crate::clean::{self, utils::find_nearest_parent_module, PrimitiveType}; use crate::formats::cache::cache; use crate::formats::item_type::ItemType; use crate::html::escape::Escape; @@ -1097,7 +1097,7 @@ impl clean::Visibility { clean::Inherited => Ok(()), clean::Visibility::Restricted(vis_did) => { - let parent_module = find_closest_parent_module(tcx, item_did); + let parent_module = find_nearest_parent_module(tcx, item_did); if vis_did.index == CRATE_DEF_INDEX { write!(f, "pub(crate) ") @@ -1106,7 +1106,7 @@ impl clean::Visibility { // is the same as no visibility modifier Ok(()) } else if parent_module - .map(|parent| find_closest_parent_module(tcx, parent)) + .map(|parent| find_nearest_parent_module(tcx, parent)) .flatten() == Some(vis_did) { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 4e261c3fd193..e225eb47b12b 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -31,7 +31,7 @@ use std::cell::Cell; use std::mem; use std::ops::Range; -use crate::clean::{self, utils::find_closest_parent_module, Crate, Item, ItemLink, PrimitiveType}; +use crate::clean::{self, utils::find_nearest_parent_module, Crate, Item, ItemLink, PrimitiveType}; use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::markdown_links; @@ -767,7 +767,12 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { fn fold_item(&mut self, mut item: Item) -> Option { use rustc_middle::ty::DefIdTree; - let parent_node = find_closest_parent_module(self.cx.tcx, item.def_id); + let parent_node = if item.is_fake() { + // FIXME: is this correct? + None + } else { + find_nearest_parent_module(self.cx.tcx, item.def_id) + }; if parent_node.is_some() { trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id); From 6c86ebab7491b34f4048af07722afa9301326dd1 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 30 Dec 2020 16:29:47 -0800 Subject: [PATCH 10/14] Remove FIXME Co-authored-by: Joshua Nelson --- src/librustdoc/passes/collect_intra_doc_links.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index e225eb47b12b..2e116da6ff5d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -768,7 +768,6 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { use rustc_middle::ty::DefIdTree; let parent_node = if item.is_fake() { - // FIXME: is this correct? None } else { find_nearest_parent_module(self.cx.tcx, item.def_id) From 6f6afae41aee721fd259a8a764ec96f93bc07911 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 30 Dec 2020 16:41:18 -0800 Subject: [PATCH 11/14] Small refactor --- src/librustdoc/clean/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 666d418cc0e2..ae329dc62f14 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2295,20 +2295,19 @@ impl Clean for (&hir::MacroDef<'_>, Option) { ) } else { let vis = item.vis.clean(cx); - let vis_printed_with_space = - vis.print_with_space(cx.tcx, cx.tcx.hir().local_def_id(item.hir_id).to_def_id()); + let def_id = cx.tcx.hir().local_def_id(item.hir_id).to_def_id(); if matchers.len() <= 1 { format!( "{}macro {}{} {{\n ...\n}}", - vis_printed_with_space, + vis.print_with_space(cx.tcx, def_id), name, matchers.iter().map(|span| span.to_src(cx)).collect::(), ) } else { format!( "{}macro {} {{\n{}}}", - vis_printed_with_space, + vis.print_with_space(cx.tcx, def_id), name, matchers .iter() From f7f14f6f0b6cde5ab9f5b644d0939fb1232b9c15 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 30 Dec 2020 17:39:03 -0800 Subject: [PATCH 12/14] Add note on panic behavior --- src/librustdoc/clean/utils.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 4009a42955f8..17f12d0a82f2 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -626,6 +626,8 @@ where } /// Find the nearest parent module of a [`DefId`]. +/// +/// **Panics if the item it belongs to [is fake][Item::is_fake].** crate fn find_nearest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Option { if def_id.is_top_level_module() { // The crate root has no parent. Use it as the root instead. From dda887a02c7df86f6e713d853b887d79be8cf803 Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 31 Dec 2020 12:00:23 -0800 Subject: [PATCH 13/14] Add FIXME for visibility of a module --- src/librustdoc/html/format.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 9fca005c34ee..9b2fb8582f54 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1097,6 +1097,9 @@ impl clean::Visibility { clean::Inherited => Ok(()), clean::Visibility::Restricted(vis_did) => { + // FIXME(camelid): This may not work correctly if `item_did` is a module. + // However, rustdoc currently never displays a module's + // visibility, so it shouldn't matter. let parent_module = find_nearest_parent_module(tcx, item_did); if vis_did.index == CRATE_DEF_INDEX { From 5604a18a60fc99a4a7feb82a280546971adda52d Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 31 Dec 2020 12:04:13 -0800 Subject: [PATCH 14/14] Add `@!has` checks to ensure private items don't have `pub` --- src/test/rustdoc/visibility.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/rustdoc/visibility.rs b/src/test/rustdoc/visibility.rs index ebb314a7941b..59427693c5a5 100644 --- a/src/test/rustdoc/visibility.rs +++ b/src/test/rustdoc/visibility.rs @@ -23,8 +23,10 @@ mod a { // @has 'foo/a/struct.FooAInSuper.html' '//pre' 'pub(crate) struct FooAInSuper' pub(in super) struct FooAInSuper; // @has 'foo/a/struct.FooAInA.html' '//pre' 'struct FooAInA' + // @!has 'foo/a/struct.FooAInA.html' '//pre' 'pub' pub(in a) struct FooAInA; // @has 'foo/a/struct.FooAPriv.html' '//pre' 'struct FooAPriv' + // @!has 'foo/a/struct.FooAPriv.html' '//pre' 'pub' struct FooAPriv; mod b { @@ -33,8 +35,10 @@ mod a { // @has 'foo/a/b/struct.FooBInSuperSuper.html' '//pre' 'pub(crate) struct FooBInSuperSuper' pub(in super::super) struct FooBInSuperSuper; // @has 'foo/a/b/struct.FooBInAB.html' '//pre' 'struct FooBInAB' + // @!has 'foo/a/b/struct.FooBInAB.html' '//pre' 'pub' pub(in a::b) struct FooBInAB; // @has 'foo/a/b/struct.FooBPriv.html' '//pre' 'struct FooBPriv' + // @!has 'foo/a/b/struct.FooBPriv.html' '//pre' 'pub' struct FooBPriv; } }