-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustdoc: Split out rustdoc pass to strip private imports #32055
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,7 +106,7 @@ pub fn strip_private(mut krate: clean::Crate) -> plugins::PluginResult { | |
retained: &mut retained, | ||
access_levels: &access_levels, | ||
}; | ||
krate = stripper.fold_crate(krate); | ||
krate = ImportStripper.fold_crate(stripper.fold_crate(krate)); | ||
} | ||
|
||
// strip all private implementations of traits | ||
|
@@ -144,12 +144,6 @@ impl<'a> fold::DocFolder for Stripper<'a> { | |
} | ||
} | ||
|
||
clean::ExternCrateItem(..) | clean::ImportItem(_) => { | ||
if i.visibility != Some(hir::Public) { | ||
return None | ||
} | ||
} | ||
|
||
clean::StructFieldItem(..) => { | ||
if i.visibility != Some(hir::Public) { | ||
return Some(clean::Item { | ||
|
@@ -170,6 +164,9 @@ impl<'a> fold::DocFolder for Stripper<'a> { | |
return None; | ||
} | ||
} | ||
// handled in the `strip-priv-imports` pass | ||
clean::ExternCrateItem(..) | clean::ImportItem(_) => {} | ||
|
||
clean::DefaultImplItem(..) | clean::ImplItem(..) => {} | ||
|
||
// tymethods/macros have no control over privacy | ||
|
@@ -242,6 +239,21 @@ impl<'a> fold::DocFolder for ImplStripper<'a> { | |
} | ||
} | ||
|
||
// This stripper discards all private import statements (`use`, `extern crate`) | ||
struct ImportStripper; | ||
impl fold::DocFolder for ImportStripper { | ||
fn fold_item(&mut self, i: Item) -> Option<Item> { | ||
match i.inner { | ||
clean::ExternCrateItem(..) | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also test visibility, right? #31362 made There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the pattern guard on the next line should take care of that (?) 😃 Or do you mean a rustdoc test? I've added one since there doesn't appear to be any rustdoc test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind, I read this code too quickly and didn’t realize these two lines are the same |
||
clean::ImportItem(..) if i.visibility != Some(hir::Public) => None, | ||
_ => self.fold_item_recur(i) | ||
} | ||
} | ||
} | ||
|
||
pub fn strip_priv_imports(krate: clean::Crate) -> plugins::PluginResult { | ||
(ImportStripper.fold_crate(krate), None) | ||
} | ||
|
||
pub fn unindent_comments(krate: clean::Crate) -> plugins::PluginResult { | ||
struct CommentCleaner; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
// compile-flags:--no-defaults --passes strip-priv-imports | ||
// aux-build:empty.rs | ||
// ignore-cross-compile | ||
|
||
// @has issue_27104/index.html | ||
// @!has - 'extern crate std' | ||
// @!has - 'use std::prelude::' | ||
|
||
// @has - 'pub extern crate empty' | ||
pub extern crate empty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that this change hit Nightly I’m trying it out but I’m surprised to not see
strip-priv-imports
listed in the default passes inrustdoc --passes list
. As far as I understand it’s indeed not a default pass, but this line makes it implied bystrip-private
. This is not exactly wrong, but at least confusing.Maybe add “, implies
strip-priv-imports
” should be added to the description forstrip-private
insrc/librustdoc/lib.rs
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that'd be a good idea. I'll submit a PR later if you or someone else hasn't by then.