Merge #523

523: html!: fix mixed self-closing and non-sc tags r=jstarry a=totorigolo

Before, self-closing tags where considered as open tags in `verify_end`, causing codes like this to not compile:

```rust
html! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

```
error: this open tag has no corresponding close tag
   --> src/lib.rs:264:17
    |
... | <div>
    | ^^^^^
```

However, this fix isn't ideal because it peeks the buffer twice for non self-closing tags. I did it that way in order to keep the peek thing. An alternative would be to turn `HtmlSelfClosingTag::peek` into a standard function returning `(ident, is_self_closing)`.

Fixes: #522

Co-authored-by: Thomas Lacroix <[email protected]>

crates/macro/src/html_tree/html_tag/mod.rs

@@ -6,7 +6,7 @@ use super::HtmlPropSuffix as TagSuffix;
 use super::HtmlTree;
 use crate::Peek;
 use boolinator::Boolinator;
-use proc_macro2::Span;
+use proc_macro2::{Delimiter, Span};
 use quote::{quote, quote_spanned, ToTokens};
 use syn::buffer::Cursor;
 use syn::parse;
@@ -42,6 +42,7 @@ impl Parse for HtmlTag {
         }
 
         let open = input.parse::<HtmlTagOpen>()?;
+        // Return early if it's a self-closing tag
         if open.div.is_some() {
             return Ok(HtmlTag {
                 ident: open.ident,
@@ -60,7 +61,7 @@ impl Parse for HtmlTag {
         let mut children: Vec<HtmlTree> = vec![];
         loop {
             if let Some(next_close_ident) = HtmlTagClose::peek(input.cursor()) {
-                if open.ident.to_string() == next_close_ident.to_string() {
+                if open.ident == next_close_ident {
                     break;
                 }
             }
@@ -162,7 +163,9 @@ impl HtmlTag {
     fn verify_end(mut cursor: Cursor, open_ident: &Ident) -> bool {
         let mut tag_stack_count = 1;
         loop {
-            if let Some(next_open_ident) = HtmlTagOpen::peek(cursor) {
+            if HtmlSelfClosingTag::peek(cursor).is_some() {
+                // Do nothing
+            } else if let Some(next_open_ident) = HtmlTagOpen::peek(cursor) {
                 if open_ident.to_string() == next_open_ident.to_string() {
                     tag_stack_count += 1;
                 }
@@ -185,6 +188,66 @@ impl HtmlTag {
     }
 }
 
+/// This struct is only used for its Peek implementation in verify_end. Parsing
+/// is done with HtmlTagOpen with `div` set to true.
+struct HtmlSelfClosingTag;
+
+impl Peek<Ident> for HtmlSelfClosingTag {
+    fn peek(cursor: Cursor) -> Option<Ident> {
+        let (punct, cursor) = cursor.punct()?;
+        (punct.as_char() == '<').as_option()?;
+
+        let (ident, cursor) = cursor.ident()?;
+        (ident.to_string().to_lowercase() == ident.to_string()).as_option()?;
+
+        let mut cursor = cursor;
+        let mut after_slash = false;
+        loop {
+            if let Some((punct, next_cursor)) = cursor.punct() {
+                match punct.as_char() {
+                    '/' => after_slash = true,
+                    '>' if after_slash => return Some(ident),
+                    '>' if !after_slash => {
+                        // We need to read after the '>' for cases like this:
+                        // <div onblur=|_| 2 > 1 />
+                        //                   ^ in order to handle this
+                        //
+                        // Because those cases are NOT handled by the html!
+                        // macro, so we want nice error messages.
+                        //
+                        // This idea here is that, in valid "JSX", after a tag,
+                        // only '<' or '{ ... }' can follow. (that should be
+                        // enough for reasonable cases)
+                        //
+                        let is_next_lt = next_cursor
+                            .punct()
+                            .map(|(p, _)| p.as_char() == '<')
+                            .unwrap_or(false);
+                        let is_next_brace = next_cursor.group(Delimiter::Brace).is_some();
+                        let no_next = next_cursor.token_tree().is_none();
+                        if is_next_lt || is_next_brace || no_next {
+                            return None;
+                        } else {
+                            // TODO: Use proc-macro's Diagnostic when stable
+                            eprintln!(
+                                "HELP: You must wrap expressions containing \
+                                 '>' in braces or parenthesis. See #523."
+                            );
+                        }
+                    }
+                    _ => after_slash = false,
+                }
+                cursor = next_cursor;
+            } else if let Some((_, next)) = cursor.token_tree() {
+                after_slash = false;
+                cursor = next;
+            } else {
+                return None;
+            }
+        }
+    }
+}
+
 struct HtmlTagOpen {
     lt: Token![<],
     ident: Ident,

tests/macro/html-tag-fail.rs

@@ -30,6 +30,10 @@ fn compile_fail() {
     html! { <input onclick=|| () /> };
     html! { <input onclick=|a, b| () /> };
     html! { <input onclick=|a: String| () /> };
+
+    // This is a known limitation. Put braces or parenthesis around expressions
+    // that contain '>'.
+    html! { <div> <div onblur=|_| 2 > 1 /> </div> };
 }
 
 fn main() {}

tests/macro/html-tag-fail.stderr

@@ -118,6 +118,13 @@ error: invalid closure argument
 32 |     html! { <input onclick=|a: String| () /> };
    |                            ^^^^^^^^^^^
 
+HELP: You must wrap expressions containing '>' in braces or parenthesis. See #523.
+error: expected valid html element
+  --> $DIR/html-tag-fail.rs:36:39
+   |
+36 |     html! { <div> <div onblur=|_| 2 > 1 /> </div> };
+   |                                       ^
+
 error[E0308]: mismatched types
   --> $DIR/html-tag-fail.rs:22:28
    |

tests/vtag_test.rs

@@ -28,6 +28,48 @@ impl Renderable<Comp> for Comp {
     }
 }
 
+struct CompInt;
+
+impl Component for CompInt {
+    type Message = u32;
+    type Properties = ();
+
+    fn create(_: Self::Properties, _: ComponentLink<Self>) -> Self {
+        CompInt
+    }
+
+    fn update(&mut self, _: Self::Message) -> ShouldRender {
+        unimplemented!();
+    }
+}
+
+impl Renderable<CompInt> for CompInt {
+    fn view(&self) -> Html<Self> {
+        unimplemented!();
+    }
+}
+
+struct CompBool;
+
+impl Component for CompBool {
+    type Message = bool;
+    type Properties = ();
+
+    fn create(_: Self::Properties, _: ComponentLink<Self>) -> Self {
+        CompBool
+    }
+
+    fn update(&mut self, _: Self::Message) -> ShouldRender {
+        unimplemented!();
+    }
+}
+
+impl Renderable<CompBool> for CompBool {
+    fn view(&self) -> Html<Self> {
+        unimplemented!();
+    }
+}
+
 #[test]
 fn it_compares_tags() {
     let a: VNode<Comp> = html! {
@@ -258,3 +300,36 @@ fn it_allows_aria_attributes() {
         panic!("vtag expected");
     }
 }
+
+#[test]
+fn it_checks_mixed_closing_tags() {
+    let a: VNode<Comp> = html! { <div> <div/>      </div> };
+    let b: VNode<Comp> = html! { <div> <div></div> </div> };
+    assert_eq!(a, b);
+
+    let a: VNode<Comp> = html! { <div> <div data-val={ 2 / 1 }/>  </div> };
+    let b: VNode<Comp> = html! { <div> <div data-val={ 2 }></div> </div> };
+    assert_eq!(a, b);
+
+    let a: VNode<Comp> = html! { <div> <div data-val={ 2 > 1 }/>  </div> };
+    let b: VNode<Comp> = html! { <div> <div data-val={ true }></div> </div> };
+    assert_eq!(a, b);
+
+    let a: VNode<CompInt> = html! { <div> <div onblur=|_| 2 / 1/>  </div> };
+    let b: VNode<CompInt> = html! { <div> <div onblur=|_| 3></div> </div> };
+    assert_eq!(a, b); // NB: assert_eq! doesn't (cannot) compare the closures
+
+    // This is a known limitation of the html! macro:
+    //
+    //   html! { <div> <img onblur=|_| 2 > 1 /> </div> }
+    //
+    // You have to put braces or parenthesis around the expression:
+    //
+    //   html! { <div> <img onblur=|_| { 2 > 1 } /> </div> }
+    //
+    let a: VNode<CompBool> = html! { <div> <div onblur=|_| { 2 > 1 } />  </div> };
+    let b: VNode<CompBool> = html! { <div> <div onblur=|_| ( 2 > 1 ) />  </div> };
+    let c: VNode<CompBool> = html! { <div> <div onblur=|_| false></div> </div> };
+    assert_eq!(a, c); // NB: assert_eq! doesn't (cannot) compare the closures
+    assert_eq!(b, c);
+}