Merge #1986

1986: don't special case module attrs r=matklad a=matklad

bors r+

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_hir/src/attr.rs

@@ -63,14 +63,24 @@ impl Attr {
         self.path.as_ident().map_or(false, |s| s.to_string() == name)
     }
 
+    // FIXME: handle cfg_attr :-)
     pub(crate) fn as_cfg(&self) -> Option<&Subtree> {
-        if self.is_simple_atom("cfg") {
+        if !self.is_simple_atom("cfg") {
+            return None;
+        }
         match &self.input {
             Some(AttrInput::TokenTree(subtree)) => Some(subtree),
             _ => None,
         }
-        } else {
+    }
-            None
+
+    pub(crate) fn as_path(&self) -> Option<&SmolStr> {
+        if !self.is_simple_atom("path") {
+            return None;
+        }
+        match &self.input {
+            Some(AttrInput::Literal(it)) => Some(it),
+            _ => None,
         }
     }
 

crates/ra_hir/src/nameres/collector.rs

@@ -2,7 +2,7 @@
 
 use ra_cfg::CfgOptions;
 use ra_db::FileId;
-use ra_syntax::ast;
+use ra_syntax::{ast, SmolStr};
 use rustc_hash::FxHashMap;
 use test_utils::tested_by;
 
@@ -546,7 +546,9 @@ where
         for item in items {
             if self.is_cfg_enabled(item.attrs()) {
                 match item.kind {
-                    raw::RawItemKind::Module(m) => self.collect_module(&self.raw_items[m]),
+                    raw::RawItemKind::Module(m) => {
+                        self.collect_module(&self.raw_items[m], item.attrs())
+                    }
                     raw::RawItemKind::Import(import_id) => self
                         .def_collector
                         .unresolved_imports
@@ -558,10 +560,12 @@ where
         }
     }
 
-    fn collect_module(&mut self, module: &raw::ModuleData) {
+    fn collect_module(&mut self, module: &raw::ModuleData, attrs: &[Attr]) {
+        let path_attr = self.path_attr(attrs);
+        let is_macro_use = self.is_macro_use(attrs);
         match module {
             // inline module, just recurse
-            raw::ModuleData::Definition { name, items, ast_id, attr_path, is_macro_use } => {
+            raw::ModuleData::Definition { name, items, ast_id } => {
                 let module_id =
                     self.push_child_module(name.clone(), ast_id.with_file_id(self.file_id), None);
 
@@ -570,21 +574,21 @@ where
                     module_id,
                     file_id: self.file_id,
                     raw_items: self.raw_items,
-                    mod_dir: self.mod_dir.descend_into_definition(name, attr_path.as_ref()),
+                    mod_dir: self.mod_dir.descend_into_definition(name, path_attr),
                 }
                 .collect(&*items);
-                if *is_macro_use {
+                if is_macro_use {
                     self.import_all_legacy_macros(module_id);
                 }
             }
             // out of line module, resolve, parse and recurse
-            raw::ModuleData::Declaration { name, ast_id, attr_path, is_macro_use } => {
+            raw::ModuleData::Declaration { name, ast_id } => {
                 let ast_id = ast_id.with_file_id(self.file_id);
                 match self.mod_dir.resolve_submodule(
                     self.def_collector.db,
                     self.file_id,
                     name,
-                    attr_path.as_ref(),
+                    path_attr,
                 ) {
                     Ok((file_id, mod_dir)) => {
                         let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id));
@@ -597,7 +601,7 @@ where
                             mod_dir,
                         }
                         .collect(raw_items.items());
-                        if *is_macro_use {
+                        if is_macro_use {
                             self.import_all_legacy_macros(module_id);
                         }
                     }
@@ -713,6 +717,14 @@ where
     fn is_cfg_enabled(&self, attrs: &[Attr]) -> bool {
         attrs.iter().all(|attr| attr.is_cfg_enabled(&self.def_collector.cfg_options) != Some(false))
     }
+
+    fn path_attr<'a>(&self, attrs: &'a [Attr]) -> Option<&'a SmolStr> {
+        attrs.iter().find_map(|attr| attr.as_path())
+    }
+
+    fn is_macro_use<'a>(&self, attrs: &'a [Attr]) -> bool {
+        attrs.iter().any(|attr| attr.is_simple_atom("macro_use"))
+    }
 }
 
 fn is_macro_rules(path: &Path) -> bool {

crates/ra_hir/src/nameres/raw.rs

@@ -5,7 +5,7 @@ use std::{ops::Index, sync::Arc};
 use ra_arena::{impl_arena_id, map::ArenaMap, Arena, RawId};
 use ra_syntax::{
     ast::{self, AttrsOwner, NameOwner},
-    AstNode, AstPtr, SmolStr, SourceFile,
+    AstNode, AstPtr, SourceFile,
 };
 use test_utils::tested_by;
 
@@ -149,19 +149,8 @@ impl_arena_id!(Module);
 
 #[derive(Debug, PartialEq, Eq)]
 pub(super) enum ModuleData {
-    Declaration {
+    Declaration { name: Name, ast_id: FileAstId<ast::Module> },
-        name: Name,
+    Definition { name: Name, ast_id: FileAstId<ast::Module>, items: Vec<RawItem> },
-        ast_id: FileAstId<ast::Module>,
-        attr_path: Option<SmolStr>,
-        is_macro_use: bool,
-    },
-    Definition {
-        name: Name,
-        ast_id: FileAstId<ast::Module>,
-        items: Vec<RawItem>,
-        attr_path: Option<SmolStr>,
-        is_macro_use: bool,
-    },
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -292,28 +281,17 @@ impl<DB: AstDatabase> RawItemsCollector<&DB> {
         let attrs = self.parse_attrs(&module);
 
         let ast_id = self.source_ast_id_map.ast_id(&module);
-        // FIXME: cfg_attr
-        let is_macro_use = module.has_atom_attr("macro_use");
         if module.has_semi() {
-            let attr_path = extract_mod_path_attribute(&module);
+            let item = self.raw_items.modules.alloc(ModuleData::Declaration { name, ast_id });
-            let item = self.raw_items.modules.alloc(ModuleData::Declaration {
-                name,
-                ast_id,
-                attr_path,
-                is_macro_use,
-            });
             self.push_item(current_module, attrs, RawItemKind::Module(item));
             return;
         }
 
         if let Some(item_list) = module.item_list() {
-            let attr_path = extract_mod_path_attribute(&module);
             let item = self.raw_items.modules.alloc(ModuleData::Definition {
                 name,
                 ast_id,
                 items: Vec::new(),
-                attr_path,
-                is_macro_use,
             });
             self.process_module(Some(item), item_list);
             self.push_item(current_module, attrs, RawItemKind::Module(item));
@@ -423,16 +401,3 @@ impl<DB: AstDatabase> RawItemsCollector<&DB> {
         Attr::from_attrs_owner(self.file_id, item, self.db)
     }
 }
-
-fn extract_mod_path_attribute(module: &ast::Module) -> Option<SmolStr> {
-    module.attrs().into_iter().find_map(|attr| {
-        attr.as_simple_key_value().and_then(|(name, value)| {
-            let is_path = name == "path";
-            if is_path {
-                Some(value)
-            } else {
-                None
-            }
-        })
-    })
-}