Auto merge of #16347 - Veykril:find-path, r=Veykril

internal: Consider all kinds of explicit private imports in find_path

Builds on top of https://github.com/rust-lang/rust-analyzer/pull/16265 to make things a bit more general, now we consider all explicit private imports.

crates/base-db/src/lib.rs

@@ -7,7 +7,6 @@ mod change;
 
 use std::panic;
 
-use rustc_hash::FxHashSet;
 use syntax::{ast, Parse, SourceFile};
 use triomphe::Arc;
 
@@ -90,15 +89,16 @@ pub trait SourceDatabaseExt: SourceDatabase {
 
 fn source_root_crates(db: &dyn SourceDatabaseExt, id: SourceRootId) -> Arc<[CrateId]> {
     let graph = db.crate_graph();
-    graph
+    let mut crates = graph
         .iter()
         .filter(|&krate| {
             let root_file = graph[krate].root_file_id;
             db.file_source_root(root_file) == id
         })
-        .collect::<FxHashSet<_>>()
+        .collect::<Vec<_>>();
-        .into_iter()
+    crates.sort();
-        .collect()
+    crates.dedup();
+    crates.into_iter().collect()
 }
 
 /// Silly workaround for cyclic deps between the traits

crates/hir-def/src/body/lower.rs

@@ -965,11 +965,10 @@ impl ExprCollector<'_> {
 
         let res = match self.def_map.modules[module]
             .scope
-            .macro_invocations
+            .macro_invoc(InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr)))
-            .get(&InFile::new(outer_file, self.ast_id_map.ast_id_for_ptr(syntax_ptr)))
         {
             // fast path, macro call is in a block module
-            Some(&call) => Ok(self.expander.enter_expand_id(self.db, call)),
+            Some(call) => Ok(self.expander.enter_expand_id(self.db, call)),
             None => self.expander.enter_expand(self.db, mcall, |path| {
                 self.def_map
                     .resolve_path(

crates/hir-def/src/child_by_source.rs

@@ -92,7 +92,7 @@ impl ChildBySource for ItemScope {
         self.impls().for_each(|imp| add_impl(db, res, file_id, imp));
         self.extern_crate_decls().for_each(|ext| add_extern_crate(db, res, file_id, ext));
         self.use_decls().for_each(|ext| add_use(db, res, file_id, ext));
-        self.unnamed_consts().for_each(|konst| {
+        self.unnamed_consts(db).for_each(|konst| {
             let loc = konst.lookup(db);
             if loc.id.file_id() == file_id {
                 res[keys::CONST].insert(loc.source(db).value, konst);

crates/hir-def/src/find_path.rs

@@ -10,7 +10,7 @@ use crate::{
     item_scope::ItemInNs,
     nameres::DefMap,
     path::{ModPath, PathKind},
-    visibility::Visibility,
+    visibility::{Visibility, VisibilityExplicity},
     CrateRootModuleId, ModuleDefId, ModuleId,
 };
 
@@ -24,7 +24,7 @@ pub fn find_path(
     prefer_prelude: bool,
 ) -> Option<ModPath> {
     let _p = profile::span("find_path");
-    find_path_inner(db, item, from, None, prefer_no_std, prefer_prelude)
+    find_path_inner(FindPathCtx { db, prefixed: None, prefer_no_std, prefer_prelude }, item, from)
 }
 
 pub fn find_path_prefixed(
@@ -36,7 +36,11 @@ pub fn find_path_prefixed(
     prefer_prelude: bool,
 ) -> Option<ModPath> {
     let _p = profile::span("find_path_prefixed");
-    find_path_inner(db, item, from, Some(prefix_kind), prefer_no_std, prefer_prelude)
+    find_path_inner(
+        FindPathCtx { db, prefixed: Some(prefix_kind), prefer_no_std, prefer_prelude },
+        item,
+        from,
+    )
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -83,64 +87,60 @@ impl PrefixKind {
     }
 }
 
-/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId
+#[derive(Copy, Clone)]
-fn find_path_inner(
+struct FindPathCtx<'db> {
-    db: &dyn DefDatabase,
+    db: &'db dyn DefDatabase,
-    item: ItemInNs,
-    from: ModuleId,
     prefixed: Option<PrefixKind>,
     prefer_no_std: bool,
     prefer_prelude: bool,
-) -> Option<ModPath> {
+}
+
+/// Attempts to find a path to refer to the given `item` visible from the `from` ModuleId
+fn find_path_inner(ctx: FindPathCtx<'_>, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
     // - if the item is a builtin, it's in scope
     if let ItemInNs::Types(ModuleDefId::BuiltinType(builtin)) = item {
         return Some(ModPath::from_segments(PathKind::Plain, Some(builtin.as_name())));
     }
 
-    let def_map = from.def_map(db);
+    let def_map = from.def_map(ctx.db);
     let crate_root = def_map.crate_root();
     // - if the item is a module, jump straight to module search
     if let ItemInNs::Types(ModuleDefId::ModuleId(module_id)) = item {
         let mut visited_modules = FxHashSet::default();
         return find_path_for_module(
-            db,
+            FindPathCtx {
+                prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate),
+                ..ctx
+            },
             &def_map,
             &mut visited_modules,
             crate_root,
             from,
             module_id,
             MAX_PATH_LEN,
-            prefixed,
-            prefer_no_std || db.crate_supports_no_std(crate_root.krate),
-            prefer_prelude,
         )
         .map(|(item, _)| item);
     }
 
     // - if the item is already in scope, return the name under which it is
-    let scope_name = find_in_scope(db, &def_map, from, item);
+    let scope_name = find_in_scope(ctx.db, &def_map, from, item);
-    if prefixed.is_none() {
+    if ctx.prefixed.is_none() {
         if let Some(scope_name) = scope_name {
             return Some(ModPath::from_segments(PathKind::Plain, Some(scope_name)));
         }
     }
 
     // - if the item is in the prelude, return the name from there
-    if let value @ Some(_) = find_in_prelude(db, &crate_root.def_map(db), &def_map, item, from) {
+    if let value @ Some(_) =
+        find_in_prelude(ctx.db, &crate_root.def_map(ctx.db), &def_map, item, from)
+    {
         return value;
     }
 
     if let Some(ModuleDefId::EnumVariantId(variant)) = item.as_module_def_id() {
         // - if the item is an enum variant, refer to it via the enum
-        if let Some(mut path) = find_path_inner(
+        if let Some(mut path) = find_path_inner(ctx, ItemInNs::Types(variant.parent.into()), from) {
-            db,
+            let data = ctx.db.enum_data(variant.parent);
-            ItemInNs::Types(variant.parent.into()),
-            from,
-            prefixed,
-            prefer_no_std,
-            prefer_prelude,
-        ) {
-            let data = db.enum_data(variant.parent);
             path.push_segment(data.variants[variant.local_id].name.clone());
             return Some(path);
         }
@@ -152,32 +152,29 @@ fn find_path_inner(
     let mut visited_modules = FxHashSet::default();
 
     calculate_best_path(
-        db,
+        FindPathCtx {
+            prefer_no_std: ctx.prefer_no_std || ctx.db.crate_supports_no_std(crate_root.krate),
+            ..ctx
+        },
         &def_map,
         &mut visited_modules,
         crate_root,
         MAX_PATH_LEN,
         item,
         from,
-        prefixed,
-        prefer_no_std || db.crate_supports_no_std(crate_root.krate),
-        prefer_prelude,
         scope_name,
     )
     .map(|(item, _)| item)
 }
 
 fn find_path_for_module(
-    db: &dyn DefDatabase,
+    ctx: FindPathCtx<'_>,
     def_map: &DefMap,
     visited_modules: &mut FxHashSet<ModuleId>,
     crate_root: CrateRootModuleId,
     from: ModuleId,
     module_id: ModuleId,
     max_len: usize,
-    prefixed: Option<PrefixKind>,
-    prefer_no_std: bool,
-    prefer_prelude: bool,
 ) -> Option<(ModPath, Stability)> {
     if max_len == 0 {
         return None;
@@ -185,8 +182,8 @@ fn find_path_for_module(
 
     // Base cases:
     // - if the item is already in scope, return the name under which it is
-    let scope_name = find_in_scope(db, def_map, from, ItemInNs::Types(module_id.into()));
+    let scope_name = find_in_scope(ctx.db, def_map, from, ItemInNs::Types(module_id.into()));
-    if prefixed.is_none() {
+    if ctx.prefixed.is_none() {
         if let Some(scope_name) = scope_name {
             return Some((ModPath::from_segments(PathKind::Plain, Some(scope_name)), Stable));
         }
@@ -198,20 +195,20 @@ fn find_path_for_module(
     }
 
     // - if relative paths are fine, check if we are searching for a parent
-    if prefixed.filter(PrefixKind::is_absolute).is_none() {
+    if ctx.prefixed.filter(PrefixKind::is_absolute).is_none() {
         if let modpath @ Some(_) = find_self_super(def_map, module_id, from) {
             return modpath.zip(Some(Stable));
         }
     }
 
     // - if the item is the crate root of a dependency crate, return the name from the extern prelude
-    let root_def_map = crate_root.def_map(db);
+    let root_def_map = crate_root.def_map(ctx.db);
     for (name, (def_id, _extern_crate)) in root_def_map.extern_prelude() {
         if module_id == def_id {
             let name = scope_name.unwrap_or_else(|| name.clone());
 
             let name_already_occupied_in_type_ns = def_map
-                .with_ancestor_maps(db, from.local_id, &mut |def_map, local_id| {
+                .with_ancestor_maps(ctx.db, from.local_id, &mut |def_map, local_id| {
                     def_map[local_id]
                         .scope
                         .type_(&name)
@@ -229,21 +226,18 @@ fn find_path_for_module(
     }
 
     if let value @ Some(_) =
-        find_in_prelude(db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from)
+        find_in_prelude(ctx.db, &root_def_map, &def_map, ItemInNs::Types(module_id.into()), from)
     {
         return value.zip(Some(Stable));
     }
     calculate_best_path(
-        db,
+        ctx,
         def_map,
         visited_modules,
         crate_root,
         max_len,
         ItemInNs::Types(module_id.into()),
         from,
-        prefixed,
-        prefer_no_std,
-        prefer_prelude,
         scope_name,
     )
 }
@@ -256,7 +250,7 @@ fn find_in_scope(
     item: ItemInNs,
 ) -> Option<Name> {
     def_map.with_ancestor_maps(db, from.local_id, &mut |def_map, local_id| {
-        def_map[local_id].scope.name_of(item).map(|(name, _)| name.clone())
+        def_map[local_id].scope.name_of(item).map(|(name, _, _)| name.clone())
     })
 }
 
@@ -273,7 +267,7 @@ fn find_in_prelude(
     // Preludes in block DefMaps are ignored, only the crate DefMap is searched
     let prelude_def_map = prelude_module.def_map(db);
     let prelude_scope = &prelude_def_map[prelude_module.local_id].scope;
-    let (name, vis) = prelude_scope.name_of(item)?;
+    let (name, vis, _declared) = prelude_scope.name_of(item)?;
     if !vis.is_visible_from(db, from) {
         return None;
     }
@@ -315,16 +309,13 @@ fn find_self_super(def_map: &DefMap, item: ModuleId, from: ModuleId) -> Option<M
 }
 
 fn calculate_best_path(
-    db: &dyn DefDatabase,
+    ctx: FindPathCtx<'_>,
     def_map: &DefMap,
     visited_modules: &mut FxHashSet<ModuleId>,
     crate_root: CrateRootModuleId,
     max_len: usize,
     item: ItemInNs,
     from: ModuleId,
-    mut prefixed: Option<PrefixKind>,
-    prefer_no_std: bool,
-    prefer_prelude: bool,
     scope_name: Option<Name>,
 ) -> Option<(ModPath, Stability)> {
     if max_len <= 1 {
@@ -341,32 +332,29 @@ fn calculate_best_path(
         };
     // Recursive case:
     // - otherwise, look for modules containing (reexporting) it and import it from one of those
-    if item.krate(db) == Some(from.krate) {
+    if item.krate(ctx.db) == Some(from.krate) {
         let mut best_path_len = max_len;
         // Item was defined in the same crate that wants to import it. It cannot be found in any
         // dependency in this case.
-        for (module_id, name) in find_local_import_locations(db, item, from) {
+        for (module_id, name) in find_local_import_locations(ctx.db, item, from) {
             if !visited_modules.insert(module_id) {
                 cov_mark::hit!(recursive_imports);
                 continue;
             }
             if let Some(mut path) = find_path_for_module(
-                db,
+                ctx,
                 def_map,
                 visited_modules,
                 crate_root,
                 from,
                 module_id,
                 best_path_len - 1,
-                prefixed,
-                prefer_no_std,
-                prefer_prelude,
             ) {
                 path.0.push_segment(name);
 
                 let new_path = match best_path.take() {
                     Some(best_path) => {
-                        select_best_path(best_path, path, prefer_no_std, prefer_prelude)
+                        select_best_path(best_path, path, ctx.prefer_no_std, ctx.prefer_prelude)
                     }
                     None => path,
                 };
@@ -379,8 +367,8 @@ fn calculate_best_path(
         // too (unless we can't name it at all). It could *also* be (re)exported by the same crate
         // that wants to import it here, but we always prefer to use the external path here.
 
-        for dep in &db.crate_graph()[from.krate].dependencies {
+        for dep in &ctx.db.crate_graph()[from.krate].dependencies {
-            let import_map = db.import_map(dep.crate_id);
+            let import_map = ctx.db.import_map(dep.crate_id);
             let Some(import_info_for) = import_map.import_info_for(item) else { continue };
             for info in import_info_for {
                 if info.is_doc_hidden {
@@ -391,16 +379,13 @@ fn calculate_best_path(
                 // Determine best path for containing module and append last segment from `info`.
                 // FIXME: we should guide this to look up the path locally, or from the same crate again?
                 let Some((mut path, path_stability)) = find_path_for_module(
-                    db,
+                    ctx,
                     def_map,
                     visited_modules,
                     crate_root,
                     from,
                     info.container,
                     max_len - 1,
-                    prefixed,
-                    prefer_no_std,
-                    prefer_prelude,
                 ) else {
                     continue;
                 };
@@ -413,17 +398,21 @@ fn calculate_best_path(
                 );
 
                 let new_path_with_stab = match best_path.take() {
-                    Some(best_path) => {
+                    Some(best_path) => select_best_path(
-                        select_best_path(best_path, path_with_stab, prefer_no_std, prefer_prelude)
+                        best_path,
-                    }
+                        path_with_stab,
+                        ctx.prefer_no_std,
+                        ctx.prefer_prelude,
+                    ),
                     None => path_with_stab,
                 };
                 update_best_path(&mut best_path, new_path_with_stab);
             }
         }
     }
-    if let Some(module) = item.module(db) {
+    let mut prefixed = ctx.prefixed;
-        if module.containing_block().is_some() && prefixed.is_some() {
+    if let Some(module) = item.module(ctx.db) {
+        if module.containing_block().is_some() && ctx.prefixed.is_some() {
             cov_mark::hit!(prefixed_in_block_expression);
             prefixed = Some(PrefixKind::Plain);
         }
@@ -548,48 +537,38 @@ fn find_local_import_locations(
             &ext_def_map[module.local_id]
         };
 
-        if let Some((name, vis)) = data.scope.name_of(item) {
+        if let Some((name, vis, declared)) = data.scope.name_of(item) {
             if vis.is_visible_from(db, from) {
-                let is_private = match vis {
+                let is_pub_or_explicit = match vis {
-                    Visibility::Module(private_mod, private_vis) => {
+                    Visibility::Module(_, VisibilityExplicity::Explicit) => {
-                        if private_mod == def_map.module_id(DefMap::ROOT)
+                        cov_mark::hit!(explicit_private_imports);
-                            && private_vis.is_explicit()
+                        true
-                        {
+                    }
-                            // Treat `pub(crate)` imports as non-private, so
+                    Visibility::Module(_, VisibilityExplicity::Implicit) => {
-                            // that we suggest adding `use crate::Foo;` instead
+                        cov_mark::hit!(discount_private_imports);
-                            // of `use crate::foo::Foo;` etc.
                         false
-                        } else {
-                            private_mod.local_id == module.local_id
                     }
-                    }
+                    Visibility::Public => true,
-                    Visibility::Public => false,
-                };
-                let is_original_def = match item.as_module_def_id() {
-                    Some(module_def_id) => data.scope.declarations().any(|it| it == module_def_id),
-                    None => false,
                 };
 
-                // Ignore private imports. these could be used if we are
+                // Ignore private imports unless they are explicit. these could be used if we are
                 // in a submodule of this module, but that's usually not
                 // what the user wants; and if this module can import
                 // the item and we're a submodule of it, so can we.
                 // Also this keeps the cached data smaller.
-                if !is_private || is_original_def {
+                if is_pub_or_explicit || declared {
                     locations.push((module, name.clone()));
                 }
             }
         }
 
         // Descend into all modules visible from `from`.
-        for (ty, vis) in data.scope.types() {
+        for (module, vis) in data.scope.modules_in_scope() {
-            if let ModuleDefId::ModuleId(module) = ty {
             if vis.is_visible_from(db, from) {
                 worklist.push(module);
             }
         }
     }
-    }
 
     locations
 }
@@ -636,16 +615,14 @@ mod tests {
             .expect("path does not resolve to a type");
 
         let found_path = find_path_inner(
-            &db,
+            FindPathCtx { prefer_no_std: false, db: &db, prefixed: prefix_kind, prefer_prelude },
             ItemInNs::Types(resolved),
             module,
-            prefix_kind,
-            false,
-            prefer_prelude,
         );
         assert_eq!(found_path, Some(mod_path), "on kind: {prefix_kind:?}");
     }
 
+    #[track_caller]
     fn check_found_path(
         ra_fixture: &str,
         unprefixed: &str,
@@ -1015,6 +992,7 @@ pub use crate::foo::bar::S;
 
     #[test]
     fn discount_private_imports() {
+        cov_mark::check!(discount_private_imports);
         check_found_path(
             r#"
 //- /main.rs
@@ -1033,7 +1011,8 @@ $0
     }
 
     #[test]
-    fn promote_pub_crate_imports() {
+    fn explicit_private_imports_crate() {
+        cov_mark::check!(explicit_private_imports);
         check_found_path(
             r#"
 //- /main.rs
@@ -1050,6 +1029,28 @@ $0
         );
     }
 
+    #[test]
+    fn explicit_private_imports() {
+        cov_mark::check!(explicit_private_imports);
+        check_found_path(
+            r#"
+//- /main.rs
+pub mod bar {
+    mod foo;
+    pub mod baz { pub struct S; }
+    pub(self) use baz::S;
+}
+
+//- /bar/foo.rs
+$0
+        "#,
+            "super::S",
+            "super::S",
+            "crate::bar::S",
+            "super::S",
+        );
+    }
+
     #[test]
     fn import_cycle() {
         check_found_path(

crates/hir-def/src/item_scope.rs

@@ -15,9 +15,11 @@ use stdx::format_to;
 use syntax::ast;
 
 use crate::{
-    db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, ConstId,
+    db::DefDatabase,
-    ExternCrateId, HasModule, ImplId, LocalModuleId, Lookup, MacroId, ModuleDefId, ModuleId,
+    per_ns::PerNs,
-    TraitId, UseId,
+    visibility::{Visibility, VisibilityExplicity},
+    AdtId, BuiltinType, ConstId, ExternCrateId, HasModule, ImplId, LocalModuleId, Lookup, MacroId,
+    ModuleDefId, ModuleId, TraitId, UseId,
 };
 
 #[derive(Debug, Default)]
@@ -105,7 +107,7 @@ pub struct ItemScope {
     /// The attribute macro invocations in this scope.
     attr_macros: FxHashMap<AstId<ast::Item>, MacroCallId>,
     /// The macro invocations in this scope.
-    pub macro_invocations: FxHashMap<AstId<ast::MacroCall>, MacroCallId>,
+    macro_invocations: FxHashMap<AstId<ast::MacroCall>, MacroCallId>,
     /// The derive macro invocations in this scope, keyed by the owner item over the actual derive attributes
     /// paired with the derive macro invocations for the specific attribute.
     derive_macros: FxHashMap<AstId<ast::Adt>, SmallVec<[DeriveMacroInvocation; 1]>>,
@@ -145,8 +147,8 @@ impl ItemScope {
             .chain(self.values.keys())
             .chain(self.macros.keys())
             .chain(self.unresolved.iter())
-            .unique()
             .sorted()
+            .dedup()
             .map(move |name| (name, self.get(name)))
     }
 
@@ -157,8 +159,8 @@ impl ItemScope {
             .filter_map(ImportOrExternCrate::into_import)
             .chain(self.use_imports_values.keys().copied())
             .chain(self.use_imports_macros.keys().copied())
-            .unique()
             .sorted()
+            .dedup()
     }
 
     pub fn fully_resolve_import(&self, db: &dyn DefDatabase, mut import: ImportId) -> PerNs {
@@ -234,20 +236,37 @@ impl ItemScope {
         self.impls.iter().copied()
     }
 
-    pub fn values(
+    pub(crate) fn modules_in_scope(&self) -> impl Iterator<Item = (ModuleId, Visibility)> + '_ {
-        &self,
+        self.types.values().copied().filter_map(|(def, vis, _)| match def {
-    ) -> impl Iterator<Item = (ModuleDefId, Visibility)> + ExactSizeIterator + '_ {
+            ModuleDefId::ModuleId(module) => Some((module, vis)),
-        self.values.values().copied().map(|(a, b, _)| (a, b))
+            _ => None,
+        })
     }
 
-    pub(crate) fn types(
+    pub fn unnamed_consts<'a>(
-        &self,
+        &'a self,
-    ) -> impl Iterator<Item = (ModuleDefId, Visibility)> + ExactSizeIterator + '_ {
+        db: &'a dyn DefDatabase,
-        self.types.values().copied().map(|(def, vis, _)| (def, vis))
+    ) -> impl Iterator<Item = ConstId> + 'a {
+        // FIXME: Also treat consts named `_DERIVE_*` as unnamed, since synstructure generates those.
+        // Should be removed once synstructure stops doing that.
+        let synstructure_hack_consts = self.values.values().filter_map(|(item, _, _)| match item {
+            &ModuleDefId::ConstId(id) => {
+                let loc = id.lookup(db);
+                let item_tree = loc.id.item_tree(db);
+                if item_tree[loc.id.value]
+                    .name
+                    .as_ref()
+                    .map_or(false, |n| n.to_smol_str().starts_with("_DERIVE_"))
+                {
+                    Some(id)
+                } else {
+                    None
                 }
+            }
+            _ => None,
+        });
 
-    pub fn unnamed_consts(&self) -> impl Iterator<Item = ConstId> + '_ {
+        self.unnamed_consts.iter().copied().chain(synstructure_hack_consts)
-        self.unnamed_consts.iter().copied()
     }
 
     /// Iterate over all module scoped macros
@@ -274,21 +293,18 @@ impl ItemScope {
     }
 
     /// XXX: this is O(N) rather than O(1), try to not introduce new usages.
-    pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility)> {
+    pub(crate) fn name_of(&self, item: ItemInNs) -> Option<(&Name, Visibility, /*declared*/ bool)> {
         match item {
-            ItemInNs::Macros(def) => self
+            ItemInNs::Macros(def) => self.macros.iter().find_map(|(name, &(other_def, vis, i))| {
-                .macros
+                (other_def == def).then_some((name, vis, i.is_none()))
-                .iter()
+            }),
-                .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))),
+            ItemInNs::Types(def) => self.types.iter().find_map(|(name, &(other_def, vis, i))| {
-            ItemInNs::Types(def) => self
+                (other_def == def).then_some((name, vis, i.is_none()))
-                .types
+            }),
-                .iter()
-                .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))),
 
-            ItemInNs::Values(def) => self
+            ItemInNs::Values(def) => self.values.iter().find_map(|(name, &(other_def, vis, i))| {
-                .values
+                (other_def == def).then_some((name, vis, i.is_none()))
-                .iter()
+            }),
-                .find_map(|(name, &(other_def, vis, _))| (other_def == def).then_some((name, vis))),
         }
     }
 
@@ -316,6 +332,10 @@ impl ItemScope {
             }),
         )
     }
+
+    pub(crate) fn macro_invoc(&self, call: AstId<ast::MacroCall>) -> Option<MacroCallId> {
+        self.macro_invocations.get(&call).copied()
+    }
 }
 
 impl ItemScope {
@@ -624,18 +644,17 @@ impl ItemScope {
     pub(crate) fn censor_non_proc_macros(&mut self, this_module: ModuleId) {
         self.types
             .values_mut()
-            .map(|(def, vis, _)| (def, vis))
+            .map(|(_, vis, _)| vis)
-            .chain(self.values.values_mut().map(|(def, vis, _)| (def, vis)))
+            .chain(self.values.values_mut().map(|(_, vis, _)| vis))
-            .map(|(_, v)| v)
             .chain(self.unnamed_trait_imports.values_mut().map(|(vis, _)| vis))
-            .for_each(|vis| *vis = Visibility::Module(this_module, Default::default()));
+            .for_each(|vis| *vis = Visibility::Module(this_module, VisibilityExplicity::Implicit));
 
         for (mac, vis, import) in self.macros.values_mut() {
             if matches!(mac, MacroId::ProcMacroId(_) if import.is_none()) {
                 continue;
             }
 
-            *vis = Visibility::Module(this_module, Default::default());
+            *vis = Visibility::Module(this_module, VisibilityExplicity::Implicit);
         }
     }
 

crates/hir-def/src/item_tree.rs

@@ -69,7 +69,7 @@ use crate::{
     generics::{GenericParams, LifetimeParamData, TypeOrConstParamData},
     path::{path, AssociatedTypeBinding, GenericArgs, ImportAlias, ModPath, Path, PathKind},
     type_ref::{Mutability, TraitRef, TypeBound, TypeRef},
-    visibility::RawVisibility,
+    visibility::{RawVisibility, VisibilityExplicity},
     BlockId, Lookup,
 };
 
@@ -78,8 +78,9 @@ pub struct RawVisibilityId(u32);
 
 impl RawVisibilityId {
     pub const PUB: Self = RawVisibilityId(u32::max_value());
-    pub const PRIV: Self = RawVisibilityId(u32::max_value() - 1);
+    pub const PRIV_IMPLICIT: Self = RawVisibilityId(u32::max_value() - 1);
-    pub const PUB_CRATE: Self = RawVisibilityId(u32::max_value() - 2);
+    pub const PRIV_EXPLICIT: Self = RawVisibilityId(u32::max_value() - 2);
+    pub const PUB_CRATE: Self = RawVisibilityId(u32::max_value() - 3);
 }
 
 impl fmt::Debug for RawVisibilityId {
@@ -87,7 +88,7 @@ impl fmt::Debug for RawVisibilityId {
         let mut f = f.debug_tuple("RawVisibilityId");
         match *self {
             Self::PUB => f.field(&"pub"),
-            Self::PRIV => f.field(&"pub(self)"),
+            Self::PRIV_IMPLICIT | Self::PRIV_EXPLICIT => f.field(&"pub(self)"),
             Self::PUB_CRATE => f.field(&"pub(crate)"),
             _ => f.field(&self.0),
         };
@@ -249,19 +250,30 @@ impl ItemVisibilities {
     fn alloc(&mut self, vis: RawVisibility) -> RawVisibilityId {
         match &vis {
             RawVisibility::Public => RawVisibilityId::PUB,
-            RawVisibility::Module(path) if path.segments().is_empty() => match &path.kind {
+            RawVisibility::Module(path, explicitiy) if path.segments().is_empty() => {
-                PathKind::Super(0) => RawVisibilityId::PRIV,
+                match (&path.kind, explicitiy) {
-                PathKind::Crate => RawVisibilityId::PUB_CRATE,
+                    (PathKind::Super(0), VisibilityExplicity::Explicit) => {
+                        RawVisibilityId::PRIV_EXPLICIT
+                    }
+                    (PathKind::Super(0), VisibilityExplicity::Implicit) => {
+                        RawVisibilityId::PRIV_IMPLICIT
+                    }
+                    (PathKind::Crate, _) => RawVisibilityId::PUB_CRATE,
                     _ => RawVisibilityId(self.arena.alloc(vis).into_raw().into()),
-            },
+                }
+            }
             _ => RawVisibilityId(self.arena.alloc(vis).into_raw().into()),
         }
     }
 }
 
 static VIS_PUB: RawVisibility = RawVisibility::Public;
-static VIS_PRIV: RawVisibility = RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)));
+static VIS_PRIV_IMPLICIT: RawVisibility =
-static VIS_PUB_CRATE: RawVisibility = RawVisibility::Module(ModPath::from_kind(PathKind::Crate));
+    RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Implicit);
+static VIS_PRIV_EXPLICIT: RawVisibility =
+    RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Explicit);
+static VIS_PUB_CRATE: RawVisibility =
+    RawVisibility::Module(ModPath::from_kind(PathKind::Crate), VisibilityExplicity::Explicit);
 
 #[derive(Default, Debug, Eq, PartialEq)]
 struct ItemTreeData {
@@ -540,7 +552,8 @@ impl Index<RawVisibilityId> for ItemTree {
     type Output = RawVisibility;
     fn index(&self, index: RawVisibilityId) -> &Self::Output {
         match index {
-            RawVisibilityId::PRIV => &VIS_PRIV,
+            RawVisibilityId::PRIV_IMPLICIT => &VIS_PRIV_IMPLICIT,
+            RawVisibilityId::PRIV_EXPLICIT => &VIS_PRIV_EXPLICIT,
             RawVisibilityId::PUB => &VIS_PUB,
             RawVisibilityId::PUB_CRATE => &VIS_PUB_CRATE,
             _ => &self.data().vis.arena[Idx::from_raw(index.0.into())],

crates/hir-def/src/item_tree/pretty.rs

@@ -104,7 +104,9 @@ impl Printer<'_> {
 
     fn print_visibility(&mut self, vis: RawVisibilityId) {
         match &self.tree[vis] {
-            RawVisibility::Module(path) => w!(self, "pub({}) ", path.display(self.db.upcast())),
+            RawVisibility::Module(path, _expl) => {
+                w!(self, "pub({}) ", path.display(self.db.upcast()))
+            }
             RawVisibility::Public => w!(self, "pub "),
         };
     }

crates/hir-def/src/nameres.rs

@@ -79,7 +79,7 @@ use crate::{
     nameres::{diagnostics::DefDiagnostic, path_resolution::ResolveMode},
     path::ModPath,
     per_ns::PerNs,
-    visibility::Visibility,
+    visibility::{Visibility, VisibilityExplicity},
     AstId, BlockId, BlockLoc, CrateRootModuleId, ExternCrateId, FunctionId, LocalModuleId, Lookup,
     MacroExpander, MacroId, ModuleId, ProcMacroId, UseId,
 };
@@ -332,8 +332,10 @@ impl DefMap {
         // NB: we use `None` as block here, which would be wrong for implicit
         // modules declared by blocks with items. At the moment, we don't use
         // this visibility for anything outside IDE, so that's probably OK.
-        let visibility =
+        let visibility = Visibility::Module(
-            Visibility::Module(ModuleId { krate, local_id, block: None }, Default::default());
+            ModuleId { krate, local_id, block: None },
+            VisibilityExplicity::Implicit,
+        );
         let module_data = ModuleData::new(
             ModuleOrigin::BlockExpr { block: block.ast_id, id: block_id },
             visibility,

crates/hir-def/src/nameres/path_resolution.rs

@@ -21,7 +21,7 @@ use crate::{
     nameres::{sub_namespace_match, BlockInfo, BuiltinShadowMode, DefMap, MacroSubNs},
     path::{ModPath, PathKind},
     per_ns::PerNs,
-    visibility::{RawVisibility, Visibility, VisibilityExplicity},
+    visibility::{RawVisibility, Visibility},
     AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId,
 };
 
@@ -87,20 +87,15 @@ impl DefMap {
         within_impl: bool,
     ) -> Option<Visibility> {
         let mut vis = match visibility {
-            RawVisibility::Module(path) => {
+            RawVisibility::Module(path, explicity) => {
                 let (result, remaining) =
                     self.resolve_path(db, original_module, path, BuiltinShadowMode::Module, None);
                 if remaining.is_some() {
                     return None;
                 }
                 let types = result.take_types()?;
-                let mv = if path.is_pub_crate() {
-                    VisibilityExplicity::Explicit
-                } else {
-                    VisibilityExplicity::Implicit
-                };
                 match types {
-                    ModuleDefId::ModuleId(m) => Visibility::Module(m, mv),
+                    ModuleDefId::ModuleId(m) => Visibility::Module(m, *explicity),
                     // error: visibility needs to refer to module
                     _ => {
                         return None;

crates/hir-def/src/resolver.rs

@@ -242,7 +242,7 @@ impl Resolver {
         let within_impl =
             self.scopes().find(|scope| matches!(scope, Scope::ImplDefScope(_))).is_some();
         match visibility {
-            RawVisibility::Module(_) => {
+            RawVisibility::Module(_, _) => {
                 let (item_map, module) = self.item_scope();
                 item_map.resolve_visibility(db, module, visibility, within_impl)
             }

crates/hir-def/src/visibility.rs

@@ -20,14 +20,14 @@ use crate::{
 pub enum RawVisibility {
     /// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
     /// equivalent to `pub(self)`.
-    Module(ModPath),
+    Module(ModPath, VisibilityExplicity),
     /// `pub`.
     Public,
 }
 
 impl RawVisibility {
     pub(crate) const fn private() -> RawVisibility {
-        RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)))
+        RawVisibility::Module(ModPath::from_kind(PathKind::Super(0)), VisibilityExplicity::Implicit)
     }
 
     pub(crate) fn from_ast(
@@ -41,18 +41,9 @@ impl RawVisibility {
         db: &dyn DefDatabase,
         node: Option<ast::Visibility>,
         span_map: SpanMapRef<'_>,
-    ) -> RawVisibility {
-        Self::from_ast_with_span_map_and_default(db, node, RawVisibility::private(), span_map)
-    }
-
-    pub(crate) fn from_ast_with_span_map_and_default(
-        db: &dyn DefDatabase,
-        node: Option<ast::Visibility>,
-        default: RawVisibility,
-        span_map: SpanMapRef<'_>,
     ) -> RawVisibility {
         let node = match node {
-            None => return default,
+            None => return RawVisibility::private(),
             Some(node) => node,
         };
         match node.kind() {
@@ -62,19 +53,19 @@ impl RawVisibility {
                     None => return RawVisibility::private(),
                     Some(path) => path,
                 };
-                RawVisibility::Module(path)
+                RawVisibility::Module(path, VisibilityExplicity::Explicit)
             }
             ast::VisibilityKind::PubCrate => {
                 let path = ModPath::from_kind(PathKind::Crate);
-                RawVisibility::Module(path)
+                RawVisibility::Module(path, VisibilityExplicity::Explicit)
             }
             ast::VisibilityKind::PubSuper => {
                 let path = ModPath::from_kind(PathKind::Super(1));
-                RawVisibility::Module(path)
+                RawVisibility::Module(path, VisibilityExplicity::Explicit)
             }
             ast::VisibilityKind::PubSelf => {
                 let path = ModPath::from_kind(PathKind::Super(0));
-                RawVisibility::Module(path)
+                RawVisibility::Module(path, VisibilityExplicity::Explicit)
             }
             ast::VisibilityKind::Pub => RawVisibility::Public,
         }
@@ -214,10 +205,9 @@ impl Visibility {
 }
 
 /// Whether the item was imported through `pub(crate) use` or just `use`.
-#[derive(Debug, Default, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub enum VisibilityExplicity {
     Explicit,
-    #[default]
     Implicit,
 }
 

crates/hir-expand/src/mod_path.rs

@@ -96,10 +96,6 @@ impl ModPath {
         self.kind == PathKind::Super(0) && self.segments.is_empty()
     }
 
-    pub fn is_pub_crate(&self) -> bool {
-        self.kind == PathKind::Crate && self.segments.is_empty()
-    }
-
     #[allow(non_snake_case)]
     pub fn is_Self(&self) -> bool {
         self.kind == PathKind::Plain

crates/hir-ty/src/method_resolution.rs

@@ -8,10 +8,9 @@ use base_db::{CrateId, Edition};
 use chalk_ir::{cast::Cast, Mutability, TyKind, UniverseIndex, WhereClause};
 use hir_def::{
     data::{adt::StructFlags, ImplData},
-    item_scope::ItemScope,
     nameres::DefMap,
     AssocItemId, BlockId, ConstId, FunctionId, HasModule, ImplId, ItemContainerId, Lookup,
-    ModuleDefId, ModuleId, TraitId,
+    ModuleId, TraitId,
 };
 use hir_expand::name::Name;
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -212,7 +211,7 @@ impl TraitImpls {
 
             // To better support custom derives, collect impls in all unnamed const items.
             // const _: () = { ... };
-            for konst in collect_unnamed_consts(db, &module_data.scope) {
+            for konst in module_data.scope.unnamed_consts(db.upcast()) {
                 let body = db.body(konst.into());
                 for (_, block_def_map) in body.blocks(db.upcast()) {
                     Self::collect_def_map(db, map, &block_def_map);
@@ -330,7 +329,7 @@ impl InherentImpls {
 
             // To better support custom derives, collect impls in all unnamed const items.
             // const _: () = { ... };
-            for konst in collect_unnamed_consts(db, &module_data.scope) {
+            for konst in module_data.scope.unnamed_consts(db.upcast()) {
                 let body = db.body(konst.into());
                 for (_, block_def_map) in body.blocks(db.upcast()) {
                     self.collect_def_map(db, &block_def_map);
@@ -376,34 +375,6 @@ pub(crate) fn incoherent_inherent_impl_crates(
     res
 }
 
-fn collect_unnamed_consts<'a>(
-    db: &'a dyn HirDatabase,
-    scope: &'a ItemScope,
-) -> impl Iterator<Item = ConstId> + 'a {
-    let unnamed_consts = scope.unnamed_consts();
-
-    // FIXME: Also treat consts named `_DERIVE_*` as unnamed, since synstructure generates those.
-    // Should be removed once synstructure stops doing that.
-    let synstructure_hack_consts = scope.values().filter_map(|(item, _)| match item {
-        ModuleDefId::ConstId(id) => {
-            let loc = id.lookup(db.upcast());
-            let item_tree = loc.id.item_tree(db.upcast());
-            if item_tree[loc.id.value]
-                .name
-                .as_ref()
-                .map_or(false, |n| n.to_smol_str().starts_with("_DERIVE_"))
-            {
-                Some(id)
-            } else {
-                None
-            }
-        }
-        _ => None,
-    });
-
-    unnamed_consts.chain(synstructure_hack_consts)
-}
-
 pub fn def_crates(
     db: &dyn HirDatabase,
     ty: &Ty,

crates/hir/src/lib.rs

@@ -754,7 +754,7 @@ impl Module {
         scope
             .declarations()
             .map(ModuleDef::from)
-            .chain(scope.unnamed_consts().map(|id| ModuleDef::Const(Const::from(id))))
+            .chain(scope.unnamed_consts(db.upcast()).map(|id| ModuleDef::Const(Const::from(id))))
             .collect()
     }
 

crates/hir/src/symbols.rs

@@ -196,7 +196,7 @@ impl<'a> SymbolCollector<'a> {
             });
         }
 
-        for const_id in scope.unnamed_consts() {
+        for const_id in scope.unnamed_consts(self.db.upcast()) {
             self.collect_from_body(const_id);
         }