Auto merge of rust-lang#34193 - petrochenkov:privalias, r=nikomatsakis

privacy: Substitute type aliases in private-in-public checker

Closes rust-lang#30503
Closes rust-lang#34293

Everyone in the issue discussion seemed to be in favor, @huonw also spoke about this [here](https://www.reddit.com/r/rust/comments/3xldr9/surfaces_and_signatures_component_privacy_versus/cy615wq), but the issue haven't got any movement.
I think it's reasonable to do this before turning `private_in_public` warnings into errors.

r? @nikomatsakis

Author: bors

src/librustc_privacy/lib.rs

@@ -873,19 +873,12 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
     // Return the visibility of the type alias's least visible component type when substituted
     fn substituted_alias_visibility(&self, item: &hir::Item, path: &hir::Path)
                                     -> Option<ty::Visibility> {
-        // We substitute type aliases only when determining impl publicity
-        // FIXME: This will probably change and all type aliases will be substituted,
-        // requires an amendment to RFC 136.
-        if self.required_visibility != ty::Visibility::PrivateExternal {
-            return None;
-        }
         // Type alias is considered public if the aliased type is
         // public, even if the type alias itself is private. So, something
         // like `type A = u8; pub fn f() -> A {...}` doesn't cause an error.
         if let hir::ItemTy(ref ty, ref generics) = item.node {
-            let mut check = SearchInterfaceForPrivateItemsVisitor {
-                min_visibility: ty::Visibility::Public, ..*self
-            };
+            let mut check = SearchInterfaceForPrivateItemsVisitor::new(self.tcx,
+                                                                       self.old_error_set);
             check.visit_ty(ty);
             // If a private type alias with default type parameters is used in public
             // interface we must ensure, that the defaults are public if they are actually used.

src/librustc_resolve/lib.rs

@@ -1744,6 +1744,7 @@ impl<'a> Resolver<'a> {
                     let def_id = self.definitions.local_def_id(type_parameter.id);
                     let def = Def::TyParam(space, index as u32, def_id, name);
                     function_type_rib.bindings.insert(ast::Ident::with_empty_ctxt(name), def);
+                    self.record_def(type_parameter.id, PathResolution::new(def));
                 }
                 self.type_ribs.push(function_type_rib);
             }

src/librustdoc/clean/mod.rs

@@ -36,8 +36,10 @@ use syntax_pos::{self, DUMMY_SP, Pos};
 use rustc_trans::back::link;
 use rustc::middle::cstore;
 use rustc::middle::privacy::AccessLevels;
+use rustc::middle::resolve_lifetime::DefRegion::*;
 use rustc::hir::def::Def;
 use rustc::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
+use rustc::hir::fold::Folder;
 use rustc::hir::print as pprust;
 use rustc::ty::subst::{self, ParamSpace, VecPerParamSpace};
 use rustc::ty;
@@ -1636,6 +1638,43 @@ impl PrimitiveType {
     }
 }
 
+
+// Poor man's type parameter substitution at HIR level.
+// Used to replace private type aliases in public signatures with their aliased types.
+struct SubstAlias<'a, 'tcx: 'a> {
+    tcx: &'a ty::TyCtxt<'a, 'tcx, 'tcx>,
+    // Table type parameter definition -> substituted type
+    ty_substs: HashMap<Def, hir::Ty>,
+    // Table node id of lifetime parameter definition -> substituted lifetime
+    lt_substs: HashMap<ast::NodeId, hir::Lifetime>,
+}
+
+impl<'a, 'tcx: 'a, 'b: 'tcx> Folder for SubstAlias<'a, 'tcx> {
+    fn fold_ty(&mut self, ty: P<hir::Ty>) -> P<hir::Ty> {
+        if let hir::TyPath(..) = ty.node {
+            let def = self.tcx.expect_def(ty.id);
+            if let Some(new_ty) = self.ty_substs.get(&def).cloned() {
+                return P(new_ty);
+            }
+        }
+        hir::fold::noop_fold_ty(ty, self)
+    }
+    fn fold_lifetime(&mut self, lt: hir::Lifetime) -> hir::Lifetime {
+        let def = self.tcx.named_region_map.defs.get(&lt.id).cloned();
+        match def {
+            Some(DefEarlyBoundRegion(_, _, node_id)) |
+            Some(DefLateBoundRegion(_, node_id)) |
+            Some(DefFreeRegion(_, node_id)) => {
+                if let Some(lt) = self.lt_substs.get(&node_id).cloned() {
+                    return lt;
+                }
+            }
+            _ => {}
+        }
+        hir::fold::noop_fold_lifetime(lt, self)
+    }
+}
+
 impl Clean<Type> for hir::Ty {
     fn clean(&self, cx: &DocContext) -> Type {
         use rustc::hir::*;
@@ -1665,8 +1704,46 @@ impl Clean<Type> for hir::Ty {
                 FixedVector(box ty.clean(cx), n)
             },
             TyTup(ref tys) => Tuple(tys.clean(cx)),
-            TyPath(None, ref p) => {
-                resolve_type(cx, p.clean(cx), self.id)
+            TyPath(None, ref path) => {
+                if let Some(tcx) = cx.tcx_opt() {
+                    // Substitute private type aliases
+                    let def = tcx.expect_def(self.id);
+                    if let Def::TyAlias(def_id) = def {
+                        if let Some(node_id) = tcx.map.as_local_node_id(def_id) {
+                            if !cx.access_levels.borrow().is_exported(def_id) {
+                                let item = tcx.map.expect_item(node_id);
+                                if let hir::ItemTy(ref ty, ref generics) = item.node {
+                                    let provided_params = &path.segments.last().unwrap().parameters;
+                                    let mut ty_substs = HashMap::new();
+                                    let mut lt_substs = HashMap::new();
+                                    for (i, ty_param) in generics.ty_params.iter().enumerate() {
+                                        let ty_param_def = tcx.expect_def(ty_param.id);
+                                        if let Some(ty) = provided_params.types().get(i).cloned()
+                                                                                        .cloned() {
+                                            ty_substs.insert(ty_param_def, ty.unwrap());
+                                        } else if let Some(default) = ty_param.default.clone() {
+                                            ty_substs.insert(ty_param_def, default.unwrap());
+                                        }
+                                    }
+                                    for (i, lt_param) in generics.lifetimes.iter().enumerate() {
+                                        if let Some(lt) = provided_params.lifetimes().get(i)
+                                                                                     .cloned()
+                                                                                     .cloned() {
+                                            lt_substs.insert(lt_param.lifetime.id, lt);
+                                        }
+                                    }
+                                    let mut subst_alias = SubstAlias {
+                                        tcx: &tcx,
+                                        ty_substs: ty_substs,
+                                        lt_substs: lt_substs
+                                    };
+                                    return subst_alias.fold_ty(ty.clone()).clean(cx);
+                                }
+                            }
+                        }
+                    }
+                }
+                resolve_type(cx, path.clean(cx), self.id)
             }
             TyPath(Some(ref qself), ref p) => {
                 let mut segments: Vec<_> = p.segments.clone().into();

src/test/compile-fail/private-in-public-warn.rs

@@ -205,11 +205,10 @@ mod aliases_pub {
     }
 
     pub fn f1(arg: PrivUseAlias) {} // OK
+    pub fn f2(arg: PrivAlias) {} // OK
 
     pub trait Tr1: PrivUseAliasTr {} // OK
-    // This should be OK, if type aliases are substituted
-    pub trait Tr2: PrivUseAliasTr<PrivAlias> {} //~ WARN private type in public interface
-        //~^ WARNING hard error
+    pub trait Tr2: PrivUseAliasTr<PrivAlias> {} // OK
 
     impl PrivAlias {
         pub fn f(arg: Priv) {} //~ WARN private type in public interface
@@ -285,6 +284,8 @@ mod aliases_params {
     struct Priv;
     type PrivAliasGeneric<T = Priv> = T;
     type Result<T> = ::std::result::Result<T, Priv>;
+
+    pub fn f1(arg: PrivAliasGeneric<u8>) {} // OK, not an error
 }
 
 #[rustc_error]

src/test/compile-fail/private-in-public.rs

@@ -105,8 +105,6 @@ mod aliases_pub {
     }
     impl PrivTr for Priv {}
 
-    // This should be OK, if type aliases are substituted
-    pub fn f2(arg: PrivAlias) {} //~ ERROR private type in public interface
     // This should be OK, but associated type aliases are not substituted yet
     pub fn f3(arg: <Priv as PrivTr>::AssocAlias) {} //~ ERROR private type in public interface
 
@@ -143,8 +141,6 @@ mod aliases_params {
     type PrivAliasGeneric<T = Priv> = T;
     type Result<T> = ::std::result::Result<T, Priv>;
 
-    // This should be OK, if type aliases are substituted
-    pub fn f1(arg: PrivAliasGeneric<u8>) {} //~ ERROR private type in public interface
     pub fn f2(arg: PrivAliasGeneric) {} //~ ERROR private type in public interface
     pub fn f3(arg: Result<u8>) {} //~ ERROR private type in public interface
 }

src/test/rustdoc/private-type-alias.rs

@@ -0,0 +1,41 @@
+// 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.
+
+type MyResultPriv<T> = Result<T, u16>;
+pub type MyResultPub<T> = Result<T, u64>;
+
+// @has private_type_alias/fn.get_result_priv.html '//pre' 'Result<u8, u16>'
+pub fn get_result_priv() -> MyResultPriv<u8> {
+    panic!();
+}
+
+// @has private_type_alias/fn.get_result_pub.html '//pre' 'MyResultPub<u32>'
+pub fn get_result_pub() -> MyResultPub<u32> {
+    panic!();
+}
+
+pub type PubRecursive = u16;
+type PrivRecursive3 = u8;
+type PrivRecursive2 = PubRecursive;
+type PrivRecursive1 = PrivRecursive3;
+
+// PrivRecursive1 is expanded twice and stops at u8
+// PrivRecursive2 is expanded once and stops at public type alias PubRecursive
+// @has private_type_alias/fn.get_result_recursive.html '//pre' '(u8, PubRecursive)'
+pub fn get_result_recursive() -> (PrivRecursive1, PrivRecursive2) {
+    panic!();
+}
+
+type MyLifetimePriv<'a> = &'a isize;
+
+// @has private_type_alias/fn.get_lifetime_priv.html '//pre' "&'static isize"
+pub fn get_lifetime_priv() -> MyLifetimePriv<'static> {
+    panic!();
+}