From a295c38ba390d4db56dcb2195c3d3b24fb502901 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 18 Mar 2019 15:06:56 -0600 Subject: [PATCH 1/5] Allow `#[serde(serde_path = "...")]` to override `extern crate serde` This is intended to be used by other crates which provide their own proc macros and use serde internally. Today there's no consistent way to put `#[derive(Deserialize)]` on a struct that consistently works, since crates may be using either `features = ["derive"]` or relying on `serde_derive` separately. Even if we assume that everyone is using `features = ["derive"]`, without this commit, any crate which generates `#[derive(serde::Deserialize)]` forces its consumers to put `serde` in their `Cargo.toml`, even if they aren't otherwise using serde for anything. Examples of crates which suffer from this in the real world are tower-web and swirl. With this feature, it's expected that these crates would have `pub extern crate serde;` in some accessible path, and add `#[serde(serde_path = "that_crate::wherever::serde")]` anywhere they place serde's derives. Those crates would also have to derive `that_crate::whatever::serde::Deserialize`, or `use` the macros explicitly beforehand. The test for this is a little funky, as it's testing this in a way that is not the intended use case, or even one we want to support. It has its own module which re-exports all of serde, but defines its own `Serialize` and `Deserialize` traits. We then test that we generated impls for those traits, instead of serde's. The only other way to test this would be to create a new test crate which does not depend on serde, but instead depends on `serde_derive` and a third crate which publicly re-exports serde. This feels like way too much overhead for a single test case, hence the funky test given. I didn't see anywhere in this repo to document this attribute, so I assume the docs will have to be done as a separate PR to a separate repo. Fixes #1487 --- serde_derive/src/de.rs | 2 +- serde_derive/src/dummy.rs | 16 +++++++++--- serde_derive/src/internals/attr.rs | 14 +++++++++++ serde_derive/src/ser.rs | 2 +- test_suite/tests/test_serde_path.rs | 39 +++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 test_suite/tests/test_serde_path.rs diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7fa1030b1..7e94ef255 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -60,7 +60,7 @@ pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result TokenStream { +pub fn wrap_in_const(serde_path: Option<&syn::Path>, trait_: &str, ty: &Ident, code: TokenStream) -> TokenStream { let try_replacement = try::replacement(); let dummy_const = Ident::new( @@ -10,13 +10,21 @@ pub fn wrap_in_const(trait_: &str, ty: &Ident, code: TokenStream) -> TokenStream Span::call_site(), ); - quote! { - #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)] - const #dummy_const: () = { + let use_serde = serde_path.map(|path| { + quote!(use #path as _serde;) + }).unwrap_or_else(|| { + quote! { #[allow(unknown_lints)] #[cfg_attr(feature = "cargo-clippy", allow(useless_attribute))] #[allow(rust_2018_idioms)] extern crate serde as _serde; + } + }); + + quote! { + #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)] + const #dummy_const: () = { + #use_serde #try_replacement #code }; diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index b95d490f2..8bc91bbb9 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -218,6 +218,7 @@ pub struct Container { remote: Option, identifier: Identifier, has_flatten: bool, + serde_path: Option, } /// Styles of representing an enum. @@ -298,6 +299,7 @@ impl Container { let mut remote = Attr::none(cx, "remote"); let mut field_identifier = BoolAttr::none(cx, "field_identifier"); let mut variant_identifier = BoolAttr::none(cx, "variant_identifier"); + let mut serde_path = Attr::none(cx, "serde_path"); for meta_items in item.attrs.iter().filter_map(get_serde_meta_items) { for meta_item in meta_items { @@ -582,6 +584,13 @@ impl Container { variant_identifier.set_true(word); } + // Parse `#[serde(serde_path = "foo")]` + Meta(NameValue(ref m)) if m.ident == "serde_path" => { + if let Ok(path) = parse_lit_into_path(cx, &m.ident, &m.lit) { + serde_path.set(&m.ident, path) + } + } + Meta(ref meta_item) => { cx.error_spanned_by( meta_item.name(), @@ -613,6 +622,7 @@ impl Container { remote: remote.get(), identifier: decide_identifier(cx, item, field_identifier, variant_identifier), has_flatten: false, + serde_path: serde_path.get(), } } @@ -671,6 +681,10 @@ impl Container { pub fn mark_has_flatten(&mut self) { self.has_flatten = true; } + + pub fn serde_path(&self) -> Option<&syn::Path> { + self.serde_path.as_ref() + } } fn decide_tag( diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 4d1f680db..8f3e357cb 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -51,7 +51,7 @@ pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result AssertNotSerdeDeserialize<'a> for Foo {} + + fake_serde::assert::(); +} + +mod fake_serde { + pub use serde::*; + + pub fn assert() + where + T: Serialize, + T: for<'a> Deserialize<'a>, + {} + + pub trait Serialize { + fn serialize(&self, serializer: S) -> Result; + } + + pub trait Deserialize<'a>: Sized { + fn deserialize>(deserializer: D) -> Result; + } +} + +trait AssertNotSerdeSerialize {} + +impl AssertNotSerdeSerialize for T {} + +trait AssertNotSerdeDeserialize<'a> {} + +impl<'a, T: serde::Deserialize<'a>> AssertNotSerdeDeserialize<'a> for T {} From 0e6ce8fa508a556d4aa3f2f9e6b724fcaf842440 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 20 Mar 2019 14:31:49 -0600 Subject: [PATCH 2/5] Fix for Rust 1.15 --- serde_derive/src/dummy.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/serde_derive/src/dummy.rs b/serde_derive/src/dummy.rs index f9d1e5fac..a4e0e038b 100644 --- a/serde_derive/src/dummy.rs +++ b/serde_derive/src/dummy.rs @@ -1,5 +1,6 @@ use proc_macro2::{Ident, Span, TokenStream}; +use syn; use try; pub fn wrap_in_const(serde_path: Option<&syn::Path>, trait_: &str, ty: &Ident, code: TokenStream) -> TokenStream { From b4d8a55b2ac2017598a9cccd5b4bceb4c180444e Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 28 Mar 2019 11:42:50 -0600 Subject: [PATCH 3/5] Change `serde_path` to `crate` Also changed the generated code to have at least one thing refer to the path directly, rather than via `use` -- This shows that the impl *can* work without `use`, but doesn't actually do all the work to remove the `use` lines unless we decide we need this feature to work on the 2015 edition --- serde_derive/src/de.rs | 2 +- serde_derive/src/internals/attr.rs | 15 +++++++++++---- serde_derive/src/ser.rs | 13 +++++++------ test_suite/tests/test_serde_path.rs | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7e94ef255..7243b53c4 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -60,7 +60,7 @@ pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result { + // Parse `#[serde(crate = "foo")]` + Meta(NameValue(ref m)) if m.ident == "crate" => { if let Ok(path) = parse_lit_into_path(cx, &m.ident, &m.lit) { serde_path.set(&m.ident, path) } @@ -682,9 +683,15 @@ impl Container { self.has_flatten = true; } - pub fn serde_path(&self) -> Option<&syn::Path> { + pub fn custom_serde_path(&self) -> Option<&syn::Path> { self.serde_path.as_ref() } + + pub fn serde_path<'a>(&'a self) -> Cow<'a, syn::Path> { + self.custom_serde_path() + .map(Cow::Borrowed) + .unwrap_or_else(|| Cow::Owned(parse_quote!(_serde))) + } } fn decide_tag( diff --git a/serde_derive/src/ser.rs b/serde_derive/src/ser.rs index 8f3e357cb..d5c83bc64 100644 --- a/serde_derive/src/ser.rs +++ b/serde_derive/src/ser.rs @@ -22,15 +22,16 @@ pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result(__self: &#remote #ty_generics, __serializer: __S) -> _serde::export::Result<__S::Ok, __S::Error> + #vis fn serialize<__S>(__self: &#remote #ty_generics, __serializer: __S) -> #serde::export::Result<__S::Ok, __S::Error> where - __S: _serde::Serializer, + __S: #serde::Serializer, { #used #body @@ -40,10 +41,10 @@ pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result(&self, __serializer: __S) -> _serde::export::Result<__S::Ok, __S::Error> + impl #impl_generics #serde::Serialize for #ident #ty_generics #where_clause { + fn serialize<__S>(&self, __serializer: __S) -> #serde::export::Result<__S::Ok, __S::Error> where - __S: _serde::Serializer, + __S: #serde::Serializer, { #body } @@ -51,7 +52,7 @@ pub fn expand_derive_serialize(input: &syn::DeriveInput) -> Result Date: Wed, 3 Apr 2019 09:16:17 -0700 Subject: [PATCH 4/5] Refer directly to serde_path in Deserialize impl This makes it not a breaking change if we later want to eliminate the `use #serde_path as _serde;` line. --- serde_derive/src/de.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/serde_derive/src/de.rs b/serde_derive/src/de.rs index 7243b53c4..df76ec631 100644 --- a/serde_derive/src/de.rs +++ b/serde_derive/src/de.rs @@ -27,15 +27,16 @@ pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result(__deserializer: __D) -> _serde::export::Result<#remote #ty_generics, __D::Error> + #vis fn deserialize<__D>(__deserializer: __D) -> #serde::export::Result<#remote #ty_generics, __D::Error> where - __D: _serde::Deserializer<#delife>, + __D: #serde::Deserializer<#delife>, { #used #body @@ -47,10 +48,10 @@ pub fn expand_derive_deserialize(input: &syn::DeriveInput) -> Result for #ident #ty_generics #where_clause { - fn deserialize<__D>(__deserializer: __D) -> _serde::export::Result + impl #de_impl_generics #serde::Deserialize<#delife> for #ident #ty_generics #where_clause { + fn deserialize<__D>(__deserializer: __D) -> #serde::export::Result where - __D: _serde::Deserializer<#delife>, + __D: #serde::Deserializer<#delife>, { #body } From f3c6b9f05a8df75f050680cc077368d723f8854c Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Wed, 3 Apr 2019 09:18:45 -0700 Subject: [PATCH 5/5] Simplify signature of Container::serde_path --- serde_derive/src/internals/attr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde_derive/src/internals/attr.rs b/serde_derive/src/internals/attr.rs index 0b96118d2..db763e22c 100644 --- a/serde_derive/src/internals/attr.rs +++ b/serde_derive/src/internals/attr.rs @@ -687,7 +687,7 @@ impl Container { self.serde_path.as_ref() } - pub fn serde_path<'a>(&'a self) -> Cow<'a, syn::Path> { + pub fn serde_path(&self) -> Cow { self.custom_serde_path() .map(Cow::Borrowed) .unwrap_or_else(|| Cow::Owned(parse_quote!(_serde)))