Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a proc macro derive for Encode and Decode supporting _only_ 1-arity tuple structs #34

Closed
mehcode opened this issue Jan 7, 2020 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mehcode
Copy link
Member

mehcode commented Jan 7, 2020

I don't want to define what anything but 1-arity tuple structs means at this time. We are going to explore derives for custom types in the near future.

From #5

@mehcode mehcode added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Jan 7, 2020
@abonander
Copy link
Collaborator

abonander commented Jan 15, 2020

This is considered a "good first issue" because it's conceptually very simple for a derive macro; it should only forward the Encode/Decode call to the inner field. (Let me know if these instructions are too dense.)

A rundown of implementing a simple derive is available in the new Rust book here: https://doc.rust-lang.org/book/ch19-06-macros.html#how-to-write-a-custom-derive-macro

However, instead of creating a new macro library, add #[proc_macro_derive(Encode)] pub fn derive_encode() and #[proc_macro_derive(Decode)] pub fn derive_decode() to sqlx-macros/src/lib.rs.
The syn and quote dependencies are already available for you.

These functions, however, should only be shims. Add a new module derives to sqlx-macros and place the implementations there. There's also an easier way to parse DeriveInput that handles the errors better:

sqlx-macros/src/lib.rs:

mod derives;

#[proc_macro_derive(Encode)]
pub fn derive_encode(tokenstream: TokenStream) -> TokenStream {
    let input = syn::parse_macro_input!(tokenstream as syn::DeriveInput);
    match derives::expand_derive_encode(input) {
        Ok(ts) => ts.into(),
        Err(e) => e.to_compile_error(),
    }
}

#[proc_macro_derive(Decode)]
pub fn derive_decode(tokenstream: TokenStream) -> TokenStream {
    let input = syn::parse_macro_input!(tokenstream as syn::DeriveInput);
    match derives::expand_derive_decode(input) {
        Ok(ts) => ts.into(),
        Err(e) => e.to_compile_error(),
    }
}

Then in sqlx-macros/src/derives.rs you can implement expand_derive_encode and expand_derive_decode which should both return syn::Result<proc_macro2::TokenStream> (the proc_macro2::TokenStream type is what quote! {} evaluates to).

Some recommendations:
* For DeriveInput::data, we only care about Data::Struct(Struct { fields: Fields::Unnamed(fields) }); anything else should return Err(syn::Error::new_spanned(input, "expected a tuple struct with a single field"))
* Check that fields.unnamed.len() == 1, if not return Err(syn::Error::new_spanned(input, "expected a tuple struct with a single field")
* The emitted impls should be generic over DB: Database, which means you need to add a where clause for the internal field type asserting that it implements the trait being derived.
* If you want to support the input struct being generic, that is up to you;l however, if you decide not to then you should return an error if input.generics.params is non-empty.
* You can add the input.generics.where_clause to the output impls verbatim if it's not None.

Here's the kind of expected input and output:

use sqlx::{Encode, Decode};

#[derive(Decode, Encode)]
struct Foo(i32);

// expected output for the above derives:
impl<DB> Encode<DB> for Foo where DB: sqlx::Database, i32: sqlx::encode::Encode<DB> {
    fn encode(&self, buf: &mut Vec<u8>) -> sqlx::encode::IsNull {
        self.0.encode(buf)
    }
}

impl<DB> Decode<DB> for Foo where DB: sqlx::Database, i32: sqlx::decode::Decode<DB> {
    fn decode(raw: &[u8]) -> Result<Self, sqlx::decode::DecodeError> {
        <i32 as Decode<DB>>::decode(raw).map(Self)
    }
}

To make the usage nice, you should change the decode and encode module re-exports in the sqlx/src/lib.rs facade to be inline modules that pub use ..::* from their counterparts in sqlx-core and
also have #[cfg(feature = "macros")] pub use sqlx_macros::<Decode | Encode>;
What this will do is allow people to use sqlx::encode::Encode or sqlx::decode::Decode and get both the trait and the derive macro in a single import. (Serde uses this trick.)

When you're done with that, open a PR and ping me (@abonander) for review; I'll walk you through adding tests. Of course also if you get stuck, feel free to @ me. I wrote all the other macro code there.

@abonander
Copy link
Collaborator

Closed by #71.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants