From 093f99b809d377274e6626a24124caa741ce07f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 9 Oct 2021 13:42:32 +0300 Subject: [PATCH] internal: start new macro test suite I don't like our macro tests -- they are brittle and don't inspire confidence. I think the reason for that is that we try to unit-test them, but that is at odds with reality, where macro expansion fundamentally depends on name resolution. --- crates/hir_def/src/lib.rs | 2 + crates/hir_def/src/macro_expansion_tests.rs | 87 +++++++++++++++++++++ crates/hir_expand/src/lib.rs | 2 +- crates/mbe/src/expander.rs | 61 --------------- 4 files changed, 90 insertions(+), 62 deletions(-) create mode 100644 crates/hir_def/src/macro_expansion_tests.rs diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs index afcf1bc7078..7d5f63afd04 100644 --- a/crates/hir_def/src/lib.rs +++ b/crates/hir_def/src/lib.rs @@ -49,6 +49,8 @@ macro_rules! eprintln { #[cfg(test)] mod test_db; +#[cfg(test)] +mod macro_expansion_tests; use std::{ hash::{Hash, Hasher}, diff --git a/crates/hir_def/src/macro_expansion_tests.rs b/crates/hir_def/src/macro_expansion_tests.rs new file mode 100644 index 00000000000..861a059a14c --- /dev/null +++ b/crates/hir_def/src/macro_expansion_tests.rs @@ -0,0 +1,87 @@ +//! This module contains tests for macro expansion. Effectively, it covers `tt`, +//! `mbe`, `proc_macro_api` and `hir_expand` crates. This might seem like a +//! wrong architecture at the first glance, but is intentional. +//! +//! Physically, macro expansion process is intertwined with name resolution. You +//! can not expand *just* the syntax. So, to be able to write integration tests +//! of the "expand this code please" form, we have to do it after name +//! resolution. That is, in this crate. We *could* fake some dependencies and +//! write unit-tests (in fact, we used to do that), but that makes tests brittle +//! and harder to understand. + +use std::ops::Range; + +use base_db::{fixture::WithFixture, SourceDatabase}; +use expect_test::{expect, Expect}; +use hir_expand::{db::AstDatabase, InFile, MacroFile}; +use stdx::format_to; +use syntax::{ast, AstNode}; + +use crate::{ + db::DefDatabase, nameres::ModuleSource, resolver::HasResolver, test_db::TestDB, AsMacroCall, +}; + +fn check(ra_fixture: &str, expect: Expect) { + let db = TestDB::with_files(ra_fixture); + let krate = db.crate_graph().iter().next().unwrap(); + let def_map = db.crate_def_map(krate); + let local_id = def_map.root(); + let module = def_map.module_id(local_id); + let resolver = module.resolver(&db); + let source = def_map[local_id].definition_source(&db); + let source_file = match source.value { + ModuleSource::SourceFile(it) => it, + ModuleSource::Module(_) | ModuleSource::BlockExpr(_) => panic!(), + }; + + let mut expansions = Vec::new(); + for macro_call in source_file.syntax().descendants().filter_map(ast::MacroCall::cast) { + let macro_call = InFile::new(source.file_id, ¯o_call); + let macro_call_id = macro_call + .as_call_id_with_errors( + &db, + krate, + |path| resolver.resolve_path_as_macro(&db, &path), + &mut |err| panic!("{}", err), + ) + .unwrap() + .unwrap(); + let macro_file = MacroFile { macro_call_id }; + let expansion_result = db.parse_macro_expansion(macro_file); + expansions.push((macro_call.value.clone(), expansion_result)); + } + + let mut expanded_text = source_file.to_string(); + for (call, exp) in expansions.into_iter().rev() { + let mut expn_text = String::new(); + if let Some(err) = exp.err { + format_to!(expn_text, "/* error: {} */", err); + } + if let Some((parse, _token_map)) = exp.value { + format_to!(expn_text, "{}", parse.syntax_node()); + } + let range = call.syntax().text_range(); + let range: Range = range.into(); + expanded_text.replace_range(range, &expn_text) + } + + expect.assert_eq(&expanded_text); +} + +#[test] +fn test_expand_rule() { + check( + r#" +macro_rules! m { + ($($i:ident);*) => ($i) +} +m!{a} +"#, + expect![[r#" +macro_rules! m { + ($($i:ident);*) => ($i) +} +/* error: expected simple binding, found nested binding `i` */ +"#]], + ); +} diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index d903725d884..ba4ef8b70fe 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -204,7 +204,7 @@ pub fn is_macro(self) -> bool { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct MacroFile { - macro_call_id: MacroCallId, + pub macro_call_id: MacroCallId, } /// `MacroCallId` identifies a particular macro invocation, like diff --git a/crates/mbe/src/expander.rs b/crates/mbe/src/expander.rs index 230d33e538c..7244d211610 100644 --- a/crates/mbe/src/expander.rs +++ b/crates/mbe/src/expander.rs @@ -114,64 +114,3 @@ enum Fragment { /// like `$i * 2` where `$i = 1 + 1` work as expectd. Ast(tt::TokenTree), } - -#[cfg(test)] -mod tests { - use syntax::{ast, AstNode}; - - use super::*; - use crate::syntax_node_to_token_tree; - - #[test] - fn test_expand_rule() { - assert_err( - "($($i:ident);*) => ($i)", - "foo!{a}", - ExpandError::BindingError(String::from( - "expected simple binding, found nested binding `i`", - )), - ); - - // FIXME: - // Add an err test case for ($($i:ident)) => ($()) - } - - fn assert_err(macro_body: &str, invocation: &str, err: ExpandError) { - assert_eq!( - expand_first(&create_rules(&format_macro(macro_body)), invocation).err, - Some(err) - ); - } - - fn format_macro(macro_body: &str) -> String { - format!( - " - macro_rules! foo {{ - {} - }} -", - macro_body - ) - } - - fn create_rules(macro_definition: &str) -> crate::MacroRules { - let source_file = ast::SourceFile::parse(macro_definition).ok().unwrap(); - let macro_definition = - source_file.syntax().descendants().find_map(ast::MacroRules::cast).unwrap(); - - let (definition_tt, _) = - syntax_node_to_token_tree(macro_definition.token_tree().unwrap().syntax()); - crate::MacroRules::parse(&definition_tt).unwrap() - } - - fn expand_first(rules: &crate::MacroRules, invocation: &str) -> ExpandResult { - let source_file = ast::SourceFile::parse(invocation).ok().unwrap(); - let macro_invocation = - source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap(); - - let (invocation_tt, _) = - syntax_node_to_token_tree(macro_invocation.token_tree().unwrap().syntax()); - - expand_rules(&rules.rules, &invocation_tt) - } -}