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

derive_Elastic*Mapping Macro #59

Closed
KodrAus opened this issue Mar 3, 2016 · 4 comments
Closed

derive_Elastic*Mapping Macro #59

KodrAus opened this issue Mar 3, 2016 · 4 comments
Assignees
Milestone

Comments

@KodrAus
Copy link
Member

KodrAus commented Mar 3, 2016

A compiler plugin macro that implements the appropriate Elastic*Mapping trait, moves the body fns to the impl, and implements the correct serde::Serialize format.

This will be a nuke-macro that makes it easy for a user to just do it as far as defining custom mapping goes. Doing it manually isn't a huge chore anyways. This is just for extra convenience.

@KodrAus KodrAus added the types label Mar 3, 2016
@KodrAus KodrAus added this to the 0.3 - Rotor / Types milestone Mar 3, 2016
@KodrAus KodrAus self-assigned this Mar 3, 2016
@KodrAus
Copy link
Member Author

KodrAus commented Mar 4, 2016

So I'm thinking that having an Elastic*Mapping macro is a dumb idea. An ElasticMapping macro will be fine, all it needs to do is derive serde::Serialize and use the given Visitor...

@KodrAus KodrAus modified the milestones: 0.2 - CI, 0.3 - Rotor / Types Mar 4, 2016
@KodrAus
Copy link
Member Author

KodrAus commented Mar 7, 2016

This should also cover building up the Visitor for main type mappings, in basically the same way. If possible, just reuse the ElasticMapping trait, so the Visitor is available to some automatic mapping function.

@KodrAus
Copy link
Member Author

KodrAus commented Mar 8, 2016

So this whole mapping design needs to be revisited.

See: rust-lang/rfcs#1148

Because we can't convince the compiler that StringFormat can't possibly also be a DateFormat, it's not possible to use the generics required on the ElasticMapping<T>, which are required to work around the T is not bound by trait impls...:

src/date/mapping.rs:110:1: 114:2 error: conflicting implementations of trait `mapping::TypeEllision<_>`: [E0119]
src/date/mapping.rs:110 impl <T: DateFormat, M: ElasticDateMapping<T>> TypeEllision<T> for M {
src/date/mapping.rs:111     fn get_ellision() -> TypeEllisionKind {
src/date/mapping.rs:112         TypeEllisionKind::Ellided
src/date/mapping.rs:113     }
src/date/mapping.rs:114 }
src/date/mapping.rs:110:1: 114:2 help: run `rustc --explain E0119` to see a detailed explanation
src/string/mapping.rs:173:1: 177:2 note: conflicting implementation is here:
src/string/mapping.rs:173 impl <T: StringFormat, M: ElasticStringMapping<T>> TypeEllision<T> for M {
src/string/mapping.rs:174   fn get_ellision() -> TypeEllisionKind {
src/string/mapping.rs:175       TypeEllisionKind::Ellided
src/string/mapping.rs:176   }
src/string/mapping.rs:177 }
src/date/mapping.rs:102:1: 108:2 error: conflicting implementations of trait `mapping::ElasticMapping<_>`: [E0119]
src/date/mapping.rs:102 impl <T: DateFormat, M: ElasticDateMapping<T>> ElasticMapping<T> for M {
src/date/mapping.rs:103     type Visitor = ElasticDateMappingVisitor<T, M>;
src/date/mapping.rs:104 
src/date/mapping.rs:105     fn data_type() -> &'static str {
src/date/mapping.rs:106         "date"
src/date/mapping.rs:107     }
                        ...
src/date/mapping.rs:102:1: 108:2 help: run `rustc --explain E0119` to see a detailed explanation
src/string/mapping.rs:165:1: 171:2 note: conflicting implementation is here:
src/string/mapping.rs:165 impl <T: StringFormat, M: ElasticStringMapping<T>> ElasticMapping<T> for M {
src/string/mapping.rs:166   type Visitor = ElasticStringMappingVisitor<T, M>;
src/string/mapping.rs:167 
src/string/mapping.rs:168   fn data_type() -> &'static str {
src/string/mapping.rs:169       "string"
src/string/mapping.rs:170   }

Possible options include not auto deriving formats and mappings. Just make it easy enough to do on the client side with a macro, so only concrete impls are available. But that almost defeats the purpose of using traits in the first place because it makes it almost not worth it for people who don't want to use macros.

This can at least be tested.

@KodrAus
Copy link
Member Author

KodrAus commented Mar 8, 2016

So this has been reworked to not use any auto impls and is looking alright so far. I'll get this all to work properly, then tidy it up (ElasticStringType should inherit StringFormat, make it the second generic arg)

@KodrAus KodrAus closed this as completed Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant