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!: handle misleading '>' in tag attributes #565

Merged
merged 1 commit into from Aug 7, 2019

Conversation

totorigolo
Copy link
Contributor

Problem

The html! macro didn't handle cases like this one:

html! {
    <div onclick=|_| 2 > 1 />
    //                 ^ this
}

Moreover, the introduction of handling of mixed self-closing and not
self-closing tags introduced a buggy error message, which is now fixed.

Solution

The parser only allows <, { or nothing after a tag's closing >.

verify_end is removed, and the presence of a closing tag is checked
when parsing the children.

Regression

Unfortunately, this change has a regression: the error message is less
good now, like here in html-tag-fail.stderr:

diff --git a/tests/macro/html-tag-fail.stderr b/tests/macro/html-tag-fail.stderr
index b14fc14..d59e8f4 100644
--- a/tests/macro/html-tag-fail.stderr
+++ b/tests/macro/html-tag-fail.stderr
@@ -52,11 +52,13 @@ error: only one root html element allowed
 12 |     html! { <img /></img> };
    |                    ^^^^^^

-error: expected valid html element
-  --> $DIR/html-tag-fail.rs:13:18
+error: unexpected end of input, expected token tree
+  --> $DIR/html-tag-fail.rs:13:5
    |
 13 |     html! { <div>Invalid</div> };
-   |                  ^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

 error: only one `attr` attribute allowed
   --> $DIR/html-tag-fail.rs:15:27

@totorigolo
Copy link
Contributor Author

I'll rebase on #564 once it gets merged, in order to move its tests into it_checks_misleading_gt.

### Problem
The html! macro didn't handle cases like this one:

```rust
html! {
    <div onclick=|_| 2 > 1 />
    //                 ^ this
}
```

Moreover, the introduction of handling of mixed self-closing and not
self-closing tags introduced a buggy error message, which is now fixed.

### Solution
The parser only allows '<', '{' or nothing after a tag's closing '>'.

`verify_end` is removed, and the presence of a closing tag is checked
when parsing the children.

Note: I also moved some tests around.

### Regression
Unfortunately, this change has a regression: the error message is less
good now, like here in `html-tag-fail.stderr`:

```
diff --git a/tests/macro/html-tag-fail.stderr b/tests/macro/html-tag-fail.stderr
index b14fc14..d59e8f4 100644
--- a/tests/macro/html-tag-fail.stderr
+++ b/tests/macro/html-tag-fail.stderr
@@ -52,11 +52,13 @@ error: only one root html element allowed
 12 |     html! { <img /></img> };
    |                    ^^^^^^

-error: expected valid html element
-  --> $DIR/html-tag-fail.rs:13:18
+error: unexpected end of input, expected token tree
+  --> $DIR/html-tag-fail.rs:13:5
    |
 13 |     html! { <div>Invalid</div> };
-   |                  ^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

 error: only one `attr` attribute allowed
   --> $DIR/html-tag-fail.rs:15:27
```
@totorigolo totorigolo force-pushed the fix-selfclosing-false-positive branch from 58030e3 to 78d4376 Compare August 7, 2019 09:53
@totorigolo
Copy link
Contributor Author

Done!

@jstarry
Copy link
Member

jstarry commented Aug 7, 2019

bors r+

bors bot added a commit that referenced this pull request Aug 7, 2019
565: html!: handle misleading '>' in tag attributes r=jstarry a=totorigolo

### Problem
The `html!` macro didn't handle cases like this one:

```rust
html! {
    <div onclick=|_| 2 > 1 />
    //                 ^ this
}
```

Moreover, the introduction of handling of mixed self-closing and not
self-closing tags introduced a buggy error message, which is now fixed.

### Solution
The parser only allows `<`, `{` or nothing after a tag's closing `>`.

`verify_end` is removed, and the presence of a closing tag is checked
when parsing the children.

### Regression
Unfortunately, this change has a regression: the error message is less
good now, like here in `html-tag-fail.stderr`:

```diff
diff --git a/tests/macro/html-tag-fail.stderr b/tests/macro/html-tag-fail.stderr
index b14fc14..d59e8f4 100644
--- a/tests/macro/html-tag-fail.stderr
+++ b/tests/macro/html-tag-fail.stderr
@@ -52,11 +52,13 @@ error: only one root html element allowed
 12 |     html! { <img /></img> };
    |                    ^^^^^^

-error: expected valid html element
-  --> $DIR/html-tag-fail.rs:13:18
+error: unexpected end of input, expected token tree
+  --> $DIR/html-tag-fail.rs:13:5
    |
 13 |     html! { <div>Invalid</div> };
-   |                  ^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

 error: only one `attr` attribute allowed
   --> $DIR/html-tag-fail.rs:15:27
```

Co-authored-by: Thomas Lacroix <toto.rigolo@free.fr>
@bors
Copy link
Contributor

bors bot commented Aug 7, 2019

Build succeeded

  • continuous-integration/travis-ci/push

@bors bors bot merged commit 78d4376 into yewstack:master Aug 7, 2019
@totorigolo totorigolo deleted the fix-selfclosing-false-positive branch May 24, 2020 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants