Silence some warnings from derive(Properties) (#2266)

* silence some warnings from derive(Properties)

* for consistency preserve [#deny(..)]

* preserve some attributes on the struct def

adds a test case to check for several warnings

* add #[deny] in test cases to error instead of warning on failure

Co-authored-by: Julius Lungys <32368314+voidpumpkin@users.noreply.github.com>
This commit is contained in:
WorldSEnder 2021-12-16 10:38:53 +01:00 committed by GitHub
parent 1ef364ffda
commit f5b921984a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 94 additions and 14 deletions

View File

@ -9,6 +9,7 @@ use super::generics::{to_arguments, with_param_bounds, GenericArguments};
use super::{DerivePropsInput, PropField};
use proc_macro2::{Ident, Span};
use quote::{format_ident, quote, ToTokens};
use syn::Attribute;
pub struct PropsBuilder<'a> {
builder_name: &'a Ident,
@ -16,6 +17,7 @@ pub struct PropsBuilder<'a> {
step_names: Vec<Ident>,
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,
}
}
}
@ -138,6 +143,7 @@ impl PropsBuilder<'_> {
builder_name,
props,
step_names,
extra_attrs,
..
} = self;
let DerivePropsInput {
@ -183,6 +189,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)*

View File

@ -1,11 +1,12 @@
use super::generics::GenericArguments;
use super::should_preserve_attr;
use proc_macro2::{Ident, Span};
use quote::{format_ident, quote, quote_spanned};
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 +23,7 @@ pub struct PropField {
ty: Type,
name: Ident,
attr: PropAttr,
extra_attrs: Vec<Attribute>,
}
impl PropField {
@ -51,7 +53,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),
@ -77,21 +79,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>,
}
}
@ -101,7 +111,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,
}
}
@ -113,13 +125,13 @@ 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)]
#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,
@ -130,8 +142,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
}
}
@ -139,12 +151,17 @@ 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
}
}
}
};
let extra_attrs = &self.extra_attrs;
quote! {
#( #extra_attrs )*
#build_fn
}
}
@ -214,8 +231,16 @@ impl TryFrom<Field> for PropField {
type Error = Error;
fn try_from(field: Field) -> Result<Self> {
let extra_attrs = field
.attrs
.iter()
.filter(|a| should_preserve_attr(a))
.cloned()
.collect();
Ok(PropField {
attr: Self::attribute(&field)?,
extra_attrs,
ty: field.ty,
name: field.ident.unwrap(),
})

View File

@ -9,7 +9,7 @@ use proc_macro2::{Ident, Span};
use quote::{format_ident, 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<PropField>,
preserved_attrs: Vec<Attribute>,
}
/// 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 = format_ident!("{}Wrapper", props_name, span = 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 = format_ident!("{}Builder", props_name, span = Span::call_site());
let builder_step = format_ident!("{}BuilderStep", props_name, span = 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());

View File

@ -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,
}
}
}

View File

@ -251,12 +251,24 @@ 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
struct Props {
#[allow(dead_code)]
create_message: ::std::option::Option<bool>,
NonSnakeCase: u32,
}
}
mod raw_field_names {
#[derive(::yew::Properties, ::std::cmp::PartialEq)]
pub struct Props {
r#true: u32,
r#pointless_raw_name: u32,
}
}
fn main() {}