3 files changed, 37 insertions, 31 deletions

diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs
index 5c1b151f7..53456fc08 100644
--- a/crates/ra_hir_def/src/attr.rs
+++ b/crates/ra_hir_def/src/attr.rs
@@ -4,7 +4,6 @@ use std::{ops, sync::Arc};
 
 use hir_expand::{either::Either, hygiene::Hygiene, AstId, Source};
 use mbe::ast_to_token_tree;
-use ra_cfg::CfgOptions;
 use ra_syntax::{
     ast::{self, AstNode, AttrsOwner},
     SmolStr,
@@ -85,17 +84,8 @@ impl Attrs {
         Attrs { entries }
     }
 
-    pub fn has_atom(&self, atom: &str) -> bool {
-        self.iter().any(|it| it.is_simple_atom(atom))
-    }
-
-    pub fn find_string_value(&self, key: &str) -> Option<SmolStr> {
-        self.iter().filter(|attr| attr.is_simple_atom(key)).find_map(|attr| {
-            match attr.input.as_ref()? {
-                AttrInput::Literal(it) => Some(it.clone()),
-                _ => None,
-            }
-        })
+    pub fn by_key(&self, key: &'static str) -> AttrQuery<'_> {
+        AttrQuery { attrs: self, key }
     }
 }
 
@@ -128,25 +118,37 @@ impl Attr {
 
         Some(Attr { path, input })
     }
+}
+
+pub struct AttrQuery<'a> {
+    attrs: &'a Attrs,
+    key: &'static str,
+}
 
-    pub fn is_simple_atom(&self, name: &str) -> bool {
-        // FIXME: Avoid cloning
-        self.path.as_ident().map_or(false, |s| s.to_string() == name)
+impl<'a> AttrQuery<'a> {
+    pub fn tt_values(self) -> impl Iterator<Item = &'a Subtree> {
+        self.attrs().filter_map(|attr| match attr.input.as_ref()? {
+            AttrInput::TokenTree(it) => Some(it),
+            _ => None,
+        })
     }
 
-    // FIXME: handle cfg_attr :-)
-    pub fn as_cfg(&self) -> Option<&Subtree> {
-        if !self.is_simple_atom("cfg") {
-            return None;
-        }
-        match &self.input {
-            Some(AttrInput::TokenTree(subtree)) => Some(subtree),
+    pub fn string_value(self) -> Option<&'a SmolStr> {
+        self.attrs().find_map(|attr| match attr.input.as_ref()? {
+            AttrInput::Literal(it) => Some(it),
             _ => None,
-        }
+        })
+    }
+
+    pub fn exists(self) -> bool {
+        self.attrs().next().is_some()
     }
 
-    pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> Option<bool> {
-        cfg_options.is_cfg_enabled(self.as_cfg()?)
+    fn attrs(self) -> impl Iterator<Item = &'a Attr> {
+        let key = self.key;
+        self.attrs
+            .iter()
+            .filter(move |attr| attr.path.as_ident().map_or(false, |s| s.to_string() == key))
     }
 }
 
diff --git a/crates/ra_hir_def/src/lang_item.rs b/crates/ra_hir_def/src/lang_item.rs
index 69d7bf21a..3b9fb0328 100644
--- a/crates/ra_hir_def/src/lang_item.rs
+++ b/crates/ra_hir_def/src/lang_item.rs
@@ -113,7 +113,7 @@ impl LangItems {
         T: Into<AttrDefId> + Copy,
     {
         let attrs = db.attrs(item.into());
-        if let Some(lang_item_name) = attrs.find_string_value("lang") {
+        if let Some(lang_item_name) = attrs.by_key("lang").string_value() {
             self.items.entry(lang_item_name.clone()).or_insert_with(|| constructor(item));
         }
     }
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 15941a1cb..7a5f90327 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -599,8 +599,8 @@ where
     }
 
     fn collect_module(&mut self, module: &raw::ModuleData, attrs: &Attrs) {
-        let path_attr = attrs.find_string_value("path");
-        let is_macro_use = attrs.has_atom("macro_use");
+        let path_attr = attrs.by_key("path").string_value();
+        let is_macro_use = attrs.by_key("macro_use").exists();
         match module {
             // inline module, just recurse
             raw::ModuleData::Definition { name, items, ast_id } => {
@@ -612,7 +612,7 @@ where
                     module_id,
                     file_id: self.file_id,
                     raw_items: self.raw_items,
-                    mod_dir: self.mod_dir.descend_into_definition(name, path_attr.as_ref()),
+                    mod_dir: self.mod_dir.descend_into_definition(name, path_attr),
                 }
                 .collect(&*items);
                 if is_macro_use {
@@ -626,7 +626,7 @@ where
                     self.def_collector.db,
                     self.file_id,
                     name,
-                    path_attr.as_ref(),
+                    path_attr,
                 ) {
                     Ok((file_id, mod_dir)) => {
                         let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id));
@@ -796,7 +796,11 @@ where
     }
 
     fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
-        attrs.iter().all(|attr| attr.is_cfg_enabled(&self.def_collector.cfg_options) != Some(false))
+        // FIXME: handle cfg_attr :-)
+        attrs
+            .by_key("cfg")
+            .tt_values()
+            .all(|tt| self.def_collector.cfg_options.is_cfg_enabled(tt) != Some(false))
     }
 }