From ab1260c8e0016890c34f50feab78bcde3da0ba4f Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sat, 9 Nov 2019 14:36:56 -0500 Subject: [PATCH 1/4] hide helper traits from calling code Makes a couple of changes to avoid helper traits from appearing anywhere in the caller code. It changes the name of the `Display` trait method to reduce the chance that an inherent method conflicts with the type in question, causing undesirable results. It also wraps the helper trait and declarations into an anonymous const context, meaning the helper traits aren't visible to the calling context. --- src/expand.rs | 48 +++++++++++++++++++++--------------------------- src/fmt.rs | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/src/expand.rs b/src/expand.rs index 2dda1fd..ec91558 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -4,38 +4,46 @@ use quote::{format_ident, quote}; use syn::{Data, DataEnum, DataStruct, DeriveInput, Error, Fields, Result}; pub fn derive(input: &DeriveInput) -> Result { - match &input.data { + let impls = match &input.data { Data::Struct(data) => impl_struct(input, data), Data::Enum(data) => impl_enum(input, data), Data::Union(_) => Err(Error::new_spanned(input, "Unions are not supported")), - } + }?; + + let helpers = specialization(); + Ok(quote! { + const _: () = { + #helpers + #impls + }; + }) } #[cfg(feature = "std")] fn specialization() -> TokenStream { quote! { trait DisplayToDisplayDoc { - fn get_display(&self) -> Self; + fn __displaydoc_display(&self) -> &Self; } - impl DisplayToDisplayDoc for &T { - fn get_display(&self) -> Self { + impl DisplayToDisplayDoc for T { + fn __displaydoc_display(&self) -> &Self { self } } trait PathToDisplayDoc { - fn get_display(&self) -> std::path::Display<'_>; + fn __displaydoc_display(&self) -> std::path::Display<'_>; } impl PathToDisplayDoc for std::path::Path { - fn get_display(&self) -> std::path::Display<'_> { + fn __displaydoc_display(&self) -> std::path::Display<'_> { self.display() } } impl PathToDisplayDoc for std::path::PathBuf { - fn get_display(&self) -> std::path::Display<'_> { + fn __displaydoc_display(&self) -> std::path::Display<'_> { self.display() } } @@ -74,13 +82,7 @@ fn impl_struct(input: &DeriveInput, data: &DataStruct) -> Result { } }); - let needed_traits = specialization(); - - Ok(quote! { - #needed_traits - - #display - }) + Ok(quote! { #display }) } fn impl_enum(input: &DeriveInput, data: &DataEnum) -> Result { @@ -93,7 +95,7 @@ fn impl_enum(input: &DeriveInput, data: &DataEnum) -> Result { .map(|variant| attr::display(&variant.attrs)) .collect::>>()?; - let display = if displays.iter().any(Option::is_some) { + if displays.iter().any(Option::is_some) { let arms = data .variants .iter() @@ -115,7 +117,7 @@ fn impl_enum(input: &DeriveInput, data: &DataEnum) -> Result { }) }) .collect::>>()?; - Some(quote! { + Ok(quote! { impl #impl_generics core::fmt::Display for #ty #ty_generics #where_clause { fn fmt(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result { #[allow(unused_variables)] @@ -126,14 +128,6 @@ fn impl_enum(input: &DeriveInput, data: &DataEnum) -> Result { } }) } else { - return Err(Error::new_spanned(input, "Missing doc comments")); - }; - - let needed_traits = specialization(); - - Ok(quote! { - #needed_traits - - #display - }) + Err(Error::new_spanned(input, "Missing doc comments")) + } } diff --git a/src/fmt.rs b/src/fmt.rs index 3ff3c2e..8d3a583 100644 --- a/src/fmt.rs +++ b/src/fmt.rs @@ -3,11 +3,6 @@ use proc_macro2::TokenStream; use quote::quote_spanned; use syn::{Ident, LitStr}; -#[cfg(feature = "std")] -const IS_STD: bool = true; -#[cfg(not(feature = "std"))] -const IS_STD: bool = false; - macro_rules! peek_next { ($read:ident) => { match $read.chars().next() { @@ -49,8 +44,8 @@ impl Display { let next = peek_next!(read); - let arg = if IS_STD && next == '}' { - quote_spanned!(span=> , (&#ident).get_display()) + let arg = if cfg!(feature = "std") && next == '}' { + quote_spanned!(span=> , #ident.__displaydoc_display()) } else { quote_spanned!(span=> , #ident) }; @@ -120,11 +115,33 @@ mod tests { assert( "{v} {v:?} {0} {0:?}", "{} {:?} {} {:?}", - ", ( & v ) . get_display ( ) , v , ( & _0 ) . get_display ( ) , _0", + ", v . __displaydoc_display ( ) , v , _0 . __displaydoc_display ( ) , _0", + ); + assert( + "error {var}", + "error {}", + ", var . __displaydoc_display ( )", + ); + + assert( + "The path {0}", + "The path {}", + ", _0 . __displaydoc_display ( )", + ); + assert("The path {0:?}", "The path {:?}", ", _0"); + } + + #[test] + #[cfg_attr(feature = "std", ignore)] + fn test_nostd_expand() { + assert( + "{v} {v:?} {0} {0:?}", + "{} {:?} {} {:?}", + ", v , v , _0 , _0", ); - assert("error {var}", "error {}", ", ( & var ) . get_display ( )"); + assert("error {var}", "error {}", ", var"); - // assert("The path {0.display()}", "The path {}", "0.display()"); - // assert("The path {0.display():?}", "The path {:?}", "0.display()"); + assert("The path {0}", "The path {}", ", _0"); + assert("The path {0:?}", "The path {:?}", ", _0"); } } From 12def5321fc5e4aca5732fe659ee001536106672 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sat, 9 Nov 2019 14:43:04 -0500 Subject: [PATCH 2/4] tests for path and struct display impls --- tests/happy.rs | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/happy.rs b/tests/happy.rs index 054f4e4..118eb39 100644 --- a/tests/happy.rs +++ b/tests/happy.rs @@ -1,5 +1,13 @@ use displaydoc::Display; -// use std::path::PathBuf; + +#[cfg(feature = "std")] +use std::path::PathBuf; + +#[derive(Display)] +/// Just a basic struct {thing} +struct HappyStruct { + thing: &'static str, +} #[derive(Display)] enum Happy { @@ -18,8 +26,10 @@ enum Happy { /// Variant5 just has {0} many problems /// but multi line comments aren't one of them Variant5(u32), - // /// The path {0.display()} - // Variant6(PathBuf), + + /// The path {0} + #[cfg(feature = "std")] + Variant6(PathBuf), } fn assert_display(input: T, expected: &'static str) { @@ -37,8 +47,15 @@ fn does_it_print() { "Variant4 wants to have a lot of lines\n\n Lets see how this works out for it", ); assert_display(Happy::Variant5(2), "Variant5 just has 2 many problems"); - // assert_display( - // Happy::Variant6(PathBuf::from("/var/log/happy")), - // "The path /var/log/happy", - // ); + + assert_display(HappyStruct { thing: "hi" }, "Just a basic struct hi"); +} + +#[test] +#[cfg(feature = "std")] +fn does_it_print_path() { + assert_display( + Happy::Variant6(PathBuf::from("/var/log/happy")), + "The path /var/log/happy", + ); } From 894d1d57ff8abf3be4424d1d6074c7c431e33275 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sat, 9 Nov 2019 16:23:12 -0500 Subject: [PATCH 3/4] use a dummy const to support rustc < 1.37 --- src/expand.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/expand.rs b/src/expand.rs index ec91558..5e6a6b5 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -11,8 +11,10 @@ pub fn derive(input: &DeriveInput) -> Result { }?; let helpers = specialization(); + let dummy_const = format_ident!("_DERIVE_Display_FOR_{}", input.ident); Ok(quote! { - const _: () = { + #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)] + const #dummy_const: () = { #helpers #impls }; From 5db9364fbd6f55a00729e25c8bffd433c2b722da Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sat, 9 Nov 2019 19:06:52 -0500 Subject: [PATCH 4/4] run ci on pull_request --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a549fa..2852c0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,4 @@ -on: [push] +on: [push, pull_request] name: Continuous integration