Parse cfg_attr and cfg specially

This allows us to simplify expansion of cfg_attr in `expand_cfg_attr()`, and also fixes a bunch of bugs like expansion of `doc = macro!()` inside `cfg_attr`.

Author: ChayimFriedman2

Cargo.lock

@@ -19,7 +19,6 @@ tracing.workspace = true
 # locals deps
 tt = { workspace = true, optional = true }
 syntax = { workspace = true, optional = true }
-span = { path = "../span", version = "0.0", optional = true }
 intern.workspace = true
 
 [dev-dependencies]
@@ -36,7 +35,7 @@ cfg = { path = ".", default-features = false, features = ["tt"] }
 
 [features]
 default = []
-syntax = ["dep:syntax", "dep:span"]
+syntax = ["dep:syntax"]
 tt = ["dep:tt"]
 in-rust-tree = []
 

crates/cfg/Cargo.toml

@@ -106,10 +106,54 @@ impl CfgExpr {
     }
 
     #[cfg(feature = "syntax")]
-    pub fn parse_from_ast(
-        ast: &mut std::iter::Peekable<syntax::ast::TokenTreeChildren>,
-    ) -> CfgExpr {
-        next_cfg_expr_from_ast(ast).unwrap_or(CfgExpr::Invalid)
+    pub fn parse_from_ast(ast: syntax::ast::CfgPredicate) -> CfgExpr {
+        use intern::sym;
+        use syntax::ast::{self, AstToken};
+
+        match ast {
+            ast::CfgPredicate::CfgAtom(atom) => {
+                let atom = match atom.key() {
+                    Some(ast::CfgAtomKey::True) => CfgAtom::Flag(sym::true_),
+                    Some(ast::CfgAtomKey::False) => CfgAtom::Flag(sym::false_),
+                    Some(ast::CfgAtomKey::Ident(key)) => {
+                        let key = Symbol::intern(key.text());
+                        match atom.string_token().and_then(ast::String::cast) {
+                            Some(value) => {
+                                if let Ok(value) = value.value() {
+                                    CfgAtom::KeyValue { key, value: Symbol::intern(&value) }
+                                } else {
+                                    return CfgExpr::Invalid;
+                                }
+                            }
+                            None => CfgAtom::Flag(key),
+                        }
+                    }
+                    None => return CfgExpr::Invalid,
+                };
+                CfgExpr::Atom(atom)
+            }
+            ast::CfgPredicate::CfgComposite(composite) => {
+                let Some(keyword) = composite.keyword() else {
+                    return CfgExpr::Invalid;
+                };
+                match keyword.text() {
+                    "all" => CfgExpr::All(
+                        composite.cfg_predicates().map(CfgExpr::parse_from_ast).collect(),
+                    ),
+                    "any" => CfgExpr::Any(
+                        composite.cfg_predicates().map(CfgExpr::parse_from_ast).collect(),
+                    ),
+                    "not" => {
+                        let mut inner = composite.cfg_predicates();
+                        let (Some(inner), None) = (inner.next(), inner.next()) else {
+                            return CfgExpr::Invalid;
+                        };
+                        CfgExpr::Not(Box::new(CfgExpr::parse_from_ast(inner)))
+                    }
+                    _ => CfgExpr::Invalid,
+                }
+            }
+        }
     }
 
     /// Fold the cfg by querying all basic `Atom` and `KeyValue` predicates.
@@ -128,65 +172,6 @@ impl CfgExpr {
     }
 }
 
-#[cfg(feature = "syntax")]
-fn next_cfg_expr_from_ast(
-    it: &mut std::iter::Peekable<syntax::ast::TokenTreeChildren>,
-) -> Option<CfgExpr> {
-    use intern::sym;
-    use syntax::{NodeOrToken, SyntaxKind, T, ast};
-
-    let name = match it.next() {
-        None => return None,
-        Some(NodeOrToken::Token(ident)) if ident.kind().is_any_identifier() => {
-            Symbol::intern(ident.text())
-        }
-        Some(_) => return Some(CfgExpr::Invalid),
-    };
-
-    let ret = match it.peek() {
-        Some(NodeOrToken::Token(eq)) if eq.kind() == T![=] => {
-            it.next();
-            if let Some(NodeOrToken::Token(literal)) = it.peek()
-                && matches!(literal.kind(), SyntaxKind::STRING)
-            {
-                let dummy_span = span::Span {
-                    range: span::TextRange::empty(span::TextSize::new(0)),
-                    anchor: span::SpanAnchor {
-                        file_id: span::EditionedFileId::from_raw(0),
-                        ast_id: span::FIXUP_ERASED_FILE_AST_ID_MARKER,
-                    },
-                    ctx: span::SyntaxContext::root(span::Edition::Edition2015),
-                };
-                let literal =
-                    Symbol::intern(tt::token_to_literal(literal.text(), dummy_span).text());
-                it.next();
-                CfgAtom::KeyValue { key: name, value: literal.clone() }.into()
-            } else {
-                return Some(CfgExpr::Invalid);
-            }
-        }
-        Some(NodeOrToken::Node(subtree)) => {
-            let mut subtree_iter = ast::TokenTreeChildren::new(subtree).peekable();
-            it.next();
-            let mut subs = std::iter::from_fn(|| next_cfg_expr_from_ast(&mut subtree_iter));
-            match name {
-                s if s == sym::all => CfgExpr::All(subs.collect()),
-                s if s == sym::any => CfgExpr::Any(subs.collect()),
-                s if s == sym::not => {
-                    CfgExpr::Not(Box::new(subs.next().unwrap_or(CfgExpr::Invalid)))
-                }
-                _ => CfgExpr::Invalid,
-            }
-        }
-        _ => CfgAtom::Flag(name).into(),
-    };
-
-    // Eat comma separator
-    while it.next().is_some_and(|it| it.as_token().is_none_or(|it| it.kind() != T![,])) {}
-
-    Some(ret)
-}
-
 #[cfg(feature = "tt")]
 fn next_cfg_expr(it: &mut tt::iter::TtIter<'_>) -> Option<CfgExpr> {
     use intern::sym;

crates/cfg/src/cfg_expr.rs

@@ -1,10 +1,7 @@
 use arbitrary::{Arbitrary, Unstructured};
 use expect_test::{Expect, expect};
 use intern::Symbol;
-use syntax::{
-    AstNode, Edition,
-    ast::{self, TokenTreeChildren},
-};
+use syntax::{AstNode, Edition, ast};
 use syntax_bridge::{
     DocCommentDesugarMode,
     dummy_test_span_utils::{DUMMY, DummyTestSpanMap},
@@ -14,50 +11,50 @@ use syntax_bridge::{
 use crate::{CfgAtom, CfgExpr, CfgOptions, DnfExpr};
 
 #[track_caller]
-fn parse_ast_cfg(tt: &ast::TokenTree) -> CfgExpr {
-    CfgExpr::parse_from_ast(&mut TokenTreeChildren::new(tt).peekable())
+fn parse_ast_cfg(pred: ast::CfgPredicate) -> CfgExpr {
+    CfgExpr::parse_from_ast(pred)
 }
 
 #[track_caller]
 fn assert_parse_result(input: &str, expected: CfgExpr) {
-    let source_file = ast::SourceFile::parse(input, Edition::CURRENT).ok().unwrap();
-    let tt_ast = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
+    let source_file = ast::SourceFile::parse(input, Edition::CURRENT).syntax_node();
+    let pred_ast = source_file.descendants().find_map(ast::CfgPredicate::cast).unwrap();
     let tt = syntax_node_to_token_tree(
-        tt_ast.syntax(),
+        pred_ast.syntax(),
         DummyTestSpanMap,
         DUMMY,
         DocCommentDesugarMode::ProcMacro,
     );
     let cfg = CfgExpr::parse(&tt);
     assert_eq!(cfg, expected);
-    let cfg = parse_ast_cfg(&tt_ast);
+    let cfg = parse_ast_cfg(pred_ast);
     assert_eq!(cfg, expected);
 }
 
 #[track_caller]
 fn check_dnf(input: &str, expect: Expect) {
     let source_file = ast::SourceFile::parse(input, Edition::CURRENT).ok().unwrap();
-    let tt_ast = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
+    let pred_ast = source_file.syntax().descendants().find_map(ast::CfgPredicate::cast).unwrap();
     let tt = syntax_node_to_token_tree(
-        tt_ast.syntax(),
+        pred_ast.syntax(),
         DummyTestSpanMap,
         DUMMY,
         DocCommentDesugarMode::ProcMacro,
     );
     let cfg = CfgExpr::parse(&tt);
     let actual = format!("#![cfg({})]", DnfExpr::new(&cfg));
     expect.assert_eq(&actual);
-    let cfg = parse_ast_cfg(&tt_ast);
+    let cfg = parse_ast_cfg(pred_ast);
     let actual = format!("#![cfg({})]", DnfExpr::new(&cfg));
     expect.assert_eq(&actual);
 }
 
 #[track_caller]
 fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
     let source_file = ast::SourceFile::parse(input, Edition::CURRENT).ok().unwrap();
-    let tt_ast = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
+    let pred_ast = source_file.syntax().descendants().find_map(ast::CfgPredicate::cast).unwrap();
     let tt = syntax_node_to_token_tree(
-        tt_ast.syntax(),
+        pred_ast.syntax(),
         DummyTestSpanMap,
         DUMMY,
         DocCommentDesugarMode::ProcMacro,
@@ -66,7 +63,7 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
     let dnf = DnfExpr::new(&cfg);
     let why_inactive = dnf.why_inactive(opts).unwrap().to_string();
     expect.assert_eq(&why_inactive);
-    let cfg = parse_ast_cfg(&tt_ast);
+    let cfg = parse_ast_cfg(pred_ast);
     let dnf = DnfExpr::new(&cfg);
     let why_inactive = dnf.why_inactive(opts).unwrap().to_string();
     expect.assert_eq(&why_inactive);
@@ -75,9 +72,9 @@ fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
 #[track_caller]
 fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
     let source_file = ast::SourceFile::parse(input, Edition::CURRENT).ok().unwrap();
-    let tt_ast = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
+    let pred_ast = source_file.syntax().descendants().find_map(ast::CfgPredicate::cast).unwrap();
     let tt = syntax_node_to_token_tree(
-        tt_ast.syntax(),
+        pred_ast.syntax(),
         DummyTestSpanMap,
         DUMMY,
         DocCommentDesugarMode::ProcMacro,
@@ -86,7 +83,7 @@ fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
     let dnf = DnfExpr::new(&cfg);
     let hints = dnf.compute_enable_hints(opts).map(|diff| diff.to_string()).collect::<Vec<_>>();
     assert_eq!(hints, expected_hints);
-    let cfg = parse_ast_cfg(&tt_ast);
+    let cfg = parse_ast_cfg(pred_ast);
     let dnf = DnfExpr::new(&cfg);
     let hints = dnf.compute_enable_hints(opts).map(|diff| diff.to_string()).collect::<Vec<_>>();
     assert_eq!(hints, expected_hints);
@@ -119,20 +116,6 @@ fn test_cfg_expr_parser() {
             .into_boxed_slice(),
         ),
     );
-
-    assert_parse_result(
-        r#"#![cfg(any(not(), all(), , bar = "baz",))]"#,
-        CfgExpr::Any(
-            vec![
-                CfgExpr::Not(Box::new(CfgExpr::Invalid)),
-                CfgExpr::All(Box::new([])),
-                CfgExpr::Invalid,
-                CfgAtom::KeyValue { key: Symbol::intern("bar"), value: Symbol::intern("baz") }
-                    .into(),
-            ]
-            .into_boxed_slice(),
-        ),
-    );
 }
 
 #[test]