Auto merge of #57586 - Aaron1011:feature/pub-priv-dep, r=petrochenkov

Implement public/private dependency feature

Implements #44663

The core implementation is done - however, there are a few issues that still need to be resolved:

- [x] The `EXTERNAL_PRIVATE_DEPENDENCY` lint currently does notthing when the `public_private_dependencies` is not enabled. Should mentioning the lint (in an `allow` or `deny` attribute) be an error if the feature is not enabled? (Resolved- the feature was removed)
- [x] Crates with the name `core` and `std` are always marked public, without the need to explcitily specify them on the command line. Is this what we want to do? Do we want to allow`no_std`/`no_core` crates to explicitly control this in some way? (Resolved - private crates are now explicitly specified)
- [x] Should I add additional UI tests? (Resolved - added more tests)
- [x] Does it make sense to be able to allow/deny the `EXTERNAL_PRIVATE_DEPENDENCY` on an individual item? (Resolved - this is implemented)

Author: bors

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",

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,

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<String> [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<RustcOptGroup> {
             "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<usize>);
     impl_dep_tracking_hash_via_hash!(Option<String>);
     impl_dep_tracking_hash_via_hash!(Option<(String, u64)>);
+    impl_dep_tracking_hash_via_hash!(Option<Vec<String>>);
     impl_dep_tracking_hash_via_hash!(Option<MergeFunctions>);
     impl_dep_tracking_hash_via_hash!(Option<PanicStrategy>);
     impl_dep_tracking_hash_via_hash!(Option<RelroLevel>);

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" }
+rustc_data_structures = { path = "../librustc_data_structures" }
+log = "0.4"

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,13 +1452,15 @@ 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.
     required_visibility: ty::Visibility,
     has_pub_restricted: bool,
     has_old_errors: bool,
     in_assoc_ty: bool,
+    private_crates: FxHashSet<CrateNum>
 }
 
 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<CrateNum>
 }
 
 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<AccessLevels> {
 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>(
         tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module));
     }
 
+    let private_crates: FxHashSet<CrateNum> = 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));
     }

src/test/ui/feature-gates/auxiliary/pub_dep.rs

@@ -0,0 +1 @@
+pub struct PubType;

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() {}

src/test/ui/privacy/pub-priv-dep/auxiliary/priv_dep.rs

@@ -0,0 +1,2 @@
+pub struct OtherType;
+pub trait OtherTrait {}

src/test/ui/privacy/pub-priv-dep/auxiliary/pub_dep.rs

@@ -0,0 +1 @@
+pub struct PubType;

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() {}

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
+

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<u8>
+}
+
+fn main() {}