Encapsulate Attrs

6 files changed, 57 insertions, 42 deletions

diff --git a/crates/ra_hir/src/code_model/attrs.rs b/crates/ra_hir/src/code_model/attrs.rs
index 9e304217c..9923cde81 100644
--- a/crates/ra_hir/src/code_model/attrs.rs
+++ b/crates/ra_hir/src/code_model/attrs.rs
@@ -8,7 +8,6 @@ use crate::{
 use hir_def::attr::Attr;
 use hir_expand::hygiene::Hygiene;
 use ra_syntax::ast;
-use std::sync::Arc;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum AttrDef {
@@ -38,16 +37,19 @@ impl_froms!(
 );
 
 pub trait Attrs {
-    fn attrs(&self, db: &impl HirDatabase) -> Option<Arc<[Attr]>>;
+    fn attrs(&self, db: &impl HirDatabase) -> hir_def::attr::Attrs;
 }
 
 pub(crate) fn attributes_query(
     db: &(impl DefDatabase + AstDatabase),
     def: AttrDef,
-) -> Option<Arc<[Attr]>> {
+) -> hir_def::attr::Attrs {
     match def {
         AttrDef::Module(it) => {
-            let src = it.declaration_source(db)?;
+            let src = match it.declaration_source(db) {
+                Some(it) => it,
+                None => return hir_def::attr::Attrs::default(),
+            };
             let hygiene = Hygiene::new(db, src.file_id);
             Attr::from_attrs_owner(&src.value, &hygiene)
         }
@@ -57,7 +59,7 @@ pub(crate) fn attributes_query(
                 let hygiene = Hygiene::new(db, src.file_id);
                 Attr::from_attrs_owner(&named, &hygiene)
             }
-            FieldSource::Pos(..) => None,
+            FieldSource::Pos(..) => hir_def::attr::Attrs::default(),
         },
         AttrDef::Adt(it) => match it {
             Adt::Struct(it) => attrs_from_ast(it, db),
@@ -74,7 +76,7 @@ pub(crate) fn attributes_query(
     }
 }
 
-fn attrs_from_ast<T, D>(node: T, db: &D) -> Option<Arc<[Attr]>>
+fn attrs_from_ast<T, D>(node: T, db: &D) -> hir_def::attr::Attrs
 where
     T: HasSource,
     T::Ast: ast::AttrsOwner,
@@ -86,7 +88,7 @@ where
 }
 
 impl<T: Into<AttrDef> + Copy> Attrs for T {
-    fn attrs(&self, db: &impl HirDatabase) -> Option<Arc<[Attr]>> {
+    fn attrs(&self, db: &impl HirDatabase) -> hir_def::attr::Attrs {
         db.attrs((*self).into())
     }
 }
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index ed0d68001..1f63be3b9 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -2,7 +2,7 @@
 
 use std::sync::Arc;
 
-use hir_def::attr::Attr;
+use hir_def::attr::Attrs;
 use ra_db::salsa;
 use ra_syntax::SmolStr;
 
@@ -61,7 +61,7 @@ pub trait DefDatabase: HirDebugDatabase + DefDatabase2 {
     fn documentation(&self, def: crate::DocDef) -> Option<crate::Documentation>;
 
     #[salsa::invoke(crate::code_model::attrs::attributes_query)]
-    fn attrs(&self, def: crate::AttrDef) -> Option<Arc<[Attr]>>;
+    fn attrs(&self, def: crate::AttrDef) -> Attrs;
 }
 
 #[salsa::query_group(HirDatabaseStorage)]
diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs
index 0e961ca12..7a9d0fdf4 100644
--- a/crates/ra_hir_def/src/attr.rs
+++ b/crates/ra_hir_def/src/attr.rs
@@ -1,6 +1,6 @@
 //! A higher level attributes based on TokenTree, with also some shortcuts.
 
-use std::sync::Arc;
+use std::{ops, sync::Arc};
 
 use hir_expand::hygiene::Hygiene;
 use mbe::ast_to_token_tree;
@@ -13,6 +13,28 @@ use tt::Subtree;
 
 use crate::path::Path;
 
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
+pub struct Attrs {
+    entries: Option<Arc<[Attr]>>,
+}
+
+impl ops::Deref for Attrs {
+    type Target = [Attr];
+
+    fn deref(&self) -> &[Attr] {
+        match &self.entries {
+            Some(it) => &*it,
+            None => &[],
+        }
+    }
+}
+
+impl Attrs {
+    pub fn has_atom(&self, atom: &str) -> bool {
+        self.iter().any(|it| it.is_simple_atom(atom))
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Attr {
     pub(crate) path: Path,
@@ -43,13 +65,15 @@ impl Attr {
         Some(Attr { path, input })
     }
 
-    pub fn from_attrs_owner(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Option<Arc<[Attr]>> {
+    pub fn from_attrs_owner(owner: &dyn AttrsOwner, hygiene: &Hygiene) -> Attrs {
         let mut attrs = owner.attrs().peekable();
-        if attrs.peek().is_none() {
+        let entries = if attrs.peek().is_none() {
             // Avoid heap allocation
-            return None;
-        }
-        Some(attrs.flat_map(|ast| Attr::from_src(ast, hygiene)).collect())
+            None
+        } else {
+            Some(attrs.flat_map(|ast| Attr::from_src(ast, hygiene)).collect())
+        };
+        Attrs { entries }
     }
 
     pub fn is_simple_atom(&self, name: &str) -> bool {
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index aae3dcadf..7902293e8 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -12,7 +12,7 @@ use rustc_hash::FxHashMap;
 use test_utils::tested_by;
 
 use crate::{
-    attr::Attr,
+    attr::Attrs,
     db::DefDatabase2,
     nameres::{
         diagnostics::DefDiagnostic, mod_resolution::ModDir, path_resolution::ReachedFixedPoint,
@@ -549,7 +549,7 @@ where
         // `#[macro_use] extern crate` is hoisted to imports macros before collecting
         // any other items.
         for item in items {
-            if self.is_cfg_enabled(item.attrs()) {
+            if self.is_cfg_enabled(&item.attrs) {
                 if let raw::RawItemKind::Import(import_id) = item.kind {
                     let import = self.raw_items[import_id].clone();
                     if import.is_extern_crate && import.is_macro_use {
@@ -560,10 +560,10 @@ where
         }
 
         for item in items {
-            if self.is_cfg_enabled(item.attrs()) {
+            if self.is_cfg_enabled(&item.attrs) {
                 match item.kind {
                     raw::RawItemKind::Module(m) => {
-                        self.collect_module(&self.raw_items[m], item.attrs())
+                        self.collect_module(&self.raw_items[m], &item.attrs)
                     }
                     raw::RawItemKind::Import(import_id) => self
                         .def_collector
@@ -585,9 +585,9 @@ where
         }
     }
 
-    fn collect_module(&mut self, module: &raw::ModuleData, attrs: &[Attr]) {
+    fn collect_module(&mut self, module: &raw::ModuleData, attrs: &Attrs) {
         let path_attr = self.path_attr(attrs);
-        let is_macro_use = self.is_macro_use(attrs);
+        let is_macro_use = attrs.has_atom("macro_use");
         match module {
             // inline module, just recurse
             raw::ModuleData::Definition { name, items, ast_id } => {
@@ -779,17 +779,13 @@ where
         }
     }
 
-    fn is_cfg_enabled(&self, attrs: &[Attr]) -> bool {
+    fn is_cfg_enabled(&self, attrs: &Attrs) -> 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> {
+    fn path_attr<'a>(&self, attrs: &'a Attrs) -> 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 {
diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs
index 7c68fd638..55a9634f8 100644
--- a/crates/ra_hir_def/src/nameres/raw.rs
+++ b/crates/ra_hir_def/src/nameres/raw.rs
@@ -16,7 +16,12 @@ use ra_syntax::{
 };
 use test_utils::tested_by;
 
-use crate::{attr::Attr, db::DefDatabase2, path::Path, FileAstId, HirFileId, ModuleSource, Source};
+use crate::{
+    attr::{Attr, Attrs},
+    db::DefDatabase2,
+    path::Path,
+    FileAstId, HirFileId, ModuleSource, Source,
+};
 
 /// `RawItems` is a set of top-level items in a file (except for impls).
 ///
@@ -129,21 +134,12 @@ impl Index<Impl> for RawItems {
     }
 }
 
-// Avoid heap allocation on items without attributes.
-type Attrs = Option<Arc<[Attr]>>;
-
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub(super) struct RawItem {
-    attrs: Attrs,
+    pub(super) attrs: Attrs,
     pub(super) kind: RawItemKind,
 }
 
-impl RawItem {
-    pub(super) fn attrs(&self) -> &[Attr] {
-        self.attrs.as_ref().map_or(&[], |it| &*it)
-    }
-}
-
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub(super) enum RawItemKind {
     Module(Module),
diff --git a/crates/ra_ide_api/src/completion/presentation.rs b/crates/ra_ide_api/src/completion/presentation.rs
index b20329459..cbaf169bf 100644
--- a/crates/ra_ide_api/src/completion/presentation.rs
+++ b/crates/ra_ide_api/src/completion/presentation.rs
@@ -286,10 +286,7 @@ impl Completions {
 }
 
 fn is_deprecated(node: impl Attrs, db: &impl HirDatabase) -> bool {
-    match node.attrs(db) {
-        None => false,
-        Some(attrs) => attrs.iter().any(|x| x.is_simple_atom("deprecated")),
-    }
+    node.attrs(db).has_atom("deprecated")
 }
 
 fn has_non_default_type_params(def: hir::GenericDef, db: &db::RootDatabase) -> bool {