From a295c38ba390d4db56dcb2195c3d3b24fb502901 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 18 Mar 2019 15:06:56 -0600 Subject: [PATCH] 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 7fa1030b..7e94ef25 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 b95d490f..8bc91bbb 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 4d1f680d..8f3e357c 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 {}