Repository: rust-analyzer/rust-analyzer
Pull request #6935 (2020-12-18 19:37:50)

Don't look at attributes when lowering to ItemTree
Resolves 2 `cfg_attr` FIXMEs

bors r+


Patch:
diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs
index 1b9c64ee55..9cd0b72aa9 100644
--- a/crates/hir_def/src/attr.rs
+++ b/crates/hir_def/src/attr.rs
@@ -12,7 +12,6 @@ use syntax::{
     ast::{self, AstNode, AttrsOwner},
     match_ast, AstToken, SmolStr, SyntaxNode,
 };
-use test_utils::mark;
 use tt::Subtree;
 
 use crate::{
@@ -123,69 +122,9 @@ impl RawAttrs {
     }
 
     /// Processes `cfg_attr`s, returning the resulting semantic `Attrs`.
-    pub(crate) fn filter(self, db: &dyn DefDatabase, krate: CrateId) -> Attrs {
-        let has_cfg_attrs = self.iter().any(|attr| {
-            attr.path.as_ident().map_or(false, |name| *name == hir_expand::name![cfg_attr])
-        });
-        if !has_cfg_attrs {
-            return Attrs(self);
-        }
-
-        let crate_graph = db.crate_graph();
-        let new_attrs = self
-            .iter()
-            .filter_map(|attr| {
-                let attr = attr.clone();
-                let is_cfg_attr =
-                    attr.path.as_ident().map_or(false, |name| *name == hir_expand::name![cfg_attr]);
-                if !is_cfg_attr {
-                    return Some(attr);
-                }
-
-                let subtree = match &attr.input {
-                    Some(AttrInput::TokenTree(it)) => it,
-                    _ => return Some(attr),
-                };
-
-                // Input subtree is: `(cfg, attr)`
-                // Split it up into a `cfg` and an `attr` subtree.
-                // FIXME: There should be a common API for this.
-                let mut saw_comma = false;
-                let (mut cfg, attr): (Vec<_>, Vec<_>) =
-                    subtree.clone().token_trees.into_iter().partition(|tree| {
-                        if saw_comma {
-                            return false;
-                        }
-
-                        match tree {
-                            tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
-                                saw_comma = true;
-                            }
-                            _ => {}
-                        }
-
-                        true
-                    });
-                cfg.pop(); // `,` ends up in here
-
-                let attr = Subtree { delimiter: None, token_trees: attr };
-                let cfg = Subtree { delimiter: subtree.delimiter, token_trees: cfg };
-                let cfg = CfgExpr::parse(&cfg);
-
-                let cfg_options = &crate_graph[krate].cfg_options;
-                if cfg_options.check(&cfg) == Some(false) {
-                    None
-                } else {
-                    mark::hit!(cfg_attr_active);
-
-                    let attr = ast::Attr::parse(&format!("#[{}]", attr)).ok()?;
-                    let hygiene = Hygiene::new_unhygienic(); // FIXME
-                    Attr::from_src(attr, &hygiene)
-                }
-            })
-            .collect();
-
-        Attrs(RawAttrs { entries: Some(new_attrs) })
+    pub(crate) fn filter(self, _db: &dyn DefDatabase, _krate: CrateId) -> Attrs {
+        // FIXME actually implement this
+        Attrs(self)
     }
 }
 
diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
index 5eb7cae7f0..100dbf5d65 100644
--- a/crates/hir_def/src/item_tree.rs
+++ b/crates/hir_def/src/item_tree.rs
@@ -12,7 +12,7 @@ use std::{
 };
 
 use arena::{Arena, Idx, RawId};
-use ast::{AstNode, AttrsOwner, NameOwner, StructKind};
+use ast::{AstNode, NameOwner, StructKind};
 use base_db::CrateId;
 use either::Either;
 use hir_expand::{
@@ -495,7 +495,6 @@ pub struct Import {
     pub alias: Option<ImportAlias>,
     pub visibility: RawVisibilityId,
     pub is_glob: bool,
-    pub is_prelude: bool,
     /// AST ID of the `use` or `extern crate` item this import was derived from. Note that many
     /// `Import`s can map to the same `use` item.
     pub ast_id: FileAstId<ast::Use>,
@@ -511,8 +510,6 @@ pub struct ExternCrate {
     pub name: Name,
     pub alias: Option<ImportAlias>,
     pub visibility: RawVisibilityId,
-    /// Whether this is a `#[macro_use] extern crate ...`.
-    pub is_macro_use: bool,
     pub ast_id: FileAstId<ast::ExternCrate>,
 }
 
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index c8f090c22c..3b206ef85e 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -485,8 +485,6 @@ impl Ctx {
     }
 
     fn lower_use(&mut self, use_item: &ast::Use) -> Vec<FileItemTreeId<Import>> {
-        // FIXME: cfg_attr
-        let is_prelude = use_item.has_atom_attr("prelude_import");
         let visibility = self.lower_visibility(use_item);
         let ast_id = self.source_ast_id_map.ast_id(use_item);
 
@@ -502,7 +500,6 @@ impl Ctx {
                     alias,
                     visibility,
                     is_glob,
-                    is_prelude,
                     ast_id,
                     index: imports.len(),
                 })));
@@ -522,10 +519,8 @@ impl Ctx {
         });
         let visibility = self.lower_visibility(extern_crate);
         let ast_id = self.source_ast_id_map.ast_id(extern_crate);
-        // FIXME: cfg_attr
-        let is_macro_use = extern_crate.has_atom_attr("macro_use");
 
-        let res = ExternCrate { name, alias, visibility, is_macro_use, ast_id };
+        let res = ExternCrate { name, alias, visibility, ast_id };
         Some(id(self.data().extern_crates.alloc(res)))
     }
 
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index b114a6fe4a..55228e480a 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -136,23 +136,35 @@ struct Import {
 }
 
 impl Import {
-    fn from_use(tree: &ItemTree, id: ItemTreeId<item_tree::Import>) -> Self {
+    fn from_use(
+        db: &dyn DefDatabase,
+        krate: CrateId,
+        tree: &ItemTree,
+        id: ItemTreeId<item_tree::Import>,
+    ) -> Self {
         let it = &tree[id.value];
+        let attrs = &tree.attrs(db, krate, ModItem::from(id.value).into());
         let visibility = &tree[it.visibility];
         Self {
             path: it.path.clone(),
             alias: it.alias.clone(),
             visibility: visibility.clone(),
             is_glob: it.is_glob,
-            is_prelude: it.is_prelude,
+            is_prelude: attrs.by_key("prelude_import").exists(),
             is_extern_crate: false,
             is_macro_use: false,
             source: ImportSource::Import(id),
         }
     }
 
-    fn from_extern_crate(tree: &ItemTree, id: ItemTreeId<item_tree::ExternCrate>) -> Self {
+    fn from_extern_crate(
+        db: &dyn DefDatabase,
+        krate: CrateId,
+        tree: &ItemTree,
+        id: ItemTreeId<item_tree::ExternCrate>,
+    ) -> Self {
         let it = &tree[id.value];
+        let attrs = &tree.attrs(db, krate, ModItem::from(id.value).into());
         let visibility = &tree[it.visibility];
         Self {
             path: ModPath::from_segments(PathKind::Plain, iter::once(it.name.clone())),
@@ -161,7 +173,7 @@ impl Import {
             is_glob: false,
             is_prelude: false,
             is_extern_crate: true,
-            is_macro_use: it.is_macro_use,
+            is_macro_use: attrs.by_key("macro_use").exists(),
             source: ImportSource::ExternCrate(id),
         }
     }
@@ -930,7 +942,12 @@ impl ModCollector<'_, '_> {
             if attrs.cfg().map_or(true, |cfg| self.is_cfg_enabled(&cfg)) {
                 if let ModItem::ExternCrate(id) = item {
                     let import = self.item_tree[*id].clone();
-                    if import.is_macro_use {
+                    let attrs = self.item_tree.attrs(
+                        self.def_collector.db,
+                        krate,
+                        ModItem::from(*id).into(),
+                    );
+                    if attrs.by_key("macro_use").exists() {
                         self.def_collector.import_macros_from_extern_crate(self.module_id, &import);
                     }
                 }
@@ -956,6 +973,8 @@ impl ModCollector<'_, '_> {
                     self.def_collector.unresolved_imports.push(ImportDirective {
                         module_id: self.module_id,
                         import: Import::from_use(
+                            self.def_collector.db,
+                            krate,
                             &self.item_tree,
                             InFile::new(self.file_id, import_id),
                         ),
@@ -966,6 +985,8 @@ impl ModCollector<'_, '_> {
                     self.def_collector.unresolved_imports.push(ImportDirective {
                         module_id: self.module_id,
                         import: Import::from_extern_crate(
+                            self.def_collector.db,
+                            krate,
                             &self.item_tree,
                             InFile::new(self.file_id, import_id),
                         ),
diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs
index 77eeee3fe9..7fb4caea36 100644
--- a/crates/hir_expand/src/name.rs
+++ b/crates/hir_expand/src/name.rs
@@ -153,7 +153,6 @@ pub mod known {
         // Special names
         macro_rules,
         doc,
-        cfg_attr,
         // Components of known path (value or mod name)
         std,
         core,
diff --git a/crates/parser/src/grammar.rs b/crates/parser/src/grammar.rs
index 1a078f6b4c..23039eba49 100644
--- a/crates/parser/src/grammar.rs
+++ b/crates/parser/src/grammar.rs
@@ -133,10 +133,6 @@ pub(crate) mod fragments {
 
         m.complete(p, MACRO_STMTS);
     }
-
-    pub(crate) fn attr(p: &mut Parser) {
-        attributes::outer_attrs(p)
-    }
 }
 
 pub(crate) fn reparser(
diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs
index ab8e4c70e8..41e62116f8 100644
--- a/crates/parser/src/lib.rs
+++ b/crates/parser/src/lib.rs
@@ -99,8 +99,6 @@ pub enum FragmentKind {
     // FIXME: use separate fragment kinds for macro inputs and outputs?
     Items,
     Statements,
-
-    Attr,
 }
 
 pub fn parse_fragment(
@@ -120,7 +118,6 @@ pub fn parse_fragment(
         FragmentKind::Statement => grammar::fragments::stmt,
         FragmentKind::Items => grammar::fragments::macro_items,
         FragmentKind::Statements => grammar::fragments::macro_stmts,
-        FragmentKind::Attr => grammar::fragments::attr,
     };
     parse_from_tokens(token_source, tree_sink, parser)
 }
diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs
index 4d272f367a..e753b11bb8 100644
--- a/crates/syntax/src/lib.rs
+++ b/crates/syntax/src/lib.rs
@@ -205,13 +205,6 @@ impl ast::Type {
     }
 }
 
-impl ast::Attr {
-    /// Returns `text`, parsed as an attribute, but only if it has no errors.
-    pub fn parse(text: &str) -> Result<Self, ()> {
-        parsing::parse_text_fragment(text, parser::FragmentKind::Attr)
-    }
-}
-
 /// Matches a `SyntaxNode` against an `ast` type.
 ///
 /// # Example:


Test patch:
diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs
index 58d69d3c6b..1a7b988318 100644
--- a/crates/hir_def/src/nameres/tests/diagnostics.rs
+++ b/crates/hir_def/src/nameres/tests/diagnostics.rs
@@ -1,5 +1,4 @@
 use base_db::fixture::WithFixture;
-use test_utils::mark;
 
 use crate::test_db::TestDB;
 
@@ -120,20 +119,3 @@ fn inactive_item() {
         "#,
     );
 }
-
-/// Tests that `cfg` attributes behind `cfg_attr` is handled properly.
-#[test]
-fn inactive_via_cfg_attr() {
-    mark::check!(cfg_attr_active);
-    check_diagnostics(
-        r#"
-        //- /lib.rs
-          #[cfg_attr(not(never), cfg(no))] fn f() {}
-        //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled
-
-          #[cfg_attr(not(never), cfg(not(no)))] fn f() {}
-
-          #[cfg_attr(never, cfg(no))] fn g() {}
-        "#,
-    );
-}