Disallows Event name collisions (#108)

* feat: disallow event name collisions, tests

* chore: clippy fix

.github/workflows/rust.yml

@@ -43,7 +43,7 @@ jobs:
         with:
           toolchain: 1.65
       - name: Run unit and integration tests
-        run: cargo test --verbose
+        run: cargo test --workspace --exclude workspaces-tests
   workspaces-test:
     runs-on: ubuntu-latest
 

Cargo.toml

@@ -13,13 +13,13 @@ version = "0.7.1"
 
 [dependencies]
 near-contract-tools-macros = {version = "=0.7.1", path = "./macros"}
-near-sdk = { version = "4.1.0", default-features = false }
+near-sdk = {version = "4.1.0", default-features = false}
 serde = "1.0.144"
 serde_json = "1.0.85"
 thiserror = "1.0.35"
 
 [dev-dependencies]
-near-sdk = { version = "4.1.0", default-features = false, features = ["unit-testing", "legacy"] }
+near-sdk = {version = "4.1.0", default-features = false, features = ["unit-testing", "legacy"]}
 
 [features]
 unstable = ["near-sdk/unstable"]

macros/src/standard/nep297.rs

@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use darling::{FromDeriveInput, FromVariant};
 use proc_macro2::TokenStream;
 use quote::quote;
@@ -80,7 +82,7 @@ pub fn expand(meta: Nep297Meta) -> Result<TokenStream, darling::Error> {
     let (imp, ty, wher) = generics.split_for_impl();
 
     // Variant attributes
-    let event = match data {
+    let (event, used_names) = match data {
         darling::ast::Data::Struct(_) => {
             let transformed_name = if let Some(name) = name {
                 name
@@ -90,43 +92,64 @@ pub fn expand(meta: Nep297Meta) -> Result<TokenStream, darling::Error> {
                 ident.to_string()
             };
 
-            quote! { #transformed_name }
+            (quote! { #transformed_name }, vec![transformed_name])
         }
         darling::ast::Data::Enum(variants) => {
-            let arms = variants.into_iter().map(|variant| {
-                let i = &variant.ident;
-
-                // This could be a function chain, but I found it to be unreadable
-                let transformed_name = if let Some(name) = variant.name {
-                    name
-                } else if let Some(rename) = variant.rename.as_ref().or(rename_all.as_ref()) {
-                    rename.transform(i.to_string())
-                } else {
-                    i.to_string()
-                };
-
-                match variant.fields.style {
-                    darling::ast::Style::Tuple => {
-                        quote! { Self :: #i ( .. ) => #transformed_name , }
-                    }
-                    darling::ast::Style::Struct => {
-                        quote! { Self :: #i { .. } => #transformed_name , }
+            let (arms, used_names) = variants
+                .into_iter()
+                .map(|variant| {
+                    let i = &variant.ident;
+
+                    // This could be a function chain, but I found it to be unreadable
+                    let transformed_name = if let Some(name) = variant.name {
+                        name
+                    } else if let Some(rename) = variant.rename.as_ref().or(rename_all.as_ref()) {
+                        rename.transform(i.to_string())
+                    } else {
+                        i.to_string()
+                    };
+
+                    (
+                        match variant.fields.style {
+                            darling::ast::Style::Tuple => {
+                                quote! { Self :: #i ( .. ) => #transformed_name , }
+                            }
+                            darling::ast::Style::Struct => {
+                                quote! { Self :: #i { .. } => #transformed_name , }
+                            }
+                            darling::ast::Style::Unit => {
+                                quote! { Self :: #i  => #transformed_name , }
+                            }
+                        },
+                        transformed_name,
+                    )
+                })
+                .unzip::<_, _, Vec<_>, Vec<_>>();
+
+            (
+                quote! {
+                    match self {
+                        #(#arms)*
                     }
-                    darling::ast::Style::Unit => {
-                        quote! { Self :: #i  => #transformed_name , }
-                    }
-                }
-            });
-
-            quote! {
-                match self {
-                    #(#arms)*
-                }
-            }
+                },
+                used_names,
+            )
         }
     };
 
-    Ok(quote! {
+    let mut e = darling::Error::accumulator();
+
+    let mut no_duplicate_names = HashSet::<&String>::new();
+    for used_name in used_names.iter() {
+        let fresh_insertion = no_duplicate_names.insert(used_name);
+        if !fresh_insertion {
+            e.push(darling::Error::custom(format!(
+                "Event name collision: `{used_name}`",
+            )))
+        }
+    }
+
+    e.finish_with(quote! {
         impl #imp #me::standard::nep297::ToEventLog for #ident #ty #wher {
             type Data = #ident #ty;
 
@@ -141,3 +164,31 @@ pub fn expand(meta: Nep297Meta) -> Result<TokenStream, darling::Error> {
         }
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use darling::FromDeriveInput;
+
+    use super::Nep297Meta;
+
+    #[test]
+    #[should_panic = "Event name collision: `first`"]
+    fn disallow_duplicate_names() {
+        let ast = syn::parse_str(
+            r#"
+            #[derive(Nep297)]
+            #[nep297(standard = "x-name-collision", version = "1.0.0")]
+            enum NameCollision {
+                #[nep297(name = "first")]
+                First,
+                #[nep297(name = "first")]
+                Second,
+            }
+        "#,
+        )
+        .unwrap();
+
+        let meta = Nep297Meta::from_derive_input(&ast).unwrap();
+        super::expand(meta).unwrap();
+    }
+}

src/approval/mod.rs

@@ -442,7 +442,7 @@ mod tests {
         let mut contract = Contract::new(2);
 
         contract.add_role(alice.clone(), &Role::Multisig);
-        contract.add_role(bob.clone(), &Role::Multisig);
+        contract.add_role(bob, &Role::Multisig);
         contract.add_role(charlie.clone(), &Role::Multisig);
 
         predecessor(&alice);

src/approval/simple_multisig.rs

@@ -291,9 +291,7 @@ mod tests {
                 Action::SayGoodbye
             };
 
-            let request_id = self.create_request(action, ApprovalState::new()).unwrap();
-
-            request_id
+            self.create_request(action, ApprovalState::new()).unwrap()
         }
 
         pub fn approve(&mut self, request_id: u32) {
@@ -373,7 +371,7 @@ mod tests {
 
         let mut context = VMContextBuilder::new();
         context
-            .predecessor_account_id(alice.clone())
+            .predecessor_account_id(alice)
             .block_timestamp(created_at + 10000);
         testing_env!(context.build());
 
@@ -401,7 +399,7 @@ mod tests {
 
         let mut context = VMContextBuilder::new();
         context
-            .predecessor_account_id(alice.clone())
+            .predecessor_account_id(alice)
             .block_timestamp(created_at + 9999);
         testing_env!(context.build());
 
@@ -430,7 +428,7 @@ mod tests {
 
         let mut context = VMContextBuilder::new();
         context
-            .predecessor_account_id(bob.clone())
+            .predecessor_account_id(bob)
             .block_timestamp(created_at + 10000);
         testing_env!(context.build());
 

src/standard/nep148.rs

@@ -59,11 +59,11 @@ mod tests {
         };
 
         let m2 = FungibleTokenMetadata {
-            spec: "spec".to_owned().into(),
-            name: "name".to_owned().into(),
-            symbol: "symbol".to_owned().into(),
-            icon: Some("icon".to_owned().into()),
-            reference: Some("reference".to_owned().into()),
+            spec: "spec".to_owned(),
+            name: "name".to_owned(),
+            symbol: "symbol".to_owned(),
+            icon: Some("icon".to_owned()),
+            reference: Some("reference".to_owned()),
             reference_hash: Some(b"reference_hash".to_vec().into()),
             decimals: 18,
         };

tests/macros/event.rs

@@ -29,6 +29,7 @@ mod test_events {
 
     #[derive(Nep297, Serialize)]
     #[nep297(standard = "enum-event", version = "1.0.0")]
+    #[allow(clippy::enum_variant_names)]
     pub enum EnumEvent {
         VariantOne,
         #[nep297(name = "genuine_variant_two")]

tests/mod.rs

@@ -1,5 +1,5 @@
 // They're tests: who cares if we use "foo"
-#[allow(clippy::blacklisted_name)]
+#[allow(clippy::disallowed_names)]
 // We don't care about test performance so much and makes for better diffs
 #[allow(clippy::redundant_clone)]
 mod macros;