Skip to content

Commit f3e0e7a

Browse files
committed
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]: rust-lang/rust#55208
1 parent 949f213 commit f3e0e7a

File tree

7 files changed

+117
-64
lines changed

7 files changed

+117
-64
lines changed

derive/CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
Unreleased
22
----------
3+
- Made `Event = ...` attribute support actual event type and not just
4+
string representation of it
35
- Bumped minimum required Rust version to `1.34.0`
46

57

derive/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ version = "0.6"
2828
[dependencies.syn]
2929
version = "0.15"
3030
default-features = false
31-
features = ["clone-impls", "derive", "parsing", "printing"]
31+
features = ["clone-impls", "derive", "extra-traits", "parsing", "printing"]
3232

3333
[dev-dependencies.gui]
3434
version = "0.3"

derive/src/lib.rs

+104-52
Original file line numberDiff line numberDiff line change
@@ -50,17 +50,20 @@ use proc_macro2::Span;
5050
use quote::__rt::TokenStream as Tokens;
5151
use quote::quote;
5252
use syn::Attribute;
53+
use syn::Binding;
5354
use syn::Data;
5455
use syn::DeriveInput;
5556
use syn::Fields;
5657
use syn::GenericParam;
5758
use syn::Generics;
58-
use syn::Lit;
59-
use syn::Meta;
60-
use syn::NestedMeta;
59+
use syn::parenthesized;
60+
use syn::parse::Parse;
61+
use syn::parse::ParseStream;
6162
use syn::parse2;
6263
use syn::punctuated::Punctuated;
6364
use syn::token::Comma;
65+
use syn::token::Eq;
66+
use syn::Type;
6467
use syn::TypeGenerics;
6568
use syn::WhereClause;
6669
use syn::WherePredicate;
@@ -69,7 +72,7 @@ use syn::WherePredicate;
6972
/// A type indicating whether or not to create a default implementation of Type::new().
7073
type New = Option<()>;
7174
/// A type representing an event type to parametrize a widget with.
72-
type Event = Option<String>;
75+
type Event = Option<Type>;
7376

7477

7578
/// The error type used internally by this module.
@@ -185,57 +188,83 @@ fn parse_attributes(attributes: &[Attribute]) -> Result<(New, Event)> {
185188
}
186189

187190
/// Parse a single item in a #[gui(list...)] attribute list.
188-
fn parse_gui_attribute(item: &NestedMeta) -> Result<(New, Event)> {
189-
match *item {
190-
NestedMeta::Meta(ref meta_item) => {
191-
match *meta_item {
192-
Meta::NameValue(ref name_val) if name_val.ident == "Event" => {
193-
match name_val.lit {
194-
Lit::Str(ref string) => Ok((None, Some(string.value()))),
195-
_ => Ok((None, None)),
196-
}
197-
},
198-
Meta::Word(ref ident) if ident == "default_new" => Ok((Some(()), None)),
199-
_ => Err(Error::from(format!("unsupported attribute: {}", meta_item.name()))),
191+
fn parse_gui_attribute(item: Attr) -> Result<(New, Event)> {
192+
match item {
193+
Attr::Ident(ref ident) if ident == "default_new" => {
194+
Ok((Some(()), None))
195+
},
196+
Attr::Binding(binding) => {
197+
// Unfortunately we can't use a pattern guard here. See issue
198+
// https://github.com/rust-lang/rust/issues/15287 for more
199+
// details.
200+
if binding.ident == "Event" {
201+
Ok((None, Some(binding.ty)))
202+
} else {
203+
Err(Error::from("encountered unknown binding attribute"))
200204
}
201205
},
202-
NestedMeta::Literal(_) => Err(Error::from("unsupported literal")),
206+
_ => Err(Error::from("encountered unknown attribute")),
203207
}
204208
}
205209

206210
/// Parse a #[gui(list...)] attribute list.
207-
fn parse_gui_attributes(list: &Punctuated<NestedMeta, Comma>) -> Result<(New, Event)> {
211+
fn parse_gui_attributes(list: AttrList) -> Result<(New, Event)> {
208212
let mut new = None;
209213
let mut event = None;
210214

211-
for item in list {
215+
for item in list.0 {
212216
let (this_new, this_event) = parse_gui_attribute(item)?;
213217
new = this_new.or(new);
214218
event = this_event.or(event);
215219
}
216220
Ok((new, event))
217221
}
218222

219-
/// Parse a single attribute, e.g., #[GuiType = "Widget"].
220-
// TODO: Once https://github.com/rust-lang/rust/pull/57367 lands in
221-
// stable we should migrate to using the actual type and not a
222-
// textual representation of it.
223+
224+
/// An attribute list representing an syn::Attribute::tts.
225+
struct AttrList(Punctuated<Attr, Comma>);
226+
227+
impl Parse for AttrList {
228+
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
229+
let content;
230+
let _ = parenthesized!(content in input);
231+
let list = content.parse_terminated(Attr::parse)?;
232+
233+
Ok(Self(list))
234+
}
235+
}
236+
237+
238+
enum Attr {
239+
Ident(Ident),
240+
Binding(Binding),
241+
}
242+
243+
impl Parse for Attr {
244+
fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
245+
// We need to peek at the second token coming up next first, because
246+
// attempting to parse it would advance the position in the buffer.
247+
if input.peek2(Eq) {
248+
let bind = input.parse::<Binding>()?;
249+
Ok(Attr::Binding(bind))
250+
} else {
251+
input.parse::<Ident>().map(Attr::Ident)
252+
}
253+
}
254+
}
255+
256+
257+
/// Parse a single attribute, e.g., #[Event = MyEvent].
223258
fn parse_attribute(attribute: &Attribute) -> Result<(New, Event)> {
224-
// We don't care about the other meta data elements, inner/outer,
225-
// doc/non-doc, it's all fine by us.
226-
227-
match attribute.interpret_meta() {
228-
Some(x) => {
229-
match x {
230-
Meta::List(ref list) if list.ident == "gui" => {
231-
// Here we have found an attribute of the form #[gui(xxx, yyy,
232-
// ...)]. Parse the inner list.
233-
parse_gui_attributes(&list.nested)
234-
},
235-
_ => Ok((None, None)),
236-
}
237-
},
238-
None => Ok((None, None)),
259+
if attribute.path.is_ident("gui") {
260+
let tokens = attribute.tts.clone();
261+
let attr = parse2::<AttrList>(tokens).or_else(|err| {
262+
Err(format!("unable to parse attributes: {:?}", err))
263+
})?;
264+
265+
parse_gui_attributes(attr)
266+
} else {
267+
Ok((None, None))
239268
}
240269
}
241270

@@ -353,12 +382,12 @@ fn expand_widget_trait(event: &Event, input: &DeriveInput) -> Tokens {
353382
let generic = event.is_none();
354383
let (generics, ty_generics, where_clause) = split_for_impl(&input.generics, generic);
355384

356-
let event = if let Some(event) = event {
357-
Ident::new(&event, Span::call_site())
385+
let widget = if let Some(event) = event {
386+
quote! { ::gui::Widget<#event> }
358387
} else {
359-
Ident::new("__E", Span::call_site())
388+
let event = Ident::new("__E", Span::call_site());
389+
quote! { ::gui::Widget<#event> }
360390
};
361-
let widget = quote! { ::gui::Widget<#event> };
362391

363392
quote! {
364393
impl #generics #widget for #name #ty_generics #where_clause {
@@ -382,7 +411,7 @@ fn expand_widget_trait(event: &Event, input: &DeriveInput) -> Tokens {
382411
/// # use gui_derive::Widget;
383412
/// # type Event = ();
384413
/// # #[derive(Debug, Widget)]
385-
/// # #[gui(Event = "Event")]
414+
/// # #[gui(Event = Event)]
386415
/// # struct TestWidget {
387416
/// # id: gui::Id,
388417
/// # }
@@ -470,12 +499,12 @@ fn expand_handleable_trait(event: &Event, input: &DeriveInput) -> Tokens {
470499
let generic = event.is_none();
471500
let (generics, ty_generics, where_clause) = split_for_impl(&input.generics, generic);
472501

473-
let event = if let Some(event) = event {
474-
Ident::new(&event, Span::call_site())
502+
let handleable = if let Some(event) = event {
503+
quote! { ::gui::Handleable<#event> }
475504
} else {
476-
Ident::new("__E", Span::call_site())
505+
let event = Ident::new("__E", Span::call_site());
506+
quote! { ::gui::Handleable<#event> }
477507
};
478-
let handleable = quote! { ::gui::Handleable<#event> };
479508

480509
quote! {
481510
impl #generics #handleable for #name #ty_generics #where_clause {}
@@ -515,26 +544,49 @@ mod tests {
515544
#[test]
516545
fn custom_event() {
517546
let tokens = quote! {
518-
#[gui(Event = "FooBarBazEvent")]
547+
#[gui(Event = FooBarBazEvent)]
519548
struct Bar { }
520549
};
521550

522551
let input = parse2::<DeriveInput>(tokens).unwrap();
523552
let (new, event) = parse_attributes(&input.attrs).unwrap();
524553
assert_eq!(new, None);
525-
assert_eq!(event, Some("FooBarBazEvent".to_string()));
554+
555+
let tokens = quote! { FooBarBazEvent };
556+
let foobar = parse2::<Type>(tokens).unwrap();
557+
assert_eq!(event, Some(foobar));
558+
}
559+
560+
#[test]
561+
fn default_new_and_event_with_ignore() {
562+
let tokens = quote! {
563+
#[allow(an_attribute_to_be_ignored)]
564+
#[gui(default_new, Event = ())]
565+
struct Baz { }
566+
};
567+
568+
let input = parse2::<DeriveInput>(tokens).unwrap();
569+
let (new, event) = parse_attributes(&input.attrs).unwrap();
570+
assert_eq!(new, Some(()));
571+
572+
let tokens = quote! { () };
573+
let parens = parse2::<Type>(tokens).unwrap();
574+
assert_eq!(event, Some(parens));
526575
}
527576

528577
#[test]
529578
fn last_event_type_takes_precedence() {
530579
let tokens = quote! {
531-
#[gui(Event = "Event1")]
532-
#[gui(Event = "Event2")]
580+
#[gui(Event = Event1)]
581+
#[gui(Event = Event2)]
533582
struct Foo { }
534583
};
535584

536585
let input = parse2::<DeriveInput>(tokens).unwrap();
537586
let (_, event) = parse_attributes(&input.attrs).unwrap();
538-
assert_eq!(event.as_ref().map(String::as_str), Some("Event2"));
587+
588+
let tokens = quote! { Event2 };
589+
let event2 = parse2::<Type>(tokens).unwrap();
590+
assert_eq!(event, Some(event2));
539591
}
540592
}

derive/tests/test_derive.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type Event = ();
4848

4949

5050
#[derive(Debug, Widget, Handleable)]
51-
#[gui(default_new, Event = "Event")]
51+
#[gui(default_new, Event = ())]
5252
struct TestWidget {
5353
id: Id,
5454
}
@@ -58,7 +58,7 @@ struct TestWidget {
5858
// purposes.
5959
#[deny(unused_imports)]
6060
#[derive(Debug, Widget)]
61-
#[gui(default_new, Event = "Event")]
61+
#[gui(default_new, Event = ())]
6262
struct TestWidgetCustom {
6363
id: Id,
6464
}
@@ -67,7 +67,7 @@ impl Handleable<Event> for TestWidgetCustom {}
6767

6868

6969
#[derive(Debug, Widget, Handleable)]
70-
#[gui(Event = "Event")]
70+
#[gui(Event = Event)]
7171
struct TestWidgetT<T>
7272
where
7373
T: 'static + Debug,
@@ -90,7 +90,7 @@ where
9090

9191

9292
#[derive(Debug, Handleable)]
93-
#[gui(Event = "Event")]
93+
#[gui(Event = Event)]
9494
struct TestHandleable {
9595
id: Id,
9696
}
@@ -158,7 +158,7 @@ impl MyEvent for CustomEvent {
158158

159159

160160
#[derive(Debug, Widget)]
161-
#[gui(Event = "E")]
161+
#[gui(Event = E)]
162162
struct TestGenericEvent<E>
163163
where
164164
E: Debug + MyEvent + 'static,

src/renderer.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,13 @@ pub trait Renderer {
6161
/// ```rust
6262
/// # use gui::{BBox, Cap, Id, Renderer, Renderable};
6363
/// # use gui::derive::{Handleable, Widget};
64-
/// # type Event = ();
6564
/// # #[derive(Debug, Widget, Handleable)]
66-
/// # #[gui(Event = "Event")]
65+
/// # #[gui(Event = ())]
6766
/// # struct ConcreteWidget1 {
6867
/// # id: Id,
6968
/// # }
7069
/// # #[derive(Debug, Widget, Handleable)]
71-
/// # #[gui(Event = "Event")]
70+
/// # #[gui(Event = ())]
7271
/// # struct ConcreteWidget2 {
7372
/// # id: Id,
7473
/// # }

tests/common/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ impl TestWidgetBuilder {
132132

133133

134134
#[derive(Debug, Widget)]
135-
#[gui(Event = "Event")]
135+
#[gui(Event = Event)]
136136
pub struct TestWidget {
137137
id: Id,
138138
event_handler: Option<EventHandler>,

tests/test_ui.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ fn need_more(id: Id, cap: &mut dyn MutCap<Event>) -> bool {
422422
}
423423

424424
#[derive(Debug, Widget)]
425-
#[gui(Event = "Event")]
425+
#[gui(Event = Event)]
426426
struct CreatingWidget {
427427
id: Id,
428428
}
@@ -481,7 +481,7 @@ fn recursive_widget_creation() {
481481
struct Moveable {}
482482

483483
#[derive(Debug, Widget, Handleable)]
484-
#[gui(Event = "Event")]
484+
#[gui(Event = Event)]
485485
struct MovingWidget {
486486
id: Id,
487487
object: Moveable,

0 commit comments

Comments
 (0)