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

fix(language-service): correctly parse expressions in an attribute #34517

Closed

Conversation

@ayazhafiz
Copy link
Member

ayazhafiz commented Dec 20, 2019

Currently, the language service provides completions in a template node
attribute by first checking if the attribute contains template bindings
to provide completions for, and then providing completions for the
expression in the attribute.

In the latter case, the expression AST was being constructed
"synthetically" inside the language service, in particular declaring the
expression to be a PropertyRead with an implicit receiver.
Unfortunately, this AST can be incorrect if the expression is actually a
property read on a component property receiver (e.g. when reading
key in the expression obj.key, obj is the receiver).

The fix is pretty simple - rather than a synthetic construction of the
AST, ask the expression parser to parse the expression in the attribute.

Fixes angular/vscode-ng-language-service#523

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No
@ayazhafiz ayazhafiz requested a review from kyliau Dec 20, 2019
@ayazhafiz ayazhafiz requested a review from angular/tools-language-service as a code owner Dec 20, 2019
@ayazhafiz ayazhafiz self-assigned this Dec 20, 2019
@ayazhafiz ayazhafiz force-pushed the ayazhafiz:fix/expression-in-template-binding branch from cc10625 to 4d88597 Dec 20, 2019
@ngbot ngbot bot added this to the needsTriage milestone Dec 20, 2019
@googlebot googlebot added the cla: yes label Dec 20, 2019
@ayazhafiz ayazhafiz force-pushed the ayazhafiz:fix/expression-in-template-binding branch from 4d88597 to abb3d57 Dec 20, 2019
Currently, the language service provides completions in a template node
attribute by first checking if the attribute contains template bindings
to provide completions for, and then providing completions for the
expression in the attribute.

In the latter case, the expression AST was being constructed
"synthetically" inside the language service, in particular declaring the
expression to be a `PropertyRead` with an implicit receiver.
Unfortunately, this AST can be incorrect if the expression is actually a
property read on a component property receiver (e.g. when reading
`key` in the expression `obj.key`, `obj` is the receiver).

The fix is pretty simple - rather than a synthetic construction of the
AST, ask the expression parser to parse the expression in the attribute.

Fixes angular/vscode-ng-language-service#523
@ayazhafiz ayazhafiz force-pushed the ayazhafiz:fix/expression-in-template-binding branch from abb3d57 to 0b366e2 Dec 20, 2019
…bute
@ayazhafiz ayazhafiz requested a review from kyliau Dec 20, 2019
@ayazhafiz ayazhafiz removed their assignment Dec 20, 2019
…bute
@kyliau
kyliau approved these changes Dec 21, 2019
Copy link
Member

kyliau left a comment

thank you, this is really clean.

@alxhub alxhub closed this in ee46b9b Jan 6, 2020
alxhub added a commit that referenced this pull request Jan 6, 2020
…34517)

Currently, the language service provides completions in a template node
attribute by first checking if the attribute contains template bindings
to provide completions for, and then providing completions for the
expression in the attribute.

In the latter case, the expression AST was being constructed
"synthetically" inside the language service, in particular declaring the
expression to be a `PropertyRead` with an implicit receiver.
Unfortunately, this AST can be incorrect if the expression is actually a
property read on a component property receiver (e.g. when reading
`key` in the expression `obj.key`, `obj` is the receiver).

The fix is pretty simple - rather than a synthetic construction of the
AST, ask the expression parser to parse the expression in the attribute.

Fixes angular/vscode-ng-language-service#523

PR Close #34517
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.