From 227882451a6f44519970ee02c6f9b1ad7252e7e9 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 18 Nov 2021 23:28:33 +0100 Subject: [PATCH 1/4] silence some warnings from derive(Properties) --- packages/yew-macro/src/derive_props/field.rs | 39 ++++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/packages/yew-macro/src/derive_props/field.rs b/packages/yew-macro/src/derive_props/field.rs index 54be0a4d542..6fedf52f8ba 100644 --- a/packages/yew-macro/src/derive_props/field.rs +++ b/packages/yew-macro/src/derive_props/field.rs @@ -5,7 +5,7 @@ use std::cmp::{Ord, Ordering, PartialEq, PartialOrd}; use std::convert::TryFrom; use syn::parse::Result; use syn::spanned::Spanned; -use syn::{Error, Expr, Field, Path, Type, TypePath, Visibility}; +use syn::{Attribute, Error, Expr, Field, Path, Type, TypePath, Visibility}; #[allow(clippy::large_enum_variant)] #[derive(PartialEq, Eq)] @@ -22,6 +22,7 @@ pub struct PropField { ty: Type, name: Ident, attr: PropAttr, + extra_attrs: Vec, } impl PropField { @@ -49,7 +50,7 @@ impl PropField { /// Used to transform the `PropWrapper` struct into `Properties` pub fn to_field_setter(&self) -> proc_macro2::TokenStream { let name = &self.name; - match &self.attr { + let setter = match &self.attr { PropAttr::Required { wrapped_name } => { quote! { #name: ::std::option::Option::unwrap(self.wrapped.#wrapped_name), @@ -75,21 +76,29 @@ impl PropField { #name: ::std::option::Option::unwrap_or_default(self.wrapped.#name), } } + }; + let extra_attrs = &self.extra_attrs; + quote! { + #( #extra_attrs )* + #setter } } /// Wrap all required props in `Option` pub fn to_field_def(&self) -> proc_macro2::TokenStream { let ty = &self.ty; + let extra_attrs = &self.extra_attrs; let wrapped_name = self.wrapped_name(); match &self.attr { PropAttr::Option => { quote! { + #( #extra_attrs )* #wrapped_name: #ty, } } _ => { quote! { + #( #extra_attrs )* #wrapped_name: ::std::option::Option<#ty>, } } @@ -99,7 +108,9 @@ impl PropField { /// All optional props must implement the `Default` trait pub fn to_default_setter(&self) -> proc_macro2::TokenStream { let wrapped_name = self.wrapped_name(); + let extra_attrs = &self.extra_attrs; quote! { + #( #extra_attrs )* #wrapped_name: ::std::option::Option::None, } } @@ -111,8 +122,8 @@ impl PropField { generic_arguments: &GenericArguments, vis: &Visibility, ) -> proc_macro2::TokenStream { - let Self { name, ty, attr } = self; - match attr { + let Self { name, ty, attr, .. } = self; + let build_fn = match attr { PropAttr::Required { wrapped_name } => { quote! { #[doc(hidden)] @@ -143,6 +154,11 @@ impl PropField { } } } + }; + let extra_attrs = &self.extra_attrs; + quote! { + #( #extra_attrs )* + #build_fn } } @@ -176,6 +192,20 @@ impl PropField { Ok(PropAttr::Required { wrapped_name }) } } + + /// Some attributes on the original struct should be preserved and added to the builder struct, + /// in order to avoid warnings (sometimes reported as errors) in the output. + fn preserved_attrs(named_field: &Field) -> Vec { + // #[cfg(...)]: does not usually appear in macro inputs, but rust-analyzer seems to generate it sometimes. + // If not preserved, results in "no-such-field" errors generating the field setter for `build` + // #[allow(...)]: silences warnings from clippy, such as dead_code etc. + named_field + .attrs + .iter() + .filter(|a| a.path.is_ident("allow") || a.path.is_ident("cfg")) + .cloned() + .collect() + } } fn is_path_segments_an_option(path_segments: impl Iterator) -> bool { @@ -214,6 +244,7 @@ impl TryFrom for PropField { fn try_from(field: Field) -> Result { Ok(PropField { attr: Self::attribute(&field)?, + extra_attrs: Self::preserved_attrs(&field), ty: field.ty, name: field.ident.unwrap(), }) From e3953b57a486adc7362c25c6344cfa4c89c7e33d Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Tue, 14 Dec 2021 04:51:45 +0100 Subject: [PATCH 2/4] for consistency preserve [#deny(..)] --- packages/yew-macro/src/derive_props/field.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/yew-macro/src/derive_props/field.rs b/packages/yew-macro/src/derive_props/field.rs index 6fedf52f8ba..119dbedaf40 100644 --- a/packages/yew-macro/src/derive_props/field.rs +++ b/packages/yew-macro/src/derive_props/field.rs @@ -193,16 +193,19 @@ impl PropField { } } - /// Some attributes on the original struct should be preserved and added to the builder struct, + /// Some attributes on the original struct are to be preserved and added to the builder struct, /// in order to avoid warnings (sometimes reported as errors) in the output. fn preserved_attrs(named_field: &Field) -> Vec { // #[cfg(...)]: does not usually appear in macro inputs, but rust-analyzer seems to generate it sometimes. // If not preserved, results in "no-such-field" errors generating the field setter for `build` // #[allow(...)]: silences warnings from clippy, such as dead_code etc. + // #[deny(...)]: enable additional warnings from clippy named_field .attrs .iter() - .filter(|a| a.path.is_ident("allow") || a.path.is_ident("cfg")) + .filter(|a| { + a.path.is_ident("allow") || a.path.is_ident("deny") || a.path.is_ident("cfg") + }) .cloned() .collect() } From 8934fa0e4eceb89a29202f298fd9f456b65adb3b Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 15 Dec 2021 07:51:53 +0100 Subject: [PATCH 3/4] preserve some attributes on the struct def adds a test case to check for several warnings --- .../yew-macro/src/derive_props/builder.rs | 7 ++++ packages/yew-macro/src/derive_props/field.rs | 39 +++++++------------ packages/yew-macro/src/derive_props/mod.rs | 37 ++++++++++++++++-- .../yew-macro/src/derive_props/wrapper.rs | 7 +++- packages/yew-macro/tests/derive_props/pass.rs | 11 ++++++ 5 files changed, 73 insertions(+), 28 deletions(-) diff --git a/packages/yew-macro/src/derive_props/builder.rs b/packages/yew-macro/src/derive_props/builder.rs index 3b41a3de0cf..69ee96df282 100644 --- a/packages/yew-macro/src/derive_props/builder.rs +++ b/packages/yew-macro/src/derive_props/builder.rs @@ -9,6 +9,7 @@ use super::generics::{to_arguments, with_param_bounds, GenericArguments}; use super::{DerivePropsInput, PropField}; use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; +use syn::Attribute; pub struct PropsBuilder<'a> { builder_name: &'a Ident, @@ -16,6 +17,7 @@ pub struct PropsBuilder<'a> { step_names: Vec, props: &'a DerivePropsInput, wrapper_name: &'a Ident, + extra_attrs: &'a [Attribute], } impl ToTokens for PropsBuilder<'_> { @@ -26,6 +28,7 @@ impl ToTokens for PropsBuilder<'_> { step_names, props, wrapper_name, + .. } = self; let DerivePropsInput { @@ -91,6 +94,7 @@ impl<'a> PropsBuilder<'_> { step_trait: &'a Ident, props: &'a DerivePropsInput, wrapper_name: &'a Ident, + extra_attrs: &'a [Attribute], ) -> PropsBuilder<'a> { PropsBuilder { builder_name: name, @@ -98,6 +102,7 @@ impl<'a> PropsBuilder<'_> { step_names: Self::build_step_names(step_trait, &props.prop_fields), props, wrapper_name, + extra_attrs, } } } @@ -137,6 +142,7 @@ impl PropsBuilder<'_> { builder_name, props, step_names, + extra_attrs, .. } = self; let DerivePropsInput { @@ -182,6 +188,7 @@ impl PropsBuilder<'_> { }); token_stream.extend(quote! { + #( #extra_attrs )* impl#impl_generics #builder_name<#current_step_arguments> #where_clause { #(#optional_prop_fn)* #(#required_prop_fn)* diff --git a/packages/yew-macro/src/derive_props/field.rs b/packages/yew-macro/src/derive_props/field.rs index 119dbedaf40..1014e900472 100644 --- a/packages/yew-macro/src/derive_props/field.rs +++ b/packages/yew-macro/src/derive_props/field.rs @@ -1,4 +1,5 @@ use super::generics::GenericArguments; +use super::should_preserve_attr; use proc_macro2::{Ident, Span}; use quote::{quote, quote_spanned}; use std::cmp::{Ord, Ordering, PartialEq, PartialOrd}; @@ -127,8 +128,8 @@ impl PropField { PropAttr::Required { wrapped_name } => { quote! { #[doc(hidden)] - #vis fn #name(mut self, #name: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { - self.wrapped.#wrapped_name = ::std::option::Option::Some(#name.into_prop_value()); + #vis fn #name(mut self, value: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { + self.wrapped.#wrapped_name = ::std::option::Option::Some(value.into_prop_value()); #builder_name { wrapped: self.wrapped, _marker: ::std::marker::PhantomData, @@ -139,8 +140,8 @@ impl PropField { PropAttr::Option => { quote! { #[doc(hidden)] - #vis fn #name(mut self, #name: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { - self.wrapped.#name = #name.into_prop_value(); + #vis fn #name(mut self, value: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { + self.wrapped.#name = value.into_prop_value(); self } } @@ -148,8 +149,8 @@ impl PropField { _ => { quote! { #[doc(hidden)] - #vis fn #name(mut self, #name: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { - self.wrapped.#name = ::std::option::Option::Some(#name.into_prop_value()); + #vis fn #name(mut self, value: impl ::yew::html::IntoPropValue<#ty>) -> #builder_name<#generic_arguments> { + self.wrapped.#name = ::std::option::Option::Some(value.into_prop_value()); self } } @@ -192,23 +193,6 @@ impl PropField { Ok(PropAttr::Required { wrapped_name }) } } - - /// Some attributes on the original struct are to be preserved and added to the builder struct, - /// in order to avoid warnings (sometimes reported as errors) in the output. - fn preserved_attrs(named_field: &Field) -> Vec { - // #[cfg(...)]: does not usually appear in macro inputs, but rust-analyzer seems to generate it sometimes. - // If not preserved, results in "no-such-field" errors generating the field setter for `build` - // #[allow(...)]: silences warnings from clippy, such as dead_code etc. - // #[deny(...)]: enable additional warnings from clippy - named_field - .attrs - .iter() - .filter(|a| { - a.path.is_ident("allow") || a.path.is_ident("deny") || a.path.is_ident("cfg") - }) - .cloned() - .collect() - } } fn is_path_segments_an_option(path_segments: impl Iterator) -> bool { @@ -245,9 +229,16 @@ impl TryFrom for PropField { type Error = Error; fn try_from(field: Field) -> Result { + let extra_attrs = field + .attrs + .iter() + .filter(|a| should_preserve_attr(a)) + .cloned() + .collect(); + Ok(PropField { attr: Self::attribute(&field)?, - extra_attrs: Self::preserved_attrs(&field), + extra_attrs, ty: field.ty, name: field.ident.unwrap(), }) diff --git a/packages/yew-macro/src/derive_props/mod.rs b/packages/yew-macro/src/derive_props/mod.rs index fa347f5a67b..65875b1e860 100644 --- a/packages/yew-macro/src/derive_props/mod.rs +++ b/packages/yew-macro/src/derive_props/mod.rs @@ -9,7 +9,7 @@ use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use std::convert::TryInto; use syn::parse::{Parse, ParseStream, Result}; -use syn::{DeriveInput, Generics, Visibility}; +use syn::{Attribute, DeriveInput, Generics, Visibility}; use wrapper::PropsWrapper; pub struct DerivePropsInput { @@ -17,6 +17,18 @@ pub struct DerivePropsInput { generics: Generics, props_name: Ident, prop_fields: Vec, + preserved_attrs: Vec, +} + +/// Some attributes on the original struct are to be preserved and added to the builder struct, +/// in order to avoid warnings (sometimes reported as errors) in the output. +fn should_preserve_attr(attr: &Attribute) -> bool { + // #[cfg(...)]: does not usually appear in macro inputs, but rust-analyzer seems to generate it sometimes. + // If not preserved, results in "no-such-field" errors generating the field setter for `build` + // #[allow(...)]: silences warnings from clippy, such as dead_code etc. + // #[deny(...)]: enable additional warnings from clippy + let path = &attr.path; + path.is_ident("allow") || path.is_ident("deny") || path.is_ident("cfg") } impl Parse for DerivePropsInput { @@ -42,11 +54,19 @@ impl Parse for DerivePropsInput { _ => unimplemented!("only structs are supported"), }; + let preserved_attrs = input + .attrs + .iter() + .filter(|a| should_preserve_attr(a)) + .cloned() + .collect(); + Ok(Self { vis: input.vis, props_name: input.ident, generics: input.generics, prop_fields, + preserved_attrs, }) } } @@ -61,13 +81,24 @@ impl ToTokens for DerivePropsInput { // The wrapper is a new struct which wraps required props in `Option` let wrapper_name = Ident::new(&format!("{}Wrapper", props_name), Span::call_site()); - let wrapper = PropsWrapper::new(&wrapper_name, generics, &self.prop_fields); + let wrapper = PropsWrapper::new( + &wrapper_name, + generics, + &self.prop_fields, + &self.preserved_attrs, + ); tokens.extend(wrapper.into_token_stream()); // The builder will only build if all required props have been set let builder_name = Ident::new(&format!("{}Builder", props_name), Span::call_site()); let builder_step = Ident::new(&format!("{}BuilderStep", props_name), Span::call_site()); - let builder = PropsBuilder::new(&builder_name, &builder_step, self, &wrapper_name); + let builder = PropsBuilder::new( + &builder_name, + &builder_step, + self, + &wrapper_name, + &self.preserved_attrs, + ); let builder_generic_args = builder.first_step_generic_args(); tokens.extend(builder.into_token_stream()); diff --git a/packages/yew-macro/src/derive_props/wrapper.rs b/packages/yew-macro/src/derive_props/wrapper.rs index d9b85cd3b12..dc5f24f5bb0 100644 --- a/packages/yew-macro/src/derive_props/wrapper.rs +++ b/packages/yew-macro/src/derive_props/wrapper.rs @@ -1,12 +1,13 @@ use super::PropField; use proc_macro2::Ident; use quote::{quote, ToTokens}; -use syn::Generics; +use syn::{Attribute, Generics}; pub struct PropsWrapper<'a> { wrapper_name: &'a Ident, generics: &'a Generics, prop_fields: &'a [PropField], + extra_attrs: &'a [Attribute], } impl ToTokens for PropsWrapper<'_> { @@ -14,6 +15,7 @@ impl ToTokens for PropsWrapper<'_> { let Self { generics, wrapper_name, + extra_attrs, .. } = self; @@ -24,6 +26,7 @@ impl ToTokens for PropsWrapper<'_> { let wrapper_default_setters = self.default_setters(); let wrapper = quote! { + #(#extra_attrs)* struct #wrapper_name#generics #where_clause { @@ -47,11 +50,13 @@ impl<'a> PropsWrapper<'_> { name: &'a Ident, generics: &'a Generics, prop_fields: &'a [PropField], + extra_attrs: &'a [Attribute], ) -> PropsWrapper<'a> { PropsWrapper { wrapper_name: name, generics, prop_fields, + extra_attrs, } } } diff --git a/packages/yew-macro/tests/derive_props/pass.rs b/packages/yew-macro/tests/derive_props/pass.rs index ce99ee6787d..c2bde88f047 100644 --- a/packages/yew-macro/tests/derive_props/pass.rs +++ b/packages/yew-macro/tests/derive_props/pass.rs @@ -251,4 +251,15 @@ mod t12 { } } +mod t13 { + #[derive(::std::cmp::PartialEq, ::yew::Properties)] + #[allow(non_snake_case)] // putting this on fields directly does not work, even in normal rust + struct Props { + #[allow(dead_code)] + create_message: ::std::option::Option, + NonSnakeCase: u32, + } + +} + fn main() {} From 58cb457c4b4f93c076bcf74890fbb3f296b3233e Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 15 Dec 2021 08:00:41 +0100 Subject: [PATCH 4/4] add #[deny] in test cases to error instead of warning on failure --- packages/yew-macro/tests/derive_props/pass.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/yew-macro/tests/derive_props/pass.rs b/packages/yew-macro/tests/derive_props/pass.rs index c2bde88f047..7026736b6b3 100644 --- a/packages/yew-macro/tests/derive_props/pass.rs +++ b/packages/yew-macro/tests/derive_props/pass.rs @@ -251,6 +251,7 @@ mod t12 { } } +#[deny(non_snake_case, dead_code)] mod t13 { #[derive(::std::cmp::PartialEq, ::yew::Properties)] #[allow(non_snake_case)] // putting this on fields directly does not work, even in normal rust