Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

html!: fix mixed self-closing and non-sc tags #523

Merged
merged 1 commit into from
Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
jstarry marked this conversation as resolved.
Show resolved Hide resolved
//
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);
}