Auto merge of #32439 - jseyfried:visible_suggestions, r=nrc

diagnostics: make paths to external items more visible

This PR changes the reported path for an external item so that it is visible from at least one local module (i.e. it does not use any inaccessible external modules) if possible. If the external item's crate was declared with an `extern crate`, the path is guarenteed to use the `extern crate`.

Fixes #23224, fixes #23355, fixes #26635, fixes #27165.

r? @nrc

Author: bors

src/librustc/middle/cstore.rs

@@ -32,7 +32,7 @@ use mir::repr::Mir;
 use mir::mir_map::MirMap;
 use session::Session;
 use session::search_paths::PathKind;
-use util::nodemap::{FnvHashMap, NodeMap, NodeSet};
+use util::nodemap::{FnvHashMap, NodeMap, NodeSet, DefIdMap};
 use std::any::Any;
 use std::cell::RefCell;
 use std::rc::Rc;
@@ -169,6 +169,7 @@ pub trait CrateStore<'tcx> : Any {
     fn item_type(&self, tcx: &TyCtxt<'tcx>, def: DefId)
                  -> ty::TypeScheme<'tcx>;
     fn relative_item_path(&self, def: DefId) -> Vec<hir_map::PathElem>;
+    fn visible_parent_map<'a>(&'a self) -> ::std::cell::RefMut<'a, DefIdMap<DefId>>;
     fn extern_item_path(&self, def: DefId) -> Vec<hir_map::PathElem>;
     fn item_name(&self, def: DefId) -> ast::Name;
     fn item_predicates(&self, tcx: &TyCtxt<'tcx>, def: DefId)
@@ -347,6 +348,9 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     fn item_type(&self, tcx: &TyCtxt<'tcx>, def: DefId)
                  -> ty::TypeScheme<'tcx> { unimplemented!() }
     fn relative_item_path(&self, def: DefId) -> Vec<hir_map::PathElem> { unimplemented!() }
+    fn visible_parent_map<'a>(&'a self) -> ::std::cell::RefMut<'a, DefIdMap<DefId>> {
+        unimplemented!()
+    }
     fn extern_item_path(&self, def: DefId) -> Vec<hir_map::PathElem> { unimplemented!() }
     fn item_name(&self, def: DefId) -> ast::Name { unimplemented!() }
     fn item_predicates(&self, tcx: &TyCtxt<'tcx>, def: DefId)

src/librustc/ty/item_path.rs

@@ -10,7 +10,7 @@
 
 use front::map::DefPathData;
 use middle::cstore::LOCAL_CRATE;
-use middle::def_id::DefId;
+use middle::def_id::{DefId, CRATE_DEF_INDEX};
 use ty::{self, Ty, TyCtxt};
 use syntax::ast;
 
@@ -75,9 +75,51 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
+    /// If possible, this pushes a global path resolving to `external_def_id` that is visible
+    /// from at least one local module and returns true. If the crate defining `external_def_id` is
+    /// declared with an `extern crate`, the path is guarenteed to use the `extern crate`.
+    pub fn try_push_visible_item_path<T>(&self, buffer: &mut T, external_def_id: DefId) -> bool
+        where T: ItemPathBuffer
+    {
+        let visible_parent_map = self.sess.cstore.visible_parent_map();
+
+        let (mut cur_def, mut cur_path) = (external_def_id, Vec::<ast::Name>::new());
+        loop {
+            // If `cur_def` is a direct or injected extern crate, push the path to the crate
+            // followed by the path to the item within the crate and return.
+            if cur_def.index == CRATE_DEF_INDEX {
+                match self.sess.cstore.extern_crate(cur_def.krate) {
+                    Some(extern_crate) if extern_crate.direct => {
+                        self.push_item_path(buffer, extern_crate.def_id);
+                        cur_path.iter().rev().map(|segment| buffer.push(&segment.as_str())).count();
+                        return true;
+                    }
+                    None => {
+                        buffer.push(&self.crate_name(cur_def.krate));
+                        cur_path.iter().rev().map(|segment| buffer.push(&segment.as_str())).count();
+                        return true;
+                    }
+                    _ => {},
+                }
+            }
+
+            cur_path.push(self.sess.cstore.item_name(cur_def));
+            match visible_parent_map.get(&cur_def) {
+                Some(&def) => cur_def = def,
+                None => return false,
+            };
+        }
+    }
+
     pub fn push_item_path<T>(&self, buffer: &mut T, def_id: DefId)
         where T: ItemPathBuffer
     {
+        match *buffer.root_mode() {
+            RootMode::Local if !def_id.is_local() =>
+                if self.try_push_visible_item_path(buffer, def_id) { return },
+            _ => {}
+        }
+
         let key = self.def_key(def_id);
         match key.disambiguated_data.data {
             DefPathData::CrateRoot => {

src/librustc_metadata/csearch.rs

@@ -13,17 +13,17 @@ use decoder;
 use encoder;
 use loader;
 
-use middle::cstore::{CrateStore, CrateSource, ChildItem, ExternCrate, FoundAst};
+use middle::cstore::{CrateStore, CrateSource, ChildItem, ExternCrate, FoundAst, DefLike};
 use middle::cstore::{NativeLibraryKind, LinkMeta, LinkagePreference};
 use middle::def;
 use middle::lang_items;
 use rustc::ty::{self, Ty, TyCtxt, VariantKind};
-use middle::def_id::{DefId, DefIndex};
+use middle::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
 
 use rustc::front::map as hir_map;
 use rustc::mir::repr::Mir;
 use rustc::mir::mir_map::MirMap;
-use rustc::util::nodemap::{FnvHashMap, NodeMap, NodeSet};
+use rustc::util::nodemap::{FnvHashMap, NodeMap, NodeSet, DefIdMap};
 
 use std::cell::RefCell;
 use std::rc::Rc;
@@ -544,4 +544,60 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore {
     {
         encoder::metadata_encoding_version
     }
+
+    /// Returns a map from a sufficiently visible external item (i.e. an external item that is
+    /// visible from at least one local module) to a sufficiently visible parent (considering
+    /// modules that re-export the external item to be parents).
+    fn visible_parent_map<'a>(&'a self) -> ::std::cell::RefMut<'a, DefIdMap<DefId>> {
+        let mut visible_parent_map = self.visible_parent_map.borrow_mut();
+        if !visible_parent_map.is_empty() { return visible_parent_map; }
+
+        use rustc_front::hir;
+        use rustc::middle::cstore::{CrateStore, ChildItem};
+        use std::collections::vec_deque::VecDeque;
+        use std::collections::hash_map::Entry;
+        for cnum in 1 .. self.next_crate_num() {
+            let cdata = self.get_crate_data(cnum);
+
+            match cdata.extern_crate.get() {
+                // Ignore crates without a corresponding local `extern crate` item.
+                Some(extern_crate) if !extern_crate.direct => continue,
+                _ => {},
+            }
+
+            let mut bfs_queue = &mut VecDeque::new();
+            let mut add_child = |bfs_queue: &mut VecDeque<_>, child: ChildItem, parent: DefId| {
+                let child = match child.def {
+                    DefLike::DlDef(def) if child.vis == hir::Public => def.def_id(),
+                    _ => return,
+                };
+
+                match visible_parent_map.entry(child) {
+                    Entry::Occupied(mut entry) => {
+                        // If `child` is defined in crate `cnum`, ensure
+                        // that it is mapped to a parent in `cnum`.
+                        if child.krate == cnum && entry.get().krate != cnum {
+                            entry.insert(parent);
+                        }
+                    }
+                    Entry::Vacant(entry) => {
+                        entry.insert(parent);
+                        bfs_queue.push_back(child);
+                    }
+                }
+            };
+
+            let croot = DefId { krate: cnum, index: CRATE_DEF_INDEX };
+            for child in self.crate_top_level_items(cnum) {
+                add_child(bfs_queue, child, croot);
+            }
+            while let Some(def) = bfs_queue.pop_front() {
+                for child in self.item_children(def) {
+                    add_child(bfs_queue, child, def);
+                }
+            }
+        }
+
+        visible_parent_map
+    }
 }

src/librustc_metadata/cstore.rs

@@ -22,7 +22,8 @@ use loader;
 
 use rustc::back::svh::Svh;
 use rustc::middle::cstore::{ExternCrate};
-use rustc::util::nodemap::{FnvHashMap, NodeMap, NodeSet};
+use rustc::util::nodemap::{FnvHashMap, NodeMap, NodeSet, DefIdMap};
+use rustc::middle::def_id::DefId;
 
 use std::cell::{RefCell, Ref, Cell};
 use std::rc::Rc;
@@ -92,6 +93,7 @@ pub struct CStore {
     used_link_args: RefCell<Vec<String>>,
     statically_included_foreign_items: RefCell<NodeSet>,
     pub intr: Rc<IdentInterner>,
+    pub visible_parent_map: RefCell<DefIdMap<DefId>>,
 }
 
 impl CStore {
@@ -104,6 +106,7 @@ impl CStore {
             used_link_args: RefCell::new(Vec::new()),
             intr: intr,
             statically_included_foreign_items: RefCell::new(NodeSet()),
+            visible_parent_map: RefCell::new(FnvHashMap()),
         }
     }
 

src/test/compile-fail/associated-types-unsized.rs

@@ -14,7 +14,7 @@ trait Get {
 }
 
 fn foo<T:Get>(t: T) {
-    let x = t.get(); //~ ERROR the trait `core::marker::Sized` is not implemented
+    let x = t.get(); //~ ERROR the trait `std::marker::Sized` is not implemented
 }
 
 fn main() {

src/test/compile-fail/bad-const-type.rs

@@ -10,8 +10,8 @@
 
 static i: String = 10;
 //~^ ERROR mismatched types
-//~| expected `collections::string::String`
+//~| expected `std::string::String`
 //~| found `_`
-//~| expected struct `collections::string::String`
+//~| expected struct `std::string::String`
 //~| found integral variable
 fn main() { println!("{}", i); }

src/test/compile-fail/bad-method-typaram-kind.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 fn foo<T:'static>() {
-    1.bar::<T>(); //~ ERROR `core::marker::Send` is not implemented
+    1.bar::<T>(); //~ ERROR `std::marker::Send` is not implemented
 }
 
 trait bar {

src/test/compile-fail/bad-sized.rs

@@ -12,7 +12,7 @@ trait Trait {}
 
 pub fn main() {
     let x: Vec<Trait + Sized> = Vec::new();
-    //~^ ERROR the trait `core::marker::Sized` is not implemented
-    //~| ERROR the trait `core::marker::Sized` is not implemented
-    //~| ERROR the trait `core::marker::Sized` is not implemented
+    //~^ ERROR the trait `std::marker::Sized` is not implemented
+    //~| ERROR the trait `std::marker::Sized` is not implemented
+    //~| ERROR the trait `std::marker::Sized` is not implemented
 }

src/test/compile-fail/binop-bitxor-str.rs

@@ -8,6 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// error-pattern:`^` cannot be applied to type `collections::string::String`
+// error-pattern:`^` cannot be applied to type `std::string::String`
 
 fn main() { let x = "a".to_string() ^ "b".to_string(); }

src/test/compile-fail/builtin-superkinds-double-superkind.rs

@@ -13,9 +13,9 @@
 
 trait Foo : Send+Sync { }
 
-impl <T: Sync+'static> Foo for (T,) { } //~ ERROR the trait `core::marker::Send` is not implemented
+impl <T: Sync+'static> Foo for (T,) { } //~ ERROR the trait `std::marker::Send` is not implemented
 
-impl <T: Send> Foo for (T,T) { } //~ ERROR the trait `core::marker::Sync` is not implemented
+impl <T: Send> Foo for (T,T) { } //~ ERROR the trait `std::marker::Sync` is not implemented
 
 impl <T: Send+Sync> Foo for (T,T,T) { } // (ok)