Support cfg attribute on impl blocks

Author: oxalica

crates/ra_hir/src/attr.rs

@@ -1,11 +1,14 @@
+use std::sync::Arc;
+
 use mbe::ast_to_token_tree;
+use ra_cfg::CfgOptions;
 use ra_syntax::{
-    ast::{self, AstNode},
+    ast::{self, AstNode, AttrsOwner},
     SmolStr,
 };
 use tt::Subtree;
 
-use crate::{db::AstDatabase, path::Path, Source};
+use crate::{db::AstDatabase, path::Path, HirFileId, Source};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub(crate) struct Attr {
@@ -40,6 +43,14 @@ impl Attr {
         Some(Attr { path, input })
     }
 
+    pub(crate) fn from_attrs_owner(
+        file_id: HirFileId,
+        owner: &impl AttrsOwner,
+        db: &impl AstDatabase,
+    ) -> Arc<[Attr]> {
+        owner.attrs().flat_map(|ast| Attr::from_src(Source { file_id, ast }, db)).collect()
+    }
+
     pub(crate) fn is_simple_atom(&self, name: &str) -> bool {
         // FIXME: Avoid cloning
         self.path.as_ident().map_or(false, |s| s.to_string() == name)
@@ -55,4 +66,8 @@ impl Attr {
             None
         }
     }
+
+    pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> Option<bool> {
+        cfg_options.is_cfg_enabled(self.as_cfg()?)
+    }
 }

crates/ra_hir/src/impl_block.rs

@@ -4,12 +4,14 @@ use rustc_hash::FxHashMap;
 use std::sync::Arc;
 
 use ra_arena::{impl_arena_id, map::ArenaMap, Arena, RawId};
+use ra_cfg::CfgOptions;
 use ra_syntax::{
     ast::{self, AstNode},
     AstPtr,
 };
 
 use crate::{
+    attr::Attr,
     code_model::{Module, ModuleSource},
     db::{AstDatabase, DefDatabase, HirDatabase},
     generics::HasGenericParams,
@@ -176,6 +178,7 @@ pub struct ModuleImplBlocks {
 impl ModuleImplBlocks {
     fn collect(
         db: &(impl DefDatabase + AstDatabase),
+        cfg_options: &CfgOptions,
         module: Module,
         source_map: &mut ImplSourceMap,
     ) -> Self {
@@ -188,11 +191,11 @@ impl ModuleImplBlocks {
         let src = m.module.definition_source(db);
         match &src.ast {
             ModuleSource::SourceFile(node) => {
-                m.collect_from_item_owner(db, source_map, node, src.file_id)
+                m.collect_from_item_owner(db, cfg_options, source_map, node, src.file_id)
             }
             ModuleSource::Module(node) => {
                 let item_list = node.item_list().expect("inline module should have item list");
-                m.collect_from_item_owner(db, source_map, &item_list, src.file_id)
+                m.collect_from_item_owner(db, cfg_options, source_map, &item_list, src.file_id)
             }
         };
         m
@@ -201,13 +204,19 @@ impl ModuleImplBlocks {
     fn collect_from_item_owner(
         &mut self,
         db: &(impl DefDatabase + AstDatabase),
+        cfg_options: &CfgOptions,
         source_map: &mut ImplSourceMap,
         owner: &dyn ast::ModuleItemOwner,
         file_id: HirFileId,
     ) {
         for item in owner.items_with_macros() {
             match item {
                 ast::ItemOrMacro::Item(ast::ModuleItem::ImplBlock(impl_block_ast)) => {
+                    let attrs = Attr::from_attrs_owner(file_id, &impl_block_ast, db);
+                    if attrs.iter().any(|attr| attr.is_cfg_enabled(cfg_options) == Some(false)) {
+                        continue;
+                    }
+
                     let impl_block = ImplData::from_ast(db, file_id, self.module, &impl_block_ast);
                     let id = self.impls.alloc(impl_block);
                     for &impl_item in &self.impls[id].items {
@@ -218,6 +227,11 @@ impl ModuleImplBlocks {
                 }
                 ast::ItemOrMacro::Item(_) => (),
                 ast::ItemOrMacro::Macro(macro_call) => {
+                    let attrs = Attr::from_attrs_owner(file_id, &macro_call, db);
+                    if attrs.iter().any(|attr| attr.is_cfg_enabled(cfg_options) == Some(false)) {
+                        continue;
+                    }
+
                     //FIXME: we should really cut down on the boilerplate required to process a macro
                     let ast_id = db.ast_id_map(file_id).ast_id(&macro_call).with_file_id(file_id);
                     if let Some(path) = macro_call
@@ -231,7 +245,13 @@ impl ModuleImplBlocks {
                             if let Some(item_list) =
                                 db.parse_or_expand(file_id).and_then(ast::MacroItems::cast)
                             {
-                                self.collect_from_item_owner(db, source_map, &item_list, file_id)
+                                self.collect_from_item_owner(
+                                    db,
+                                    cfg_options,
+                                    source_map,
+                                    &item_list,
+                                    file_id,
+                                )
                             }
                         }
                     }
@@ -246,8 +266,10 @@ pub(crate) fn impls_in_module_with_source_map_query(
     module: Module,
 ) -> (Arc<ModuleImplBlocks>, Arc<ImplSourceMap>) {
     let mut source_map = ImplSourceMap::default();
+    let crate_graph = db.crate_graph();
+    let cfg_options = crate_graph.cfg_options(module.krate.crate_id());
 
-    let result = ModuleImplBlocks::collect(db, module, &mut source_map);
+    let result = ModuleImplBlocks::collect(db, cfg_options, module, &mut source_map);
     (Arc::new(result), Arc::new(source_map))
 }
 

crates/ra_hir/src/nameres/collector.rs

@@ -716,10 +716,7 @@ where
     }
 
     fn is_cfg_enabled(&self, attrs: &[Attr]) -> bool {
-        attrs
-            .iter()
-            .flat_map(|attr| attr.as_cfg())
-            .all(|cfg| self.def_collector.cfg_options.is_cfg_enabled(cfg).unwrap_or(true))
+        attrs.iter().all(|attr| attr.is_cfg_enabled(&self.def_collector.cfg_options) != Some(false))
     }
 }
 

crates/ra_hir/src/nameres/raw.rs

@@ -411,9 +411,7 @@ impl<DB: AstDatabase> RawItemsCollector<&DB> {
     }
 
     fn parse_attrs(&self, item: &impl ast::AttrsOwner) -> Arc<[Attr]> {
-        item.attrs()
-            .flat_map(|ast| Attr::from_src(Source { ast, file_id: self.file_id }, self.db))
-            .collect()
+        Attr::from_attrs_owner(self.file_id, item, self.db)
     }
 }
 

crates/ra_hir/src/ty/tests.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use insta::assert_snapshot;
 
+use ra_cfg::CfgOptions;
 use ra_db::{salsa::Database, FilePosition, SourceDatabase};
 use ra_syntax::{
     algo,
@@ -23,6 +24,50 @@ use crate::{
 mod never_type;
 mod coercion;
 
+#[test]
+fn cfg_impl_block() {
+    let (mut db, pos) = MockDatabase::with_position(
+        r#"
+//- /main.rs
+use foo::S as T;
+struct S;
+
+#[cfg(test)]
+impl S {
+    fn foo1(&self) -> i32 { 0 }
+}
+
+#[cfg(not(test))]
+impl S {
+    fn foo2(&self) -> i32 { 0 }
+}
+
+fn test() {
+    let t = (S.foo1(), S.foo2(), T.foo3(), T.foo4());
+    t<|>;
+}
+
+//- /foo.rs
+struct S;
+
+#[cfg(not(test))]
+impl S {
+    fn foo3(&self) -> i32 { 0 }
+}
+
+#[cfg(test)]
+impl S {
+    fn foo4(&self) -> i32 { 0 }
+}
+"#,
+    );
+    db.set_crate_graph_from_fixture(crate_graph! {
+        "main": ("/main.rs", ["foo"], CfgOptions::default().atom("test".into())),
+        "foo": ("/foo.rs", []),
+    });
+    assert_eq!("(i32, {unknown}, i32, {unknown})", type_at_pos(&db, pos));
+}
+
 #[test]
 fn infer_await() {
     let (mut db, pos) = MockDatabase::with_position(