Merge #9499

9499: fix: Fix cycle in visibility computation with modules from the same block r=jonas-schievink a=jonas-schievink

fixes #9481

bors r+

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

Author: bors[bot]

crates/hir_def/src/body/tests/block.rs

@@ -344,3 +344,40 @@ fn foo() {
         "#]],
     )
 }
+
+#[test]
+fn is_visible_from_same_def_map() {
+    // Regression test for https://github.com/rust-analyzer/rust-analyzer/issues/9481
+    check_at(
+        r#"
+fn outer() {
+    mod command {
+        use crate::name;
+    }
+
+    mod tests {
+        use super::*;
+    }
+    $0
+}
+        "#,
+        expect![[r#"
+            block scope
+            command: t
+            name: _
+            tests: t
+
+            block scope::command
+            name: _
+
+            block scope::tests
+            name: _
+            outer: v
+
+            crate
+            outer: v
+        "#]],
+    );
+    // FIXME: `name` should not be visible in the block scope. This happens because ItemTrees store
+    // inner items incorrectly.
+}

crates/hir_def/src/lib.rs

@@ -115,18 +115,6 @@ impl ModuleId {
     pub fn containing_block(&self) -> Option<BlockId> {
         self.block
     }
-
-    /// Returns `true` if this module represents a block expression.
-    ///
-    /// Returns `false` if this module is a submodule *inside* a block expression
-    /// (eg. `m` in `{ mod m {} }`).
-    pub fn is_block_root(&self, db: &dyn db::DefDatabase) -> bool {
-        if self.block.is_none() {
-            return false;
-        }
-
-        self.def_map(db)[self.local_id].parent.is_none()
-    }
 }
 
 /// An ID of a module, **local** to a specific crate

crates/hir_def/src/visibility.rs

@@ -134,8 +134,22 @@ impl Visibility {
         // visibility as the containing module (even though no items are directly nameable from
         // there, getting this right is important for method resolution).
         // In that case, we adjust the visibility of `to_module` to point to the containing module.
-        if to_module.is_block_root(db) {
-            to_module = to_module.containing_module(db).unwrap();
+        // Additional complication: `to_module` might be in `from_module`'s `DefMap`, which we're
+        // currently computing, so we must not call the `def_map` query for it.
+        let arc;
+        let to_module_def_map =
+            if to_module.krate == def_map.krate() && to_module.block == def_map.block_id() {
+                def_map
+            } else {
+                arc = to_module.def_map(db);
+                &arc
+            };
+        let is_block_root = match to_module.block {
+            Some(_) => to_module_def_map[to_module.local_id].parent.is_none(),
+            None => false,
+        };
+        if is_block_root {
+            to_module = to_module_def_map.containing_module(to_module.local_id).unwrap();
         }
 
         // from_module needs to be a descendant of to_module