Merge #5475

5475: Support `Trait as _` imports r=matklad a=jonas-schievink

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/2736

Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>

crates/ra_hir_def/src/item_scope.rs

@@ -36,6 +36,8 @@ pub struct ItemScope {
 
     defs: Vec<ModuleDefId>,
     impls: Vec<ImplId>,
+    /// Traits imported via `use Trait as _;`.
+    unnamed_trait_imports: FxHashMap<TraitId, Visibility>,
     /// Macros visible in current module in legacy textual scope
     ///
     /// For macros invoked by an unqualified identifier like `bar!()`, `legacy_macros` will be searched in first.
@@ -126,10 +128,13 @@ impl ItemScope {
     }
 
     pub(crate) fn traits<'a>(&'a self) -> impl Iterator<Item = TraitId> + 'a {
-        self.types.values().filter_map(|(def, _)| match def {
+        self.types
-            ModuleDefId::TraitId(t) => Some(*t),
+            .values()
-            _ => None,
+            .filter_map(|(def, _)| match def {
-        })
+                ModuleDefId::TraitId(t) => Some(*t),
+                _ => None,
+            })
+            .chain(self.unnamed_trait_imports.keys().copied())
     }
 
     pub(crate) fn define_def(&mut self, def: ModuleDefId) {
@@ -148,6 +153,14 @@ impl ItemScope {
         self.legacy_macros.insert(name, mac);
     }
 
+    pub(crate) fn unnamed_trait_vis(&self, tr: TraitId) -> Option<Visibility> {
+        self.unnamed_trait_imports.get(&tr).copied()
+    }
+
+    pub(crate) fn push_unnamed_trait(&mut self, tr: TraitId, vis: Visibility) {
+        self.unnamed_trait_imports.insert(tr, vis);
+    }
+
     pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool {
         let mut changed = false;
 
@@ -241,8 +254,12 @@ impl ItemScope {
         changed
     }
 
-    pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator<Item = (Name, PerNs)> + 'a {
+    pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator<Item = (Option<Name>, PerNs)> + 'a {
-        self.entries().map(|(name, res)| (name.clone(), res))
+        self.entries().map(|(name, res)| (Some(name.clone()), res)).chain(
+            self.unnamed_trait_imports
+                .iter()
+                .map(|(tr, vis)| (None, PerNs::types(ModuleDefId::TraitId(*tr), *vis))),
+        )
     }
 
     pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {

crates/ra_hir_def/src/nameres.rs

@@ -239,7 +239,7 @@ impl CrateDefMap {
             entries.sort_by_key(|(name, _)| name.clone());
 
             for (name, def) in entries {
-                format_to!(buf, "{}:", name);
+                format_to!(buf, "{}:", name.map_or("_".to_string(), |name| name.to_string()));
 
                 if def.types.is_some() {
                     buf.push_str(" t");

crates/ra_hir_def/src/nameres/collector.rs

@@ -310,7 +310,7 @@ impl DefCollector<'_> {
         if export {
             self.update(
                 self.def_map.root,
-                &[(name, PerNs::macros(macro_, Visibility::Public))],
+                &[(Some(name), PerNs::macros(macro_, Visibility::Public))],
                 Visibility::Public,
                 ImportType::Named,
             );
@@ -336,7 +336,7 @@ impl DefCollector<'_> {
     fn define_proc_macro(&mut self, name: Name, macro_: MacroDefId) {
         self.update(
             self.def_map.root,
-            &[(name, PerNs::macros(macro_, Visibility::Public))],
+            &[(Some(name), PerNs::macros(macro_, Visibility::Public))],
             Visibility::Public,
             ImportType::Named,
         );
@@ -534,7 +534,7 @@ impl DefCollector<'_> {
                             let name = variant_data.name.clone();
                             let variant = EnumVariantId { parent: e, local_id };
                             let res = PerNs::both(variant.into(), variant.into(), vis);
-                            (name, res)
+                            (Some(name), res)
                         })
                         .collect::<Vec<_>>();
                     self.update(module_id, &resolutions, vis, ImportType::Glob);
@@ -550,15 +550,15 @@ impl DefCollector<'_> {
             match import.path.segments.last() {
                 Some(last_segment) => {
                     let name = match &import.alias {
-                        Some(ImportAlias::Alias(name)) => name.clone(),
+                        Some(ImportAlias::Alias(name)) => Some(name.clone()),
-                        Some(ImportAlias::Underscore) => last_segment.clone(), // FIXME rust-analyzer#2736
+                        Some(ImportAlias::Underscore) => None,
-                        None => last_segment.clone(),
+                        None => Some(last_segment.clone()),
                     };
                     log::debug!("resolved import {:?} ({:?}) to {:?}", name, import, def);
 
                     // extern crates in the crate root are special-cased to insert entries into the extern prelude: rust-lang/rust#54658
                     if import.is_extern_crate && module_id == self.def_map.root {
-                        if let Some(def) = def.take_types() {
+                        if let (Some(def), Some(name)) = (def.take_types(), name.as_ref()) {
                             self.def_map.extern_prelude.insert(name.clone(), def);
                         }
                     }
@@ -573,7 +573,7 @@ impl DefCollector<'_> {
     fn update(
         &mut self,
         module_id: LocalModuleId,
-        resolutions: &[(Name, PerNs)],
+        resolutions: &[(Option<Name>, PerNs)],
         vis: Visibility,
         import_type: ImportType,
     ) {
@@ -584,7 +584,7 @@ impl DefCollector<'_> {
     fn update_recursive(
         &mut self,
         module_id: LocalModuleId,
-        resolutions: &[(Name, PerNs)],
+        resolutions: &[(Option<Name>, PerNs)],
         // All resolutions are imported with this visibility; the visibilies in
         // the `PerNs` values are ignored and overwritten
         vis: Visibility,
@@ -595,15 +595,51 @@ impl DefCollector<'_> {
             // prevent stack overflows (but this shouldn't be possible)
             panic!("infinite recursion in glob imports!");
         }
-        let scope = &mut self.def_map.modules[module_id].scope;
         let mut changed = false;
+
         for (name, res) in resolutions {
-            changed |= scope.push_res_with_import(
+            match name {
-                &mut self.from_glob_import,
+                Some(name) => {
-                (module_id, name.clone()),
+                    let scope = &mut self.def_map.modules[module_id].scope;
-                res.with_visibility(vis),
+                    changed |= scope.push_res_with_import(
-                import_type,
+                        &mut self.from_glob_import,
-            );
+                        (module_id, name.clone()),
+                        res.with_visibility(vis),
+                        import_type,
+                    );
+                }
+                None => {
+                    let tr = match res.take_types() {
+                        Some(ModuleDefId::TraitId(tr)) => tr,
+                        Some(other) => {
+                            log::debug!("non-trait `_` import of {:?}", other);
+                            continue;
+                        }
+                        None => continue,
+                    };
+                    let old_vis = self.def_map.modules[module_id].scope.unnamed_trait_vis(tr);
+                    let should_update = match old_vis {
+                        None => true,
+                        Some(old_vis) => {
+                            let max_vis = old_vis.max(vis, &self.def_map).unwrap_or_else(|| {
+                                panic!("`Tr as _` imports with unrelated visibilities {:?} and {:?} (trait {:?})", old_vis, vis, tr);
+                            });
+
+                            if max_vis == old_vis {
+                                false
+                            } else {
+                                mark::hit!(upgrade_underscore_visibility);
+                                true
+                            }
+                        }
+                    };
+
+                    if should_update {
+                        changed = true;
+                        self.def_map.modules[module_id].scope.push_unnamed_trait(tr, vis);
+                    }
+                }
+            }
         }
 
         if !changed {
@@ -950,7 +986,7 @@ impl ModCollector<'_, '_> {
                         .unwrap_or(Visibility::Public);
                     self.def_collector.update(
                         self.module_id,
-                        &[(name.clone(), PerNs::from_def(id, vis, has_constructor))],
+                        &[(Some(name.clone()), PerNs::from_def(id, vis, has_constructor))],
                         vis,
                         ImportType::Named,
                     )
@@ -1057,7 +1093,7 @@ impl ModCollector<'_, '_> {
         self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
         self.def_collector.update(
             self.module_id,
-            &[(name, PerNs::from_def(def, vis, false))],
+            &[(Some(name), PerNs::from_def(def, vis, false))],
             vis,
             ImportType::Named,
         );

crates/ra_hir_def/src/nameres/tests.rs

@@ -558,3 +558,133 @@ mod b {
         "#]],
     );
 }
+
+#[test]
+fn underscore_import() {
+    check(
+        r#"
+//- /main.rs
+use tr::Tr as _;
+use tr::Tr2 as _;
+
+mod tr {
+    pub trait Tr {}
+    pub trait Tr2 {}
+}
+    "#,
+        expect![[r#"
+            crate
+            _: t
+            _: t
+            tr: t
+
+            crate::tr
+            Tr: t
+            Tr2: t
+        "#]],
+    );
+}
+
+#[test]
+fn underscore_reexport() {
+    check(
+        r#"
+//- /main.rs
+mod tr {
+    pub trait PubTr {}
+    pub trait PrivTr {}
+}
+mod reex {
+    use crate::tr::PrivTr as _;
+    pub use crate::tr::PubTr as _;
+}
+use crate::reex::*;
+    "#,
+        expect![[r#"
+            crate
+            _: t
+            reex: t
+            tr: t
+
+            crate::tr
+            PrivTr: t
+            PubTr: t
+
+            crate::reex
+            _: t
+            _: t
+        "#]],
+    );
+}
+
+#[test]
+fn underscore_pub_crate_reexport() {
+    mark::check!(upgrade_underscore_visibility);
+    check(
+        r#"
+//- /main.rs crate:main deps:lib
+use lib::*;
+
+//- /lib.rs crate:lib
+use tr::Tr as _;
+pub use tr::Tr as _;
+
+mod tr {
+    pub trait Tr {}
+}
+    "#,
+        expect![[r#"
+            crate
+            _: t
+        "#]],
+    );
+}
+
+#[test]
+fn underscore_nontrait() {
+    check(
+        r#"
+//- /main.rs
+mod m {
+    pub struct Struct;
+    pub enum Enum {}
+    pub const CONST: () = ();
+}
+use crate::m::{Struct as _, Enum as _, CONST as _};
+    "#,
+        expect![[r#"
+            crate
+            m: t
+
+            crate::m
+            CONST: v
+            Enum: t
+            Struct: t v
+        "#]],
+    );
+}
+
+#[test]
+fn underscore_name_conflict() {
+    check(
+        r#"
+//- /main.rs
+struct Tr;
+
+use tr::Tr as _;
+
+mod tr {
+    pub trait Tr {}
+}
+    "#,
+        expect![[r#"
+            crate
+            _: t
+            Tr: t v
+            tr: t
+
+            crate::tr
+            Tr: t
+        "#]],
+    );
+}

crates/ra_hir_def/src/visibility.rs

@@ -5,6 +5,7 @@ use ra_syntax::ast;
 
 use crate::{
     db::DefDatabase,
+    nameres::CrateDefMap,
     path::{ModPath, PathKind},
     ModuleId,
 };
@@ -115,7 +116,7 @@ impl Visibility {
 
     pub(crate) fn is_visible_from_def_map(
         self,
-        def_map: &crate::nameres::CrateDefMap,
+        def_map: &CrateDefMap,
         from_module: crate::LocalModuleId,
     ) -> bool {
         let to_module = match self {
@@ -129,4 +130,42 @@ impl Visibility {
         });
         ancestors.any(|m| m == to_module.local_id)
     }
+
+    /// Returns the most permissive visibility of `self` and `other`.
+    ///
+    /// If there is no subset relation between `self` and `other`, returns `None` (ie. they're only
+    /// visible in unrelated modules).
+    pub(crate) fn max(self, other: Visibility, def_map: &CrateDefMap) -> Option<Visibility> {
+        match (self, other) {
+            (Visibility::Module(_), Visibility::Public)
+            | (Visibility::Public, Visibility::Module(_))
+            | (Visibility::Public, Visibility::Public) => Some(Visibility::Public),
+            (Visibility::Module(mod_a), Visibility::Module(mod_b)) => {
+                if mod_a.krate != mod_b.krate {
+                    return None;
+                }
+
+                let mut a_ancestors = std::iter::successors(Some(mod_a.local_id), |m| {
+                    let parent_id = def_map[*m].parent?;
+                    Some(parent_id)
+                });
+                let mut b_ancestors = std::iter::successors(Some(mod_b.local_id), |m| {
+                    let parent_id = def_map[*m].parent?;
+                    Some(parent_id)
+                });
+
+                if a_ancestors.any(|m| m == mod_b.local_id) {
+                    // B is above A
+                    return Some(Visibility::Module(mod_b));
+                }
+
+                if b_ancestors.any(|m| m == mod_a.local_id) {
+                    // A is above B
+                    return Some(Visibility::Module(mod_a));
+                }
+
+                None
+            }
+        }
+    }
 }

crates/ra_hir_ty/src/tests/traits.rs

@@ -3089,3 +3089,25 @@ fn test() {
         "#,
     );
 }
+
+#[test]
+fn underscore_import() {
+    check_types(
+        r#"
+mod tr {
+    pub trait Tr {
+        fn method(&self) -> u8 { 0 }
+    }
+}
+
+struct Tr;
+impl crate::tr::Tr for Tr {}
+
+use crate::tr::Tr as _;
+fn test() {
+    Tr.method();
+  //^^^^^^^^^^^ u8
+}
+    "#,
+    );
+}