Allow renaming macro attributes (#1891)

Signed-off-by: maciektr <[email protected]> Co-authored-by: Rémy Baranx <[email protected]> Co-authored-by: Maksim Zdobnikau <[email protected]>
software-mansion · Jan 17, 2025 · 56999bb · 56999bb
1 parent ce86046
commit 56999bb
Show file tree

Hide file tree

Showing 12 changed files with 242 additions and 12 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -29,7 +29,7 @@ jobs:
       - uses: Swatinem/rust-cache@v2
       - uses: taiki-e/install-action@nextest
       - name: nextest archive
-        run: cargo nextest archive --workspace --all-features --cargo-profile ci --archive-file 'nextest-archive-${{ matrix.platform.os }}.tar.zst'
+        run: cargo nextest archive --workspace --exclude cairo-lang-macro --all-features --cargo-profile ci --archive-file 'nextest-archive-${{ matrix.platform.os }}.tar.zst'
       - uses: actions/upload-artifact@v4
         with:
           name: nextest-archive-${{ matrix.platform.os }}
@@ -82,6 +82,27 @@ jobs:
       - name: run tests
         run: cargo test -p scarb-metadata
 
+  test-cairo-lang-macro:
+    name: test cairo-lang-macro ${{ matrix.platform.name }}
+    runs-on: ${{ matrix.platform.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        platform:
+          - name: linux x86-64
+            os: ubuntu-latest
+          - name: windows x86-64
+            os: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: Swatinem/rust-cache@v2
+      - name: Run tests
+        # Note tests depending on trybuild crate cannot be run with nextest,
+        # as they require access to cargo build cache of the package,
+        # which is not archived with nextest-archive.
+        run: cargo test -p cairo-lang-macro --all-features
+
   test-prebuilt-plugins:
     name: test prebuilt plugins ${{ matrix.platform.name }}
     runs-on: ${{ matrix.platform.os }}

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -149,6 +149,7 @@ tower-http = { version = "0.4", features = ["fs"] }
 tracing = "0.1"
 tracing-core = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
+trybuild = "1.0.101"
 typed-builder = ">=0.17"
 url = { version = "2", features = ["serde"] }
 walkdir = "2"

diff --git a/plugins/cairo-lang-macro-attributes/src/lib.rs b/plugins/cairo-lang-macro-attributes/src/lib.rs
@@ -2,17 +2,21 @@ use proc_macro::TokenStream;
 use quote::{quote, ToTokens};
 use scarb_stable_hash::short_hash;
 use syn::spanned::Spanned;
-use syn::{parse_macro_input, Expr, ItemFn, LitStr, Meta};
+use syn::{
+    parse::Parse, parse::ParseStream, parse_macro_input, Expr, Ident, ItemFn, LitStr, Meta, Result,
+    Token,
+};
 
 /// Constructs the attribute macro implementation.
 ///
 /// This macro hides the conversion to stable ABI structs from the user.
 ///
 /// Note, that this macro can be used multiple times, to define multiple independent attribute macros.
 #[proc_macro_attribute]
-pub fn attribute_macro(_args: TokenStream, input: TokenStream) -> TokenStream {
+pub fn attribute_macro(args: TokenStream, input: TokenStream) -> TokenStream {
     macro_helper(
         input,
+        parse_macro_input!(args as AttributeArgs),
         quote!(::cairo_lang_macro::ExpansionKind::Attr),
         quote!(::cairo_lang_macro::ExpansionFunc::Attr),
     )
@@ -24,9 +28,18 @@ pub fn attribute_macro(_args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// Note, that this macro can be used multiple times, to define multiple independent attribute macros.
 #[proc_macro_attribute]
-pub fn inline_macro(_args: TokenStream, input: TokenStream) -> TokenStream {
+pub fn inline_macro(args: TokenStream, input: TokenStream) -> TokenStream {
+    // Emit compilation error if `parent` argument is used.
+    let attribute_args = parse_macro_input!(args as AttributeArgs);
+    if let Some(path) = attribute_args.parent_module_path {
+        return syn::Error::new(path.span(), "inline macro cannot use `parent` argument")
+            .to_compile_error()
+            .into();
+    }
+    // Otherwise, proceed with the macro expansion.
     macro_helper(
         input,
+        Default::default(),
         quote!(::cairo_lang_macro::ExpansionKind::Inline),
         quote!(::cairo_lang_macro::ExpansionFunc::Other),
     )
@@ -38,18 +51,35 @@ pub fn inline_macro(_args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// Note, that this macro can be used multiple times, to define multiple independent attribute macros.
 #[proc_macro_attribute]
-pub fn derive_macro(_args: TokenStream, input: TokenStream) -> TokenStream {
+pub fn derive_macro(args: TokenStream, input: TokenStream) -> TokenStream {
     macro_helper(
         input,
+        parse_macro_input!(args as AttributeArgs),
         quote!(::cairo_lang_macro::ExpansionKind::Derive),
         quote!(::cairo_lang_macro::ExpansionFunc::Other),
     )
 }
 
-fn macro_helper(input: TokenStream, kind: impl ToTokens, func: impl ToTokens) -> TokenStream {
+fn macro_helper(
+    input: TokenStream,
+    args: AttributeArgs,
+    kind: impl ToTokens,
+    func: impl ToTokens,
+) -> TokenStream {
     let item: ItemFn = parse_macro_input!(input as ItemFn);
 
     let original_item_name = item.sig.ident.to_string();
+    let expansion_name = if let Some(path) = args.parent_module_path {
+        let value = path.value();
+        if !is_valid_path(&value) {
+            return syn::Error::new(path.span(), "`parent` argument is not a valid path")
+                .to_compile_error()
+                .into();
+        }
+        format!("{}::{}", value, original_item_name)
+    } else {
+        original_item_name
+    };
     let doc = item
         .attrs
         .iter()
@@ -74,7 +104,7 @@ fn macro_helper(input: TokenStream, kind: impl ToTokens, func: impl ToTokens) ->
         item_name.to_string().to_uppercase()
     );
 
-    let callback_link = syn::Ident::new(callback_link.as_str(), item.span());
+    let callback_link = Ident::new(callback_link.as_str(), item.span());
 
     let expanded = quote! {
         #item
@@ -83,7 +113,7 @@ fn macro_helper(input: TokenStream, kind: impl ToTokens, func: impl ToTokens) ->
         #[linkme(crate = ::cairo_lang_macro::linkme)]
         static #callback_link: ::cairo_lang_macro::ExpansionDefinition =
             ::cairo_lang_macro::ExpansionDefinition{
-                name: #original_item_name,
+                name: #expansion_name,
                 doc: #doc,
                 kind: #kind,
                 fun: #func(#item_name),
@@ -92,6 +122,55 @@ fn macro_helper(input: TokenStream, kind: impl ToTokens, func: impl ToTokens) ->
     TokenStream::from(expanded)
 }
 
+#[derive(Default)]
+struct AttributeArgs {
+    parent_module_path: Option<LitStr>,
+}
+
+impl Parse for AttributeArgs {
+    fn parse(input: ParseStream) -> Result<Self> {
+        if input.is_empty() {
+            return Ok(Self {
+                parent_module_path: None,
+            });
+        }
+        let parent_identifier: Ident = input.parse()?;
+        if parent_identifier != "parent" {
+            return Err(input.error("only `parent` argument is supported"));
+        }
+        let _eq_token: Token![=] = input.parse()?;
+        let parent_module_path: LitStr = input.parse()?;
+        Ok(Self {
+            parent_module_path: Some(parent_module_path),
+        })
+    }
+}
+
+fn is_valid_path(path: &str) -> bool {
+    let mut chars = path.chars().peekable();
+    let mut last_was_colon = false;
+    while let Some(c) = chars.next() {
+        if c.is_alphanumeric() || c == '_' {
+            last_was_colon = false;
+        } else if c == ':' {
+            if last_was_colon {
+                // If the last character was also a colon, continue
+                last_was_colon = false;
+            } else {
+                // If the next character is not a colon, it's an error
+                if chars.peek() != Some(&':') {
+                    return false;
+                }
+                last_was_colon = true;
+            }
+        } else {
+            return false;
+        }
+    }
+    // If the loop ends with a colon flag still true, it means the string ended with a single colon.
+    !last_was_colon
+}
+
 /// Constructs the post-processing callback.
 ///
 /// This callback will be called after the source code compilation (and thus after all the procedural
@@ -123,7 +202,7 @@ pub fn post_process(_args: TokenStream, input: TokenStream) -> TokenStream {
         "POST_PROCESS_DESERIALIZE_{}",
         item_name.to_string().to_uppercase()
     );
-    let callback_link = syn::Ident::new(callback_link.as_str(), item.span());
+    let callback_link = Ident::new(callback_link.as_str(), item.span());
 
     let expanded = quote! {
         #item
@@ -140,7 +219,7 @@ pub fn post_process(_args: TokenStream, input: TokenStream) -> TokenStream {
 fn hide_name(mut item: ItemFn) -> ItemFn {
     let id = short_hash(item.sig.ident.to_string());
     let item_name = format!("{}_{}", item.sig.ident, id);
-    item.sig.ident = syn::Ident::new(item_name.as_str(), item.sig.ident.span());
+    item.sig.ident = Ident::new(item_name.as_str(), item.sig.ident.span());
     item
 }
 
@@ -150,9 +229,9 @@ const EXEC_ATTR_PREFIX: &str = "__exec_attr_";
 pub fn executable_attribute(input: TokenStream) -> TokenStream {
     let input: LitStr = parse_macro_input!(input as LitStr);
     let callback_link = format!("EXEC_ATTR_DESERIALIZE{}", input.value().to_uppercase());
-    let callback_link = syn::Ident::new(callback_link.as_str(), input.span());
+    let callback_link = Ident::new(callback_link.as_str(), input.span());
     let item_name = format!("{EXEC_ATTR_PREFIX}{}", input.value());
-    let org_name = syn::Ident::new(item_name.as_str(), input.span());
+    let org_name = Ident::new(item_name.as_str(), input.span());
     let expanded = quote! {
         fn #org_name() {
             // No op to ensure no function with the same name is created.

diff --git a/plugins/cairo-lang-macro/Cargo.toml b/plugins/cairo-lang-macro/Cargo.toml
@@ -25,6 +25,7 @@ serde = { workspace = true, optional = true }
 [dev-dependencies]
 serde.workspace = true
 serde_json.workspace = true
+trybuild.workspace = true
 
 [features]
 serde = ["dep:serde"]
diff --git a/plugins/cairo-lang-macro/tests/args/args_01.rs b/plugins/cairo-lang-macro/tests/args/args_01.rs
@@ -0,0 +1,8 @@
+use cairo_lang_macro::{attribute_macro, ProcMacroResult, TokenStream};
+
+#[attribute_macro(unsupported_key = "some::path")]
+fn t1(_a: TokenStream, _b: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+fn main() {}
diff --git a/plugins/cairo-lang-macro/tests/args/args_01.stderr b/plugins/cairo-lang-macro/tests/args/args_01.stderr
@@ -0,0 +1,13 @@
+error: only `parent` argument is supported
+ --> tests/args/args_01.rs:3:35
+  |
+3 | #[attribute_macro(unsupported_key = "some::path")]
+  |                                   ^
+
+warning: unused imports: `ProcMacroResult` and `TokenStream`
+ --> tests/args/args_01.rs:1:41
+  |
+1 | use cairo_lang_macro::{attribute_macro, ProcMacroResult, TokenStream};
+  |                                         ^^^^^^^^^^^^^^^  ^^^^^^^^^^^
+  |
+  = note: `#[warn(unused_imports)]` on by default
diff --git a/plugins/cairo-lang-macro/tests/args/args_02.rs b/plugins/cairo-lang-macro/tests/args/args_02.rs
@@ -0,0 +1,8 @@
+use cairo_lang_macro::{attribute_macro, ProcMacroResult, TokenStream};
+
+#[attribute_macro(parent = "a-b")]
+fn t1(_a: TokenStream, _b: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+fn main() {}
diff --git a/plugins/cairo-lang-macro/tests/args/args_02.stderr b/plugins/cairo-lang-macro/tests/args/args_02.stderr
@@ -0,0 +1,13 @@
+error: `parent` argument is not a valid path
+ --> tests/args/args_02.rs:3:28
+  |
+3 | #[attribute_macro(parent = "a-b")]
+  |                            ^^^^^
+
+warning: unused imports: `ProcMacroResult` and `TokenStream`
+ --> tests/args/args_02.rs:1:41
+  |
+1 | use cairo_lang_macro::{attribute_macro, ProcMacroResult, TokenStream};
+  |                                         ^^^^^^^^^^^^^^^  ^^^^^^^^^^^
+  |
+  = note: `#[warn(unused_imports)]` on by default
diff --git a/plugins/cairo-lang-macro/tests/args/args_03.rs b/plugins/cairo-lang-macro/tests/args/args_03.rs
@@ -0,0 +1,8 @@
+use cairo_lang_macro::{inline_macro, ProcMacroResult, TokenStream, MACRO_DEFINITIONS_SLICE};
+
+#[inline_macro(parent = "parent")]
+fn t1(_a: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+fn main() {}
diff --git a/plugins/cairo-lang-macro/tests/args/args_03.stderr b/plugins/cairo-lang-macro/tests/args/args_03.stderr
@@ -0,0 +1,13 @@
+error: inline macro cannot use `parent` argument
+ --> tests/args/args_03.rs:3:25
+  |
+3 | #[inline_macro(parent = "parent")]
+  |                         ^^^^^^^^
+
+warning: unused imports: `MACRO_DEFINITIONS_SLICE`, `ProcMacroResult`, and `TokenStream`
+ --> tests/args/args_03.rs:1:38
+  |
+1 | use cairo_lang_macro::{inline_macro, ProcMacroResult, TokenStream, MACRO_DEFINITIONS_SLICE};
+  |                                      ^^^^^^^^^^^^^^^  ^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: `#[warn(unused_imports)]` on by default
diff --git a/plugins/cairo-lang-macro/tests/arguments_parsing.rs b/plugins/cairo-lang-macro/tests/arguments_parsing.rs
@@ -0,0 +1,40 @@
+use cairo_lang_macro::{attribute_macro, ProcMacroResult, TokenStream, MACRO_DEFINITIONS_SLICE};
+use cairo_lang_macro_attributes::derive_macro;
+
+#[attribute_macro]
+fn t1(_a: TokenStream, _b: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+#[attribute_macro(parent = "parent_1::module")]
+fn t2(_a: TokenStream, _b: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+#[attribute_macro(parent = "::parent")]
+fn t3(_a: TokenStream, _b: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+#[derive_macro(parent = "parent")]
+fn t4(_a: TokenStream) -> ProcMacroResult {
+    ProcMacroResult::new(TokenStream::empty())
+}
+
+#[test]
+fn happy_path() {
+    let list: Vec<String> = MACRO_DEFINITIONS_SLICE
+        .iter()
+        .map(|m| m.name.to_string())
+        .collect();
+    assert_eq!(
+        list,
+        vec!["t1", "parent_1::module::t2", "::parent::t3", "parent::t4"]
+    );
+}
+
+#[test]
+fn test_parsing_errors() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/args/args_*.rs");
+}