Repository: antoyo/relm (language: rust)

Issue:
Confusing error message on empty view! macro
This gives the following error:
```
unexpected end of input, expected identifier
```
and is at the wrong position.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
index 590931b5..b5218f88 100644
--- a/.github/workflows/CI.yml
+++ b/.github/workflows/CI.yml
@@ -49,6 +49,10 @@ jobs:
             openbox &
             cargo test --manifest-path relm-examples/Cargo.toml -- --nocapture
 
+      - name: "relm-derive: tests"
+        run: |
+            cargo test --manifest-path relm-derive/Cargo.toml -- --nocapture
+
       - name: "relm: test examples"
         run: |
             Xvfb :99 &
diff --git a/relm-derive/Cargo.toml b/relm-derive/Cargo.toml
index 07db9af1..921830a4 100644
--- a/relm-derive/Cargo.toml
+++ b/relm-derive/Cargo.toml
@@ -20,3 +20,8 @@ quote = "1.0"
 [dependencies.syn]
 features = ["extra-traits", "fold", "full", "visit"]
 version = "^1.0"
+
+[dev-dependencies]
+gtk = "^0.9.0"
+relm = { path = ".." }
+trybuild = "1.0.42"
diff --git a/relm-derive/src/gen/mod.rs b/relm-derive/src/gen/mod.rs
index ee154165..15a1aa72 100644
--- a/relm-derive/src/gen/mod.rs
+++ b/relm-derive/src/gen/mod.rs
@@ -63,7 +63,7 @@ use syn::visit::Visit;
 use self::adder::{Adder, Message, Property};
 pub use self::generator::gen_where_clause;
 use self::parser::EitherWidget::{Gtk, Relm};
-use self::parser::{Widget, parse_widgets};
+use self::parser::{Widget, WidgetList};
 use self::walker::ModelVariableVisitor;
 
 const MODEL_IDENT: &str = "__relm_model";
@@ -429,57 +429,12 @@ impl Driver {
     }
 
     fn get_view(&mut self, name: &Ident, typ: &Type) -> Result<View> {
-        // This method should probably just be replaced with `impl_view` and
-        // `view_validation_before_impl` should be put inside `impl_view`
-        self.view_validation_before_impl();
-        self.impl_view(name, typ)
-    }
-
-    fn view_validation_before_impl(&mut self) {
-        /*
-        // This is what comes immediately after `view!` e.g. `{ ... }`
-        let macro_token_tree: Vec<_> = self.view_macro.as_ref().expect("`view!` macro not yet set").tts
-            .clone()
-            .into_iter()
-            .collect();
-        // Panic if the macro is declared as anything other than `view! { ... }` or equivalent
-        if macro_token_tree.len() != 1 {
-            panic!("Invalid `view!` syntax, must be `view! { ... }`, `view! ( ... )`, or `view! [ ... ]`");
-        }
-        // Reach inside the brackets and bind the contents (the top level items) of `view!`
-        let top_level_items: Vec<_> = match macro_token_tree[0].kind {
-            TokenNode::Group(_, ref tts) => tts.clone().into_iter().collect(),
-            _ => panic!("Contents of `view!` should be a comma-delimitted series of items")
-        };
-        if let Some(index) = top_level_items.iter().position(|item|
-            match item.kind {
-                TokenNode::Op(',', _) => true,
-                _ => false,
-            })
-        {
-            // Find a comma (meaning more than one top level item) and panic unless it's just a trailing comma
-            if index != top_level_items.len() - 1 {
-                panic!("There may only be one top-level item in `view!`");
-            }
-        } else if top_level_items.len() == 0 {
-            // Panic if `view!` is empty e.g. `view! {}`
-            panic!("`view!` macro is empty, must contain one top-level item");
-        }
-        let macro_name_segments = &self.view_macro.as_ref().expect("`view!` macro not yet set").path.segments;
-        let last_segment = &macro_name_segments[macro_name_segments.len() - 1];
-        if (macro_name_segments.len() != 1) || (last_segment.ident.as_ref() != "view") {
-            let joined_path = macro_name_segments.iter()
-                .map(|seg| seg.ident.as_ref())
-                .collect::<Vec<&str>>()
-                .join("::");
-            panic!("Expected `view!` macro, found `{}` instead", joined_path);
-        }
-        */
-    }
+        let WidgetList { mut widgets } = self
+            .view_macro
+            .take()
+            .expect("view_macro in impl_view()")
+            .parse_body()?;
 
-    fn impl_view(&mut self, name: &Ident, typ: &Type) -> Result<View> {
-        let tts = self.view_macro.take().expect("view_macro in impl_view()").tokens;
-        let mut widgets = parse_widgets(tts)?;
         self.widget_parent_id = widgets[0].parent_id.clone();
 
         let mut msg_model_map = HashMap::new();
@@ -700,61 +655,3 @@ fn gen_set_child_prop_calls(widget: &Widget) -> Option<ImplItem> {
         None
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use syn::parse_expr;
-    use syn::{ExprKind};
-
-    #[test]
-    #[should_panic(expected = "Expected `view!` macro, found `foo` instead")]
-    fn incorrect_view_macro_name() {
-        let macro_text = "foo! {
-            gtk::Window {}
-        }";
-        let parsed_expr: ExprKind = parse_expr(macro_text)
-                                        .expect("incorrect_view_macro_name > parse_expr failed").node;
-        let mac = match parsed_expr {
-            ExprKind::Mac(mac) => mac,
-            _ => panic!("Expected ExprKind::Mac(mac), found {:#?}", parsed_expr),
-        };
-        let mut driver = Driver::new();
-        driver.view_macro = Some(mac);
-        driver.view_validation_before_impl();
-    }
-
-    #[test]
-    #[should_panic(expected = "`view!` macro is empty, must contain one top-level item")]
-    fn empty_view_macro() {
-        let macro_text = "view! {
-        }";
-        let parsed_expr: ExprKind = parse_expr(macro_text)
-                                        .expect("empty_view_macro > parse_expr failed").node;
-        let mac = match parsed_expr {
-            ExprKind::Mac(mac) => mac,
-            _ => panic!("Expected ExprKind::Mac(mac), found {:#?}", parsed_expr),
-        };
-        let mut driver = Driver::new();
-        driver.view_macro = Some(mac);
-        driver.view_validation_before_impl();
-    }
-
-    #[test]
-    #[should_panic(expected = "There may only be one top-level item in `view!`")]
-    fn multiple_top_level_items() {
-        let macro_text = "view! {
-            gtk::Window {},
-            gtk::Window {}
-        }";
-        let parsed_expr: ExprKind = parse_expr(macro_text)
-                                        .expect("multiple_top_level_items > parse_expr failed").node;
-        let mac = match parsed_expr {
-            ExprKind::Mac(mac) => mac,
-            _ => panic!("Expected ExprKind::Mac(mac), found {:#?}", parsed_expr),
-        };
-        let mut driver = Driver::new();
-        driver.view_macro = Some(mac);
-        driver.view_validation_before_impl();
-    }
-}
diff --git a/relm-derive/src/gen/parser.rs b/relm-derive/src/gen/parser.rs
index ff4a66d6..f4c09a85 100644
--- a/relm-derive/src/gen/parser.rs
+++ b/relm-derive/src/gen/parser.rs
@@ -28,7 +28,7 @@ use std::str::FromStr;
 use std::sync::Mutex;
 
 use lazy_static::lazy_static;
-use proc_macro2::{Span, TokenTree, TokenStream};
+use proc_macro2::{Span, TokenTree};
 use quote::{quote, ToTokens};
 use syn::{
     Expr,
@@ -227,39 +227,37 @@ impl RelmWidget {
     }
 }
 
-struct WidgetList {
-    widgets: Vec<Widget>,
+pub struct WidgetList {
+    pub widgets: Vec<Widget>,
 }
 
 impl Parse for WidgetList {
     fn parse(input: ParseStream) -> Result<Self> {
-        let widget: Widget = input.parse()?;
-        let mut widgets = vec![widget];
-        while !input.is_empty() {
-            let widget: Widget = input.parse()?;
-            widgets.push(widget);
-        }
-        Ok(WidgetList {
-            widgets,
-        })
-    }
-}
-
-pub fn parse_widgets(tokens: TokenStream) -> Result<Vec<Widget>> {
-    if let Ok(literal) = parse2::<LitStr>(tokens.clone()) {
-        // TODO: also support glade file.
-        let mut file = File::open(literal.value()).expect("File::open() in parse()");
-        let mut file_content = String::new();
-        file.read_to_string(&mut file_content).expect("read_to_string() in parse()");
-        let tokens = proc_macro::TokenStream::from_str(&file_content).expect("convert string to TokenStream");
-        let tokens = respan_with(tokens, literal.span().unwrap());
+        let lookahead = input.lookahead1();
+        if lookahead.peek(LitStr) {
+            let literal: LitStr = input.parse()?;
+
+            // TODO: also support glade file.
+            let mut file = File::open(literal.value()).expect("File::open() in parse()");
+            let mut file_content = String::new();
+            file.read_to_string(&mut file_content)
+                .expect("read_to_string() in parse()");
+            let tokens = proc_macro::TokenStream::from_str(&file_content)
+                .expect("convert string to TokenStream");
+            let tokens = respan_with(tokens, literal.span().unwrap());
+
+            syn::parse(tokens)
+        } else if lookahead.peek(Ident) || lookahead.peek(token::Pound) {
+            let mut widgets = vec![];
+
+            while !input.is_empty() {
+                widgets.push(input.parse()?);
+            }
 
-        let widgets: WidgetList = parse(tokens)?;
-        Ok(widgets.widgets)
-    }
-    else {
-        let widgets: WidgetList = parse2(tokens)?;
-        Ok(widgets.widgets)
+            Ok(WidgetList { widgets })
+        } else {
+            Err(lookahead.error())
+        }
     }
 }
 


Test patch:
diff --git a/relm-derive/tests/ui.rs b/relm-derive/tests/ui.rs
new file mode 100644
index 00000000..870c2f95
--- /dev/null
+++ b/relm-derive/tests/ui.rs
@@ -0,0 +1,5 @@
+#[test]
+fn ui() {
+    let t = trybuild::TestCases::new();
+    t.compile_fail("tests/ui/*.rs");
+}
diff --git a/relm-derive/tests/ui/empty_view_macro.rs b/relm-derive/tests/ui/empty_view_macro.rs
new file mode 100644
index 00000000..c0762ca0
--- /dev/null
+++ b/relm-derive/tests/ui/empty_view_macro.rs
@@ -0,0 +1,15 @@
+#![allow(unused_imports)]
+
+use relm::Widget;
+use relm_derive::widget;
+
+#[widget]
+impl Widget for Foo {
+    fn model() {}
+
+    fn update(&mut self, _: ()) {}
+
+    view! {}
+}
+
+fn main() {}
diff --git a/relm-derive/tests/ui/empty_view_macro.stderr b/relm-derive/tests/ui/empty_view_macro.stderr
new file mode 100644
index 00000000..7298e5c5
--- /dev/null
+++ b/relm-derive/tests/ui/empty_view_macro.stderr
@@ -0,0 +1,5 @@
+error: unexpected end of input, expected one of: string literal, identifier, `#`
+  --> $DIR/empty_view_macro.rs:12:11
+   |
+12 |     view! {}
+   |           ^^