Skip to content

Commit

Permalink
Silence some warnings from derive(Properties) (#2266)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
WorldSEnder and voidpumpkin committed Dec 16, 2021
1 parent 1ef364f commit f5b9219
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 14 deletions.
7 changes: 7 additions & 0 deletions packages/yew-macro/src/derive_props/builder.rs
Expand Up @@ -9,13 +9,15 @@ 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,
step_trait: &'a Ident,
step_names: Vec<Ident>,
props: &'a DerivePropsInput,
wrapper_name: &'a Ident,
extra_attrs: &'a [Attribute],
}

impl ToTokens for PropsBuilder<'_> {
Expand All @@ -26,6 +28,7 @@ impl ToTokens for PropsBuilder<'_> {
step_names,
props,
wrapper_name,
..
} = self;

let DerivePropsInput {
Expand Down Expand Up @@ -91,13 +94,15 @@ impl<'a> PropsBuilder<'_> {
step_trait: &'a Ident,
props: &'a DerivePropsInput,
wrapper_name: &'a Ident,
extra_attrs: &'a [Attribute],
) -> PropsBuilder<'a> {
PropsBuilder {
builder_name: name,
step_trait,
step_names: Self::build_step_names(step_trait, &props.prop_fields),
props,
wrapper_name,
extra_attrs,
}
}
}
Expand Down Expand Up @@ -138,6 +143,7 @@ impl PropsBuilder<'_> {
builder_name,
props,
step_names,
extra_attrs,
..
} = self;
let DerivePropsInput {
Expand Down Expand Up @@ -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)*
Expand Down
45 changes: 35 additions & 10 deletions packages/yew-macro/src/derive_props/field.rs
@@ -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)]
Expand All @@ -22,6 +23,7 @@ pub struct PropField {
ty: Type,
name: Ident,
attr: PropAttr,
extra_attrs: Vec<Attribute>,
}

impl PropField {
Expand Down Expand Up @@ -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),
Expand All @@ -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>,
}
}
Expand All @@ -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,
}
}
Expand All @@ -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,
Expand All @@ -130,21 +142,26 @@ 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
}
}
}
_ => {
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
}
}

Expand Down Expand Up @@ -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(),
})
Expand Down
37 changes: 34 additions & 3 deletions packages/yew-macro/src/derive_props/mod.rs
Expand Up @@ -9,14 +9,26 @@ 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 {
vis: Visibility,
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 {
Expand All @@ -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,
})
}
}
Expand All @@ -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());

Expand Down
7 changes: 6 additions & 1 deletion packages/yew-macro/src/derive_props/wrapper.rs
@@ -1,19 +1,21 @@
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<'_> {
fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
let Self {
generics,
wrapper_name,
extra_attrs,
..
} = self;

Expand All @@ -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
{
Expand All @@ -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,
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions packages/yew-macro/tests/derive_props/pass.rs
Expand Up @@ -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() {}

0 comments on commit f5b9219

Please sign in to comment.