Skip to content

Commit

Permalink
Use an actual type for the gui(Event = ...) attribute
Browse files Browse the repository at this point in the history
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]: rust-lang/rust#55208
  • Loading branch information
d-e-s-o committed Sep 7, 2019
1 parent 949f213 commit ebd18c9
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 64 deletions.
2 changes: 2 additions & 0 deletions derive/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`


Expand Down
2 changes: 1 addition & 1 deletion derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
154 changes: 102 additions & 52 deletions derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String>;
type Event = Option<Type>;


/// The error type used internally by this module.
Expand Down Expand Up @@ -185,57 +188,81 @@ 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<NestedMeta, Comma>) -> 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);
}
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<Attr, Comma>);

impl Parse for AttrList {
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
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<Self> {
if input.peek2(Eq) {
let bind = input.parse::<Binding>()?;
Ok(Attr::Binding(bind))
} else {
input.parse::<Ident>().map(|ident| Attr::Ident(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::<AttrList>(tokens).or_else(|err| {
Err(format!("unable to parse attributes: {:?}", err))
})?;

parse_gui_attributes(attr)
} else {
Ok((None, None))
}
}

Expand Down Expand Up @@ -353,12 +380,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 {
Expand All @@ -382,7 +409,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,
/// # }
Expand Down Expand Up @@ -470,12 +497,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 {}
Expand Down Expand Up @@ -515,26 +542,49 @@ mod tests {
#[test]
fn custom_event() {
let tokens = quote! {
#[gui(Event = "FooBarBazEvent")]
#[gui(Event = FooBarBazEvent)]
struct Bar { }
};

let input = parse2::<DeriveInput>(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::<Type>(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::<DeriveInput>(tokens).unwrap();
let (new, event) = parse_attributes(&input.attrs).unwrap();
assert_eq!(new, Some(()));

let tokens = quote! { () };
let parens = parse2::<Type>(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::<DeriveInput>(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::<Type>(tokens).unwrap();
assert_eq!(event, Some(event2));
}
}
10 changes: 5 additions & 5 deletions derive/tests/test_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Event = ();


#[derive(Debug, Widget, Handleable)]
#[gui(default_new, Event = "Event")]
#[gui(default_new, Event = ())]
struct TestWidget {
id: Id,
}
Expand All @@ -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,
}
Expand All @@ -67,7 +67,7 @@ impl Handleable<Event> for TestWidgetCustom {}


#[derive(Debug, Widget, Handleable)]
#[gui(Event = "Event")]
#[gui(Event = Event)]
struct TestWidgetT<T>
where
T: 'static + Debug,
Expand All @@ -90,7 +90,7 @@ where


#[derive(Debug, Handleable)]
#[gui(Event = "Event")]
#[gui(Event = Event)]
struct TestHandleable {
id: Id,
}
Expand Down Expand Up @@ -158,7 +158,7 @@ impl MyEvent for CustomEvent {


#[derive(Debug, Widget)]
#[gui(Event = "E")]
#[gui(Event = E)]
struct TestGenericEvent<E>
where
E: Debug + MyEvent + 'static,
Expand Down
5 changes: 2 additions & 3 deletions src/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
/// # }
Expand Down
2 changes: 1 addition & 1 deletion tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl TestWidgetBuilder {


#[derive(Debug, Widget)]
#[gui(Event = "Event")]
#[gui(Event = Event)]
pub struct TestWidget {
id: Id,
event_handler: Option<EventHandler>,
Expand Down
4 changes: 2 additions & 2 deletions tests/test_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ fn need_more(id: Id, cap: &mut dyn MutCap<Event>) -> bool {
}

#[derive(Debug, Widget)]
#[gui(Event = "Event")]
#[gui(Event = Event)]
struct CreatingWidget {
id: Id,
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit ebd18c9

Please sign in to comment.