Unnamed repository; edit this file 'description' to name the repository.
Review Updates and added tests.
| -rw-r--r-- | crates/cfg/Cargo.toml | 2 | ||||
| -rw-r--r-- | crates/cfg/src/cfg_expr.rs | 77 | ||||
| -rw-r--r-- | crates/cfg/src/tests.rs | 26 | ||||
| -rw-r--r-- | crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs | 118 | ||||
| -rw-r--r-- | crates/hir-expand/src/cfg_process.rs | 171 | ||||
| -rw-r--r-- | crates/hir-expand/src/db.rs | 6 |
6 files changed, 274 insertions, 126 deletions
diff --git a/crates/cfg/Cargo.toml b/crates/cfg/Cargo.toml index 059fd85440..9b3a5026ac 100644 --- a/crates/cfg/Cargo.toml +++ b/crates/cfg/Cargo.toml @@ -16,7 +16,6 @@ rustc-hash.workspace = true # locals deps tt.workspace = true -syntax.workspace = true [dev-dependencies] expect-test = "1.4.1" @@ -29,6 +28,7 @@ derive_arbitrary = "1.3.2" # local deps mbe.workspace = true +syntax.workspace = true [lints] workspace = true diff --git a/crates/cfg/src/cfg_expr.rs b/crates/cfg/src/cfg_expr.rs index 91731c29d2..b7dbb7b5fd 100644 --- a/crates/cfg/src/cfg_expr.rs +++ b/crates/cfg/src/cfg_expr.rs @@ -2,12 +2,8 @@ //! //! See: <https://doc.rust-lang.org/reference/conditional-compilation.html#conditional-compilation> -use std::{fmt, iter::Peekable, slice::Iter as SliceIter}; +use std::{fmt, slice::Iter as SliceIter}; -use syntax::{ - ast::{self, Meta}, - NodeOrToken, -}; use tt::SmolStr; /// A simple configuration value passed in from the outside. @@ -51,12 +47,7 @@ impl CfgExpr { pub fn parse<S>(tt: &tt::Subtree<S>) -> CfgExpr { next_cfg_expr(&mut tt.token_trees.iter()).unwrap_or(CfgExpr::Invalid) } - /// Parses a `cfg` attribute from the meta - pub fn parse_from_attr_meta(meta: Meta) -> Option<CfgExpr> { - let tt = meta.token_tree()?; - let mut iter = tt.token_trees_and_tokens().skip(1).peekable(); - next_cfg_expr_from_syntax(&mut iter) - } + /// Fold the cfg by querying all basic `Atom` and `KeyValue` predicates. pub fn fold(&self, query: &dyn Fn(&CfgAtom) -> bool) -> Option<bool> { match self { @@ -72,70 +63,6 @@ impl CfgExpr { } } } -fn next_cfg_expr_from_syntax<I>(iter: &mut Peekable<I>) -> Option<CfgExpr> -where - I: Iterator<Item = NodeOrToken<ast::TokenTree, syntax::SyntaxToken>>, -{ - let name = match iter.next() { - None => return None, - Some(NodeOrToken::Token(element)) => match element.kind() { - syntax::T![ident] => SmolStr::new(element.text()), - _ => return Some(CfgExpr::Invalid), - }, - Some(_) => return Some(CfgExpr::Invalid), - }; - let result = match name.as_str() { - "all" | "any" | "not" => { - let mut preds = Vec::new(); - let Some(NodeOrToken::Node(tree)) = iter.next() else { - return Some(CfgExpr::Invalid); - }; - let mut tree_iter = tree.token_trees_and_tokens().skip(1).peekable(); - while tree_iter - .peek() - .filter( - |element| matches!(element, NodeOrToken::Token(token) if (token.kind() != syntax::T![')'])), - ) - .is_some() - { - let pred = next_cfg_expr_from_syntax(&mut tree_iter); - if let Some(pred) = pred { - preds.push(pred); - } - } - let group = match name.as_str() { - "all" => CfgExpr::All(preds), - "any" => CfgExpr::Any(preds), - "not" => CfgExpr::Not(Box::new(preds.pop().unwrap_or(CfgExpr::Invalid))), - _ => unreachable!(), - }; - Some(group) - } - _ => match iter.peek() { - Some(NodeOrToken::Token(element)) if (element.kind() == syntax::T![=]) => { - iter.next(); - match iter.next() { - Some(NodeOrToken::Token(value_token)) - if (value_token.kind() == syntax::SyntaxKind::STRING) => - { - let value = value_token.text(); - let value = SmolStr::new(value.trim_matches('"')); - Some(CfgExpr::Atom(CfgAtom::KeyValue { key: name, value })) - } - _ => None, - } - } - _ => Some(CfgExpr::Atom(CfgAtom::Flag(name))), - }, - }; - if let Some(NodeOrToken::Token(element)) = iter.peek() { - if element.kind() == syntax::T![,] { - iter.next(); - } - } - result -} - fn next_cfg_expr<S>(it: &mut SliceIter<'_, tt::TokenTree<S>>) -> Option<CfgExpr> { let name = match it.next() { None => return None, diff --git a/crates/cfg/src/tests.rs b/crates/cfg/src/tests.rs index 8eca907d8b..62fb429a63 100644 --- a/crates/cfg/src/tests.rs +++ b/crates/cfg/src/tests.rs @@ -1,10 +1,7 @@ use arbitrary::{Arbitrary, Unstructured}; use expect_test::{expect, Expect}; use mbe::{syntax_node_to_token_tree, DummyTestSpanMap, DUMMY}; -use syntax::{ - ast::{self, Attr}, - AstNode, SourceFile, -}; +use syntax::{ast, AstNode}; use crate::{CfgAtom, CfgExpr, CfgOptions, DnfExpr}; @@ -15,22 +12,6 @@ fn assert_parse_result(input: &str, expected: CfgExpr) { let cfg = CfgExpr::parse(&tt); assert_eq!(cfg, expected); } -fn check_dnf_from_syntax(input: &str, expect: Expect) { - let parse = SourceFile::parse(input); - let node = match parse.tree().syntax().descendants().find_map(Attr::cast) { - Some(it) => it, - None => { - let node = std::any::type_name::<Attr>(); - panic!("Failed to make ast node `{node}` from text {input}") - } - }; - let node = node.clone_subtree(); - assert_eq!(node.syntax().text_range().start(), 0.into()); - - let cfg = CfgExpr::parse_from_attr_meta(node.meta().unwrap()).unwrap(); - let actual = format!("#![cfg({})]", DnfExpr::new(cfg)); - expect.assert_eq(&actual); -} fn check_dnf(input: &str, expect: Expect) { let source_file = ast::SourceFile::parse(input).ok().unwrap(); @@ -105,11 +86,6 @@ fn smoke() { check_dnf("#![cfg(not(a))]", expect![[r#"#![cfg(not(a))]"#]]); } -#[test] -fn cfg_from_attr() { - check_dnf_from_syntax(r#"#[cfg(test)]"#, expect![[r#"#![cfg(test)]"#]]); - check_dnf_from_syntax(r#"#[cfg(not(never))]"#, expect![[r#"#![cfg(not(never))]"#]]); -} #[test] fn distribute() { diff --git a/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs b/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs index 86b4466153..89c1b44608 100644 --- a/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs +++ b/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs @@ -528,3 +528,121 @@ impl < > $crate::fmt::Debug for Command< > where { }"#]], ); } +#[test] +fn test_debug_expand_with_cfg() { + check( + r#" + //- minicore: derive, fmt + use core::fmt::Debug; + + #[derive(Debug)] + struct HideAndShow { + #[cfg(never)] + always_hide: u32, + #[cfg(not(never))] + always_show: u32, + } + #[derive(Debug)] + enum HideAndShowEnum { + #[cfg(never)] + AlwaysHide, + #[cfg(not(never))] + AlwaysShow{ + #[cfg(never)] + always_hide: u32, + #[cfg(not(never))] + always_show: u32, + } + } + "#, + expect![[r#" +use core::fmt::Debug; + +#[derive(Debug)] +struct HideAndShow { + #[cfg(never)] + always_hide: u32, + #[cfg(not(never))] + always_show: u32, +} +#[derive(Debug)] +enum HideAndShowEnum { + #[cfg(never)] + AlwaysHide, + #[cfg(not(never))] + AlwaysShow{ + #[cfg(never)] + always_hide: u32, + #[cfg(not(never))] + always_show: u32, + } +} + +impl < > $crate::fmt::Debug for HideAndShow< > where { + fn fmt(&self , f: &mut $crate::fmt::Formatter) -> $crate::fmt::Result { + match self { + HideAndShow { + always_show: always_show, + } + =>f.debug_struct("HideAndShow").field("always_show", &always_show).finish() + } + } +} +impl < > $crate::fmt::Debug for HideAndShowEnum< > where { + fn fmt(&self , f: &mut $crate::fmt::Formatter) -> $crate::fmt::Result { + match self { + HideAndShowEnum::AlwaysShow { + always_show: always_show, + } + =>f.debug_struct("AlwaysShow").field("always_show", &always_show).finish(), + } + } +}"#]], + ); +} +#[test] +fn test_default_expand_with_cfg() { + check( + r#" +//- minicore: derive, default +#[derive(Default)] +struct Foo { + field1: i32, + #[cfg(never)] + field2: (), +} +#[derive(Default)] +enum Bar { + Foo, + #[cfg_attr(not(never), default)] + Bar, +} +"#, + expect![[r#" +#[derive(Default)] +struct Foo { + field1: i32, + #[cfg(never)] + field2: (), +} +#[derive(Default)] +enum Bar { + Foo, + #[cfg_attr(not(never), default)] + Bar, +} + +impl < > $crate::default::Default for Foo< > where { + fn default() -> Self { + Foo { + field1: $crate::default::Default::default(), + } + } +} +impl < > $crate::default::Default for Bar< > where { + fn default() -> Self { + Bar::Bar + } +}"#]], + ); +} diff --git a/crates/hir-expand/src/cfg_process.rs b/crates/hir-expand/src/cfg_process.rs index 1d088ada64..c74c13a6fd 100644 --- a/crates/hir-expand/src/cfg_process.rs +++ b/crates/hir-expand/src/cfg_process.rs @@ -1,10 +1,14 @@ //! Processes out #[cfg] and #[cfg_attr] attributes from the input for the derive macro +use std::iter::Peekable; + +use cfg::{CfgAtom, CfgExpr}; use rustc_hash::FxHashSet; use syntax::{ ast::{self, Attr, HasAttrs, Meta, VariantList}, - AstNode, SyntaxElement, SyntaxNode, T, + AstNode, NodeOrToken, SyntaxElement, SyntaxNode, T, }; use tracing::{debug, warn}; +use tt::SmolStr; use crate::{db::ExpandDatabase, MacroCallKind, MacroCallLoc}; @@ -13,7 +17,7 @@ fn check_cfg_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) -> O return None; } debug!("Evaluating cfg {}", attr); - let cfg = cfg::CfgExpr::parse_from_attr_meta(attr.meta()?)?; + let cfg = parse_from_attr_meta(attr.meta()?)?; debug!("Checking cfg {:?}", cfg); let enabled = db.crate_graph()[loc.krate].cfg_options.check(&cfg) != Some(false); Some(enabled) @@ -24,7 +28,7 @@ fn check_cfg_attr_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) return None; } debug!("Evaluating cfg_attr {}", attr); - let cfg_expr = cfg::CfgExpr::parse_from_attr_meta(attr.meta()?)?; + let cfg_expr = parse_from_attr_meta(attr.meta()?)?; debug!("Checking cfg_attr {:?}", cfg_expr); let enabled = db.crate_graph()[loc.krate].cfg_options.check(&cfg_expr) != Some(false); Some(enabled) @@ -43,7 +47,7 @@ fn process_has_attrs_with_possible_comma<I: HasAttrs>( debug!("censoring type {:?}", item.syntax()); remove.insert(item.syntax().clone().into()); // We need to remove the , as well - add_comma(&item, remove); + remove_possible_comma(&item, remove); break 'attrs; } @@ -63,7 +67,18 @@ fn process_has_attrs_with_possible_comma<I: HasAttrs>( } Some(()) } - +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +enum CfgExprStage { + /// Stripping the CFGExpr part of the attribute + StrippigCfgExpr, + /// Found the comma after the CFGExpr. Will keep all tokens until the next comma or the end of the attribute + FoundComma, + /// Everything following the attribute. This could be another attribute or the end of the attribute. + // FIXME: cfg_attr with multiple attributes will not be handled correctly. We will only keep the first attribute + // Related Issue: https://github.com/rust-lang/rust-analyzer/issues/10110 + EverythingElse, +} +/// This function creates its own set of tokens to remove. To help prevent malformed syntax as input. fn remove_tokens_within_cfg_attr(meta: Meta) -> Option<FxHashSet<SyntaxElement>> { let mut remove: FxHashSet<SyntaxElement> = FxHashSet::default(); debug!("Enabling attribute {}", meta); @@ -73,36 +88,44 @@ fn remove_tokens_within_cfg_attr(meta: Meta) -> Option<FxHashSet<SyntaxElement>> let meta_tt = meta.token_tree()?; debug!("meta_tt {}", meta_tt); - // Remove the left paren - remove.insert(meta_tt.l_paren_token()?.into()); - let mut found_comma = false; - for tt in meta_tt.token_trees_and_tokens().skip(1) { - debug!("Checking {:?}", tt); - // Check if it is a subtree or a token. If it is a token check if it is a comma. If so, remove it and break. - match tt { - syntax::NodeOrToken::Node(node) => { - // Remove the entire subtree + let mut stage = CfgExprStage::StrippigCfgExpr; + for tt in meta_tt.token_trees_and_tokens() { + debug!("Checking {:?}. Stage: {:?}", tt, stage); + match (stage, tt) { + (CfgExprStage::StrippigCfgExpr, syntax::NodeOrToken::Node(node)) => { remove.insert(node.syntax().clone().into()); } - syntax::NodeOrToken::Token(token) => { + (CfgExprStage::StrippigCfgExpr, syntax::NodeOrToken::Token(token)) => { if token.kind() == T![,] { - found_comma = true; - remove.insert(token.into()); - break; + stage = CfgExprStage::FoundComma; } remove.insert(token.into()); } + (CfgExprStage::FoundComma, syntax::NodeOrToken::Token(token)) + if (token.kind() == T![,] || token.kind() == T![')']) => + { + // The end of the attribute or separator for the next attribute + stage = CfgExprStage::EverythingElse; + remove.insert(token.into()); + } + (CfgExprStage::EverythingElse, syntax::NodeOrToken::Node(node)) => { + remove.insert(node.syntax().clone().into()); + } + (CfgExprStage::EverythingElse, syntax::NodeOrToken::Token(token)) => { + remove.insert(token.into()); + } + // This is an actual attribute + _ => {} } } - if !found_comma { - warn!("No comma found in {}", meta_tt); + if stage != CfgExprStage::EverythingElse { + warn!("Invalid cfg_attr attribute. {:?}", meta_tt); return None; } - // Remove the right paren - remove.insert(meta_tt.r_paren_token()?.into()); Some(remove) } -fn add_comma(item: &impl AstNode, res: &mut FxHashSet<SyntaxElement>) { +/// Removes a possible comma after the [AstNode] +fn remove_possible_comma(item: &impl AstNode, res: &mut FxHashSet<SyntaxElement>) { if let Some(comma) = item.syntax().next_sibling_or_token().filter(|it| it.kind() == T![,]) { res.insert(comma); } @@ -120,7 +143,7 @@ fn process_enum( debug!("censoring type {:?}", variant.syntax()); remove.insert(variant.syntax().clone().into()); // We need to remove the , as well - add_comma(&variant, remove); + remove_possible_comma(&variant, remove); continue 'variant; }; @@ -202,3 +225,103 @@ pub(crate) fn process_cfg_attrs( } Some(remove) } +/// Parses a `cfg` attribute from the meta +fn parse_from_attr_meta(meta: Meta) -> Option<CfgExpr> { + let tt = meta.token_tree()?; + let mut iter = tt.token_trees_and_tokens().skip(1).peekable(); + next_cfg_expr_from_syntax(&mut iter) +} + +fn next_cfg_expr_from_syntax<I>(iter: &mut Peekable<I>) -> Option<CfgExpr> +where + I: Iterator<Item = NodeOrToken<ast::TokenTree, syntax::SyntaxToken>>, +{ + let name = match iter.next() { + None => return None, + Some(NodeOrToken::Token(element)) => match element.kind() { + syntax::T![ident] => SmolStr::new(element.text()), + _ => return Some(CfgExpr::Invalid), + }, + Some(_) => return Some(CfgExpr::Invalid), + }; + let result = match name.as_str() { + "all" | "any" | "not" => { + let mut preds = Vec::new(); + let Some(NodeOrToken::Node(tree)) = iter.next() else { + return Some(CfgExpr::Invalid); + }; + let mut tree_iter = tree.token_trees_and_tokens().skip(1).peekable(); + while tree_iter + .peek() + .filter( + |element| matches!(element, NodeOrToken::Token(token) if (token.kind() != syntax::T![')'])), + ) + .is_some() + { + let pred = next_cfg_expr_from_syntax(&mut tree_iter); + if let Some(pred) = pred { + preds.push(pred); + } + } + let group = match name.as_str() { + "all" => CfgExpr::All(preds), + "any" => CfgExpr::Any(preds), + "not" => CfgExpr::Not(Box::new(preds.pop().unwrap_or(CfgExpr::Invalid))), + _ => unreachable!(), + }; + Some(group) + } + _ => match iter.peek() { + Some(NodeOrToken::Token(element)) if (element.kind() == syntax::T![=]) => { + iter.next(); + match iter.next() { + Some(NodeOrToken::Token(value_token)) + if (value_token.kind() == syntax::SyntaxKind::STRING) => + { + let value = value_token.text(); + let value = SmolStr::new(value.trim_matches('"')); + Some(CfgExpr::Atom(CfgAtom::KeyValue { key: name, value })) + } + _ => None, + } + } + _ => Some(CfgExpr::Atom(CfgAtom::Flag(name))), + }, + }; + if let Some(NodeOrToken::Token(element)) = iter.peek() { + if element.kind() == syntax::T![,] { + iter.next(); + } + } + result +} +#[cfg(test)] +mod tests { + use cfg::DnfExpr; + use expect_test::{expect, Expect}; + use syntax::{ast::Attr, AstNode, SourceFile}; + + use crate::cfg_process::parse_from_attr_meta; + + fn check_dnf_from_syntax(input: &str, expect: Expect) { + let parse = SourceFile::parse(input); + let node = match parse.tree().syntax().descendants().find_map(Attr::cast) { + Some(it) => it, + None => { + let node = std::any::type_name::<Attr>(); + panic!("Failed to make ast node `{node}` from text {input}") + } + }; + let node = node.clone_subtree(); + assert_eq!(node.syntax().text_range().start(), 0.into()); + + let cfg = parse_from_attr_meta(node.meta().unwrap()).unwrap(); + let actual = format!("#![cfg({})]", DnfExpr::new(cfg)); + expect.assert_eq(&actual); + } + #[test] + fn cfg_from_attr() { + check_dnf_from_syntax(r#"#[cfg(test)]"#, expect![[r#"#![cfg(test)]"#]]); + check_dnf_from_syntax(r#"#[cfg(not(never))]"#, expect![[r#"#![cfg(not(never))]"#]]); + } +} diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs index bb69c72be4..a7469ae5c8 100644 --- a/crates/hir-expand/src/db.rs +++ b/crates/hir-expand/src/db.rs @@ -151,12 +151,16 @@ pub fn expand_speculative( ), MacroCallKind::Derive { .. } | MacroCallKind::Attr { .. } => { let censor = censor_for_macro_input(&loc, speculative_args); + let censor_cfg = + cfg_process::process_cfg_attrs(speculative_args, &loc, db).unwrap_or_default(); let mut fixups = fixup::fixup_syntax(span_map, speculative_args, loc.call_site); fixups.append.retain(|it, _| match it { syntax::NodeOrToken::Token(_) => true, - it => !censor.contains(it), + it => !censor.contains(it) && !censor_cfg.contains(it), }); fixups.remove.extend(censor); + fixups.remove.extend(censor_cfg); + ( mbe::syntax_node_to_token_tree_modified( speculative_args, |