Keep track of visibility during def collection

6 files changed, 120 insertions, 54 deletions

diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 5323af097..88c4a1216 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -543,7 +543,10 @@ where
             };
             self.body.item_scope.define_def(def);
             if let Some(name) = name {
-                self.body.item_scope.push_res(name.as_name(), def.into());
+                let vis = crate::visibility::ResolvedVisibility::Public; // FIXME determine correctly
+                self.body
+                    .item_scope
+                    .push_res(name.as_name(), crate::per_ns::PerNs::from_def(def, vis));
             }
         }
     }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index b0288ee8d..d77f37f67 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -5,7 +5,10 @@ use hir_expand::name::Name;
 use once_cell::sync::Lazy;
 use rustc_hash::FxHashMap;
 
-use crate::{per_ns::PerNs, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId};
+use crate::{
+    per_ns::PerNs, visibility::ResolvedVisibility, AdtId, BuiltinType, ImplId, MacroDefId,
+    ModuleDefId, TraitId,
+};
 
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ItemScope {
@@ -30,7 +33,9 @@ pub struct ItemScope {
 static BUILTIN_SCOPE: Lazy<FxHashMap<Name, PerNs>> = Lazy::new(|| {
     BuiltinType::ALL
         .iter()
-        .map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into())))
+        .map(|(name, ty)| {
+            (name.clone(), PerNs::types(ty.clone().into(), ResolvedVisibility::Public))
+        })
         .collect()
 });
 
@@ -144,29 +149,37 @@ impl ItemScope {
         changed
     }
 
+    #[cfg(test)]
     pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> {
         self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect()
     }
 
+    pub(crate) fn collect_resolutions_with_vis(
+        &self,
+        vis: ResolvedVisibility,
+    ) -> Vec<(Name, PerNs)> {
+        self.visible.iter().map(|(name, res)| (name.clone(), res.with_visibility(vis))).collect()
+    }
+
     pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {
         self.legacy_macros.clone()
     }
 }
 
-impl From<ModuleDefId> for PerNs {
-    fn from(def: ModuleDefId) -> PerNs {
+impl PerNs {
+    pub(crate) fn from_def(def: ModuleDefId, v: ResolvedVisibility) -> PerNs {
         match def {
-            ModuleDefId::ModuleId(_) => PerNs::types(def),
-            ModuleDefId::FunctionId(_) => PerNs::values(def),
+            ModuleDefId::ModuleId(_) => PerNs::types(def, v),
+            ModuleDefId::FunctionId(_) => PerNs::values(def, v),
             ModuleDefId::AdtId(adt) => match adt {
-                AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def),
-                AdtId::EnumId(_) => PerNs::types(def),
+                AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def, v),
+                AdtId::EnumId(_) => PerNs::types(def, v),
             },
-            ModuleDefId::EnumVariantId(_) => PerNs::both(def, def),
-            ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def),
-            ModuleDefId::TraitId(_) => PerNs::types(def),
-            ModuleDefId::TypeAliasId(_) => PerNs::types(def),
-            ModuleDefId::BuiltinType(_) => PerNs::types(def),
+            ModuleDefId::EnumVariantId(_) => PerNs::both(def, def, v),
+            ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def, v),
+            ModuleDefId::TraitId(_) => PerNs::types(def, v),
+            ModuleDefId::TypeAliasId(_) => PerNs::types(def, v),
+            ModuleDefId::BuiltinType(_) => PerNs::types(def, v),
         }
     }
 }
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index b9f40d3dd..5b8478037 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -24,6 +24,7 @@ use crate::{
     },
     path::{ModPath, PathKind},
     per_ns::PerNs,
+    visibility::ResolvedVisibility,
     AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern,
     LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
 };
@@ -214,7 +215,10 @@ where
         // In Rust, `#[macro_export]` macros are unconditionally visible at the
         // crate root, even if the parent modules is **not** visible.
         if export {
-            self.update(self.def_map.root, &[(name, PerNs::macros(macro_))]);
+            self.update(
+                self.def_map.root,
+                &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))],
+            );
         }
     }
 
@@ -351,6 +355,10 @@ where
         let import_id = directive.import_id;
         let import = &directive.import;
         let def = directive.status.namespaces();
+        let vis = self
+            .def_map
+            .resolve_visibility(self.db, module_id, &directive.import.visibility)
+            .unwrap_or(ResolvedVisibility::Public);
 
         if import.is_glob {
             log::debug!("glob import: {:?}", import);
@@ -365,8 +373,10 @@ where
                         let item_map = self.db.crate_def_map(m.krate);
                         let scope = &item_map[m.local_id].scope;
 
+                        // TODO: only use names we can see
+
                         // Module scoped macros is included
-                        let items = scope.collect_resolutions();
+                        let items = scope.collect_resolutions_with_vis(vis);
 
                         self.update(module_id, &items);
                     } else {
@@ -375,8 +385,10 @@ where
                         // additions
                         let scope = &self.def_map[m.local_id].scope;
 
+                        // TODO: only use names we can see
+
                         // Module scoped macros is included
-                        let items = scope.collect_resolutions();
+                        let items = scope.collect_resolutions_with_vis(vis);
 
                         self.update(module_id, &items);
                         // record the glob import in case we add further items
@@ -396,7 +408,7 @@ where
                         .map(|(local_id, variant_data)| {
                             let name = variant_data.name.clone();
                             let variant = EnumVariantId { parent: e, local_id };
-                            let res = PerNs::both(variant.into(), variant.into());
+                            let res = PerNs::both(variant.into(), variant.into(), vis);
                             (name, res)
                         })
                         .collect::<Vec<_>>();
@@ -422,7 +434,7 @@ where
                         }
                     }
 
-                    self.update(module_id, &[(name, def)]);
+                    self.update(module_id, &[(name, def.with_visibility(vis))]);
                 }
                 None => tested_by!(bogus_paths),
             }
@@ -701,8 +713,9 @@ where
         modules[self.module_id].children.insert(name.clone(), res);
         let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res };
         let def: ModuleDefId = module.into();
+        let vis = ResolvedVisibility::Public; // TODO handle module visibility
         self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
-        self.def_collector.update(self.module_id, &[(name, def.into())]);
+        self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]);
         res
     }
 
@@ -716,6 +729,7 @@ where
 
         let name = def.name.clone();
         let container = ContainerId::ModuleId(module);
+        let vis = &def.visibility;
         let def: ModuleDefId = match def.kind {
             raw::DefKind::Function(ast_id) => FunctionLoc {
                 container: container.into(),
@@ -761,7 +775,12 @@ where
             .into(),
         };
         self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
-        self.def_collector.update(self.module_id, &[(name, def.into())])
+        let vis = self
+            .def_collector
+            .def_map
+            .resolve_visibility(self.def_collector.db, self.module_id, vis)
+            .unwrap_or(ResolvedVisibility::Public);
+        self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))])
     }
 
     fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {
diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs
index d88076aa7..a56e3f08b 100644
--- a/crates/ra_hir_def/src/nameres/path_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/path_resolution.rs
@@ -62,7 +62,9 @@ impl ResolvePathResult {
 
 impl CrateDefMap {
     pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs {
-        self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it))
+        self.extern_prelude
+            .get(name)
+            .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public))
     }
 
     pub(crate) fn resolve_visibility(
@@ -115,17 +117,21 @@ impl CrateDefMap {
             PathKind::DollarCrate(krate) => {
                 if krate == self.krate {
                     tested_by!(macro_dollar_crate_self);
-                    PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into())
+                    PerNs::types(
+                        ModuleId { krate: self.krate, local_id: self.root }.into(),
+                        ResolvedVisibility::Public,
+                    )
                 } else {
                     let def_map = db.crate_def_map(krate);
                     let module = ModuleId { krate, local_id: def_map.root };
                     tested_by!(macro_dollar_crate_other);
-                    PerNs::types(module.into())
+                    PerNs::types(module.into(), ResolvedVisibility::Public)
                 }
             }
-            PathKind::Crate => {
-                PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into())
-            }
+            PathKind::Crate => PerNs::types(
+                ModuleId { krate: self.krate, local_id: self.root }.into(),
+                ResolvedVisibility::Public,
+            ),
             // plain import or absolute path in 2015: crate-relative with
             // fallback to extern prelude (with the simplification in
             // rust-lang/rust#57745)
@@ -153,7 +159,10 @@ impl CrateDefMap {
                 let m = successors(Some(original_module), |m| self.modules[*m].parent)
                     .nth(lvl as usize);
                 if let Some(local_id) = m {
-                    PerNs::types(ModuleId { krate: self.krate, local_id }.into())
+                    PerNs::types(
+                        ModuleId { krate: self.krate, local_id }.into(),
+                        ResolvedVisibility::Public,
+                    )
                 } else {
                     log::debug!("super path in root module");
                     return ResolvePathResult::empty(ReachedFixedPoint::Yes);
@@ -167,7 +176,7 @@ impl CrateDefMap {
                 };
                 if let Some(def) = self.extern_prelude.get(&segment) {
                     log::debug!("absolute path {:?} resolved to crate {:?}", path, def);
-                    PerNs::types(*def)
+                    PerNs::types(*def, ResolvedVisibility::Public)
                 } else {
                     return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude
                 }
@@ -175,7 +184,7 @@ impl CrateDefMap {
         };
 
         for (i, segment) in segments {
-            let curr = match curr_per_ns.take_types() {
+            let (curr, vis) = match curr_per_ns.take_types_vis() {
                 Some(r) => r,
                 None => {
                     // we still have path segments left, but the path so far
@@ -216,11 +225,11 @@ impl CrateDefMap {
                     match enum_data.variant(&segment) {
                         Some(local_id) => {
                             let variant = EnumVariantId { parent: e, local_id };
-                            PerNs::both(variant.into(), variant.into())
+                            PerNs::both(variant.into(), variant.into(), ResolvedVisibility::Public)
                         }
                         None => {
                             return ResolvePathResult::with(
-                                PerNs::types(e.into()),
+                                PerNs::types(e.into(), vis),
                                 ReachedFixedPoint::Yes,
                                 Some(i),
                                 Some(self.krate),
@@ -238,7 +247,7 @@ impl CrateDefMap {
                     );
 
                     return ResolvePathResult::with(
-                        PerNs::types(s),
+                        PerNs::types(s, vis),
                         ReachedFixedPoint::Yes,
                         Some(i),
                         Some(self.krate),
@@ -262,11 +271,15 @@ impl CrateDefMap {
         //  - current module / scope
         //  - extern prelude
         //  - std prelude
-        let from_legacy_macro =
-            self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros);
+        let from_legacy_macro = self[module]
+            .scope
+            .get_legacy_macro(name)
+            .map_or_else(PerNs::none, |m| PerNs::macros(m, ResolvedVisibility::Public));
         let from_scope = self[module].scope.get(name, shadow);
-        let from_extern_prelude =
-            self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
+        let from_extern_prelude = self
+            .extern_prelude
+            .get(name)
+            .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public));
         let from_prelude = self.resolve_in_prelude(db, name, shadow);
 
         from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude)
diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs
index 3a5105028..16e61a6a5 100644
--- a/crates/ra_hir_def/src/per_ns.rs
+++ b/crates/ra_hir_def/src/per_ns.rs
@@ -5,13 +5,13 @@
 
 use hir_expand::MacroDefId;
 
-use crate::ModuleDefId;
+use crate::{visibility::ResolvedVisibility, ModuleDefId};
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct PerNs {
-    pub types: Option<ModuleDefId>,
-    pub values: Option<ModuleDefId>,
-    pub macros: Option<MacroDefId>,
+    pub types: Option<(ModuleDefId, ResolvedVisibility)>,
+    pub values: Option<(ModuleDefId, ResolvedVisibility)>,
+    pub macros: Option<(MacroDefId, ResolvedVisibility)>,
 }
 
 impl Default for PerNs {
@@ -25,20 +25,20 @@ impl PerNs {
         PerNs { types: None, values: None, macros: None }
     }
 
-    pub fn values(t: ModuleDefId) -> PerNs {
-        PerNs { types: None, values: Some(t), macros: None }
+    pub fn values(t: ModuleDefId, v: ResolvedVisibility) -> PerNs {
+        PerNs { types: None, values: Some((t, v)), macros: None }
     }
 
-    pub fn types(t: ModuleDefId) -> PerNs {
-        PerNs { types: Some(t), values: None, macros: None }
+    pub fn types(t: ModuleDefId, v: ResolvedVisibility) -> PerNs {
+        PerNs { types: Some((t, v)), values: None, macros: None }
     }
 
-    pub fn both(types: ModuleDefId, values: ModuleDefId) -> PerNs {
-        PerNs { types: Some(types), values: Some(values), macros: None }
+    pub fn both(types: ModuleDefId, values: ModuleDefId, v: ResolvedVisibility) -> PerNs {
+        PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
     }
 
-    pub fn macros(macro_: MacroDefId) -> PerNs {
-        PerNs { types: None, values: None, macros: Some(macro_) }
+    pub fn macros(macro_: MacroDefId, v: ResolvedVisibility) -> PerNs {
+        PerNs { types: None, values: None, macros: Some((macro_, v)) }
     }
 
     pub fn is_none(&self) -> bool {
@@ -46,15 +46,27 @@ impl PerNs {
     }
 
     pub fn take_types(self) -> Option<ModuleDefId> {
+        self.types.map(|it| it.0)
+    }
+
+    pub fn take_types_vis(self) -> Option<(ModuleDefId, ResolvedVisibility)> {
         self.types
     }
 
     pub fn take_values(self) -> Option<ModuleDefId> {
-        self.values
+        self.values.map(|it| it.0)
     }
 
     pub fn take_macros(self) -> Option<MacroDefId> {
-        self.macros
+        self.macros.map(|it| it.0)
+    }
+
+    pub fn with_visibility(self, vis: ResolvedVisibility) -> PerNs {
+        PerNs {
+            types: self.types.map(|(it, _)| (it, vis)),
+            values: self.values.map(|(it, _)| (it, vis)),
+            macros: self.macros.map(|(it, _)| (it, vis)),
+        }
     }
 
     pub fn or(self, other: PerNs) -> PerNs {
diff --git a/crates/ra_hir_def/src/resolver.rs b/crates/ra_hir_def/src/resolver.rs
index b57dcf635..950bf6c79 100644
--- a/crates/ra_hir_def/src/resolver.rs
+++ b/crates/ra_hir_def/src/resolver.rs
@@ -466,10 +466,16 @@ impl Scope {
                     f(name.clone(), ScopeDef::PerNs(def));
                 });
                 m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| {
-                    f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_)));
+                    f(
+                        name.clone(),
+                        ScopeDef::PerNs(PerNs::macros(macro_, ResolvedVisibility::Public)),
+                    );
                 });
                 m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| {
-                    f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into())));
+                    f(
+                        name.clone(),
+                        ScopeDef::PerNs(PerNs::types(def.into(), ResolvedVisibility::Public)),
+                    );
                 });
                 if let Some(prelude) = m.crate_def_map.prelude {
                     let prelude_def_map = db.crate_def_map(prelude.krate);