Skip to content
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

rustdoc: clean up and test macro visibility print #84074

Merged
merged 1 commit into from
Apr 17, 2021
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
4 changes: 2 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2287,14 +2287,14 @@ impl Clean<Item> for (&hir::MacroDef<'_>, Option<Symbol>) {
if matchers.len() <= 1 {
format!(
"{}macro {}{} {{\n ...\n}}",
vis.print_with_space(cx.tcx, def_id, &cx.cache),
vis.to_src_with_space(cx.tcx, def_id),
name,
matchers.iter().map(|span| span.to_src(cx)).collect::<String>(),
)
} else {
format!(
"{}macro {} {{\n{}}}",
vis.print_with_space(cx.tcx, def_id, &cx.cache),
vis.to_src_with_space(cx.tcx, def_id),
name,
matchers
.iter()
Expand Down
48 changes: 38 additions & 10 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,8 +1186,6 @@ impl clean::Visibility {
item_did: DefId,
cache: &'a Cache,
) -> impl fmt::Display + 'a + Captures<'tcx> {
use rustc_span::symbol::kw;

let to_print = match self {
clean::Public => "pub ".to_owned(),
clean::Inherited => String::new(),
Expand All @@ -1212,18 +1210,11 @@ impl clean::Visibility {
} else {
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");
// modified from `resolved_path()` to work with `DefPathData`
let last_name = path.data.last().unwrap().data.get_opt_name().unwrap();
let anchor = anchor(vis_did, &last_name.as_str(), cache).to_string();

let mut s = "pub(".to_owned();
if path.data.len() != 1
|| (first_name != kw::SelfLower && first_name != kw::Super)
{
s.push_str("in ");
}
let mut s = "pub(in ".to_owned();
Comment on lines -1215 to +1217
Copy link
Member

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed impossible for this conditional to evaluate to false. It's checking if the path is exactly equal to self or super, but there are already conditionals above that check if it's equal to the current module or the parent module.

Removing it also didn't break any test cases, supporting my dead-code theory.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense, thanks. I think I didn't realize what it was doing when @camelid added the pub(self) handling back in #80368.

for seg in &path.data[..path.data.len() - 1] {
s.push_str(&format!("{}::", seg.data.get_opt_name().unwrap()));
}
Expand All @@ -1234,6 +1225,43 @@ impl clean::Visibility {
};
display_fn(move |f| f.write_str(&to_print))
}

/// This function is the same as print_with_space, except that it renders no links.
/// It's used for macros' rendered source view, which is syntax highlighted and cannot have
/// any HTML in it.
crate fn to_src_with_space<'a, 'tcx: 'a>(
self,
tcx: TyCtxt<'tcx>,
item_did: DefId,
) -> impl fmt::Display + 'a + Captures<'tcx> {
let to_print = match self {
clean::Public => "pub ".to_owned(),
clean::Inherited => String::new(),
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 {
"pub(crate) ".to_owned()
} else if parent_module == Some(vis_did) {
// `pub(in foo)` where `foo` is the parent module
// is the same as no visibility modifier
String::new()
} else if parent_module
.map(|parent| find_nearest_parent_module(tcx, parent))
.flatten()
== Some(vis_did)
{
"pub(super) ".to_owned()
} else {
format!("pub(in {}) ", tcx.def_path_str(vis_did))
}
}
};
display_fn(move |f| f.write_str(&to_print))
}
}

crate trait PrintWithSpace {
Expand Down
17 changes: 17 additions & 0 deletions src/test/rustdoc/decl_macro.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// compile-flags: --document-private-items

#![feature(decl_macro)]

// @has decl_macro/macro.my_macro.html //pre 'pub macro my_macro() {'
Expand Down Expand Up @@ -37,3 +39,18 @@ pub macro my_macro_multi {
pub macro by_example_single {
($foo:expr) => {}
}

mod a {
mod b {
// @has decl_macro/a/b/macro.by_example_vis.html //pre 'pub(super) macro by_example_vis($foo:expr) {'
pub(in super) macro by_example_vis {
($foo:expr) => {}
}
mod c {
// @has decl_macro/a/b/c/macro.by_example_vis_named.html //pre 'pub(in a) macro by_example_vis_named($foo:expr) {'
pub(in a) macro by_example_vis_named {
($foo:expr) => {}
}
}
}
}