Merge #3685

3685: Auto import macros r=SomeoneToIgnore a=SomeoneToIgnore

If I got it right, assists test infra does not support multiple crates snippets (2720e2374b/crates/ra_hir_def/src/nameres/tests.rs (L491)) hence no tests added for the macro import.

Co-authored-by: Kirill Bulatov <mail4score@gmail.com>

Cargo.lock

@@ -884,6 +884,7 @@ version = "0.1.0"
 name = "ra_assists"
 version = "0.1.0"
 dependencies = [
+ "either",
  "format-buf",
  "itertools 0.9.0",
  "join_to_string",
@@ -1045,6 +1046,7 @@ dependencies = [
 name = "ra_ide_db"
 version = "0.1.0"
 dependencies = [
+ "either",
  "fst",
  "log",
  "once_cell",

crates/ra_assists/Cargo.toml

@@ -12,6 +12,7 @@ format-buf = "1.0.0"
 join_to_string = "0.1.3"
 rustc-hash = "1.1.0"
 itertools = "0.9.0"
+either = "1.5.3"
 
 ra_syntax = { path = "../ra_syntax" }
 ra_text_edit = { path = "../ra_text_edit" }

crates/ra_assists/src/handlers/auto_import.rs

@@ -17,6 +17,7 @@ use crate::{
     utils::insert_use_statement,
     AssistId,
 };
+use either::Either;
 
 // Assist: auto_import
 //
@@ -58,6 +59,7 @@ pub(crate) fn auto_import(ctx: AssistCtx) -> Option<Assist> {
     group.finish()
 }
 
+#[derive(Debug)]
 struct AutoImportAssets {
     import_candidate: ImportCandidate,
     module_with_name_to_import: Module,
@@ -127,14 +129,14 @@ impl AutoImportAssets {
         ImportsLocator::new(db)
             .find_imports(&self.get_search_query())
             .into_iter()
-            .filter_map(|module_def| match &self.import_candidate {
+            .filter_map(|candidate| match &self.import_candidate {
                 ImportCandidate::TraitAssocItem(assoc_item_type, _) => {
-                    let located_assoc_item = match module_def {
-                        ModuleDef::Function(located_function) => located_function
+                    let located_assoc_item = match candidate {
+                        Either::Left(ModuleDef::Function(located_function)) => located_function
                             .as_assoc_item(db)
                             .map(|assoc| assoc.container(db))
                             .and_then(Self::assoc_to_trait),
-                        ModuleDef::Const(located_const) => located_const
+                        Either::Left(ModuleDef::Const(located_const)) => located_const
                             .as_assoc_item(db)
                             .map(|assoc| assoc.container(db))
                             .and_then(Self::assoc_to_trait),
@@ -153,10 +155,11 @@ impl AutoImportAssets {
                             |_, assoc| Self::assoc_to_trait(assoc.container(db)),
                         )
                         .map(ModuleDef::from)
+                        .map(Either::Left)
                 }
                 ImportCandidate::TraitMethod(function_callee, _) => {
                     let located_assoc_item =
-                        if let ModuleDef::Function(located_function) = module_def {
+                        if let Either::Left(ModuleDef::Function(located_function)) = candidate {
                             located_function
                                 .as_assoc_item(db)
                                 .map(|assoc| assoc.container(db))
@@ -179,10 +182,18 @@ impl AutoImportAssets {
                             },
                         )
                         .map(ModuleDef::from)
+                        .map(Either::Left)
+                }
+                _ => Some(candidate),
+            })
+            .filter_map(|candidate| match candidate {
+                Either::Left(module_def) => {
+                    self.module_with_name_to_import.find_use_path(db, module_def)
+                }
+                Either::Right(macro_def) => {
+                    self.module_with_name_to_import.find_use_path(db, macro_def)
                 }
-                _ => Some(module_def),
             })
-            .filter_map(|module_def| self.module_with_name_to_import.find_use_path(db, module_def))
             .filter(|use_path| !use_path.segments.is_empty())
             .take(20)
             .collect::<BTreeSet<_>>()
@@ -439,6 +450,30 @@ mod tests {
         );
     }
 
+    #[test]
+    fn macro_import() {
+        check_assist(
+            auto_import,
+            r"
+                    //- /lib.rs crate:crate_with_macro
+                    #[macro_export]
+                    macro_rules! foo {
+                        () => ()
+                    }
+
+                    //- /main.rs crate:main deps:crate_with_macro
+                    fn main() {
+                        foo<|>
+                    }",
+            r"use crate_with_macro::foo;
+
+fn main() {
+    foo<|>
+}
+",
+        );
+    }
+
     #[test]
     fn auto_import_target() {
         check_assist_target(

crates/ra_assists/src/handlers/fill_match_arms.rs

@@ -2,7 +2,7 @@
 
 use std::iter;
 
-use hir::{Adt, HasSource, Semantics};
+use hir::{Adt, HasSource, ModuleDef, Semantics};
 use itertools::Itertools;
 use ra_ide_db::RootDatabase;
 
@@ -154,7 +154,7 @@ fn resolve_tuple_of_enum_def(
 }
 
 fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::EnumVariant) -> Option<ast::Pat> {
-    let path = crate::ast_transform::path_to_ast(module.find_use_path(db, var.into())?);
+    let path = crate::ast_transform::path_to_ast(module.find_use_path(db, ModuleDef::from(var))?);
 
     // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though
     let pat: ast::Pat = match var.source(db).value.kind() {

crates/ra_assists/src/lib.rs

@@ -165,7 +165,6 @@ mod helpers {
 
     use ra_db::{fixture::WithFixture, FileId, FileRange, SourceDatabaseExt};
     use ra_ide_db::{symbol_index::SymbolsDatabase, RootDatabase};
-    use ra_syntax::TextRange;
     use test_utils::{add_cursor, assert_eq_text, extract_range_or_offset, RangeOrOffset};
 
     use crate::{AssistCtx, AssistHandler};
@@ -175,8 +174,7 @@ mod helpers {
         let (mut db, file_id) = RootDatabase::with_single_file(text);
         // FIXME: ideally, this should be done by the above `RootDatabase::with_single_file`,
         // but it looks like this might need specialization? :(
-        let local_roots = vec![db.file_source_root(file_id)];
-        db.set_local_roots(Arc::new(local_roots));
+        db.set_local_roots(Arc::new(vec![db.file_source_root(file_id)]));
         (db, file_id)
     }
 
@@ -206,11 +204,24 @@ mod helpers {
     }
 
     fn check(assist: AssistHandler, before: &str, expected: ExpectedResult) {
-        let (range_or_offset, before) = extract_range_or_offset(before);
-        let range: TextRange = range_or_offset.into();
+        let (text_without_caret, file_with_caret_id, range_or_offset, db) =
+            if before.contains("//-") {
+                let (mut db, position) = RootDatabase::with_position(before);
+                db.set_local_roots(Arc::new(vec![db.file_source_root(position.file_id)]));
+                (
+                    db.file_text(position.file_id).as_ref().to_owned(),
+                    position.file_id,
+                    RangeOrOffset::Offset(position.offset),
+                    db,
+                )
+            } else {
+                let (range_or_offset, text_without_caret) = extract_range_or_offset(before);
+                let (db, file_id) = with_single_file(&text_without_caret);
+                (text_without_caret, file_id, range_or_offset, db)
+            };
+
+        let frange = FileRange { file_id: file_with_caret_id, range: range_or_offset.into() };
 
-        let (db, file_id) = with_single_file(&before);
-        let frange = FileRange { file_id, range };
         let sema = Semantics::new(&db);
         let assist_ctx = AssistCtx::new(&sema, frange, true);
 
@@ -218,7 +229,7 @@ mod helpers {
             (Some(assist), ExpectedResult::After(after)) => {
                 let action = assist.0[0].action.clone().unwrap();
 
-                let mut actual = action.edit.apply(&before);
+                let mut actual = action.edit.apply(&text_without_caret);
                 match action.cursor_position {
                     None => {
                         if let RangeOrOffset::Offset(before_cursor_pos) = range_or_offset {
@@ -237,7 +248,7 @@ mod helpers {
             (Some(assist), ExpectedResult::Target(target)) => {
                 let action = assist.0[0].action.clone().unwrap();
                 let range = action.target.expect("expected target on action");
-                assert_eq_text!(&before[range], target);
+                assert_eq_text!(&text_without_caret[range], target);
             }
             (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
             (None, ExpectedResult::After(_)) | (None, ExpectedResult::Target(_)) => {

crates/ra_hir/src/code_model.rs

@@ -33,7 +33,11 @@ use ra_syntax::{
 };
 use rustc_hash::FxHashSet;
 
-use crate::{db::HirDatabase, has_source::HasSource, CallableDef, HirDisplay, InFile, Name};
+use crate::{
+    db::{DefDatabase, HirDatabase},
+    has_source::HasSource,
+    CallableDef, HirDisplay, InFile, Name,
+};
 
 /// hir::Crate describes a single crate. It's the main interface with which
 /// a crate's dependencies interact. Mostly, it should be just a proxy for the
@@ -274,20 +278,10 @@ impl Module {
     /// this module, if possible.
     pub fn find_use_path(
         self,
-        db: &dyn HirDatabase,
-        item: ModuleDef,
+        db: &dyn DefDatabase,
+        item: impl Into<ItemInNs>,
     ) -> Option<hir_def::path::ModPath> {
-        // FIXME expose namespace choice
-        hir_def::find_path::find_path(db.upcast(), determine_item_namespace(item), self.into())
-    }
-}
-
-fn determine_item_namespace(module_def: ModuleDef) -> ItemInNs {
-    match module_def {
-        ModuleDef::Static(_) | ModuleDef::Const(_) | ModuleDef::Function(_) => {
-            ItemInNs::Values(module_def.into())
-        }
-        _ => ItemInNs::Types(module_def.into()),
+        hir_def::find_path::find_path(db, item.into(), self.into())
     }
 }
 

crates/ra_hir/src/from_id.rs

@@ -9,8 +9,8 @@ use hir_def::{
 };
 
 use crate::{
-    Adt, AssocItem, AttrDef, DefWithBody, EnumVariant, GenericDef, Local, ModuleDef, StructField,
-    VariantDef,
+    code_model::ItemInNs, Adt, AssocItem, AttrDef, DefWithBody, EnumVariant, GenericDef, Local,
+    MacroDef, ModuleDef, StructField, VariantDef,
 };
 
 macro_rules! from_id {
@@ -228,3 +228,20 @@ impl From<(DefWithBodyId, PatId)> for Local {
         Local { parent, pat_id }
     }
 }
+
+impl From<MacroDef> for ItemInNs {
+    fn from(macro_def: MacroDef) -> Self {
+        ItemInNs::Macros(macro_def.into())
+    }
+}
+
+impl From<ModuleDef> for ItemInNs {
+    fn from(module_def: ModuleDef) -> Self {
+        match module_def {
+            ModuleDef::Static(_) | ModuleDef::Const(_) | ModuleDef::Function(_) => {
+                ItemInNs::Values(module_def.into())
+            }
+            _ => ItemInNs::Types(module_def.into()),
+        }
+    }
+}

crates/ra_ide_db/Cargo.toml

@@ -17,6 +17,7 @@ fst = { version = "0.4", default-features = false }
 rustc-hash = "1.1.0"
 superslice = "1.0.0"
 once_cell = "1.3.1"
+either = "1.5.3"
 
 ra_syntax = { path = "../ra_syntax" }
 ra_text_edit = { path = "../ra_text_edit" }

crates/ra_ide_db/src/imports_locator.rs

@@ -1,7 +1,7 @@
 //! This module contains an import search funcionality that is provided to the ra_assists module.
 //! Later, this should be moved away to a separate crate that is accessible from the ra_assists module.
 
-use hir::{ModuleDef, Semantics};
+use hir::{MacroDef, ModuleDef, Semantics};
 use ra_prof::profile;
 use ra_syntax::{ast, AstNode, SyntaxKind::NAME};
 
@@ -10,6 +10,7 @@ use crate::{
     symbol_index::{self, FileSymbol, Query},
     RootDatabase,
 };
+use either::Either;
 
 pub struct ImportsLocator<'a> {
     sema: Semantics<'a, RootDatabase>,
@@ -20,7 +21,7 @@ impl<'a> ImportsLocator<'a> {
         Self { sema: Semantics::new(db) }
     }
 
-    pub fn find_imports(&mut self, name_to_import: &str) -> Vec<ModuleDef> {
+    pub fn find_imports(&mut self, name_to_import: &str) -> Vec<Either<ModuleDef, MacroDef>> {
         let _p = profile("search_for_imports");
         let db = self.sema.db;
 
@@ -43,7 +44,8 @@ impl<'a> ImportsLocator<'a> {
             .chain(lib_results.into_iter())
             .filter_map(|import_candidate| self.get_name_definition(&import_candidate))
             .filter_map(|name_definition_to_import| match name_definition_to_import {
-                Definition::ModuleDef(module_def) => Some(module_def),
+                Definition::ModuleDef(module_def) => Some(Either::Left(module_def)),
+                Definition::Macro(macro_def) => Some(Either::Right(macro_def)),
                 _ => None,
             })
             .collect()