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

parser and language service fixes for unclosed element tags #42554

Closed
wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Jun 10, 2021

fix(compiler): always match close tag to the nearest open element

This commit updates the parser logic to continue to try to match an end
tag to an unclosed open tag on the stack. Previously, it would only
push an error to the list and stop looking at unclosed elements.

For example, the invalid HTML of <li><div></li>, has an unclosed
element stack of [li, div] when it encounters the close li tag.
We compare against the previously unclosed tag div and see that this is
unexpected. Instead of simply giving up here, we continue to move up the
unclosed tags until we find a match (if there is one).

fix(language-service): Use last child end span for parent without close tag

Unclosed element tags are not assigned an endSourceSpan by the parser.
As a result, the visitor which determines the target node at a position
for the language service was unable to determine that a given position
was inside an unclosed parent. This happens because we update the
endSourceSpan of template/element nodes to be the end tag (and there
is not one for unclosed tags). Consequently, the visitor then cannot
match a position to any child node location.

This change updates the visitor logic to check if there are any
children of a template/element node and updates the end span to be the
end span of the last child. This allows our isWithin logic to identify
that a child position is within the unclosed parent.

@atscott atscott added state: WIP area: language-service Issues related to Angular's VS Code language service area: compiler Issues related to `ngc`, Angular's template compiler labels Jun 10, 2021
@ngbot ngbot bot added this to the Backlog milestone Jun 10, 2021
@google-cla google-cla bot added the cla: yes label Jun 10, 2021
@pullapprove pullapprove bot requested a review from alxhub June 10, 2021 23:26
@atscott
Copy link
Contributor Author

atscott commented Jun 11, 2021

presubmit

@atscott atscott added target: patch This PR is targeted for the next patch release and removed state: WIP labels Jun 11, 2021
This commit updates the parser logic to continue to try to match an end
tag to an unclosed open tag on the stack. Previously, it would only
push an error to the list and stop looking at unclosed elements.

For example, the invalid HTML of `<li><div></li>`, has an unclosed
element stack of [`li`, `div`] when it encounters the close `li` tag.
We compare against the previously unclosed tag `div` and see that this is
unexpected. Instead of simply giving up here, we continue to move up the
unclosed tags until we find a match (if there is one).
packages/language-service/ivy/template_target.ts Outdated Show resolved Hide resolved
…se tag

Unclosed element tags are not assigned an `endSourceSpan` by the parser.
As a result, the visitor which determines the target node at a position
for the language service was unable to determine that a given position
was inside an unclosed parent. This happens because we update the
`endSourceSpan` of template/element nodes to be the end tag (and there
is not one for unclosed tags). Consequently, the visitor then cannot
match a position to any child node location.

This change updates the visitor logic to check if there are any
`children` of a template/element node and updates the end span to be the
end span of the last child. This allows our `isWithin` logic to identify
that a child position is within the unclosed parent.

Addresses one of the issues found during investigation of angular/vscode-ng-language-service#1399
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jun 14, 2021
@alxhub alxhub closed this in 8c1e0e6 Jun 14, 2021
alxhub pushed a commit that referenced this pull request Jun 14, 2021
…se tag (#42554)

Unclosed element tags are not assigned an `endSourceSpan` by the parser.
As a result, the visitor which determines the target node at a position
for the language service was unable to determine that a given position
was inside an unclosed parent. This happens because we update the
`endSourceSpan` of template/element nodes to be the end tag (and there
is not one for unclosed tags). Consequently, the visitor then cannot
match a position to any child node location.

This change updates the visitor logic to check if there are any
`children` of a template/element node and updates the end span to be the
end span of the last child. This allows our `isWithin` logic to identify
that a child position is within the unclosed parent.

Addresses one of the issues found during investigation of angular/vscode-ng-language-service#1399

PR Close #42554
alxhub pushed a commit that referenced this pull request Jun 14, 2021
…2554)

This commit updates the parser logic to continue to try to match an end
tag to an unclosed open tag on the stack. Previously, it would only
push an error to the list and stop looking at unclosed elements.

For example, the invalid HTML of `<li><div></li>`, has an unclosed
element stack of [`li`, `div`] when it encounters the close `li` tag.
We compare against the previously unclosed tag `div` and see that this is
unexpected. Instead of simply giving up here, we continue to move up the
unclosed tags until we find a match (if there is one).

PR Close #42554
alxhub pushed a commit that referenced this pull request Jun 14, 2021
…se tag (#42554)

Unclosed element tags are not assigned an `endSourceSpan` by the parser.
As a result, the visitor which determines the target node at a position
for the language service was unable to determine that a given position
was inside an unclosed parent. This happens because we update the
`endSourceSpan` of template/element nodes to be the end tag (and there
is not one for unclosed tags). Consequently, the visitor then cannot
match a position to any child node location.

This change updates the visitor logic to check if there are any
`children` of a template/element node and updates the end span to be the
end span of the last child. This allows our `isWithin` logic to identify
that a child position is within the unclosed parent.

Addresses one of the issues found during investigation of angular/vscode-ng-language-service#1399

PR Close #42554
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: language-service Issues related to Angular's VS Code language service cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants