diesel-rs/diesel

Interaction between `#[derive(AsChangeset)]` and macro in type position

clechasseur opened this issue · 2 comments

Setup

Versions

  • Rust: 1.71.1
  • Diesel: 2.1.1
  • Database: Postgres
  • Operating System macOS

Feature Flags

  • diesel: postgres

Problem Description

When trying to #[derive(AsChangeset)], if the type of a field is provided by a macro, the generated implementation of AsChangeset is incorrect.

What are you trying to accomplish?

I am trying to create a single macro to generate several structs for a similar entity type, for various operations (insert_into, full update, partial update, etc.) To do this, I am using a macro to conditionally wrap the struct's field types in Options.

What is the expected output?

See below

What is the actual output?

See below

Are you seeing any additional errors?

No

Steps to reproduce

The following code (a):

diesel::table! {
    foobars (id) {
        id -> Int4,
        example -> Bool,
    }
}

#[derive(AsChangeset)]
#[diesel(table_name = foobars)]
struct PatchFoobar1 {
    example: Option<bool>,
}

generates this (a'):

        #[diesel(table_name = foobars)]
        struct PatchFoobar1 {
            example: Option<bool>,
        }
        #[allow(unused_imports)]
        const _: () = {
            use diesel;
            use diesel::query_builder::AsChangeset;
            use diesel::prelude::*;
            impl AsChangeset for PatchFoobar1 {
                type Target = foobars::table;
                type Changeset = <(
                    std::option::Option<diesel::dsl::Eq<foobars::example, bool>>,
                ) as AsChangeset>::Changeset;
                fn as_changeset(self) -> Self::Changeset {
                    (self.example.map(|x| foobars::example.eq(x)),).as_changeset()
                }
            }
            impl<'update> AsChangeset for &'update PatchFoobar1 {
                type Target = foobars::table;
                type Changeset = <(
                    std::option::Option<
                        diesel::dsl::Eq<foobars::example, &'update bool>,
                    >,
                ) as AsChangeset>::Changeset;
                fn as_changeset(self) -> Self::Changeset {
                    (self.example.as_ref().map(|x| foobars::example.eq(x)),)
                        .as_changeset()
                }
            }
        };

This is a correct implementation which properly detected that example is an Option.

The following code (b):

diesel::table! {
    foobars (id) {
        id -> Int4,
        example -> Bool,
    }
}

macro_rules! wrap_opt {
    ($ty:ty) => { Option<$ty> };
}

#[derive(AsChangeset)]
#[diesel(table_name = foobars)]
struct PatchFoobar2 {
    example: wrap_opt!(bool),
}

generates this (b'):

        #[diesel(table_name = foobars)]
        struct PatchFoobar2 {
            example: Option<bool>,
        }
        #[allow(unused_imports)]
        const _: () = {
            use diesel;
            use diesel::query_builder::AsChangeset;
            use diesel::prelude::*;
            impl AsChangeset for PatchFoobar2 {
                type Target = foobars::table;
                type Changeset = <(
                    diesel::dsl::Eq<foobars::example, Option<bool>>,
                ) as AsChangeset>::Changeset;
                fn as_changeset(self) -> Self::Changeset {
                    (foobars::example.eq(self.example),).as_changeset()
                }
            }
            impl<'update> AsChangeset for &'update PatchFoobar2 {
                type Target = foobars::table;
                type Changeset = <(
                    diesel::dsl::Eq<foobars::example, &'update Option<bool>>,
                ) as AsChangeset>::Changeset;
                fn as_changeset(self) -> Self::Changeset {
                    (foobars::example.eq(&self.example),).as_changeset()
                }
            }
        };

As you can see, in both a' and b', the struct type is the same (one field of type Option<bool>). However, in b', the generated implementation of AsChangeset is incorrect; it did not detect that example is an Option, although it did correctly output code that compares the example field with an Option<bool>.

Compiling b gives the following compiler error:

error[E0277]: the trait bound `std::option::Option<bool>: AppearsOnTable<foobars::table>` is not satisfied
   --> src/<somewhere>
    |
130 | #[derive(AsChangeset)]
    |          ^^^^^^^^^^^ the trait `AppearsOnTable<foobars::table>` is not implemented for `std::option::Option<bool>`
    |
    = help: the following other types implement trait `AppearsOnTable<QS>`:
              <&'a T as AppearsOnTable<QS>>
              <(T0, T1) as AppearsOnTable<QS>>
              <(T0, T1, T2) as AppearsOnTable<QS>>
              <(T0, T1, T2, T3) as AppearsOnTable<QS>>
              <(T0, T1, T2, T3, T4) as AppearsOnTable<QS>>
              <(T0, T1, T2, T3, T4, T5) as AppearsOnTable<QS>>
              <(T0, T1, T2, T3, T4, T5, T6) as AppearsOnTable<QS>>
              <(T0, T1, T2, T3, T4, T5, T6, T7) as AppearsOnTable<QS>>
            and 152 others
    = note: required for `&'update std::option::Option<bool>` to implement `AppearsOnTable<foobars::table>`
    = note: required for `diesel::expression::operators::Eq<foobars::columns::example, &'update std::option::Option<bool>>` to implement `diesel::AsChangeset`
    = note: this error originates in the derive macro `AsChangeset` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.

I believe this may be caused by this:

match *ty {
Type::Path(ref ty) => {
let last_segment = ty.path.segments.iter().last().unwrap();
match last_segment.arguments {
AngleBracketed(ref args) if last_segment.ident == "Option" => {
match args.args.iter().last() {
Some(GenericArgument::Type(ty)) => Some(ty),
_ => None,
}
}
_ => None,
}
}
_ => None,
}

In the second case (b), I believe that *ty might end up being a Type::Macro instead of Type::Path.

Not knowing a lot about macros yet, I am uncertain how to fix this, or indeed if this is fixable. If it is and this is considered a big enough concern to warrant a patch, I would be happy to try to contribute a PR if I can.

Checklist

  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
  • This issue can be reproduced without requiring a third party crate

Thanks for reporting this issue. It's unfortunally not really possible to handle that in any meaningful way due to restrictions in how rust derive macros work. We have essentially only access to the string representation of the struct the derive macro is applied to, that excludes any type information or even information about how the wrap_opt macro is defined. Depending on the macro definition that kind of structure might be valid or not, so we also cannot just say: "It's a macro, let's return an error". "Fixing" this would require larger changes to how proc macros work at a language level.

Based on this I would like to close this issue as wont-fix, as there is really no solution here.

I thought that might be the case, but I just wanted to confirm this with someone with more Rust experience than I have. Thanks for looking at the report. I'll find another way of implementing what I need.