Skip to content

Commit

Permalink
[refactor]: Update FromVariant macro to syn 2.0 & improve UX
Browse files Browse the repository at this point in the history 
- use darling's traits to parse the input
- cover attributes with tests
- add a diagnostic for a common error: attribute on variant instead of field
- ensure spans for generated enums are linked to enum variants, which improves "conflicting implementation" errors

Signed-off-by: Nikita Strygin <[email protected]>
  • Loading branch information
@DCNick3
DCNick3 committed Jul 27, 2023
1 parent 94ce57d commit 5055e57
Show file tree
Hide file tree
Showing 12 changed files with 339 additions and 117 deletions.
5 changes: 4 additions & 1 deletion Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion macro/derive/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ license.workspace = true
proc-macro = true

[dependencies]
syn = { workspace = true, features = ["default", "full"] }
syn2 = { workspace = true, features = ["default", "full"] }
quote = { workspace = true }
proc-macro2 = { workspace = true }
manyhow = { workspace = true }
darling = { workspace = true }

[dev-dependencies]
iroha_macro = { workspace = true }

trybuild = { workspace = true }
impls = "1.0.3"
266 changes: 185 additions & 81 deletions macro/derive/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

#![allow(clippy::restriction)]

use proc_macro::TokenStream;
use quote::quote;
use darling::{util::SpannedValue, FromDeriveInput};
use manyhow::{manyhow, Result};
use proc_macro2::{Span, TokenStream};
use quote::{quote, quote_spanned, ToTokens};
use syn2::{spanned::Spanned, Token};

/// Attribute for skipping from attribute
const SKIP_FROM_ATTR: &str = "skip_from";
Expand All @@ -12,19 +15,117 @@ const SKIP_TRY_FROM_ATTR: &str = "skip_try_from";
const SKIP_CONTAINER: &str = "skip_container";

/// Helper macro to expand FFI functions
#[manyhow]
#[proc_macro_attribute]
pub fn ffi_impl_opaque(_: TokenStream, item: TokenStream) -> TokenStream {
let item: syn::ItemImpl = syn::parse_macro_input!(item);
pub fn ffi_impl_opaque(_: TokenStream, item: TokenStream) -> Result<TokenStream> {
let item: syn2::ItemImpl = syn2::parse2(item)?;

quote! {
Ok(quote! {
#[cfg_attr(
all(feature = "ffi_export", not(feature = "ffi_import")),
iroha_ffi::ffi_export
)]
#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)]
#item
})
}

#[derive(darling::FromDeriveInput, Debug)]
#[darling(supports(enum_any))]
struct FromVariantInput {
ident: syn2::Ident,
generics: syn2::Generics,
data: darling::ast::Data<SpannedValue<FromVariantVariant>, darling::util::Ignored>,
}

// FromVariant manually implemented for additional validation
#[derive(Debug)]
struct FromVariantVariant {
ident: syn2::Ident,
fields: darling::ast::Fields<SpannedValue<FromVariantField>>,
}

impl FromVariantVariant {
fn can_from_be_implemented(
fields: &darling::ast::Fields<SpannedValue<FromVariantField>>,
) -> bool {
fields.style == darling::ast::Style::Tuple && fields.fields.len() == 1
}
}

impl darling::FromVariant for FromVariantVariant {
fn from_variant(variant: &syn2::Variant) -> darling::Result<Self> {
let ident = variant.ident.clone();
let fields = darling::ast::Fields::try_from(&variant.fields)?;
let mut accumulator = darling::error::Accumulator::default();

let can_from_be_implemented = Self::can_from_be_implemented(&fields);

for field in &fields.fields {
if (field.skip_from || field.skip_container) && !can_from_be_implemented {
accumulator.push(darling::Error::custom("#[skip_from], #[skip_try_from] and #[skip_container] attributes are only allowed for new-type enum variants (single unnamed field). The `From` traits will not be implemented for other kinds of variants").with_span(&field.span()));
}
}

for attr in &variant.attrs {
let span = attr.span();
let attr = attr.path().to_token_stream().to_string();
match attr.as_str() {
SKIP_FROM_ATTR | SKIP_TRY_FROM_ATTR | SKIP_CONTAINER => {
accumulator.push(
darling::Error::custom(format!(
"#[{}] attribute should be applied to the field, not variant",
&attr
))
.with_span(&span),
);
}
_ => {}
}
}

accumulator.finish()?;

Ok(Self { ident, fields })
}
}

// FromField manually implemented for non-standard attributes
#[derive(Debug)]
struct FromVariantField {
ty: syn2::Type,
skip_from: bool,
skip_try_from: bool,
skip_container: bool,
}

// implementing manually, because darling can't parse attributes that are not under some unified attr
// It expects us to have a common attribute that will contain all the fields, like:
// #[hello(skip_from, skip_container)]
// The already defined macro API uses `skip_from` and `skip_container` attributes without any qualification
// Arguably, this is also more convenient for the user (?)
// Hence, we fall back to manual parsing
impl darling::FromField for FromVariantField {
fn from_field(field: &syn2::Field) -> darling::Result<Self> {
let mut skip_from = false;
let mut skip_try_from = false;
let mut skip_container = false;
for attr in &field.attrs {
match attr.path().clone().to_token_stream().to_string().as_str() {
SKIP_FROM_ATTR => skip_from = true,
SKIP_TRY_FROM_ATTR => skip_try_from = true,
SKIP_CONTAINER => skip_container = true,
// ignore unknown attributes, rustc handles them
_ => continue,
}
}
Ok(Self {
ty: field.ty.clone(),
skip_from,
skip_try_from,
skip_container,
})
}
.into()
}

/// [`FromVariant`] is used for implementing `From<Variant> for Enum`
Expand Down Expand Up @@ -56,25 +157,24 @@ pub fn ffi_impl_opaque(_: TokenStream, item: TokenStream) -> TokenStream {
/// }
/// }
/// ```
#[manyhow]
#[proc_macro_derive(FromVariant, attributes(skip_from, skip_try_from, skip_container))]
pub fn from_variant_derive(input: TokenStream) -> TokenStream {
let ast = syn::parse(input).expect("Failed to parse input Token Stream.");
impl_from_variant(&ast)
}

fn attrs_have_ident(attrs: &[syn::Attribute], ident: &str) -> bool {
attrs.iter().any(|attr| attr.path.is_ident(ident))
pub fn from_variant_derive(input: TokenStream) -> Result<TokenStream> {
let ast = syn2::parse2(input)?;
let ast = FromVariantInput::from_derive_input(&ast)?;
Ok(impl_from_variant(&ast))
}

const CONTAINERS: &[&str] = &["Box", "RefCell", "Cell", "Rc", "Arc", "Mutex", "RwLock"];

fn get_type_argument<'b>(s: &str, ty: &'b syn::TypePath) -> Option<&'b syn::GenericArgument> {
fn get_type_argument<'b>(s: &str, ty: &'b syn2::TypePath) -> Option<&'b syn2::GenericArgument> {
// NOTE: this is NOT syn2::Path::is_ident because it allows for generic parameters
let segments = &ty.path.segments;
if segments.len() != 1 || segments[0].ident != s {
return None;
}

if let syn::PathArguments::AngleBracketed(ref bracketed_arguments) = segments[0].arguments {
if let syn2::PathArguments::AngleBracketed(ref bracketed_arguments) = segments[0].arguments {
assert_eq!(bracketed_arguments.args.len(), 1);
Some(&bracketed_arguments.args[0])
} else {
Expand All @@ -83,12 +183,12 @@ fn get_type_argument<'b>(s: &str, ty: &'b syn::TypePath) -> Option<&'b syn::Gene
}

fn from_container_variant_internal(
into_ty: &syn::Ident,
into_variant: &syn::Ident,
from_ty: &syn::GenericArgument,
container_ty: &syn::TypePath,
generics: &syn::Generics,
) -> proc_macro2::TokenStream {
into_ty: &syn2::Ident,
into_variant: &syn2::Ident,
from_ty: &syn2::GenericArgument,
container_ty: &syn2::TypePath,
generics: &syn2::Generics,
) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

quote! {
Expand All @@ -101,14 +201,15 @@ fn from_container_variant_internal(
}

fn from_variant_internal(
into_ty: &syn::Ident,
into_variant: &syn::Ident,
from_ty: &syn::Type,
generics: &syn::Generics,
) -> proc_macro2::TokenStream {
span: Span,
into_ty: &syn2::Ident,
into_variant: &syn2::Ident,
from_ty: &syn2::Type,
generics: &syn2::Generics,
) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

quote! {
quote_spanned! { span =>
impl #impl_generics From<#from_ty> for #into_ty #ty_generics #where_clause {
fn from(origin: #from_ty) -> Self {
#into_ty :: #into_variant (origin)
Expand All @@ -118,15 +219,16 @@ fn from_variant_internal(
}

fn from_variant(
into_ty: &syn::Ident,
into_variant: &syn::Ident,
from_ty: &syn::Type,
generics: &syn::Generics,
span: Span,
into_ty: &syn2::Ident,
into_variant: &syn2::Ident,
from_ty: &syn2::Type,
generics: &syn2::Generics,
skip_container: bool,
) -> proc_macro2::TokenStream {
let from_orig = from_variant_internal(into_ty, into_variant, from_ty, generics);
) -> TokenStream {
let from_orig = from_variant_internal(span, into_ty, into_variant, from_ty, generics);

if let syn::Type::Path(path) = from_ty {
if let syn2::Type::Path(path) = from_ty {
let mut code = from_orig;

if skip_container {
Expand All @@ -141,19 +243,19 @@ fn from_variant(
.iter()
.map(|segment| {
let mut segment = segment.clone();
segment.arguments = syn::PathArguments::default();
segment.arguments = syn2::PathArguments::default();
segment
})
.collect::<syn::punctuated::Punctuated<_, syn::token::Colon2>>();
let path = syn::Path {
.collect::<syn2::punctuated::Punctuated<_, Token![::]>>();
let path = syn2::Path {
segments,
leading_colon: None,
};
let path = &syn::TypePath { path, qself: None };
let path = &syn2::TypePath { path, qself: None };

let from_inner =
from_container_variant_internal(into_ty, into_variant, inner, path, generics);
code = quote! {
code = quote_spanned! { span =>
#code
#from_inner
};
Expand All @@ -167,14 +269,15 @@ fn from_variant(
}

fn try_into_variant(
enum_ty: &syn::Ident,
variant: &syn::Ident,
variant_ty: &syn::Type,
generics: &syn::Generics,
) -> proc_macro2::TokenStream {
span: Span,
enum_ty: &syn2::Ident,
variant: &syn2::Ident,
variant_ty: &syn2::Type,
generics: &syn2::Generics,
) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

quote! {
quote_spanned! { span =>
impl #impl_generics TryFrom<#enum_ty #ty_generics> for #variant_ty #where_clause {
type Error = iroha_macro::error::ErrorTryFromEnum<#enum_ty #ty_generics, Self>;

Expand All @@ -189,46 +292,47 @@ fn try_into_variant(
}
}

fn impl_from_variant(ast: &syn::DeriveInput) -> TokenStream {
fn impl_from_variant(ast: &FromVariantInput) -> TokenStream {
let name = &ast.ident;

let generics = &ast.generics;
let froms = if let syn::Data::Enum(data_enum) = &ast.data {
&data_enum.variants
} else {
panic!("Only enums are supported")
}
.iter()
.filter_map(|variant| {
if let syn::Fields::Unnamed(ref unnamed) = variant.fields {
if unnamed.unnamed.len() == 1 {
let variant_type = &unnamed
.unnamed
.first()
.expect("Won't fail as we have more than one argument for variant")
.ty;

let try_into = if attrs_have_ident(&unnamed.unnamed[0].attrs, SKIP_TRY_FROM_ATTR) {
quote!()
} else {
try_into_variant(name, &variant.ident, variant_type, generics)
};
let from = if attrs_have_ident(&unnamed.unnamed[0].attrs, SKIP_FROM_ATTR) {
quote!()
} else if attrs_have_ident(&unnamed.unnamed[0].attrs, SKIP_CONTAINER) {
from_variant(name, &variant.ident, variant_type, generics, true)
} else {
from_variant(name, &variant.ident, variant_type, generics, false)
};

return Some(quote!(
#try_into
#from
));
let froms = ast
.data
.as_ref()
.take_enum()
.expect("BUG: this should've been checked when parsing FromVariantInput")
.into_iter()
.filter_map(|variant| {
if !variant.fields.is_newtype() {
return None;
}
}
None
});
let span = variant.span();
let field = variant
.fields
.iter()
.next()
.expect("BUG: this should've been checked when parsing FromVariantInput");
let variant_type = &field.ty;

let try_into = if field.skip_try_from {
quote!()
} else {
try_into_variant(span, name, &variant.ident, variant_type, generics)
};
let from = if field.skip_from {
quote!()
} else if field.skip_container {
from_variant(span, name, &variant.ident, variant_type, generics, true)
} else {
from_variant(span, name, &variant.ident, variant_type, generics, false)
};

Some(quote!(
#try_into
#from
))
});

quote! { #(#froms)* }.into()
quote! { #(#froms)* }
}
Loading

0 comments on commit 5055e57

Please sign in to comment.