Skip to content

Commit dcf23b2

Browse files
sdroegesdroege
authored
Merge pull request #965 from ranfdev/props_clean
Refactor properties macro, improve errors
2 parents 3db3e10 + 2d0a490 commit dcf23b2

File tree

1 file changed

+93
-90
lines changed

1 file changed

+93
-90
lines changed

‎glib-macros/src/properties.rs‎

Lines changed: 93 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ impl Parse for PropsMacroInput {
4242
.attrs
4343
.iter()
4444
.find(|x| x.path.is_ident("properties"))
45-
.expect("missing #[properties(wrapper_type = ...)]");
45+
.ok_or_else(|| {
46+
syn::Error::new(
47+
derive_input.span(),
48+
"missing #[properties(wrapper_type = ...)]",
49+
)
50+
})?;
4651
let wrapper_ty: PropertiesAttr = wrapper_ty.parse_args()?;
4752
let props: Vec<_> = match derive_input.data {
4853
syn::Data::Struct(struct_data) => parse_fields(struct_data.fields)?,
@@ -85,7 +90,7 @@ enum PropAttr {
8590
Get(Option<syn::Expr>),
8691
Set(Option<syn::Expr>),
8792

88-
// ident [= expr]
93+
// ident = expr
8994
OverrideClass(syn::Type),
9095
OverrideInterface(syn::Type),
9196

@@ -177,33 +182,19 @@ struct ReceivedAttrs {
177182
builder_fields: HashMap<syn::Ident, Option<syn::Expr>>,
178183
}
179184

180-
impl ReceivedAttrs {
181-
fn new(
182-
attrs_span: &proc_macro2::Span,
183-
attrs: impl IntoIterator<Item = PropAttr>,
184-
) -> syn::Result<Self> {
185+
impl Parse for ReceivedAttrs {
186+
fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
187+
let attrs = syn::punctuated::Punctuated::<PropAttr, Token![,]>::parse_terminated(input)?;
185188
let this = attrs.into_iter().fold(Self::default(), |mut this, attr| {
186189
this.set_from_attr(attr);
187190
this
188191
});
189192

190-
if this.get.is_none() && this.set.is_none() {
191-
return Err(syn::Error::new(
192-
*attrs_span,
193-
"No `get` or `set` specified: at least one is required.".to_string(),
194-
));
195-
}
196-
197-
if this.override_class.is_some() && this.override_interface.is_some() {
198-
return Err(syn::Error::new(
199-
*attrs_span,
200-
"Both `override_class` and `override_interface` specified.".to_string(),
201-
));
202-
}
203-
204193
Ok(this)
205194
}
195+
}
206196

197+
impl ReceivedAttrs {
207198
fn set_from_attr(&mut self, attr: PropAttr) {
208199
match attr {
209200
PropAttr::Get(some_fn) => self.get = Some(some_fn.into()),
@@ -258,6 +249,20 @@ impl PropDesc {
258249
builder_fields,
259250
} = attrs;
260251

252+
if get.is_none() && set.is_none() {
253+
return Err(syn::Error::new(
254+
attrs_span,
255+
"No `get` or `set` specified: at least one is required.".to_string(),
256+
));
257+
}
258+
259+
if override_class.is_some() && override_interface.is_some() {
260+
return Err(syn::Error::new(
261+
attrs_span,
262+
"Both `override_class` and `override_interface` specified.".to_string(),
263+
));
264+
}
265+
261266
// Fill needed, but missing, attributes with calculated default values
262267
let name = name.unwrap_or_else(|| {
263268
syn::LitStr::new(
@@ -284,65 +289,66 @@ impl PropDesc {
284289
}
285290
}
286291

287-
fn expand_properties_fn(props: &[PropDesc]) -> TokenStream2 {
288-
let n_props = props.len();
292+
fn expand_param_spec(prop: &PropDesc) -> TokenStream2 {
289293
let crate_ident = crate_ident_new();
290-
let properties_build_phase = props.iter().map(|prop| {
291-
let PropDesc {
292-
ty, name, builder, ..
293-
} = prop;
294-
295-
let rw_flags = match (&prop.get, &prop.set) {
296-
(Some(_), Some(_)) => quote!(.readwrite()),
297-
(Some(_), None) => quote!(.read_only()),
298-
(None, Some(_)) => quote!(.write_only()),
299-
(None, None) => unreachable!("No `get` or `set` specified"),
300-
};
294+
let PropDesc {
295+
ty, name, builder, ..
296+
} = prop;
297+
298+
match (&prop.override_class, &prop.override_interface) {
299+
(Some(c), None) => return quote!(#crate_ident::ParamSpecOverride::for_class::<#c>(#name)),
300+
(None, Some(i)) => {
301+
return quote!(#crate_ident::ParamSpecOverride::for_interface::<#i>(#name))
302+
}
303+
(Some(_), Some(_)) => {
304+
unreachable!("Both `override_class` and `override_interface` specified")
305+
}
306+
(None, None) => (),
307+
};
301308

302-
match (&prop.override_class, &prop.override_interface) {
303-
(Some(c), None) => {
304-
return quote!(#crate_ident::ParamSpecOverride::for_class::<#c>(#name))
305-
}
306-
(None, Some(i)) => {
307-
return quote!(#crate_ident::ParamSpecOverride::for_interface::<#i>(#name))
308-
}
309-
(Some(_), Some(_)) => {
310-
unreachable!("Both `override_class` and `override_interface` specified")
311-
}
312-
(None, None) => (),
313-
};
309+
let rw_flags = match (&prop.get, &prop.set) {
310+
(Some(_), Some(_)) => quote!(.readwrite()),
311+
(Some(_), None) => quote!(.read_only()),
312+
(None, Some(_)) => quote!(.write_only()),
313+
(None, None) => unreachable!("No `get` or `set` specified"),
314+
};
314315

315-
let builder_call = builder
316-
.as_ref()
317-
.cloned()
318-
.map(|(mut required_params, chained_methods)| {
319-
let name_expr = syn::ExprLit {
320-
attrs: vec![],
321-
lit: syn::Lit::Str(name.to_owned()),
322-
};
323-
required_params.insert(0, name_expr.into());
324-
let required_params = required_params.iter();
325-
326-
quote!((#(#required_params,)*)#chained_methods)
327-
})
328-
.unwrap_or(quote!((#name)));
316+
let builder_call = builder
317+
.as_ref()
318+
.cloned()
319+
.map(|(mut required_params, chained_methods)| {
320+
let name_expr = syn::ExprLit {
321+
attrs: vec![],
322+
lit: syn::Lit::Str(name.to_owned()),
323+
};
324+
required_params.insert(0, name_expr.into());
325+
let required_params = required_params.iter();
329326

330-
let builder_fields = prop.builder_fields.iter().map(|(k, v)| quote!(.#k(#v)));
327+
quote!((#(#required_params,)*)#chained_methods)
328+
})
329+
.unwrap_or(quote!((#name)));
331330

332-
let span = prop.attrs_span;
333-
quote_spanned! {span=>
334-
<<#ty as #crate_ident::Property>::Value as #crate_ident::HasParamSpec>
335-
::param_spec_builder() #builder_call
336-
#rw_flags
337-
#(#builder_fields)*
338-
.build()
339-
}
340-
});
331+
let builder_fields = prop.builder_fields.iter().map(|(k, v)| quote!(.#k(#v)));
332+
333+
let span = prop.attrs_span;
334+
quote_spanned! {span=>
335+
<<#ty as #crate_ident::Property>::Value as #crate_ident::HasParamSpec>
336+
::param_spec_builder() #builder_call
337+
#rw_flags
338+
#(#builder_fields)*
339+
.build()
340+
}
341+
}
342+
343+
fn expand_properties_fn(props: &[PropDesc]) -> TokenStream2 {
344+
let n_props = props.len();
345+
let crate_ident = crate_ident_new();
346+
let param_specs = props.iter().map(expand_param_spec);
341347
quote!(
342348
fn derived_properties() -> &'static [#crate_ident::ParamSpec] {
343349
use #crate_ident::once_cell::sync::Lazy;
344350
static PROPERTIES: Lazy<[#crate_ident::ParamSpec; #n_props]> = Lazy::new(|| [
345-
#(#properties_build_phase,)*
351+
#(#param_specs,)*
346352
]);
347353
PROPERTIES.as_ref()
348354
}
@@ -392,10 +398,10 @@ fn expand_property_fn(props: &[PropDesc]) -> TokenStream2 {
392398
pspec: &#crate_ident::ParamSpec
393399
) -> #crate_ident::Value {
394400
let prop = DerivedPropertiesEnum::try_from(id-1)
395-
.unwrap_or_else(|_| panic!("missing handler for property {}", pspec.name()));
401+
.unwrap_or_else(|_| panic!("property not defined {}", pspec.name()));
396402
match prop {
397403
#(#match_branch_get,)*
398-
_ => unreachable!(),
404+
_ => panic!("missing getter for property {}", pspec.name()),
399405
}
400406
}
401407
)
@@ -450,10 +456,10 @@ fn expand_set_property_fn(props: &[PropDesc]) -> TokenStream2 {
450456
pspec: &#crate_ident::ParamSpec
451457
){
452458
let prop = DerivedPropertiesEnum::try_from(id-1)
453-
.unwrap_or_else(|_| panic!("missing handler for property {}", pspec.name()));;
459+
.unwrap_or_else(|_| panic!("property not defined {}", pspec.name()));
454460
match prop {
455461
#(#match_branch_set,)*
456-
_ => unreachable!(),
462+
_ => panic!("missing setter for property {}", pspec.name()),
457463
}
458464
}
459465
)
@@ -469,16 +475,13 @@ fn parse_fields(fields: syn::Fields) -> syn::Result<Vec<PropDesc>> {
469475
attrs
470476
.into_iter()
471477
.filter(|a| a.path.is_ident("property"))
472-
.map(move |attrs| {
473-
let span = attrs.span();
474-
let attrs = attrs.parse_args_with(
475-
syn::punctuated::Punctuated::<PropAttr, Token![,]>::parse_terminated,
476-
)?;
478+
.map(move |prop_attrs| {
479+
let span = prop_attrs.span();
477480
PropDesc::new(
478481
span,
479482
ident.as_ref().unwrap().clone(),
480483
ty.clone(),
481-
ReceivedAttrs::new(&span, attrs)?,
484+
prop_attrs.parse_args()?,
482485
)
483486
})
484487
})
@@ -490,7 +493,7 @@ fn name_to_ident(name: &syn::LitStr) -> syn::Ident {
490493
format_ident!("{}", name.value().replace('-', "_"))
491494
}
492495

493-
fn expand_getset_properties_impl(props: &[PropDesc]) -> TokenStream2 {
496+
fn expand_wrapper_getset_properties(props: &[PropDesc]) -> TokenStream2 {
494497
let crate_ident = crate_ident_new();
495498
let defs = props.iter().map(|p| {
496499
let name = &p.name;
@@ -518,7 +521,7 @@ fn expand_getset_properties_impl(props: &[PropDesc]) -> TokenStream2 {
518521
quote!(#(#defs)*)
519522
}
520523

521-
fn expand_connect_prop_notify_impl(props: &[PropDesc]) -> TokenStream2 {
524+
fn expand_wrapper_connect_prop_notify(props: &[PropDesc]) -> TokenStream2 {
522525
let crate_ident = crate_ident_new();
523526
let connection_fns = props.iter().map(|p| {
524527
let name = &p.name;
@@ -533,7 +536,7 @@ fn expand_connect_prop_notify_impl(props: &[PropDesc]) -> TokenStream2 {
533536
quote!(#(#connection_fns)*)
534537
}
535538

536-
fn expand_notify_impl(props: &[PropDesc]) -> TokenStream2 {
539+
fn expand_wrapper_notify_prop(props: &[PropDesc]) -> TokenStream2 {
537540
let crate_ident = crate_ident_new();
538541
let emit_fns = props.iter().map(|p| {
539542
let name = &p.name;
@@ -603,9 +606,9 @@ pub fn impl_derive_props(input: PropsMacroInput) -> TokenStream {
603606
let fn_properties = expand_properties_fn(&input.props);
604607
let fn_property = expand_property_fn(&input.props);
605608
let fn_set_property = expand_set_property_fn(&input.props);
606-
let getset_properties_impl = expand_getset_properties_impl(&input.props);
607-
let connect_prop_notify_impl = expand_connect_prop_notify_impl(&input.props);
608-
let notify_impl = expand_notify_impl(&input.props);
609+
let getset_properties = expand_wrapper_getset_properties(&input.props);
610+
let connect_prop_notify = expand_wrapper_connect_prop_notify(&input.props);
611+
let notify_prop = expand_wrapper_notify_prop(&input.props);
609612
let properties_enum = expand_properties_enum(&input.props);
610613

611614
let expanded = quote! {
@@ -620,9 +623,9 @@ pub fn impl_derive_props(input: PropsMacroInput) -> TokenStream {
620623
}
621624

622625
impl #wrapper_type {
623-
#getset_properties_impl
624-
#connect_prop_notify_impl
625-
#notify_impl
626+
#getset_properties
627+
#connect_prop_notify
628+
#notify_prop
626629
}
627630

628631
};

0 commit comments

Comments
 (0)