Remove special handing of struct fields of type Option in derive(Properties) (#3398)

* removed PropAttr::Option

* fixed formatting

* removed an irrelevant test

* added a test for converting value into Some(value) in the html! macro

* added more tests for derive(Properties)

* added more tests (again)
This commit is contained in:
Tim Kurdov 2023-09-23 13:34:30 +01:00 committed by GitHub
parent b71dbfe17e
commit ce7702e9e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 94 additions and 121 deletions

View File

@ -5,10 +5,7 @@ use proc_macro2::{Ident, Span};
use quote::{format_ident, quote, quote_spanned};
use syn::parse::Result;
use syn::spanned::Spanned;
use syn::{
parse_quote, Attribute, Error, Expr, Field, GenericParam, Generics, Path, Type, TypePath,
Visibility,
};
use syn::{parse_quote, Attribute, Error, Expr, Field, GenericParam, Generics, Type, Visibility};
use super::should_preserve_attr;
use crate::derive_props::generics::push_type_param;
@ -17,7 +14,6 @@ use crate::derive_props::generics::push_type_param;
#[derive(PartialEq, Eq)]
enum PropAttr {
Required { wrapped_name: Ident },
Option,
PropOr(Expr),
PropOrElse(Expr),
PropOrDefault,
@ -82,11 +78,6 @@ impl PropField {
#name: ::std::option::Option::unwrap(this.wrapped.#wrapped_name),
}
}
PropAttr::Option => {
quote! {
#name: this.wrapped.#name,
}
}
PropAttr::PropOr(value) => {
quote_spanned! {value.span()=>
#name: ::std::option::Option::unwrap_or(this.wrapped.#name, #value),
@ -115,19 +106,9 @@ impl PropField {
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>,
}
}
quote! {
#( #extra_attrs )*
#wrapped_name: ::std::option::Option<#ty>,
}
}
@ -164,19 +145,6 @@ impl PropField {
}
}
}
PropAttr::Option => {
quote! {
#[doc(hidden)]
#vis fn #name<#token_ty>(
&mut self,
token: #token_ty,
value: impl ::yew::html::IntoPropValue<#ty>,
) -> #token_ty {
self.wrapped.#name = value.into_prop_value();
token
}
}
}
_ => {
quote! {
#[doc(hidden)]
@ -216,12 +184,6 @@ impl PropField {
} else {
unreachable!()
}
} else if matches!(
&named_field.ty,
Type::Path(TypePath { path, .. })
if is_path_an_option(path)
) {
Ok(PropAttr::Option)
} else {
let ident = named_field.ident.as_ref().unwrap();
let wrapped_name = format_ident!("{}_wrapper", ident, span = Span::mixed_site());
@ -294,36 +256,6 @@ impl<'a> PropFieldCheck<'a> {
}
}
fn is_path_segments_an_option(path_segments: impl Iterator<Item = String>) -> bool {
fn is_option_path_seg(seg_index: usize, path: &str) -> u8 {
match (seg_index, path) {
(0, "core") => 0b001,
(0, "std") => 0b001,
(0, "Option") => 0b111,
(1, "option") => 0b010,
(2, "Option") => 0b100,
_ => 0,
}
}
path_segments
.enumerate()
.fold(0, |flags, (i, ps)| flags | is_option_path_seg(i, &ps))
== 0b111
}
/// Returns true when the [`Path`] seems like an [`Option`] type.
///
/// This function considers the following paths as Options:
/// - core::option::Option
/// - std::option::Option
/// - Option::*
///
/// Users can define their own [`Option`] type and this will return true - this is unavoidable.
fn is_path_an_option(path: &Path) -> bool {
is_path_segments_an_option(path.segments.iter().take(3).map(|ps| ps.ident.to_string()))
}
impl TryFrom<Field> for PropField {
type Error = Error;
@ -369,25 +301,3 @@ impl PartialEq for PropField {
self.name == other.name
}
}
#[cfg(test)]
mod tests {
use crate::derive_props::field::is_path_segments_an_option;
#[test]
fn all_std_and_core_option_path_seg_return_true() {
assert!(is_path_segments_an_option(
vec!["core".to_owned(), "option".to_owned(), "Option".to_owned()].into_iter()
));
assert!(is_path_segments_an_option(
vec!["std".to_owned(), "option".to_owned(), "Option".to_owned()].into_iter()
));
assert!(is_path_segments_an_option(
vec!["Option".to_owned()].into_iter()
));
// why OR instead of XOR
assert!(is_path_segments_an_option(
vec!["Option".to_owned(), "Vec".to_owned(), "Option".to_owned()].into_iter()
));
}
}

View File

@ -36,6 +36,18 @@ mod t3 {
}
}
mod t4 {
use super::*;
#[derive(Clone, Properties, PartialEq)]
pub struct Props {
value: Option<String>
}
fn required_option_should_be_provided() {
::yew::props!{ Props { } };
}
}
mod t5 {
use super::*;
#[derive(Clone, Properties, PartialEq)]

View File

@ -1,7 +1,7 @@
error: unexpected end of input, expected expression
--> tests/derive_props/fail.rs:44:19
--> tests/derive_props/fail.rs:56:19
|
44 | #[prop_or()]
56 | #[prop_or()]
| ^
error: cannot find attribute `props` in this scope
@ -11,18 +11,18 @@ error: cannot find attribute `props` in this scope
| ^^^^^
error[E0425]: cannot find value `foo` in this scope
--> tests/derive_props/fail.rs:74:24
--> tests/derive_props/fail.rs:86:24
|
74 | #[prop_or_else(foo)]
86 | #[prop_or_else(foo)]
| ^^^ not found in this scope
|
note: these functions exist but are inaccessible
--> tests/derive_props/fail.rs:88:5
--> tests/derive_props/fail.rs:100:5
|
88 | fn foo(bar: i32) -> String {
100 | fn foo(bar: i32) -> String {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ `crate::t9::foo`: not accessible
...
102 | fn foo() -> i32 {
114 | fn foo() -> i32 {
| ^^^^^^^^^^^^^^^ `crate::t10::foo`: not accessible
error[E0277]: the trait bound `Value: Default` is not satisfied
@ -107,10 +107,39 @@ note: required by a bound in `html::component::properties::__macro::PreBuild::<T
| ^^^^^^^^^^^^^^^^^^^ required by this bound in `html::component::properties::__macro::PreBuild::<Token, B>::build`
= note: this error originates in the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0308]: mismatched types
--> tests/derive_props/fail.rs:54:19
error[E0277]: the trait bound `AssertAllProps: HasProp<t4::_Props::value, _>` is not satisfied
--> tests/derive_props/fail.rs:47:24
|
54 | #[prop_or(123)]
47 | ::yew::props!{ Props { } };
| ^^^^^ the trait `HasProp<t4::_Props::value, _>` is not implemented for `AssertAllProps`
|
= help: the following other types implement trait `HasProp<P, How>`:
<CheckChildrenPropsAll<B> as HasProp<P, &dyn HasProp<P, How>>>
<CheckContextProviderPropsAll<B> as HasProp<P, &dyn HasProp<P, How>>>
<HasContextProviderPropschildren<B> as HasProp<P, &dyn HasProp<P, How>>>
<HasContextProviderPropschildren<B> as HasProp<children, HasContextProviderPropschildren<B>>>
<HasContextProviderPropscontext<B> as HasProp<P, &dyn HasProp<P, How>>>
<HasContextProviderPropscontext<B> as HasProp<yew::context::_ContextProviderProps::context, HasContextProviderPropscontext<B>>>
<suspense::component::CheckSuspensePropsAll<B> as HasProp<P, &dyn HasProp<P, How>>>
<t10::CheckPropsAll<B> as HasProp<P, &dyn HasProp<P, How>>>
and $N others
note: required because of the requirements on the impl of `HasAllProps<t4::Props, (_,)>` for `t4::CheckPropsAll<AssertAllProps>`
--> tests/derive_props/fail.rs:41:21
|
41 | #[derive(Clone, Properties, PartialEq)]
| ^^^^^^^^^^
= note: required because of the requirements on the impl of `AllPropsFor<t4::PropsBuilder, (_,)>` for `AssertAllProps`
note: required by a bound in `html::component::properties::__macro::PreBuild::<Token, B>::build`
--> $WORKSPACE/packages/yew/src/html/component/properties.rs
|
| Token: AllPropsFor<B, How>,
| ^^^^^^^^^^^^^^^^^^^ required by this bound in `html::component::properties::__macro::PreBuild::<Token, B>::build`
= note: this error originates in the derive macro `Properties` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0308]: mismatched types
--> tests/derive_props/fail.rs:66:19
|
66 | #[prop_or(123)]
| ^^^
| |
| expected struct `String`, found integer
@ -119,15 +148,15 @@ error[E0308]: mismatched types
note: associated function defined here
help: try using a conversion method
|
54 | #[prop_or(123.to_string())]
66 | #[prop_or(123.to_string())]
| ++++++++++++
54 | #[prop_or(123.to_string())]
66 | #[prop_or(123.to_string())]
| ++++++++++++
error[E0277]: expected a `FnOnce<()>` closure, found `{integer}`
--> tests/derive_props/fail.rs:64:24
--> tests/derive_props/fail.rs:76:24
|
64 | #[prop_or_else(123)]
76 | #[prop_or_else(123)]
| ^^^ expected an `FnOnce<()>` closure, found `{integer}`
|
= help: the trait `FnOnce<()>` is not implemented for `{integer}`
@ -135,20 +164,20 @@ error[E0277]: expected a `FnOnce<()>` closure, found `{integer}`
note: required by a bound in `Option::<T>::unwrap_or_else`
error[E0593]: function is expected to take 0 arguments, but it takes 1 argument
--> tests/derive_props/fail.rs:84:24
|
84 | #[prop_or_else(foo)]
| ^^^ expected function that takes 0 arguments
--> tests/derive_props/fail.rs:96:24
|
96 | #[prop_or_else(foo)]
| ^^^ expected function that takes 0 arguments
...
88 | fn foo(bar: i32) -> String {
| -------------------------- takes 1 argument
|
100 | fn foo(bar: i32) -> String {
| -------------------------- takes 1 argument
|
note: required by a bound in `Option::<T>::unwrap_or_else`
error[E0271]: type mismatch resolving `<fn() -> i32 {t10::foo} as FnOnce<()>>::Output == String`
--> tests/derive_props/fail.rs:98:24
|
98 | #[prop_or_else(foo)]
| ^^^ expected struct `String`, found `i32`
|
--> tests/derive_props/fail.rs:110:24
|
110 | #[prop_or_else(foo)]
| ^^^ expected struct `String`, found `i32`
|
note: required by a bound in `Option::<T>::unwrap_or_else`

View File

@ -227,7 +227,7 @@ mod t12 {
}
fn optional_prop_generics_should_work() {
::yew::props! { Props::<::std::primitive::bool> { } };
::yew::props! { Props::<::std::primitive::bool> { value: ::std::option::Option::Some(true) } };
::yew::props! { Props::<::std::primitive::bool> { value: true } };
}
}
@ -249,7 +249,28 @@ mod raw_field_names {
r#true: u32,
r#pointless_raw_name: u32,
}
}
mod value_into_some_value_in_props {
#[derive(::std::cmp::PartialEq, ::yew::Properties)]
struct Props {
required: ::std::option::Option<usize>,
#[prop_or_default]
optional: ::std::option::Option<usize>
}
#[::yew::function_component]
fn Inner(_props: &Props) -> ::yew::html::Html {
::yew::html!{}
}
#[::yew::function_component]
fn Main() -> ::yew::html::Html {
::yew::html! {<>
<Inner required=3 optional=5/>
<Inner required={::std::option::Option::Some(6)}/>
</>}
}
}
fn main() {}

View File

@ -25,6 +25,7 @@ mod feat_csr_ssr {
#[derive(Properties, PartialEq, Debug, Clone)]
pub(crate) struct BaseSuspenseProps {
pub children: Html,
#[prop_or(None)]
pub fallback: Option<Html>,
}