From 7d7852b245a48c1c6a3cf40fab27c7a66d8c27a1 Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Sat, 7 Sep 2019 09:22:47 -0700 Subject: [PATCH] Use an actual type for the gui(Event = ...) attribute The current implementation of gui-derive's Event attribute has the problem that the user specifies the event type to use in representation of a string. That is a problem for many reasons, two of which are: - when an event is used only in an attribute (by virtue of being a string there), the compiler will flag it as unused - type lookup (using the RLS) will simply not work as expected because a string looses all the type information It was not an accident that this functionality was implemented this way as the unrestricted_attribute_tokens feature, which allows for non-string tokens to be used as attributes, was unstable at the time this functionality was implemented, causing compilation errors such as the following: > error: expected unsuffixed literal or identifier, found Event2 > --> tests/test_derive.rs:38:7 > | > 38 | #[gui(Event = Event2)] > | ^^^^^ > | > = help: try enabling `#![feature(unrestricted_attribute_tokens)]` Now that [Rust issue 55208][issue-55208] has landed and reached the stable toolchain, this change implements the functionality properly. [issue-55208]: https://github.com/rust-lang/rust/issues/55208 --- derive/CHANGELOG.md | 2 + derive/src/lib.rs | 115 +++++++++++++++++++++--------------- derive/tests/test_derive.rs | 11 ++-- src/renderer.rs | 5 +- tests/common/mod.rs | 2 +- tests/test_ui.rs | 4 +- 6 files changed, 80 insertions(+), 59 deletions(-) diff --git a/derive/CHANGELOG.md b/derive/CHANGELOG.md index 5fedb2c..2437094 100644 --- a/derive/CHANGELOG.md +++ b/derive/CHANGELOG.md @@ -1,5 +1,7 @@ Unreleased ---------- +- Made `Event = ...` attribute support actual event type and not just + string representation of it - Bumped minimum required Rust version to `1.34.0` diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 52fdaa5..643af2a 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -50,17 +50,19 @@ use proc_macro2::Span; use quote::__rt::TokenStream as Tokens; use quote::quote; use syn::Attribute; +use syn::Binding; use syn::Data; use syn::DeriveInput; use syn::Fields; use syn::GenericParam; use syn::Generics; -use syn::Lit; -use syn::Meta; -use syn::NestedMeta; +use syn::parenthesized; +use syn::parse::Parse; +use syn::parse::ParseStream; use syn::parse2; use syn::punctuated::Punctuated; use syn::token::Comma; +use syn::Type; use syn::TypeGenerics; use syn::WhereClause; use syn::WherePredicate; @@ -69,7 +71,7 @@ use syn::WherePredicate; /// A type indicating whether or not to create a default implementation of Type::new(). type New = Option<()>; /// A type representing an event type to parametrize a widget with. -type Event = Option; +type Event = Option; /// The error type used internally by this module. @@ -185,30 +187,31 @@ fn parse_attributes(attributes: &[Attribute]) -> Result<(New, Event)> { } /// Parse a single item in a #[gui(list...)] attribute list. -fn parse_gui_attribute(item: &NestedMeta) -> Result<(New, Event)> { - match *item { - NestedMeta::Meta(ref meta_item) => { - match *meta_item { - Meta::NameValue(ref name_val) if name_val.ident == "Event" => { - match name_val.lit { - Lit::Str(ref string) => Ok((None, Some(string.value()))), - _ => Ok((None, None)), - } - }, - Meta::Word(ref ident) if ident == "default_new" => Ok((Some(()), None)), - _ => Err(Error::from(format!("unsupported attribute: {}", meta_item.name()))), +fn parse_gui_attribute(item: Attr) -> Result<(New, Event)> { + match item { + Attr::Ident(ref ident) if ident == "default_new" => { + Ok((Some(()), None)) + }, + Attr::Binding(binding) => { + // Unfortunately we can't use a pattern guard here. See issue + // https://github.com/rust-lang/rust/issues/15287 for more + // details. + if binding.ident == "Event" { + Ok((None, Some(binding.ty))) + } else { + Err(Error::from("encountered unknown attribute")) } }, - NestedMeta::Literal(_) => Err(Error::from("unsupported literal")), + _ => Err(Error::from("encountered unknown attribute")), } } /// Parse a #[gui(list...)] attribute list. -fn parse_gui_attributes(list: &Punctuated) -> Result<(New, Event)> { +fn parse_gui_attributes(list: AttrList) -> Result<(New, Event)> { let mut new = None; let mut event = None; - for item in list { + for item in list.0 { let (this_new, this_event) = parse_gui_attribute(item)?; new = this_new.or(new); event = this_event.or(event); @@ -216,29 +219,47 @@ fn parse_gui_attributes(list: &Punctuated) -> Result<(New, Ev Ok((new, event)) } -/// Parse a single attribute, e.g., #[GuiType = "Widget"]. -// TODO: Once https://github.com/rust-lang/rust/pull/57367 lands in -// stable we should migrate to using the actual type and not a -// textual representation of it. -fn parse_attribute(attribute: &Attribute) -> Result<(New, Event)> { - // We don't care about the other meta data elements, inner/outer, - // doc/non-doc, it's all fine by us. - - match attribute.interpret_meta() { - Some(x) => { - match x { - Meta::List(ref list) if list.ident == "gui" => { - // Here we have found an attribute of the form #[gui(xxx, yyy, - // ...)]. Parse the inner list. - parse_gui_attributes(&list.nested) - }, - _ => Ok((None, None)), - } - }, - None => Ok((None, None)), + +/// An attribute list representing an syn::Attribute::tts. +struct AttrList(Punctuated); + +impl Parse for AttrList { + fn parse(input: ParseStream<'_>) -> syn::Result { + let content; + let _ = parenthesized!(content in input); + let list = content.parse_terminated(Attr::parse)?; + + Ok(Self(list)) } } + +enum Attr { + Ident(Ident), + Binding(Binding), +} + +impl Parse for Attr { + fn parse(input: ParseStream<'_>) -> syn::Result { + if let Ok(bind) = input.parse::() { + Ok(Attr::Binding(bind)) + } else { + input.parse::().map(|ident| Attr::Ident(ident)) + } + } +} + + +/// Parse a single attribute, e.g., #[Event = MyEvent]. +fn parse_attribute(attribute: &Attribute) -> Result<(New, Event)> { + let tokens = attribute.tts.clone(); + let attr = parse2::(tokens).or_else(|err| { + Err(format!("unable to parse attributes: {:?}", err)) + })?; + + parse_gui_attributes(attr) +} + /// Expand the input with the implementation of the required traits. fn expand_widget_input(new: New, event: &Event, input: &DeriveInput) -> Result { match input.data { @@ -353,12 +374,12 @@ fn expand_widget_trait(event: &Event, input: &DeriveInput) -> Tokens { let generic = event.is_none(); let (generics, ty_generics, where_clause) = split_for_impl(&input.generics, generic); - let event = if let Some(event) = event { - Ident::new(&event, Span::call_site()) + let widget = if let Some(event) = event { + quote! { ::gui::Widget<#event> } } else { - Ident::new("__E", Span::call_site()) + let event = Ident::new("__E", Span::call_site()); + quote! { ::gui::Widget<#event> } }; - let widget = quote! { ::gui::Widget<#event> }; quote! { impl #generics #widget for #name #ty_generics #where_clause { @@ -470,12 +491,12 @@ fn expand_handleable_trait(event: &Event, input: &DeriveInput) -> Tokens { let generic = event.is_none(); let (generics, ty_generics, where_clause) = split_for_impl(&input.generics, generic); - let event = if let Some(event) = event { - Ident::new(&event, Span::call_site()) + let handleable = if let Some(event) = event { + quote! { ::gui::Handleable<#event> } } else { - Ident::new("__E", Span::call_site()) + let event = Ident::new("__E", Span::call_site()); + quote! { ::gui::Handleable<#event> } }; - let handleable = quote! { ::gui::Handleable<#event> }; quote! { impl #generics #handleable for #name #ty_generics #where_clause {} diff --git a/derive/tests/test_derive.rs b/derive/tests/test_derive.rs index 36c0ff1..043d693 100644 --- a/derive/tests/test_derive.rs +++ b/derive/tests/test_derive.rs @@ -43,12 +43,11 @@ use gui::UnhandledEvent; use gui::Widget; -#[allow(unused)] type Event = (); #[derive(Debug, Widget, Handleable)] -#[gui(default_new, Event = "Event")] +#[gui(Event = ())] struct TestWidget { id: Id, } @@ -58,7 +57,7 @@ struct TestWidget { // purposes. #[deny(unused_imports)] #[derive(Debug, Widget)] -#[gui(default_new, Event = "Event")] +#[gui(default_new, Event = ())] struct TestWidgetCustom { id: Id, } @@ -67,7 +66,7 @@ impl Handleable for TestWidgetCustom {} #[derive(Debug, Widget, Handleable)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct TestWidgetT where T: 'static + Debug, @@ -90,7 +89,7 @@ where #[derive(Debug, Handleable)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct TestHandleable { id: Id, } @@ -158,7 +157,7 @@ impl MyEvent for CustomEvent { #[derive(Debug, Widget)] -#[gui(Event = "E")] +#[gui(Event = E)] struct TestGenericEvent where E: Debug + MyEvent + 'static, diff --git a/src/renderer.rs b/src/renderer.rs index 76cb585..be5a68e 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -61,14 +61,13 @@ pub trait Renderer { /// ```rust /// # use gui::{BBox, Cap, Id, Renderer, Renderable}; /// # use gui::derive::{Handleable, Widget}; - /// # type Event = (); /// # #[derive(Debug, Widget, Handleable)] - /// # #[gui(Event = "Event")] + /// # #[gui(Event = ())] /// # struct ConcreteWidget1 { /// # id: Id, /// # } /// # #[derive(Debug, Widget, Handleable)] - /// # #[gui(Event = "Event")] + /// # #[gui(Event = ())] /// # struct ConcreteWidget2 { /// # id: Id, /// # } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 6b62ca0..cef11ce 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -132,7 +132,7 @@ impl TestWidgetBuilder { #[derive(Debug, Widget)] -#[gui(Event = "Event")] +#[gui(Event = Event)] pub struct TestWidget { id: Id, event_handler: Option, diff --git a/tests/test_ui.rs b/tests/test_ui.rs index 1b9abb9..496ca91 100644 --- a/tests/test_ui.rs +++ b/tests/test_ui.rs @@ -422,7 +422,7 @@ fn need_more(id: Id, cap: &mut dyn MutCap) -> bool { } #[derive(Debug, Widget)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct CreatingWidget { id: Id, } @@ -481,7 +481,7 @@ fn recursive_widget_creation() { struct Moveable {} #[derive(Debug, Widget, Handleable)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct MovingWidget { id: Id, object: Moveable,