From f3e0e7a262b1b7ba547c9b6cd08886fbceefa6cf 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/Cargo.toml | 2 +- derive/src/lib.rs | 156 ++++++++++++++++++++++++------------ derive/tests/test_derive.rs | 10 +-- src/renderer.rs | 5 +- tests/common/mod.rs | 2 +- tests/test_ui.rs | 4 +- 7 files changed, 117 insertions(+), 64 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/Cargo.toml b/derive/Cargo.toml index 8576295..6aa430a 100644 --- a/derive/Cargo.toml +++ b/derive/Cargo.toml @@ -28,7 +28,7 @@ version = "0.6" [dependencies.syn] version = "0.15" default-features = false -features = ["clone-impls", "derive", "parsing", "printing"] +features = ["clone-impls", "derive", "extra-traits", "parsing", "printing"] [dev-dependencies.gui] version = "0.3" diff --git a/derive/src/lib.rs b/derive/src/lib.rs index 52fdaa5..fa22297 100644 --- a/derive/src/lib.rs +++ b/derive/src/lib.rs @@ -50,17 +50,20 @@ 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::token::Eq; +use syn::Type; use syn::TypeGenerics; use syn::WhereClause; use syn::WherePredicate; @@ -69,7 +72,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 +188,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 binding 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,26 +220,51 @@ 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. + +/// 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 { + // We need to peek at the second token coming up next first, because + // attempting to parse it would advance the position in the buffer. + if input.peek2(Eq) { + let bind = input.parse::()?; + Ok(Attr::Binding(bind)) + } else { + input.parse::().map(Attr::Ident) + } + } +} + + +/// Parse a single attribute, e.g., #[Event = MyEvent]. 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)), + if attribute.path.is_ident("gui") { + let tokens = attribute.tts.clone(); + let attr = parse2::(tokens).or_else(|err| { + Err(format!("unable to parse attributes: {:?}", err)) + })?; + + parse_gui_attributes(attr) + } else { + Ok((None, None)) } } @@ -353,12 +382,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 { @@ -382,7 +411,7 @@ fn expand_widget_trait(event: &Event, input: &DeriveInput) -> Tokens { /// # use gui_derive::Widget; /// # type Event = (); /// # #[derive(Debug, Widget)] -/// # #[gui(Event = "Event")] +/// # #[gui(Event = Event)] /// # struct TestWidget { /// # id: gui::Id, /// # } @@ -470,12 +499,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 {} @@ -515,26 +544,49 @@ mod tests { #[test] fn custom_event() { let tokens = quote! { - #[gui(Event = "FooBarBazEvent")] + #[gui(Event = FooBarBazEvent)] struct Bar { } }; let input = parse2::(tokens).unwrap(); let (new, event) = parse_attributes(&input.attrs).unwrap(); assert_eq!(new, None); - assert_eq!(event, Some("FooBarBazEvent".to_string())); + + let tokens = quote! { FooBarBazEvent }; + let foobar = parse2::(tokens).unwrap(); + assert_eq!(event, Some(foobar)); + } + + #[test] + fn default_new_and_event_with_ignore() { + let tokens = quote! { + #[allow(an_attribute_to_be_ignored)] + #[gui(default_new, Event = ())] + struct Baz { } + }; + + let input = parse2::(tokens).unwrap(); + let (new, event) = parse_attributes(&input.attrs).unwrap(); + assert_eq!(new, Some(())); + + let tokens = quote! { () }; + let parens = parse2::(tokens).unwrap(); + assert_eq!(event, Some(parens)); } #[test] fn last_event_type_takes_precedence() { let tokens = quote! { - #[gui(Event = "Event1")] - #[gui(Event = "Event2")] + #[gui(Event = Event1)] + #[gui(Event = Event2)] struct Foo { } }; let input = parse2::(tokens).unwrap(); let (_, event) = parse_attributes(&input.attrs).unwrap(); - assert_eq!(event.as_ref().map(String::as_str), Some("Event2")); + + let tokens = quote! { Event2 }; + let event2 = parse2::(tokens).unwrap(); + assert_eq!(event, Some(event2)); } } diff --git a/derive/tests/test_derive.rs b/derive/tests/test_derive.rs index 2b988ea..c56be2b 100644 --- a/derive/tests/test_derive.rs +++ b/derive/tests/test_derive.rs @@ -48,7 +48,7 @@ type Event = (); #[derive(Debug, Widget, Handleable)] -#[gui(default_new, Event = "Event")] +#[gui(default_new, Event = ())] struct TestWidget { id: Id, } @@ -58,7 +58,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 +67,7 @@ impl Handleable for TestWidgetCustom {} #[derive(Debug, Widget, Handleable)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct TestWidgetT where T: 'static + Debug, @@ -90,7 +90,7 @@ where #[derive(Debug, Handleable)] -#[gui(Event = "Event")] +#[gui(Event = Event)] struct TestHandleable { id: Id, } @@ -158,7 +158,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,