diff --git a/.github/actions/embed/Dockerfile b/.github/actions/embed/Dockerfile index 13ee7c7250..d1b0e30f19 100644 --- a/.github/actions/embed/Dockerfile +++ b/.github/actions/embed/Dockerfile @@ -10,6 +10,6 @@ ENV RUSTUP_HOME=/rust ENV CARGO_HOME=/cargo ENV PATH=/cargo/bin:/rust/bin:$PATH -RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --no-modify-path && cargo install cargo-expand ENTRYPOINT [ "/cargo/bin/cargo", "test", "--all", "--release", "--all-features", "--no-fail-fast" ] diff --git a/.github/actions/zts/Dockerfile b/.github/actions/zts/Dockerfile index e23f55bba5..28ae5d00e7 100644 --- a/.github/actions/zts/Dockerfile +++ b/.github/actions/zts/Dockerfile @@ -12,4 +12,4 @@ ENV PATH=/cargo/bin:/rust/bin:$PATH RUN (curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain nightly --no-modify-path) && rustup default nightly -ENTRYPOINT [ "/cargo/bin/cargo", "build", "--all", "--release" ] \ No newline at end of file +ENTRYPOINT [ "/cargo/bin/cargo", "build", "--all", "--release" ] diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ffb2a7bba5..3a45a3b5cb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -126,6 +126,12 @@ jobs: with: # increment this manually to force cache eviction prefix-key: ${{ env.RUST_CACHE_PREFIX }} + - name: Install Cargo expand + run: cargo install cargo-expand --locked + if: "!contains(matrix.os, 'ubuntu')" + - name: Install Cargo expand + uses: dtolnay/install@cargo-expand + if: "contains(matrix.os, 'ubuntu')" # LLVM & Clang - name: Cache LLVM and Clang id: cache-llvm diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 59e87916b1..09d93bfde7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -55,6 +55,8 @@ jobs: echo "LIBCLANG_PATH=${{ runner.temp }}/llvm-${{ env.clang }}/lib" >> $GITHUB_ENV echo "LLVM_VERSION=${{ steps.clang.outputs.version }}" >> $GITHUB_ENV echo "LLVM_CONFIG_PATH=${{ runner.temp }}/llvm-${{ env.clang }}/bin/llvm-config" >> $GITHUB_ENV + - name: Install Cargo expand + uses: dtolnay/install@cargo-expand - name: Install tarpaulin run: | cargo install cargo-tarpaulin --locked diff --git a/.lefthook.yml b/.lefthook.yml index df869bf13b..236e7d3da4 100644 --- a/.lefthook.yml +++ b/.lefthook.yml @@ -4,6 +4,8 @@ pre-commit: - name: fmt run: rustfmt --edition 2021 {staged_files} glob: "*.rs" + exclude: + - "crates/macros/tests/expand/*.expanded.rs" stage_fixed: true - name: clippy run: cargo clippy --workspace --all-targets --all-features -- -W clippy::pedantic -D warnings diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7a19c2026e..a1ec6baa6d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,17 @@ We have both unit and integration tests. When contributing, please ensure that y covered by an integration test. If possible, add unit tests as well. This might not always be possible due to the need of a running PHP interpreter. +### Testing macros + +To test macro expansion, we use [`runtime-macros`](https://github.com/jeremydavis519/runtime-macros) in conjunction +with the [`macro-test`](https://github.com/eupn/macrotest) crate. + +To add new tests add a file inside the `tests/expand` directory. After running the tests, a new `.expanded.rs` +file will be created in the same directory. This file contains the expanded macro code. Verify that the +expanded code is correct and that it matches the expected output. Commit the expanded file as well. + +If creating a new macro it needs to be added to the test contained at the bottom of the `crates/macros/src/lib.rs` file. + ### State of unit tests There are still large parts of the library that are not covered by unit tests. We strive to cover as much as possible, but this is a work in progress. If you make changes to untested code, we would diff --git a/crates/macros/Cargo.toml b/crates/macros/Cargo.toml index e79a703d5a..276830c3c0 100644 --- a/crates/macros/Cargo.toml +++ b/crates/macros/Cargo.toml @@ -22,3 +22,8 @@ convert_case = "0.8.0" [lints.rust] missing_docs = "warn" + +[dev-dependencies] +glob = "0.3.2" +macrotest = "1.1.0" +runtime-macros = "1.1.1" diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index f4b0f72aff..d058018916 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -12,9 +12,8 @@ mod syn_ext; mod zval; use proc_macro::TokenStream; -use syn::{ - parse_macro_input, DeriveInput, ItemConst, ItemFn, ItemForeignMod, ItemImpl, ItemStruct, -}; +use proc_macro2::TokenStream as TokenStream2; +use syn::{DeriveInput, ItemConst, ItemFn, ItemForeignMod, ItemImpl, ItemStruct}; extern crate proc_macro; @@ -203,14 +202,17 @@ extern crate proc_macro; // END DOCS FROM classes.md #[proc_macro_attribute] pub fn php_class(args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemStruct); + php_class_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_class_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemStruct); if !args.is_empty() { - return err!(input => "`#[php_class()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error().into(); + return err!(input => "`#[php_class()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error(); } - class::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + class::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM function.md @@ -371,14 +373,17 @@ pub fn php_class(args: TokenStream, input: TokenStream) -> TokenStream { // END DOCS FROM function.md #[proc_macro_attribute] pub fn php_function(args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemFn); + php_function_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_function_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemFn); if !args.is_empty() { - return err!(input => "`#[php_function()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error().into(); + return err!(input => "`#[php_function()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error(); } - function::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + function::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM constant.md @@ -436,14 +441,17 @@ pub fn php_function(args: TokenStream, input: TokenStream) -> TokenStream { // END DOCS FROM constant.md #[proc_macro_attribute] pub fn php_const(args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemConst); + php_const_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_const_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemConst); if !args.is_empty() { - return err!(input => "`#[php_const()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error().into(); + return err!(input => "`#[php_const()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error(); } - constant::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + constant::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM module.md @@ -516,14 +524,17 @@ pub fn php_const(args: TokenStream, input: TokenStream) -> TokenStream { // END DOCS FROM module.md #[proc_macro_attribute] pub fn php_module(args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemFn); + php_module_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_module_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemFn); if !args.is_empty() { - return err!(input => "`#[php_module()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error().into(); + return err!(input => "`#[php_module()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error(); } - module::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + module::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM impl.md @@ -713,14 +724,17 @@ pub fn php_module(args: TokenStream, input: TokenStream) -> TokenStream { // END DOCS FROM impl.md #[proc_macro_attribute] pub fn php_impl(args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemImpl); + php_impl_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_impl_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemImpl); if !args.is_empty() { - return err!(input => "`#[php_impl()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error().into(); + return err!(input => "`#[php_impl()]` args are no longer supported. Please use `#[php()]` instead.").to_compile_error(); } - impl_::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + impl_::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM extern.md @@ -789,12 +803,15 @@ pub fn php_impl(args: TokenStream, input: TokenStream) -> TokenStream { /// [`Zval`]: crate::types::Zval // END DOCS FROM extern.md #[proc_macro_attribute] -pub fn php_extern(_: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemForeignMod); +pub fn php_extern(args: TokenStream, input: TokenStream) -> TokenStream { + php_extern_internal(args.into(), input.into()).into() +} + +#[allow(clippy::needless_pass_by_value)] +fn php_extern_internal(_: TokenStream2, input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemForeignMod); - extern_::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() + extern_::parser(input).unwrap_or_else(|e| e.to_compile_error()) } // BEGIN DOCS FROM zval_convert.md @@ -953,11 +970,13 @@ pub fn php_extern(_: TokenStream, input: TokenStream) -> TokenStream { // END DOCS FROM zval_convert.md #[proc_macro_derive(ZvalConvert)] pub fn zval_convert_derive(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as DeriveInput); + zval_convert_derive_internal(input.into()).into() +} - zval::parser(input) - .unwrap_or_else(|e| e.to_compile_error()) - .into() +fn zval_convert_derive_internal(input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as DeriveInput); + + zval::parser(input).unwrap_or_else(|e| e.to_compile_error()) } /// Defines an `extern` function with the Zend fastcall convention based on @@ -992,35 +1011,61 @@ pub fn zval_convert_derive(input: TokenStream) -> TokenStream { /// Rust and the `abi_vectorcall` feature enabled. #[proc_macro] pub fn zend_fastcall(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemFn); + zend_fastcall_internal(input.into()).into() +} - fastcall::parser(input).into() +fn zend_fastcall_internal(input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as ItemFn); + + fastcall::parser(input) } /// Wraps a function to be used in the [`Module::function`] method. #[proc_macro] pub fn wrap_function(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as syn::Path); + wrap_function_internal(input.into()).into() +} + +fn wrap_function_internal(input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as syn::Path); match function::wrap(&input) { Ok(parsed) => parsed, Err(e) => e.to_compile_error(), } - .into() } /// Wraps a constant to be used in the [`ModuleBuilder::constant`] method. #[proc_macro] pub fn wrap_constant(input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as syn::Path); + wrap_constant_internal(input.into()).into() +} + +fn wrap_constant_internal(input: TokenStream2) -> TokenStream2 { + let input = parse_macro_input2!(input as syn::Path); match constant::wrap(&input) { Ok(parsed) => parsed, Err(e) => e.to_compile_error(), } - .into() } +macro_rules! parse_macro_input2 { + ($tokenstream:ident as $ty:ty) => { + match syn::parse2::<$ty>($tokenstream) { + Ok(data) => data, + Err(err) => { + return proc_macro2::TokenStream::from(err.to_compile_error()); + } + } + }; + ($tokenstream:ident) => { + $crate::parse_macro_input!($tokenstream as _) + }; +} + +pub(crate) use parse_macro_input2; + macro_rules! err { ($span:expr => $($msg:tt)*) => { ::syn::Error::new(::syn::spanned::Spanned::span(&$span), format!($($msg)*)) @@ -1061,3 +1106,62 @@ pub(crate) mod prelude { pub(crate) use crate::{bail, err}; pub(crate) type Result = std::result::Result; } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + type AttributeFn = + fn(proc_macro2::TokenStream, proc_macro2::TokenStream) -> proc_macro2::TokenStream; + type FunctionLikeFn = fn(proc_macro2::TokenStream) -> proc_macro2::TokenStream; + + #[test] + pub fn test_expand() { + macrotest::expand("tests/expand/*.rs"); + for entry in glob::glob("tests/expand/*.rs").expect("Failed to read expand tests glob") { + let entry = entry.expect("Failed to read expand test file"); + runtime_expand_attr(&entry); + runtime_expand_func(&entry); + runtime_expand_derive(&entry); + } + } + + fn runtime_expand_attr(path: &PathBuf) { + let file = std::fs::File::open(path).expect("Failed to open expand test file"); + runtime_macros::emulate_attributelike_macro_expansion( + file, + &[ + ("php_class", php_class_internal as AttributeFn), + ("php_const", php_const_internal as AttributeFn), + ("php_extern", php_extern_internal as AttributeFn), + ("php_function", php_function_internal as AttributeFn), + ("php_impl", php_impl_internal as AttributeFn), + ("php_module", php_module_internal as AttributeFn), + ], + ) + .expect("Failed to expand attribute macros in test file"); + } + + fn runtime_expand_func(path: &PathBuf) { + let file = std::fs::File::open(path).expect("Failed to open expand test file"); + runtime_macros::emulate_functionlike_macro_expansion( + file, + &[ + ("zend_fastcall", zend_fastcall_internal as FunctionLikeFn), + ("wrap_function", wrap_function_internal as FunctionLikeFn), + ("wrap_constant", wrap_constant_internal as FunctionLikeFn), + ], + ) + .expect("Failed to expand function-like macros in test file"); + } + + fn runtime_expand_derive(path: &PathBuf) { + let file = std::fs::File::open(path).expect("Failed to open expand test file"); + runtime_macros::emulate_derive_macro_expansion( + file, + &[("ZvalConvert", zval_convert_derive_internal)], + ) + .expect("Failed to expand derive macros in test file"); + } +} diff --git a/crates/macros/tests/expand/const.expanded.rs b/crates/macros/tests/expand/const.expanded.rs new file mode 100644 index 0000000000..d8cf62817d --- /dev/null +++ b/crates/macros/tests/expand/const.expanded.rs @@ -0,0 +1,10 @@ +#[macro_use] +extern crate ext_php_rs_derive; +const MY_CONST: &str = "Hello, world!"; +#[allow(non_upper_case_globals)] +const _internal_const_docs_MY_CONST: &[&str] = &[]; +#[allow(non_upper_case_globals)] +const _internal_const_name_MY_CONST: &str = "MY_CONST"; +fn main() { + (_internal_const_name_MY_CONST, MY_CONST, _internal_const_docs_MY_CONST); +} diff --git a/crates/macros/tests/expand/const.rs b/crates/macros/tests/expand/const.rs new file mode 100644 index 0000000000..740d59bb62 --- /dev/null +++ b/crates/macros/tests/expand/const.rs @@ -0,0 +1,9 @@ +#[macro_use] +extern crate ext_php_rs_derive; + +#[php_const] +const MY_CONST: &str = "Hello, world!"; + +fn main() { + wrap_constant!(MY_CONST); +}