Commit Graph
54 Commits
Author SHA1 Message Date
Ayaz Hafiz 6ae3b68acf feat(compiler): Parse and recover on incomplete opening HTML tags (#38681)
Let's say we have a code like

```html
<div<span>123</span>
```

Currently this gets parsed into a tree with the element tag `div<span`.
This has at least two downsides:

- An incorrect diagnostic that `</span>` doesn't close an element is
  emitted.
- A consumer of the parse tree using it for editor services is unable to
  provide correct completions for the opening `<span>` tag.

This patch attempts to fix both issues by instead parsing the code into
the same tree that would be parsed for `<div></div><span>123</span>`.

In particular, we do this by optimistically scanning an open tag as
usual, but if we do not notice a terminating '>', we mark the tag as
"incomplete". A parser then emits an error for the incomplete tag and
adds a synthetic (recovered) element node to the tree with the
incomplete open tag's name.

What's the downside of this? For one, a breaking change.

<ol>
<li>

The first breaking change is that `<` symbols that are ambiguously text
or opening tags will be parsed as opening tags instead of text in
element bodies. Take the code

```html
<p>a<b</p>
```

Clearly we cannot have the best of both worlds, and this patch chooses
to swap the parsing strategy to support the new feature. Of course, `<`
can still be inserted as text via the `&lt;` entity.

</li>
</ol>

Part of #38596

PR Close #38681
2020-09-21 12:27:01 -07:00
Pete Bacon Darwin 1d8c5d88cd refactor(compiler): element.sourceSpan should span the outerHTML (#38581)
Previously, the `sourceSpan` and `startSourceSpan` were the same
object, which meant that you had the following situation:

```
element = <div>some content</div>
sourceSpan = <div>
startSourceSpan = <div>
endSourceSpan = </div>
```

This made `sourceSpan` redundant and meant that if you
wanted a span for the whole element including its content
and closing tag, it had to be computed.

Now `sourceSpan` is separated from `startSourceSpan`
resulting in:

```
element = <div>some content</div>
sourceSpan = <div>some content</div>
startSourceSpan = <div>
endSourceSpan = </div>
```

PR Close #38581
2020-09-02 14:47:31 -07:00
Pete Bacon Darwin a68f1a78a7 refactor(compiler): element.startSourceSpan is required (#38581)
Previously, the `startSourceSpan` property could be null
but in reality it is always well defined - except for a legacy
case in the old i18n extraction/merging code, where the
typings for source-spans are already being undermined.

Making this property non-null, simplifies code elsewhere
in the project.

PR Close #38581
2020-09-02 14:47:28 -07:00
Pete Bacon Darwin 86e11f1110 refactor(compiler): move the line-ending handling decision (#38581)
Previously the lexer was responsible for deciding whether an "inline"
template should also have its line-endings normalized.

Now this decision is made higher up in the call stack to allow more
flexibility in the parser/lexer.

PR Close #38581
2020-09-02 14:47:25 -07:00
crisbeto f5a148b1b7 fix(compiler): incorrectly inferring namespace for HTML nodes inside SVG (#38477)
The HTML parser gets an element's namespace either from the tag name
(e.g. `<svg:rect>`) or from its parent element `<svg><rect></svg>`) which
breaks down when an element is inside of an SVG `foreignElement`,
because foreign elements allow nodes from a different namespace to be
inserted into an SVG.

These changes add another flag to the tag definitions which tells child
nodes whether to try to inherit their namespaces from their parents.
It also adds a definition for `foreignObject` with the new flag,
allowing elements placed inside it to infer their namespaces instead.

Fixes #37218.

PR Close #38477
2020-08-31 13:25:38 -07:00
Joey Perrott ff9f4de4f1 fix(compiler): update unparsable character reference entity error messages (#38319)
Within an angular template, when a character entity is unable to be parsed, previously a generic
unexpected character error was thrown.  This does not properly express the issue that was discovered
as the issue is actually caused by the discovered character making the whole of the entity unparsable.
The compiler will now instead inform via the error message what string was attempted to be parsed
and what it was attempted to be parsed as.

Example, for this template:
```
<p>
  &#x123p
</p>
```
Before this change:
`Unexpected character "p"`

After this change:
`Unable to parse entity "&#x123p" - hexadecimal character reference entities must end with ";"`

Fixes #26067

PR Close #38319
2020-07-31 15:32:53 -07:00
JoostK 1a7a7360b0 fix(compiler): properly associate source spans for implicitly closed elements (#38126)
HTML is very lenient when it comes to closing elements, so Angular's parser has
rules that specify which elements are implicitly closed when closing a tag.
The parser keeps track of the nesting of tag names using a stack and parsing
a closing tag will pop as many elements off the stack as possible, provided
that the elements can be implicitly closed.

For example, consider the following templates:

- `<div><br></div>`, the `<br>` is implicitly closed when parsing `</div>`,
  because `<br>` is a void element.
- `<div><p></div>`, the `<p>` is implicitly closed when parsing `</div>`,
  as `<p>` is allowed to be closed by the closing of its parent element.
- `<ul><li>A <li>B</ul>`, the first `<li>` is implicitly closed when parsing
  the second `<li>`, whereas the second `<li>` would be implicitly closed when
  parsing the `</ul>`.

In all the cases above the parsed structure would be correct, however the source
span of the closing `</div>` would incorrectly be assigned to the element that
is implicitly closed. The problem was that closing an element would associate
the source span with the element at the top of the stack, however this may not
be the element that is actually being closed if some elements would be
implicitly closed.

This commit fixes the issue by assigning the end source span with the element
on the stack that is actually being closed. Any implicitly closed elements that
are popped off the stack will not be assigned an end source span, as the
implicit closing implies that no ending element is present.

Note that there is a difference between self-closed elements such as `<input/>`
and implicitly closed elements such as `<input>`. The former does have an end
source span (identical to its start source span) whereas the latter does not.

Fixes #36118
Resolves FW-2004

PR Close #38126
2020-07-20 10:02:06 -07:00
JoostK 8edf5ba29d refactor(compiler): remove unused parser methods (#38126)
These methods are no longer used so they can safely be removed.

PR Close #38126
2020-07-20 10:02:06 -07:00
Joey Perrott d1ea1f4c7f build: update license headers to reference Google LLC (#37205)
Update the license headers throughout the repository to reference Google LLC
rather than Google Inc, for the required license headers.

PR Close #37205
2020-05-26 14:26:58 -04:00
Pete Bacon Darwin 70dd27ffd8 fix(compiler): normalize line endings in ICU expansions (#36741)
The html parser already normalizes line endings (converting `\r\n` to `\n`)
for most text in templates but it was missing the expressions of ICU expansions.

In ViewEngine backticked literal strings, used to define inline templates,
were already normalized by the TypeScript parser.
In Ivy we are parsing the raw text of the source file directly so the line
endings need to be manually normalized.

This change ensures that inline templates have the line endings of ICU
expression normalized correctly, which matches the ViewEngine.

In ViewEngine external templates, defined in HTML files, the behavior was
different, since TypeScript was not normalizing the line endings.
Specifically, ICU expansion "expressions" are not being normalized.
This is a problem because it means that i18n message ids can be different on
different machines that are setup with different line ending handling,
or if the developer moves a template from inline to external or vice versa.

The goal is always to normalize line endings, whether inline or external.
But this would be a breaking change since it would change i18n message ids
that have been previously computed. Therefore this commit aligns the ivy
template parsing to have the same "buggy" behavior for external templates.

There is now a compiler option `i18nNormalizeLineEndingsInICUs`, which
if set to `true` will ensure the correct non-buggy behavior. For the time
being this option defaults to `false` to ensure backward compatibility while
allowing opt-in to the desired behavior. This option's default will be
flipped in a future breaking change release.

Further, when this option is set to `false`, any ICU expression tokens,
which have not been normalized, are added to the `ParseResult` from the
`HtmlParser.parse()` method. In the future, this collection of tokens could
be used to diagnose and encourage developers to migrate their i18n message
ids. See FW-2106.

Closes #36725

PR Close #36741
2020-04-28 12:22:40 -07:00
Pete Bacon Darwin e0aa39929b refactor(compiler): simplify tokenizer and parser results (#36741)
Move the creation of the results objects into the wrapper functions.
This makes it easier to reason about what the parser and lexer classes
are responsible for - you create a new object for each tokenization or
parsing activity and they hold the state of the activity.

PR Close #36741
2020-04-28 12:22:39 -07:00
Alex Rickabaugh 83a9159063 style(compiler): reformat of codebase with new clang-format version (#36520)
This commit reformats the packages/compiler tree using the new version of
clang-format.

PR Close #36520
2020-04-08 14:51:08 -07:00
Sonu Kapoor fced8ee40e fix(localize): allow ICU expansion case to start with any character except } (#36123)
Previously, an expansion case could only start with an alpha numeric character.
This commit fixes this by allowing an expansion case to start with any character
except `}`.

The [ICU spec](http://userguide.icu-project.org/formatparse/messages) is pretty vague:

> Use a "select" argument to select sub-messages via a fixed set of keywords.

It does not specify what can be a "keyword" but from looking at the surrounding syntax it
appears that it can indeed be any string that does not contain a `}` character.

Closes #31586

PR Close #36123
2020-03-23 11:37:12 -07:00
JoostK 4945274080 perf(compiler): optimize cloning cursors state (#34332)
On a large compilation unit with big templates, the total time spent in
the `PlainCharacterCursor` constructor was 470ms. This commit applies
two optimizations to reduce this time:

1. Avoid the object spread operator within the constructor, as the
generated `__assign` helper in the emitted UMD bundle (ES5) does not
optimize well compared to a hardcoded object literal. This results in a
significant performance improvement. Because of the straight-forward
object literal, the VM is now much better able to optimize the memory
allocations which makes a significant difference as the
`PlainCharacterCursor` constructor is called in tight loops.

2. Reduce the number of `CharacterCursor` clones. Although cloning
itself is now much faster because of the optimization above, several
clone operations were not necessary.

Combined, these changes reduce the total time spent in the
`PlainCharacterCursor` constructor to just 10ms.

PR Close #34332
2019-12-12 14:06:37 -08:00
Pete Bacon Darwin aaa08f7be3 refactor(compiler): add abstract NodeWithI18n class to ML parsing (#33318)
This abstract class will be useful for identifying nodes that
can hold i18n data.

PR Close #33318
2019-10-22 13:30:16 -04:00
Pete Bacon Darwin 03103d2d59 refactor(compiler): rename i18n AST to i18nMeta (#33318)
This better reflects what this type represents and what it is used for.

PR Close #33318
2019-10-22 13:30:16 -04:00
Keen Yee Liau 65a0d2b53d fix(language-service): Preserve CRLF in templates for language-service (#33241)
This is a potential fix for https://github.com/angular/vscode-ng-language-service/issues/235
suggested by @andrius-pra in
https://github.com/andrius-pra/angular/commit/47696136e3d487098c2ba466e21b09cbb6bdfaeb.

Currently, CRLF line endings are converted to LFs and this causes the
diagnostics span to be off in templates that use CRLF. The line endings
must be preserved in order to maintain correct span offset. The solution
is to add an option to the Tokenizer to indicate such preservation.

PR Close #33241
2019-10-22 13:29:23 -04:00
Pete Bacon Darwin 0ddf0c4895 fix(compiler): do not remove whitespace wrapping i18n expansions (#31962)
Similar to interpolation, we do not want to completely remove whitespace
nodes that are siblings of an expansion.

For example, the following template

```html
<div>
  <strong>items left<strong> {count, plural, =1 {item} other {items}}
</div>
```

was being collapsed to

```html
<div><strong>items left<strong>{count, plural, =1 {item} other {items}}</div>
```

which results in the text looking like

```
items left4
```

instead it should be collapsed to

```html
<div><strong>items left<strong> {count, plural, =1 {item} other {items}}</div>
```

which results in the text looking like

```
items left 4
```

---

**Analysis of the code and manual testing has shown that this does not cause
the generated ids to change, so there is no breaking change here.**

PR Close #31962
2019-08-09 12:03:50 -07:00
Paul Gschwendtner 012b535147 refactor(compiler): ensure compatibility with typescript strict flag (#30993)
As part of FW-1265, the `@angular/compiler` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.

Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993
2019-07-18 14:21:25 -07:00
Pete Bacon Darwin 304a12f027 feat(compiler): support skipping leading trivia in template source-maps (#30095)
Leading trivia, such as whitespace or comments, is
confusing for developers looking at source-mapped
templates, since they expect the source-map segment
to start after the trivia.

This commit adds skipping trivial characters to the lexer;
and then implements that in the template parser.

PR Close #30095
2019-04-25 12:36:54 -07:00
Pawel Kozlowski dafbbf8b64 fix(core): parse incorrect ML open tag as text (#29328)
This PR alligns markup language lexer with the previous behaviour in version 7.x:
https://stackblitz.com/edit/angular-iancj2

While this behaviour is not perfect (we should be giving users an error message
here about invalid HTML instead of assuming text node) this is probably best we
can do without more substential re-write of lexing / parsing infrastructure.

This PR just fixes #29231 and restores VE behaviour - a more elaborate fix will
be done in a separate PR as it requries non-trivial rewrites.

PR Close #29328
2019-03-19 23:23:31 -04:00
Matias Niemelä a3ec058f6b revert: fix(core): parse incorrect ML open tag as text (#29328) 2019-03-19 11:12:32 -07:00
Pawel Kozlowski 4605df83e1 fix(core): parse incorrect ML open tag as text (#29328)
This PR alligns markup language lexer with the previous behaviour in version 7.x:
https://stackblitz.com/edit/angular-iancj2

While this behaviour is not perfect (we should be giving users an error message
here about invalid HTML instead of assuming text node) this is probably best we
can do without more substential re-write of lexing / parsing infrastructure.

This PR just fixes #29231 and restores VE behaviour - a more elaborate fix will
be done in a separate PR as it requries non-trivial rewrites.

PR Close #29328
2019-03-19 13:30:20 -04:00
Pawel Kozlowski f2dc32e5c7 fix(core): don't wrap <tr> and <col> elements into a required parent (#29219)
BREAKING CHANGE:

Certain elements (like `<tr>` or `<col>`) require parent elements to be of a certain type by the HTML specification
(ex. <tr> can only be inside <tbody> / <thead>). Before this change Angular template parser was auto-correcting
"invalid" HTML using the following rules:
- `<tr>` would be wrapped in `<tbody>` if not inside `<tbody>`, `<tfoot>` or `<thead>`;
- `<col>` would be wrapped in `<colgroup>` if not inside `<colgroup>`.

This meachanism of automatic wrapping / auto-correcting was problematic for several reasons:
- it is non-obvious and arbitrary (ex. there are more HTML elements that has rules for parent type);
- it is incorrect for cases where `<tr>` / `<col>` are at the root of a component's content, ex.:

```html
<projecting-tr-inside-tbody>
  <tr>...</tr>
</projecting-tr-inside-tbody>
```

In the above example the `<projecting-tr-inside-tbody>` component culd be "surprised" to see additional
`<tbody>` elements inserted by Angular HTML parser.

PR Close #29219
2019-03-14 03:07:01 -04:00
Pete Bacon Darwin cb20b3b40a docs(compiler): correct lexer argument descriptions (#28978)
PR Close #28978
2019-02-28 02:44:19 -08:00
Pete Bacon Darwin f7c867ebc2 fix(ivy): correctly tokenize escaped characters in templates (#28978)
Previously the start of a character indicated by an escape sequence
was being incorrectly computed by the lexer, which caused tokens
to include the start of the escaped character sequence in the
preceding token. In particular this affected the name extracted
from opening tags if the name was terminated by an escape sequence.
For example, `<t\n>` would have the name `t\` rather than `t`.

This fix refactors the lexer to use a "cursor" object to iterate over
the characters in the template source. There are two cursor implementations,
one expects a simple string, the other expects a string that contains
JavaScript escape sequences that need to be unescaped.

PR Close #28978
2019-02-28 02:44:19 -08:00
Pete Bacon Darwin 76979e12c9 refactor(compiler): remove not-null operator hack from lexer (#28978)
The parts of a token are supposed to be an array of not-null strings,
but we were using `null` for tags that had no prefix. This has been
fixed to use the empty string in such cases, which allows the `null !`
hack to be removed.

PR Close #28978
2019-02-28 02:44:19 -08:00
Pete Bacon Darwin c0dac184cd fix(compiler): markup lexer should not capture quotes in attribute value (#28055)
When tokenizing markup (e.g. HTML) element attributes
can have quoted or unquoted values (e.g. `a=b` or `a="b"`).
The `ATTR_VALUE` tokens were capturing the quotes, which
was inconsistent and also affected source-mapping.

Now the tokenizer captures additional `ATTR_QUOTE` tokens,
which the HTML related parsers understand and factor into their
token parsing.

PR Close #28055
2019-02-12 20:58:27 -08:00
Pete Bacon Darwin 2424184d42 feat(compiler): support tokenizing escaped strings (#28055)
In order to support source mapping of templates, we need
to be able to tokenize the template in its original context.
When the template is defined inline as a JavaScript string
in a TS/JS source file, the tokenizer must be able to handle
string escape sequences, such as `\n` and `\"` as they
appear in the original source file.

This commit teaches the lexer how to unescape these
sequences, but only when the `escapedString` option is
set to true.  Otherwise there is no change to the tokenizing
behaviour.

PR Close #28055
2019-02-12 20:58:27 -08:00
Pete Bacon Darwin eeb560ac88 feat(compiler): support tokenizing a sub-section of an input string (#28055)
The lexer that does the tokenizing can now process only a part the source
string, by passing a `range` property in the `options` argument. The
locations of the nodes that are tokenized will now take into account the
position of the span in the context of the original source string.

This `range` option is, in turn, exposed from the template parser as well.

Being able to process parts of files helps to enable SourceMap support
when compiling inline component templates.

PR Close #28055
2019-02-12 20:58:27 -08:00