Skip to content

Commit

Permalink
html!: allow mixing self-closing and non-sc tags
Browse files Browse the repository at this point in the history
#### 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! {
    <div>
        <div/> // <- considered as open tag
    </div>
}
```

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

#### 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
  • Loading branch information
totorigolo committed Jul 26, 2019
1 parent 4c30319 commit 5908d33
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 3 deletions.
69 changes: 66 additions & 3 deletions crates/macro/src/html_tree/html_tag/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions tests/macro/html-tag-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
7 changes: 7 additions & 0 deletions tests/macro/html-tag-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down
75 changes: 75 additions & 0 deletions tests/vtag_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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);
}

0 comments on commit 5908d33

Please sign in to comment.