Merge #2455

2455: Add BuiltinShadowMode r=flodiebold a=edwin0cheng

This PR try to fix #1905 by introduce an `BuiltinShadowMode` in name resolving functions. 

cc @flodiebold 

Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>

Author: bors[bot]

crates/ra_hir/src/source_binder.rs

@@ -286,7 +286,7 @@ impl SourceAnalyzer {
 
         let items = self
             .resolver
-            .resolve_module_path(db, &path)
+            .resolve_module_path_in_items(db, &path)
             .take_types()
             .map(|it| PathResolution::Def(it.into()));
         types.or(values).or(items).or_else(|| {

crates/ra_hir_def/src/body.rs

@@ -15,7 +15,7 @@ use rustc_hash::FxHashMap;
 use crate::{
     db::DefDatabase,
     expr::{Expr, ExprId, Pat, PatId},
-    nameres::CrateDefMap,
+    nameres::{BuiltinShadowMode, CrateDefMap},
     path::Path,
     src::HasSource,
     DefWithBodyId, HasModule, Lookup, ModuleId,
@@ -83,7 +83,10 @@ impl Expander {
     }
 
     fn resolve_path_as_macro(&self, db: &impl DefDatabase, path: &Path) -> Option<MacroDefId> {
-        self.crate_def_map.resolve_path(db, self.module.local_id, path).0.take_macros()
+        self.crate_def_map
+            .resolve_path(db, self.module.local_id, path, BuiltinShadowMode::Other)
+            .0
+            .take_macros()
     }
 }
 

crates/ra_hir_def/src/nameres.rs

@@ -149,6 +149,15 @@ static BUILTIN_SCOPE: Lazy<FxHashMap<Name, Resolution>> = Lazy::new(|| {
         .collect()
 });
 
+/// Shadow mode for builtin type which can be shadowed by module.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+pub enum BuiltinShadowMode {
+    // Prefer Module
+    Module,
+    // Prefer Other Types
+    Other,
+}
+
 /// Legacy macros can only be accessed through special methods like `get_legacy_macros`.
 /// Other methods will only resolve values, types and module scoped macros only.
 impl ModuleScope {
@@ -178,8 +187,20 @@ impl ModuleScope {
     }
 
     /// Get a name from current module scope, legacy macros are not included
-    pub fn get(&self, name: &Name) -> Option<&Resolution> {
-        self.items.get(name).or_else(|| BUILTIN_SCOPE.get(name))
+    pub fn get(&self, name: &Name, shadow: BuiltinShadowMode) -> Option<&Resolution> {
+        match shadow {
+            BuiltinShadowMode::Module => self.items.get(name).or_else(|| BUILTIN_SCOPE.get(name)),
+            BuiltinShadowMode::Other => {
+                let item = self.items.get(name);
+                if let Some(res) = item {
+                    if let Some(ModuleDefId::ModuleId(_)) = res.def.take_types() {
+                        return BUILTIN_SCOPE.get(name).or(item);
+                    }
+                }
+
+                item.or_else(|| BUILTIN_SCOPE.get(name))
+            }
+        }
     }
 
     pub fn traits<'a>(&'a self) -> impl Iterator<Item = TraitId> + 'a {
@@ -250,8 +271,10 @@ impl CrateDefMap {
         db: &impl DefDatabase,
         original_module: LocalModuleId,
         path: &Path,
+        shadow: BuiltinShadowMode,
     ) -> (PerNs, Option<usize>) {
-        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
+        let res =
+            self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path, shadow);
         (res.resolved_def, res.segment_index)
     }
 }

crates/ra_hir_def/src/nameres/collector.rs

@@ -19,7 +19,7 @@ use crate::{
     db::DefDatabase,
     nameres::{
         diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint,
-        raw, CrateDefMap, ModuleData, Resolution, ResolveMode,
+        raw, BuiltinShadowMode, CrateDefMap, ModuleData, Resolution, ResolveMode,
     },
     path::{Path, PathKind},
     per_ns::PerNs,
@@ -299,6 +299,7 @@ where
                 ResolveMode::Import,
                 module_id,
                 &import.path,
+                BuiltinShadowMode::Module,
             );
 
             (res.resolved_def, res.reached_fixedpoint)
@@ -477,6 +478,7 @@ where
                 ResolveMode::Other,
                 *module_id,
                 path,
+                BuiltinShadowMode::Module,
             );
 
             if let Some(def) = resolved_res.resolved_def.take_macros() {

crates/ra_hir_def/src/nameres/path_resolution.rs

@@ -16,7 +16,7 @@ use test_utils::tested_by;
 
 use crate::{
     db::DefDatabase,
-    nameres::CrateDefMap,
+    nameres::{BuiltinShadowMode, CrateDefMap},
     path::{Path, PathKind},
     per_ns::PerNs,
     AdtId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId,
@@ -68,7 +68,17 @@ impl CrateDefMap {
         mode: ResolveMode,
         original_module: LocalModuleId,
         path: &Path,
+        shadow: BuiltinShadowMode,
     ) -> ResolvePathResult {
+        // if it is not the last segment, we prefer the module to the builtin
+        let prefer_module = |index| {
+            if index == path.segments.len() - 1 {
+                shadow
+            } else {
+                BuiltinShadowMode::Module
+            }
+        };
+
         let mut segments = path.segments.iter().enumerate();
         let mut curr_per_ns: PerNs = match path.kind {
             PathKind::DollarCrate(krate) => {
@@ -96,20 +106,20 @@ impl CrateDefMap {
                 if self.edition == Edition::Edition2015
                     && (path.kind == PathKind::Abs || mode == ResolveMode::Import) =>
             {
-                let segment = match segments.next() {
-                    Some((_, segment)) => segment,
+                let (idx, segment) = match segments.next() {
+                    Some((idx, segment)) => (idx, segment),
                     None => return ResolvePathResult::empty(ReachedFixedPoint::Yes),
                 };
                 log::debug!("resolving {:?} in crate root (+ extern prelude)", segment);
-                self.resolve_name_in_crate_root_or_extern_prelude(&segment.name)
+                self.resolve_name_in_crate_root_or_extern_prelude(&segment.name, prefer_module(idx))
             }
             PathKind::Plain => {
-                let segment = match segments.next() {
-                    Some((_, segment)) => segment,
+                let (idx, segment) = match segments.next() {
+                    Some((idx, segment)) => (idx, segment),
                     None => return ResolvePathResult::empty(ReachedFixedPoint::Yes),
                 };
                 log::debug!("resolving {:?} in module", segment);
-                self.resolve_name_in_module(db, original_module, &segment.name)
+                self.resolve_name_in_module(db, original_module, &segment.name, prefer_module(idx))
             }
             PathKind::Super => {
                 if let Some(p) = self.modules[original_module].parent {
@@ -160,7 +170,7 @@ impl CrateDefMap {
                             Path { segments: path.segments[i..].to_vec(), kind: PathKind::Self_ };
                         log::debug!("resolving {:?} in other crate", path);
                         let defp_map = db.crate_def_map(module.krate);
-                        let (def, s) = defp_map.resolve_path(db, module.local_id, &path);
+                        let (def, s) = defp_map.resolve_path(db, module.local_id, &path, shadow);
                         return ResolvePathResult::with(
                             def,
                             ReachedFixedPoint::Yes,
@@ -169,7 +179,7 @@ impl CrateDefMap {
                     }
 
                     // Since it is a qualified path here, it should not contains legacy macros
-                    match self[module.local_id].scope.get(&segment.name) {
+                    match self[module.local_id].scope.get(&segment.name, prefer_module(i)) {
                         Some(res) => res.def,
                         _ => {
                             log::debug!("path segment {:?} not found", segment.name);
@@ -212,6 +222,7 @@ impl CrateDefMap {
                 }
             };
         }
+
         ResolvePathResult::with(curr_per_ns, ReachedFixedPoint::Yes, None)
     }
 
@@ -220,6 +231,7 @@ impl CrateDefMap {
         db: &impl DefDatabase,
         module: LocalModuleId,
         name: &Name,
+        shadow: BuiltinShadowMode,
     ) -> PerNs {
         // Resolve in:
         //  - legacy scope of macro
@@ -228,23 +240,33 @@ impl CrateDefMap {
         //  - std prelude
         let from_legacy_macro =
             self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros);
-        let from_scope = self[module].scope.get(name).map_or_else(PerNs::none, |res| res.def);
+        let from_scope =
+            self[module].scope.get(name, shadow).map_or_else(PerNs::none, |res| res.def);
         let from_extern_prelude =
             self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
-        let from_prelude = self.resolve_in_prelude(db, name);
+        let from_prelude = self.resolve_in_prelude(db, name, shadow);
 
         from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude)
     }
 
-    fn resolve_name_in_crate_root_or_extern_prelude(&self, name: &Name) -> PerNs {
+    fn resolve_name_in_crate_root_or_extern_prelude(
+        &self,
+        name: &Name,
+        shadow: BuiltinShadowMode,
+    ) -> PerNs {
         let from_crate_root =
-            self[self.root].scope.get(name).map_or_else(PerNs::none, |res| res.def);
+            self[self.root].scope.get(name, shadow).map_or_else(PerNs::none, |res| res.def);
         let from_extern_prelude = self.resolve_name_in_extern_prelude(name);
 
         from_crate_root.or(from_extern_prelude)
     }
 
-    fn resolve_in_prelude(&self, db: &impl DefDatabase, name: &Name) -> PerNs {
+    fn resolve_in_prelude(
+        &self,
+        db: &impl DefDatabase,
+        name: &Name,
+        shadow: BuiltinShadowMode,
+    ) -> PerNs {
         if let Some(prelude) = self.prelude {
             let keep;
             let def_map = if prelude.krate == self.krate {
@@ -254,7 +276,10 @@ impl CrateDefMap {
                 keep = db.crate_def_map(prelude.krate);
                 &keep
             };
-            def_map[prelude.local_id].scope.get(name).map_or_else(PerNs::none, |res| res.def)
+            def_map[prelude.local_id]
+                .scope
+                .get(name, shadow)
+                .map_or_else(PerNs::none, |res| res.def)
         } else {
             PerNs::none()
         }

crates/ra_hir_def/src/resolver.rs

@@ -14,7 +14,7 @@ use crate::{
     db::DefDatabase,
     expr::{ExprId, PatId},
     generics::GenericParams,
-    nameres::CrateDefMap,
+    nameres::{BuiltinShadowMode, CrateDefMap},
     path::{Path, PathKind},
     per_ns::PerNs,
     AdtId, AstItemDef, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId, FunctionId,
@@ -91,7 +91,7 @@ pub enum ValueNs {
 impl Resolver {
     /// Resolve known trait from std, like `std::futures::Future`
     pub fn resolve_known_trait(&self, db: &impl DefDatabase, path: &Path) -> Option<TraitId> {
-        let res = self.resolve_module_path(db, path).take_types()?;
+        let res = self.resolve_module_path(db, path, BuiltinShadowMode::Other).take_types()?;
         match res {
             ModuleDefId::TraitId(it) => Some(it),
             _ => None,
@@ -100,7 +100,7 @@ impl Resolver {
 
     /// Resolve known struct from std, like `std::boxed::Box`
     pub fn resolve_known_struct(&self, db: &impl DefDatabase, path: &Path) -> Option<StructId> {
-        let res = self.resolve_module_path(db, path).take_types()?;
+        let res = self.resolve_module_path(db, path, BuiltinShadowMode::Other).take_types()?;
         match res {
             ModuleDefId::AdtId(AdtId::StructId(it)) => Some(it),
             _ => None,
@@ -109,26 +109,34 @@ impl Resolver {
 
     /// Resolve known enum from std, like `std::result::Result`
     pub fn resolve_known_enum(&self, db: &impl DefDatabase, path: &Path) -> Option<EnumId> {
-        let res = self.resolve_module_path(db, path).take_types()?;
+        let res = self.resolve_module_path(db, path, BuiltinShadowMode::Other).take_types()?;
         match res {
             ModuleDefId::AdtId(AdtId::EnumId(it)) => Some(it),
             _ => None,
         }
     }
 
-    /// pub only for source-binder
-    pub fn resolve_module_path(&self, db: &impl DefDatabase, path: &Path) -> PerNs {
+    fn resolve_module_path(
+        &self,
+        db: &impl DefDatabase,
+        path: &Path,
+        shadow: BuiltinShadowMode,
+    ) -> PerNs {
         let (item_map, module) = match self.module() {
             Some(it) => it,
             None => return PerNs::none(),
         };
-        let (module_res, segment_index) = item_map.resolve_path(db, module, path);
+        let (module_res, segment_index) = item_map.resolve_path(db, module, path, shadow);
         if segment_index.is_some() {
             return PerNs::none();
         }
         module_res
     }
 
+    pub fn resolve_module_path_in_items(&self, db: &impl DefDatabase, path: &Path) -> PerNs {
+        self.resolve_module_path(db, path, BuiltinShadowMode::Module)
+    }
+
     pub fn resolve_path_in_type_ns(
         &self,
         db: &impl DefDatabase,
@@ -163,7 +171,12 @@ impl Resolver {
                     }
                 }
                 Scope::ModuleScope(m) => {
-                    let (module_def, idx) = m.crate_def_map.resolve_path(db, m.module_id, path);
+                    let (module_def, idx) = m.crate_def_map.resolve_path(
+                        db,
+                        m.module_id,
+                        path,
+                        BuiltinShadowMode::Other,
+                    );
                     let res = match module_def.take_types()? {
                         ModuleDefId::AdtId(it) => TypeNs::AdtId(it),
                         ModuleDefId::EnumVariantId(it) => TypeNs::EnumVariantId(it),
@@ -256,7 +269,12 @@ impl Resolver {
                 Scope::ImplBlockScope(_) | Scope::AdtScope(_) => continue,
 
                 Scope::ModuleScope(m) => {
-                    let (module_def, idx) = m.crate_def_map.resolve_path(db, m.module_id, path);
+                    let (module_def, idx) = m.crate_def_map.resolve_path(
+                        db,
+                        m.module_id,
+                        path,
+                        BuiltinShadowMode::Other,
+                    );
                     return match idx {
                         None => {
                             let value = match module_def.take_values()? {
@@ -310,7 +328,7 @@ impl Resolver {
 
     pub fn resolve_path_as_macro(&self, db: &impl DefDatabase, path: &Path) -> Option<MacroDefId> {
         let (item_map, module) = self.module()?;
-        item_map.resolve_path(db, module, path).0.take_macros()
+        item_map.resolve_path(db, module, path, BuiltinShadowMode::Other).0.take_macros()
     }
 
     pub fn process_all_names(&self, db: &impl DefDatabase, f: &mut dyn FnMut(Name, ScopeDef)) {

crates/ra_hir_ty/src/tests.rs

@@ -3693,6 +3693,42 @@ fn main() {
     assert_eq!(t, "Foo");
 }
 
+#[test]
+fn not_shadowing_primitive_by_module() {
+    let t = type_at(
+        r#"
+//- /str.rs
+fn foo() {}
+
+//- /main.rs
+mod str;
+fn foo() -> &'static str { "" }
+
+fn main() {
+    foo()<|>;
+}"#,
+    );
+    assert_eq!(t, "&str");
+}
+
+#[test]
+fn not_shadowing_module_by_primitive() {
+    let t = type_at(
+        r#"
+//- /str.rs
+fn foo() -> u32 {0}
+
+//- /main.rs
+mod str;
+fn foo() -> &'static str { "" }
+
+fn main() {
+    str::foo()<|>;
+}"#,
+    );
+    assert_eq!(t, "u32");
+}
+
 #[test]
 fn deref_trait() {
     let t = type_at(