diff --git a/Cargo.lock b/Cargo.lock index 058f1b1e716a9..1ac2dfe25c395 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2878,6 +2878,7 @@ dependencies = [ name = "rustc_privacy" version = "0.0.0" dependencies = [ + "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "rustc 0.0.0", "rustc_data_structures 0.0.0", "rustc_typeck 0.0.0", diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 35a0382359572..3fe544d690640 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -125,6 +125,12 @@ declare_lint! { "detect private items in public interfaces not caught by the old implementation" } +declare_lint! { + pub EXPORTED_PRIVATE_DEPENDENCIES, + Warn, + "public interface leaks type from a private dependency" +} + declare_lint! { pub PUB_USE_OF_PRIVATE_EXTERN_CRATE, Deny, @@ -405,6 +411,7 @@ impl LintPass for HardwiredLints { TRIVIAL_CASTS, TRIVIAL_NUMERIC_CASTS, PRIVATE_IN_PUBLIC, + EXPORTED_PRIVATE_DEPENDENCIES, PUB_USE_OF_PRIVATE_EXTERN_CRATE, INVALID_TYPE_PARAM_DEFAULT, CONST_ERR, diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 4b1aefb2216fb..86f676fbf888a 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -411,6 +411,10 @@ top_level_options!( remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED], edition: Edition [TRACKED], + + // The list of crates to consider private when + // checking leaked private dependency types in public interfaces + extern_private: Vec [TRACKED], } ); @@ -606,6 +610,7 @@ impl Default for Options { cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, + extern_private: Vec::new() } } } @@ -1724,6 +1729,12 @@ pub fn rustc_optgroups() -> Vec { "Specify where an external rust library is located", "NAME=PATH", ), + opt::multi_s( + "", + "extern-private", + "Specify where an extern rust library is located, marking it as a private dependency", + "NAME=PATH", + ), opt::opt_s("", "sysroot", "Override the system root", "PATH"), opt::multi("Z", "", "Set internal debugging options", "FLAG"), opt::opt_s( @@ -1905,6 +1916,7 @@ pub fn build_session_options_and_crate_config( let crate_types = parse_crate_types_from_list(unparsed_crate_types) .unwrap_or_else(|e| early_error(error_format, &e[..])); + let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format); let mut debugging_opts = build_debugging_options(matches, error_format); @@ -2218,8 +2230,18 @@ pub fn build_session_options_and_crate_config( ); } + if matches.opt_present("extern-private") && !debugging_opts.unstable_options { + early_error( + ErrorOutputType::default(), + "'--extern-private' is unstable and only \ + available for nightly builds of rustc." + ) + } + + let extern_private = matches.opt_strs("extern-private"); + let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - for arg in &matches.opt_strs("extern") { + for arg in matches.opt_strs("extern").into_iter().chain(matches.opt_strs("extern-private")) { let mut parts = arg.splitn(2, '='); let name = parts.next().unwrap_or_else(|| early_error(error_format, "--extern value must not be empty")); @@ -2287,6 +2309,7 @@ pub fn build_session_options_and_crate_config( cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, edition, + extern_private }, cfg, ) @@ -2460,6 +2483,7 @@ mod dep_tracking { impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option<(String, u64)>); + impl_dep_tracking_hash_via_hash!(Option>); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); impl_dep_tracking_hash_via_hash!(Option); diff --git a/src/librustc_privacy/Cargo.toml b/src/librustc_privacy/Cargo.toml index 62eab40f3ec9a..dfc4e5b5db45d 100644 --- a/src/librustc_privacy/Cargo.toml +++ b/src/librustc_privacy/Cargo.toml @@ -13,4 +13,5 @@ rustc = { path = "../librustc" } rustc_typeck = { path = "../librustc_typeck" } syntax = { path = "../libsyntax" } syntax_pos = { path = "../libsyntax_pos" } -rustc_data_structures = { path = "../librustc_data_structures" } \ No newline at end of file +rustc_data_structures = { path = "../librustc_data_structures" } +log = "0.4" diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index dcbb9ff4a7576..73f83eb6f7a3a 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -9,6 +9,7 @@ #[macro_use] extern crate rustc; #[macro_use] extern crate syntax; +#[macro_use] extern crate log; extern crate rustc_typeck; extern crate syntax_pos; extern crate rustc_data_structures; @@ -1451,6 +1452,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, + item_id: ast::NodeId, item_def_id: DefId, span: Span, /// The visitor checks that each component type is at least this visible. @@ -1458,6 +1460,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { has_pub_restricted: bool, has_old_errors: bool, in_assoc_ty: bool, + private_crates: FxHashSet } impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { @@ -1492,6 +1495,16 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { } fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool { + if self.leaks_private_dep(def_id) { + self.tcx.lint_node(lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES, + self.item_id, + self.span, + &format!("{} `{}` from private dependency '{}' in public \ + interface", kind, descr, + self.tcx.crate_name(def_id.krate))); + + } + let node_id = match self.tcx.hir().as_local_node_id(def_id) { Some(node_id) => node_id, None => return false, @@ -1514,9 +1527,23 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { self.tcx.lint_node(lint::builtin::PRIVATE_IN_PUBLIC, node_id, self.span, &format!("{} (error {})", msg, err_code)); } + } + false } + + /// An item is 'leaked' from a private dependency if all + /// of the following are true: + /// 1. It's contained within a public type + /// 2. It comes from a private crate + fn leaks_private_dep(&self, item_id: DefId) -> bool { + let ret = self.required_visibility == ty::Visibility::Public && + self.private_crates.contains(&item_id.krate); + + debug!("leaks_private_dep(item_id={:?})={}", item_id, ret); + return ret; + } } impl<'a, 'tcx> DefIdVisitor<'a, 'tcx> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { @@ -1530,6 +1557,7 @@ struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, has_pub_restricted: bool, old_error_set: &'a NodeSet, + private_crates: FxHashSet } impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { @@ -1560,12 +1588,14 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { SearchInterfaceForPrivateItemsVisitor { tcx: self.tcx, + item_id, item_def_id: self.tcx.hir().local_def_id(item_id), span: self.tcx.hir().span(item_id), required_visibility, has_pub_restricted: self.has_pub_restricted, has_old_errors, in_assoc_ty: false, + private_crates: self.private_crates.clone() } } @@ -1690,6 +1720,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Lrc { fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { let empty_tables = ty::TypeckTables::empty(None); + // Check privacy of names not checked in previous compilation stages. let mut visitor = NamePrivacyVisitor { tcx, @@ -1725,6 +1756,12 @@ fn privacy_access_levels<'tcx>( queries::check_mod_privacy::ensure(tcx, tcx.hir().local_def_id(module)); } + let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() + .flat_map(|c| { + tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() + }).collect(); + + // Build up a set of all exported items in the AST. This is a set of all // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { @@ -1767,6 +1804,7 @@ fn privacy_access_levels<'tcx>( tcx, has_pub_restricted, old_error_set: &visitor.old_error_set, + private_crates }; krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); } diff --git a/src/test/ui/feature-gates/auxiliary/pub_dep.rs b/src/test/ui/feature-gates/auxiliary/pub_dep.rs new file mode 100644 index 0000000000000..3ebafd953addf --- /dev/null +++ b/src/test/ui/feature-gates/auxiliary/pub_dep.rs @@ -0,0 +1 @@ +pub struct PubType; diff --git a/src/test/ui/feature-gates/feature-gate-public_private_dependencies.rs b/src/test/ui/feature-gates/feature-gate-public_private_dependencies.rs new file mode 100644 index 0000000000000..b8fb4b8dc19da --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-public_private_dependencies.rs @@ -0,0 +1,20 @@ +// This test is different from other feature gate tests. +// Instead of checking that an error occurs without the feature gate, +// it checks that *no* errors/warnings occurs without the feature gate. +// This is due to the fact that 'public_private_dependencies' just enables +// a lint, so disabling it shouldn't cause any code to stop compiling. + +// run-pass +// aux-build:pub_dep.rs + +// Without ![feature(public_private_dependencies)], +// this should do nothing/ +#![deny(exported_private_dependencies)] + +extern crate pub_dep; + +pub struct Foo { + pub field: pub_dep::PubType +} + +fn main() {} diff --git a/src/test/ui/privacy/pub-priv-dep/auxiliary/priv_dep.rs b/src/test/ui/privacy/pub-priv-dep/auxiliary/priv_dep.rs new file mode 100644 index 0000000000000..e7afeb84fb4f4 --- /dev/null +++ b/src/test/ui/privacy/pub-priv-dep/auxiliary/priv_dep.rs @@ -0,0 +1,2 @@ +pub struct OtherType; +pub trait OtherTrait {} diff --git a/src/test/ui/privacy/pub-priv-dep/auxiliary/pub_dep.rs b/src/test/ui/privacy/pub-priv-dep/auxiliary/pub_dep.rs new file mode 100644 index 0000000000000..3ebafd953addf --- /dev/null +++ b/src/test/ui/privacy/pub-priv-dep/auxiliary/pub_dep.rs @@ -0,0 +1 @@ +pub struct PubType; diff --git a/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs b/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs new file mode 100644 index 0000000000000..9ebc96017fe9c --- /dev/null +++ b/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -0,0 +1,46 @@ + // aux-build:priv_dep.rs + // aux-build:pub_dep.rs + // compile-flags: --extern-private priv_dep +#![deny(exported_private_dependencies)] + +// This crate is a private dependency +extern crate priv_dep; +// This crate is a public dependenct +extern crate pub_dep; + +use priv_dep::{OtherType, OtherTrait}; +use pub_dep::PubType; + +// Type from private dependency used in private +// type - this is fine +struct PrivateType { + field: OtherType +} + +pub struct PublicType { + pub field: OtherType, + //~^ ERROR type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface + priv_field: OtherType, // Private field - this is fine + pub other_field: PubType // Type from public dependency - this is fine +} + +impl PublicType { + pub fn pub_fn(param: OtherType) {} + //~^ ERROR type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface + + fn priv_fn(param: OtherType) {} +} + +pub trait MyPubTrait { + type Foo: OtherTrait; +} +//~^^^ ERROR trait `priv_dep::OtherTrait` from private dependency 'priv_dep' in public interface + +pub struct AllowedPrivType { + #[allow(exported_private_dependencies)] + pub allowed: OtherType +} + + + +fn main() {} diff --git a/src/test/ui/privacy/pub-priv-dep/pub-priv1.stderr b/src/test/ui/privacy/pub-priv-dep/pub-priv1.stderr new file mode 100644 index 0000000000000..b31efdbd781dc --- /dev/null +++ b/src/test/ui/privacy/pub-priv-dep/pub-priv1.stderr @@ -0,0 +1,28 @@ +error: type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:21:5 + | +LL | pub field: OtherType, + | ^^^^^^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/pub-priv1.rs:4:9 + | +LL | #![deny(exported_private_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:28:5 + | +LL | pub fn pub_fn(param: OtherType) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: trait `priv_dep::OtherTrait` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:34:1 + | +LL | / pub trait MyPubTrait { +LL | | type Foo: OtherTrait; +LL | | } + | |_^ + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/privacy/pub-priv-dep/std-pub.rs b/src/test/ui/privacy/pub-priv-dep/std-pub.rs new file mode 100644 index 0000000000000..e25aa93a02e60 --- /dev/null +++ b/src/test/ui/privacy/pub-priv-dep/std-pub.rs @@ -0,0 +1,12 @@ +// The 'std' crates should always be implicitly public, +// without having to pass any compiler arguments + +// run-pass + +#![deny(exported_private_dependencies)] + +pub struct PublicType { + pub field: Option +} + +fn main() {}