From 5908d33b2c7ba09de56f1c5647e0780b6b0fbe84 Mon Sep 17 00:00:00 2001 From: Thomas Lacroix Date: Tue, 9 Jul 2019 12:49:58 +0200 Subject: [PATCH] html!: allow mixing self-closing and non-sc tags #### Problem Self-closing tags are more convenient when there are no children. For instance, React's JSX allows it. Before, self-closing tags where considered as open tags in `verify_end`, causing codes like this to not compile: ```rust html! {
// <- considered as open tag
} ``` ``` error: this open tag has no corresponding close tag --> src/lib.rs:264:17 | ... |
| ^^^^^ ``` #### Solution Add a new `Peek`-able `HtmlSelfClosingTag`, used in `verify_end`. 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 function returning (ident, is_self_closing). #### Notes I added a TODO for when proc-macro::Diagnostic gets stabilized, in order to replace the clunky `eprintln!` with proper diagnostics. I didn't try to return a Result to get a nice error right now because this error should be fairly rare so we can just wait IMO. Fixes: #522 --- crates/macro/src/html_tree/html_tag/mod.rs | 69 +++++++++++++++++++- tests/macro/html-tag-fail.rs | 4 ++ tests/macro/html-tag-fail.stderr | 7 ++ tests/vtag_test.rs | 75 ++++++++++++++++++++++ 4 files changed, 152 insertions(+), 3 deletions(-) diff --git a/crates/macro/src/html_tree/html_tag/mod.rs b/crates/macro/src/html_tree/html_tag/mod.rs index d769753035f..4284d18ebc7 100644 --- a/crates/macro/src/html_tree/html_tag/mod.rs +++ b/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::()?; + // 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 = 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 for HtmlSelfClosingTag { + fn peek(cursor: Cursor) -> Option { + 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: + //
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, diff --git a/tests/macro/html-tag-fail.rs b/tests/macro/html-tag-fail.rs index cf01005495f..594df3851c0 100644 --- a/tests/macro/html-tag-fail.rs +++ b/tests/macro/html-tag-fail.rs @@ -30,6 +30,10 @@ fn compile_fail() { html! { }; html! { }; html! { }; + + // This is a known limitation. Put braces or parenthesis around expressions + // that contain '>'. + html! {
1 />
}; } fn main() {} diff --git a/tests/macro/html-tag-fail.stderr b/tests/macro/html-tag-fail.stderr index 135fdd15ccc..152ef6e48ce 100644 --- a/tests/macro/html-tag-fail.stderr +++ b/tests/macro/html-tag-fail.stderr @@ -118,6 +118,13 @@ error: invalid closure argument 32 | html! { }; | ^^^^^^^^^^^ +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! {
1 />
}; + | ^ + error[E0308]: mismatched types --> $DIR/html-tag-fail.rs:22:28 | diff --git a/tests/vtag_test.rs b/tests/vtag_test.rs index 5ca5d6db0b6..92cb32ca826 100644 --- a/tests/vtag_test.rs +++ b/tests/vtag_test.rs @@ -28,6 +28,48 @@ impl Renderable for Comp { } } +struct CompInt; + +impl Component for CompInt { + type Message = u32; + type Properties = (); + + fn create(_: Self::Properties, _: ComponentLink) -> Self { + CompInt + } + + fn update(&mut self, _: Self::Message) -> ShouldRender { + unimplemented!(); + } +} + +impl Renderable for CompInt { + fn view(&self) -> Html { + unimplemented!(); + } +} + +struct CompBool; + +impl Component for CompBool { + type Message = bool; + type Properties = (); + + fn create(_: Self::Properties, _: ComponentLink) -> Self { + CompBool + } + + fn update(&mut self, _: Self::Message) -> ShouldRender { + unimplemented!(); + } +} + +impl Renderable for CompBool { + fn view(&self) -> Html { + unimplemented!(); + } +} + #[test] fn it_compares_tags() { let a: VNode = html! { @@ -258,3 +300,36 @@ fn it_allows_aria_attributes() { panic!("vtag expected"); } } + +#[test] +fn it_checks_mixed_closing_tags() { + let a: VNode = html! {
}; + let b: VNode = html! {
}; + assert_eq!(a, b); + + let a: VNode = html! {
}; + let b: VNode = html! {
}; + assert_eq!(a, b); + + let a: VNode = html! {
1 }/>
}; + let b: VNode = html! {
}; + assert_eq!(a, b); + + let a: VNode = html! {
}; + let b: VNode = html! {
}; + assert_eq!(a, b); // NB: assert_eq! doesn't (cannot) compare the closures + + // This is a known limitation of the html! macro: + // + // html! {
1 />
} + // + // You have to put braces or parenthesis around the expression: + // + // html! {
1 } />
} + // + let a: VNode = html! {
1 } />
}; + let b: VNode = html! {
1 ) />
}; + let c: VNode = html! {
}; + assert_eq!(a, c); // NB: assert_eq! doesn't (cannot) compare the closures + assert_eq!(b, c); +}