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: Split out rustdoc pass to strip private imports #32055

Merged
merged 1 commit into from
Mar 6, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ const PASSES: &'static [Pass] = &[
"concatenates all document attributes into one document attribute"),
("strip-private", passes::strip_private,
"strips all private items from a crate which cannot be seen externally"),
("strip-priv-imports", passes::strip_priv_imports,
"strips all private import statements (`use`, `extern crate`) from a crate"),
];

const DEFAULT_PASSES: &'static [&'static str] = &[
Expand Down
26 changes: 19 additions & 7 deletions src/librustdoc/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

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 in rustdoc --passes list. As far as I understand it’s indeed not a default pass, but this line makes it implied by strip-private. This is not exactly wrong, but at least confusing.

Maybe add “, implies strip-priv-imports” should be added to the description for strip-private in src/librustdoc/lib.rs?

Copy link
Contributor Author

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.

}

// strip all private implementations of traits
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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(..) |
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also test visibility, right? #31362 made pub extern crate meaningful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 pub extern crate.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 match arm :)

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;
Expand Down
9 changes: 9 additions & 0 deletions src/test/auxiliary/empty.rs
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.
2 changes: 1 addition & 1 deletion src/test/rustdoc/issue-15347.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags:--no-defaults --passes "collapse-docs" --passes "unindent-comments"
// compile-flags:--no-defaults --passes collapse-docs --passes unindent-comments

// @has issue_15347/fn.foo.html
#[doc(hidden)]
Expand Down
20 changes: 20 additions & 0 deletions src/test/rustdoc/issue-27104.rs
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;