11 files changed, 139 insertions, 23 deletions

diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index ed07d6928..c0b0b7841 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -568,9 +568,13 @@ impl ExprCollector<'_> {
 
         let res = match res {
             Ok(res) => res,
-            Err(UnresolvedMacro) => {
+            Err(UnresolvedMacro { path }) => {
                 self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedMacroCall(
-                    UnresolvedMacroCall { file: outer_file, node: syntax_ptr.cast().unwrap() },
+                    UnresolvedMacroCall {
+                        file: outer_file,
+                        node: syntax_ptr.cast().unwrap(),
+                        path,
+                    },
                 ));
                 collector(self, None);
                 return;
diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs
index c1d3e998f..63f5fe88d 100644
--- a/crates/hir_def/src/body/tests.rs
+++ b/crates/hir_def/src/body/tests.rs
@@ -180,7 +180,7 @@ fn unresolved_macro_diag() {
         r#"
 fn f() {
     m!();
-  //^^^^ unresolved macro call
+  //^^^^ unresolved macro `m!`
 }
       "#,
     );
diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs
index 97abf8653..a71ae2668 100644
--- a/crates/hir_def/src/diagnostics.rs
+++ b/crates/hir_def/src/diagnostics.rs
@@ -8,7 +8,7 @@ use hir_expand::diagnostics::{Diagnostic, DiagnosticCode, DiagnosticSink};
 use hir_expand::{HirFileId, InFile};
 use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
 
-use crate::{db::DefDatabase, DefWithBodyId};
+use crate::{db::DefDatabase, path::ModPath, DefWithBodyId};
 
 pub fn validate_body(db: &dyn DefDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) {
     let source_map = db.body_with_source_map(owner).1;
@@ -103,6 +103,7 @@ impl Diagnostic for UnresolvedImport {
 pub struct UnresolvedMacroCall {
     pub file: HirFileId,
     pub node: AstPtr<ast::MacroCall>,
+    pub path: ModPath,
 }
 
 impl Diagnostic for UnresolvedMacroCall {
@@ -110,7 +111,7 @@ impl Diagnostic for UnresolvedMacroCall {
         DiagnosticCode("unresolved-macro-call")
     }
     fn message(&self) -> String {
-        "unresolved macro call".to_string()
+        format!("unresolved macro `{}!`", self.path)
     }
     fn display_source(&self) -> InFile<SyntaxNodePtr> {
         InFile::new(self.file, self.node.clone().into())
diff --git a/crates/hir_def/src/find_path.rs b/crates/hir_def/src/find_path.rs
index 41da3bc2d..2c4bbe585 100644
--- a/crates/hir_def/src/find_path.rs
+++ b/crates/hir_def/src/find_path.rs
@@ -955,6 +955,29 @@ fn main() {
     }
 
     #[test]
+    fn from_inside_module() {
+        // This worked correctly, but the test suite logic was broken.
+        cov_mark::check!(submodule_in_testdb);
+        check_found_path(
+            r#"
+mod baz {
+    pub struct Foo {}
+}
+
+mod bar {
+    fn bar() {
+        $0
+    }
+}
+            "#,
+            "crate::baz::Foo",
+            "crate::baz::Foo",
+            "crate::baz::Foo",
+            "crate::baz::Foo",
+        )
+    }
+
+    #[test]
     fn recursive_pub_mod_reexport() {
         cov_mark::check!(recursive_imports);
         check_found_path(
diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
index ffee05500..5ac1670b5 100644
--- a/crates/hir_def/src/lib.rs
+++ b/crates/hir_def/src/lib.rs
@@ -66,6 +66,7 @@ use hir_expand::{
 };
 use la_arena::Idx;
 use nameres::DefMap;
+use path::ModPath;
 use syntax::ast;
 
 use crate::builtin_type::BuiltinType;
@@ -107,6 +108,18 @@ impl ModuleId {
     pub fn containing_module(&self, db: &dyn db::DefDatabase) -> Option<ModuleId> {
         self.def_map(db).containing_module(self.local_id)
     }
+
+    /// 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
@@ -675,7 +688,9 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
     }
 }
 
-pub struct UnresolvedMacro;
+pub struct UnresolvedMacro {
+    pub path: ModPath,
+}
 
 fn macro_call_as_call_id(
     call: &AstIdWithPath<ast::MacroCall>,
@@ -684,7 +699,8 @@ fn macro_call_as_call_id(
     resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
     error_sink: &mut dyn FnMut(mbe::ExpandError),
 ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
-    let def: MacroDefId = resolver(call.path.clone()).ok_or(UnresolvedMacro)?;
+    let def: MacroDefId =
+        resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
 
     let res = if let MacroDefKind::BuiltInEager(..) = def.kind {
         let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db.upcast()));
@@ -714,8 +730,13 @@ fn derive_macro_as_call_id(
     krate: CrateId,
     resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
 ) -> Result<MacroCallId, UnresolvedMacro> {
-    let def: MacroDefId = resolver(item_attr.path.clone()).ok_or(UnresolvedMacro)?;
-    let last_segment = item_attr.path.segments().last().ok_or(UnresolvedMacro)?;
+    let def: MacroDefId = resolver(item_attr.path.clone())
+        .ok_or_else(|| UnresolvedMacro { path: item_attr.path.clone() })?;
+    let last_segment = item_attr
+        .path
+        .segments()
+        .last()
+        .ok_or_else(|| UnresolvedMacro { path: item_attr.path.clone() })?;
     let res = def
         .as_lazy_macro(
             db.upcast(),
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index 9e181751c..542f190a1 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -481,7 +481,7 @@ mod diagnostics {
 
         UnresolvedProcMacro { ast: MacroCallKind },
 
-        UnresolvedMacroCall { ast: AstId<ast::MacroCall> },
+        UnresolvedMacroCall { ast: AstId<ast::MacroCall>, path: ModPath },
 
         MacroError { ast: MacroCallKind, message: String },
     }
@@ -546,8 +546,9 @@ mod diagnostics {
         pub(super) fn unresolved_macro_call(
             container: LocalModuleId,
             ast: AstId<ast::MacroCall>,
+            path: ModPath,
         ) -> Self {
-            Self { in_module: container, kind: DiagnosticKind::UnresolvedMacroCall { ast } }
+            Self { in_module: container, kind: DiagnosticKind::UnresolvedMacroCall { ast, path } }
         }
 
         pub(super) fn add_to(
@@ -662,9 +663,13 @@ mod diagnostics {
                     });
                 }
 
-                DiagnosticKind::UnresolvedMacroCall { ast } => {
+                DiagnosticKind::UnresolvedMacroCall { ast, path } => {
                     let node = ast.to_node(db.upcast());
-                    sink.push(UnresolvedMacroCall { file: ast.file_id, node: AstPtr::new(&node) });
+                    sink.push(UnresolvedMacroCall {
+                        file: ast.file_id,
+                        node: AstPtr::new(&node),
+                        path: path.clone(),
+                    });
                 }
 
                 DiagnosticKind::MacroError { ast, message } => {
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index fb4ddff5e..05ceb1efb 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -829,7 +829,7 @@ impl DefCollector<'_> {
                             res = ReachedFixedPoint::No;
                             return false;
                         }
-                        Err(UnresolvedMacro) | Ok(Err(_)) => {}
+                        Err(UnresolvedMacro { .. }) | Ok(Err(_)) => {}
                     }
                 }
                 MacroDirectiveKind::Derive { ast_id, derive_attr } => {
@@ -845,7 +845,7 @@ impl DefCollector<'_> {
                             res = ReachedFixedPoint::No;
                             return false;
                         }
-                        Err(UnresolvedMacro) => (),
+                        Err(UnresolvedMacro { .. }) => (),
                     }
                 }
             }
@@ -943,10 +943,11 @@ impl DefCollector<'_> {
                     &mut |_| (),
                 ) {
                     Ok(_) => (),
-                    Err(UnresolvedMacro) => {
+                    Err(UnresolvedMacro { path }) => {
                         self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
                             directive.module_id,
                             ast_id.ast_id,
+                            path,
                         ));
                     }
                 },
@@ -1530,7 +1531,7 @@ impl ModCollector<'_, '_> {
                 ));
                 return;
             }
-            Err(UnresolvedMacro) => (),
+            Err(UnresolvedMacro { .. }) => (),
         }
 
         // Case 2: resolve in module scope, expand during name resolution.
diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs
index ccc9f22eb..c984148c3 100644
--- a/crates/hir_def/src/nameres/path_resolution.rs
+++ b/crates/hir_def/src/nameres/path_resolution.rs
@@ -387,7 +387,13 @@ impl DefMap {
             .get_legacy_macro(name)
             .map_or_else(PerNs::none, |m| PerNs::macros(m, Visibility::Public));
         let from_scope = self[module].scope.get(name);
-        let from_builtin = BUILTIN_SCOPE.get(name).copied().unwrap_or_else(PerNs::none);
+        let from_builtin = match self.block {
+            Some(_) => {
+                // Only resolve to builtins in the root `DefMap`.
+                PerNs::none()
+            }
+            None => BUILTIN_SCOPE.get(name).copied().unwrap_or_else(PerNs::none),
+        };
         let from_scope_or_builtin = match shadow {
             BuiltinShadowMode::Module => from_scope.or(from_builtin),
             BuiltinShadowMode::Other => {
diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs
index 1ac88fc89..543975e07 100644
--- a/crates/hir_def/src/nameres/tests/diagnostics.rs
+++ b/crates/hir_def/src/nameres/tests/diagnostics.rs
@@ -170,7 +170,7 @@ fn unresolved_legacy_scope_macro() {
 
           m!();
           m2!();
-        //^^^^^^ unresolved macro call
+        //^^^^^^ unresolved macro `self::m2!`
         "#,
     );
 }
@@ -187,7 +187,7 @@ fn unresolved_module_scope_macro() {
 
           self::m!();
           self::m2!();
-        //^^^^^^^^^^^^ unresolved macro call
+        //^^^^^^^^^^^^ unresolved macro `self::m2!`
         "#,
     );
 }
diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs
index dd36106f8..8fa703a57 100644
--- a/crates/hir_def/src/test_db.rs
+++ b/crates/hir_def/src/test_db.rs
@@ -15,7 +15,12 @@ use rustc_hash::FxHashSet;
 use syntax::{algo, ast, AstNode, TextRange, TextSize};
 use test_utils::extract_annotations;
 
-use crate::{db::DefDatabase, nameres::DefMap, src::HasSource, Lookup, ModuleDefId, ModuleId};
+use crate::{
+    db::DefDatabase,
+    nameres::{DefMap, ModuleSource},
+    src::HasSource,
+    LocalModuleId, Lookup, ModuleDefId, ModuleId,
+};
 
 #[salsa::database(
     base_db::SourceDatabaseExtStorage,
@@ -87,10 +92,11 @@ impl TestDB {
     pub(crate) fn module_at_position(&self, position: FilePosition) -> ModuleId {
         let file_module = self.module_for_file(position.file_id);
         let mut def_map = file_module.def_map(self);
+        let module = self.mod_at_position(&def_map, position);
 
         def_map = match self.block_at_position(&def_map, position) {
             Some(it) => it,
-            None => return file_module,
+            None => return def_map.module_id(module),
         };
         loop {
             let new_map = self.block_at_position(&def_map, position);
@@ -106,6 +112,47 @@ impl TestDB {
         }
     }
 
+    /// Finds the smallest/innermost module in `def_map` containing `position`.
+    fn mod_at_position(&self, def_map: &DefMap, position: FilePosition) -> LocalModuleId {
+        let mut size = None;
+        let mut res = def_map.root();
+        for (module, data) in def_map.modules() {
+            let src = data.definition_source(self);
+            if src.file_id != position.file_id.into() {
+                continue;
+            }
+
+            let range = match src.value {
+                ModuleSource::SourceFile(it) => it.syntax().text_range(),
+                ModuleSource::Module(it) => it.syntax().text_range(),
+                ModuleSource::BlockExpr(it) => it.syntax().text_range(),
+            };
+
+            if !range.contains(position.offset) {
+                continue;
+            }
+
+            let new_size = match size {
+                None => range.len(),
+                Some(size) => {
+                    if range.len() < size {
+                        range.len()
+                    } else {
+                        size
+                    }
+                }
+            };
+
+            if size != Some(new_size) {
+                cov_mark::hit!(submodule_in_testdb);
+                size = Some(new_size);
+                res = module;
+            }
+        }
+
+        res
+    }
+
     fn block_at_position(&self, def_map: &DefMap, position: FilePosition) -> Option<Arc<DefMap>> {
         // Find the smallest (innermost) function in `def_map` containing the cursor.
         let mut size = None;
diff --git a/crates/hir_def/src/visibility.rs b/crates/hir_def/src/visibility.rs
index 9908cd926..d4b7c9970 100644
--- a/crates/hir_def/src/visibility.rs
+++ b/crates/hir_def/src/visibility.rs
@@ -123,11 +123,19 @@ impl Visibility {
         def_map: &DefMap,
         mut from_module: crate::LocalModuleId,
     ) -> bool {
-        let to_module = match self {
+        let mut to_module = match self {
             Visibility::Module(m) => m,
             Visibility::Public => return true,
         };
 
+        // `to_module` might be the root module of a block expression. Those have the same
+        // 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();
+        }
+
         // from_module needs to be a descendant of to_module
         let mut def_map = def_map;
         let mut parent_arc;