[refactor]: Use darling to parse attributes in `iroha_version_deriv…

…e`. It does it pretty well =) Signed-off-by: Nikita Strygin <DCNick3@users.noreply.github.com>
hyperledger-iroha · Jul 20, 2023 · 5398793 · 5398793
1 parent cb2d401
commit 5398793
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 147 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/version/derive/Cargo.toml b/version/derive/Cargo.toml
@@ -14,7 +14,11 @@ proc-macro = true
 syn2 = { workspace = true, features = ["default", "full"] }
 quote = { workspace = true }
 proc-macro2 = { workspace = true }
-manyhow = "0.4.2"
+
+# using fork for `darling` support
+# Tracked at https://github.com/ModProg/manyhow/pull/7
+manyhow = { git = "https://github.com/DCNick3/manyhow.git", rev = "3f48c16fbf6fa8f45dd9428ff191722863867be1", features = ["darling"] }
+darling = { version = "0.20.1" }
 
 [dev-dependencies]
 iroha_version = { workspace = true, features = ["scale", "json"] }

diff --git a/version/derive/src/lib.rs b/version/derive/src/lib.rs
@@ -3,18 +3,16 @@
 
 use std::ops::Range;
 
-use manyhow::{bail, error_message, manyhow, Result};
+use darling::{ast::NestedMeta, FromMeta};
+use manyhow::{bail, manyhow, Result};
 use proc_macro2::{Span, TokenStream};
 use quote::{format_ident, quote};
 use syn2::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
-    Data, DeriveInput, Error as SynError, Expr, Ident, Lit, LitInt, Meta, Path,
-    Result as SynResult, Token,
+    Data, DeriveInput, Error as SynError, Ident, LitInt, Path, Result as SynResult, Token,
 };
 
-const VERSION_NUMBER_ARG_NAME: &str = "n";
-const VERSIONED_STRUCT_ARG_NAME: &str = "versioned";
 const VERSION_FIELD_NAME: &str = "version";
 const CONTENT_FIELD_NAME: &str = "content";
 
@@ -107,146 +105,19 @@ pub fn declare_versioned_with_json(input: TokenStream) -> Result<TokenStream> {
     Ok(impl_declare_versioned(&args, false, true))
 }
 
-// NOTE: this type is temporary and will be replaced with `manyhow::Emitter` when it's exposed
-// see https://github.com/ModProg/manyhow/issues/2#issuecomment-1643604170
-struct Accumulator(Vec<manyhow::Error>);
-impl Accumulator {
-    fn new() -> Self {
-        Self(Vec::new())
-    }
-
-    /// Emits an error
-    pub fn emit(&mut self, error: impl manyhow::ToTokensError + 'static) {
-        self.0.push(manyhow::Error::from(error));
-    }
-
-    /// Checks if any errors were emitted
-    #[must_use]
-    pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
-    }
-
-    /// Errors if not [`Self::is_empty`]
-    pub fn into_result(self) -> Result<()> {
-        if self.is_empty() {
-            Ok(())
-        } else {
-            let mut error = manyhow::Error::from(manyhow::SilentError);
-
-            for e in self.0 {
-                error.push(e);
-            }
-
-            Err(error)
-        }
-    }
-}
-
+#[derive(FromMeta)]
 struct VersionArgs {
-    version_number: u32,
-    // suggestion: change macro API to accept an `Ident` directly (idents as string literals are meh)
-    versioned_struct_name: String,
-}
-
-impl VersionArgs {
-    fn parse_from_args(args: TokenStream) -> Result<Self> {
-        struct RawArgs(Punctuated<Meta, Token![,]>);
-        impl Parse for RawArgs {
-            fn parse(input: ParseStream) -> syn2::Result<Self> {
-                Ok(Self(Punctuated::parse_terminated(input)?))
-            }
-        }
-
-        let RawArgs(args) = syn2::parse2(args)?;
-        let mut accum = Accumulator::new();
-
-        let mut version_number = None;
-        let mut versioned_struct_name = None;
-        for arg in &args {
-            let Meta::NameValue(arg) = arg else {
-                // uhh, this is kinda ugly
-                // see https://github.com/ModProg/manyhow/issues/6
-                accum.emit(error_message!(
-                    arg,
-                    "Expected an argument of form `#[...(name = value)]`"
-                ));
-                continue;
-            };
-
-            // ideally, these two should be done in "parallel": one check does not depend on another
-            // this is, actually, pretty often needed when writing proc macros...
-            let Some(name) = arg.path.get_ident() else {
-                accum.emit(error_message!(
-                    &arg.path,
-                    "Expected single identifier for attribute argument key."
-                ));
-                continue;
-            };
-            let Expr::Lit(syn2::ExprLit { lit: value, .. }) = &arg.value else {
-                accum.emit(error_message!(
-                    &arg.value,
-                    "Expected a literal for attribute argument value."
-                ));
-                continue;
-            };
-
-            match name.to_string().as_str() {
-                VERSION_NUMBER_ARG_NAME => {
-                    let Lit::Int(int) = value else {
-                        accum.emit(error_message!(
-                            value,
-                            "Expected an integer literal for version number."
-                        ));
-                        continue;
-                    };
-                    version_number = Some(int.base10_parse()?);
-                }
-                VERSIONED_STRUCT_ARG_NAME => {
-                    let Lit::Str(str) = value else {
-                        accum.emit(error_message!(
-                            value,
-                            "Expected a string literal for versioned struct name."
-                        ));
-                        continue;
-                    };
-                    versioned_struct_name = Some(str.value());
-                }
-                _ => {
-                    accum.emit(error_message!(name, "Unknown argument `{}`", name));
-                    continue;
-                }
-            }
-        }
-
-        if version_number.is_none() {
-            accum.emit(error_message!(
-                &args,
-                "Missing argument `{}`",
-                VERSION_NUMBER_ARG_NAME
-            ));
-        }
-        if versioned_struct_name.is_none() {
-            accum.emit(error_message!(
-                &args,
-                "Missing argument `{}`",
-                VERSIONED_STRUCT_ARG_NAME
-            ));
-        }
-
-        accum.into_result()?;
-
-        Ok(Self {
-            version_number: version_number.unwrap(),
-            versioned_struct_name: versioned_struct_name.unwrap(),
-        })
-    }
+    n: u32,
+    versioned: syn2::Ident,
 }
 
 fn impl_version(args: TokenStream, item: TokenStream) -> Result<TokenStream> {
+    let args = NestedMeta::parse_meta_list(args)?;
     let VersionArgs {
-        version_number,
-        versioned_struct_name,
-    } = VersionArgs::parse_from_args(args)?;
+        n: version_number,
+        versioned: versioned_struct_name,
+    } = VersionArgs::from_list(&args)?;
+
     let struct_name = {
         let item = syn2::parse2::<DeriveInput>(item.clone())?;
         match &item.data {

diff --git a/version/derive/tests/ui_fail/unknown_field_in_version_attribute.stderr b/version/derive/tests/ui_fail/unknown_field_in_version_attribute.stderr
@@ -1,8 +1,8 @@
-error: Unknown argument `derive`
+error: Unknown field: `derive`
  --> tests/ui_fail/unknown_field_in_version_attribute.rs:7:50
   |
 7 | #[version(n = 1, versioned = "VersionedMessage", derive = "Clone")]
-  |                                                  ^^^^^^
+  |                                                  ^^^^^^^^^^^^^^^^
 
 error[E0412]: cannot find type `_VersionedMessageV1` in this scope
  --> tests/ui_fail/unknown_field_in_version_attribute.rs:5:1