Merge #7431

7431: Handle `super` paths inside blocks correctly r=jonas-schievink a=jonas-schievink

We now intern `BlockLoc` and use `BlockId` to refer to block expressions. This is needed to keep `ModuleId` simple, since it would otherwise have to store an arbitrarily long chain of blocks and couldn't be `Copy`.

The `DefMap` hierarchy is now created as the caller descends into an item body. This is necessary to link the correct module as the block's parent, which is important for correct name resolution.

As a result, we can now resolve `super` paths inside block expressions by climbing the `DefMap` chain.

bors r+

Co-authored-by: Jonas Schievink <[email protected]>

3 files changed, 109 insertions, 20 deletions

diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs
index c1eded5f2..419e465ed 100644
--- a/crates/hir_def/src/nameres/path_resolution.rs
+++ b/crates/hir_def/src/nameres/path_resolution.rs
@@ -10,8 +10,6 @@
 //!
 //! `ReachedFixedPoint` signals about this.
 
-use std::iter::successors;
-
 use base_db::Edition;
 use hir_expand::name;
 use hir_expand::name::Name;
@@ -131,8 +129,8 @@ impl DefMap {
             result.krate = result.krate.or(new.krate);
             result.segment_index = result.segment_index.min(new.segment_index);
 
-            match &current_map.parent {
-                Some(map) => current_map = map,
+            match &current_map.block {
+                Some(block) => current_map = &block.parent,
                 None => return result,
             }
         }
@@ -193,14 +191,35 @@ impl DefMap {
                 self.resolve_name_in_module(db, original_module, &segment, prefer_module)
             }
             PathKind::Super(lvl) => {
-                let m = successors(Some(original_module), |m| self.modules[*m].parent)
-                    .nth(lvl as usize);
-                if let Some(local_id) = m {
-                    PerNs::types(self.module_id(local_id).into(), Visibility::Public)
-                } else {
-                    log::debug!("super path in root module");
-                    return ResolvePathResult::empty(ReachedFixedPoint::Yes);
+                let mut module = original_module;
+                for i in 0..lvl {
+                    match self.modules[module].parent {
+                        Some(it) => module = it,
+                        None => match &self.block {
+                            Some(block) => {
+                                // Look up remaining path in parent `DefMap`
+                                let new_path = ModPath {
+                                    kind: PathKind::Super(lvl - i),
+                                    segments: path.segments.clone(),
+                                };
+                                log::debug!("`super` path: {} -> {} in parent map", path, new_path);
+                                return block.parent.resolve_path_fp_with_macro(
+                                    db,
+                                    mode,
+                                    block.parent_module,
+                                    &new_path,
+                                    shadow,
+                                );
+                            }
+                            None => {
+                                log::debug!("super path in root module");
+                                return ResolvePathResult::empty(ReachedFixedPoint::Yes);
+                            }
+                        },
+                    }
                 }
+
+                PerNs::types(self.module_id(module).into(), Visibility::Public)
             }
             PathKind::Abs => {
                 // 2018-style absolute path -- only extern prelude
diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs
index 73e3a4702..b36d0b59b 100644
--- a/crates/hir_def/src/nameres/tests.rs
+++ b/crates/hir_def/src/nameres/tests.rs
@@ -8,12 +8,12 @@ mod block;
 
 use std::sync::Arc;
 
-use base_db::{fixture::WithFixture, SourceDatabase};
+use base_db::{fixture::WithFixture, FilePosition, SourceDatabase};
 use expect_test::{expect, Expect};
-use hir_expand::db::AstDatabase;
+use syntax::AstNode;
 use test_utils::mark;
 
-use crate::{db::DefDatabase, nameres::*, test_db::TestDB};
+use crate::{db::DefDatabase, nameres::*, test_db::TestDB, Lookup};
 
 fn compute_crate_def_map(ra_fixture: &str) -> Arc<DefMap> {
     let db = TestDB::with_files(ra_fixture);
@@ -23,14 +23,58 @@ fn compute_crate_def_map(ra_fixture: &str) -> Arc<DefMap> {
 
 fn compute_block_def_map(ra_fixture: &str) -> Arc<DefMap> {
     let (db, position) = TestDB::with_position(ra_fixture);
+
+    // FIXME: perhaps we should make this use body lowering tests instead?
+
     let module = db.module_for_file(position.file_id);
-    let ast_map = db.ast_id_map(position.file_id.into());
-    let ast = db.parse(position.file_id);
-    let block: ast::BlockExpr =
-        syntax::algo::find_node_at_offset(&ast.syntax_node(), position.offset).unwrap();
-    let block_id = ast_map.ast_id(&block);
+    let mut def_map = db.crate_def_map(module.krate);
+    while let Some(new_def_map) = descend_def_map_at_position(&db, position, def_map.clone()) {
+        def_map = new_def_map;
+    }
+
+    // FIXME: select the right module, not the root
+
+    def_map
+}
+
+fn descend_def_map_at_position(
+    db: &dyn DefDatabase,
+    position: FilePosition,
+    def_map: Arc<DefMap>,
+) -> Option<Arc<DefMap>> {
+    for (local_id, module_data) in def_map.modules() {
+        let mod_def = module_data.origin.definition_source(db);
+        let ast_map = db.ast_id_map(mod_def.file_id);
+        let item_tree = db.item_tree(mod_def.file_id);
+        let root = db.parse_or_expand(mod_def.file_id).unwrap();
+        for item in module_data.scope.declarations() {
+            match item {
+                ModuleDefId::FunctionId(it) => {
+                    // Technically blocks can be inside any type (due to arrays and const generics),
+                    // and also in const/static initializers. For tests we only really care about
+                    // functions though.
+
+                    let ast = ast_map.get(item_tree[it.lookup(db).id.value].ast_id).to_node(&root);
+
+                    if ast.syntax().text_range().contains(position.offset) {
+                        // Cursor inside function, descend into its body's DefMap.
+                        // Note that we don't handle block *expressions* inside function bodies.
+                        let ast_map = db.ast_id_map(position.file_id.into());
+                        let ast_id = ast_map.ast_id(&ast.body().unwrap());
+                        let block = BlockLoc {
+                            ast_id: InFile::new(position.file_id.into(), ast_id),
+                            module: def_map.module_id(local_id),
+                        };
+                        let block_id = db.intern_block(block);
+                        return Some(db.block_def_map(block_id));
+                    }
+                }
+                _ => continue,
+            }
+        }
+    }
 
-    db.block_def_map(module.krate, InFile::new(position.file_id.into(), block_id))
+    None
 }
 
 fn check(ra_fixture: &str, expect: Expect) {
diff --git a/crates/hir_def/src/nameres/tests/block.rs b/crates/hir_def/src/nameres/tests/block.rs
index 01d6326a7..470ca593e 100644
--- a/crates/hir_def/src/nameres/tests/block.rs
+++ b/crates/hir_def/src/nameres/tests/block.rs
@@ -95,3 +95,29 @@ fn outer() {
         "#]],
     );
 }
+
+#[test]
+fn super_imports() {
+    check_at(
+        r#"
+mod module {
+    fn f() {
+        use super::Struct;
+        $0
+    }
+}
+
+struct Struct {}
+"#,
+        expect![[r#"
+            block scope
+            Struct: t
+            crate
+            Struct: t
+            module: t
+
+            crate::module
+            f: v
+        "#]],
+    );
+}