Autocompletion in expression contexts #39727

alxhub · 2020-11-17T23:01:05Z

No description provided.

atscott · 2020-12-07T20:43:07Z

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

+   * autocompletion at that point in the expression, if such a location exists.
+   */
+  getExpressionCompletionLocation(
+      expr: PropertyRead|SafePropertyRead|MethodCall|SafeMethodCall,


What about KeyedRead/KeyedWrite/PropertyWrite?

KeyedRead and KeyedWrite are non-trivial - the key is itself an expression, meaning we won't target the KeyedRead but rather a LiteralPrimitive (if a string literal key is used) within the KeyedRead. It's possible to support these forms, but I want to get the 99% case working first.

I added support / a test for PropertyWrite, since it's nearly identical to PropertyRead.

packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts

atscott · 2020-12-07T20:52:51Z

packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts

@@ -79,4 +83,52 @@ export class CompletionEngine {
    this.globalCompletionCache.set(context, completion);
    return completion;
  }
+
+  getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|MethodCall|


I'm actually a bit curious about this method in general. Shouldn't TTC#getSymbolAtLocation return this same information?

Edit: Ah, I see that you use getEnd whereas the getSymbolAtLocation uses getStart. Maybe there's some more nuance to this as well that I don't see immediately.

I don't think there's any nuance with getStart() vs getEnd(). TTC#getSymbolOfNode() would almost certainly work in the cases supported by this PR. But something more complex liked KeyedRead may require a completion location that's specifically inside of the quoted key string, for example, which getSymbolOfNode() isn't obligated to provide.

We could add to getSymbolOfNode()'s contract that it provides autocompletion-compatible locations where necessary, but I think a separate method is probably preferable.

atscott · 2020-12-07T20:53:47Z

packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts

+      // Safe navigation operations are a little more complex, and involve a ternary. Completion
+      // happens in the "true" case of the ternary.
+      const ternaryExpr = findFirstMatchingNode(this.tcb, {
+        filter: ts.isParenthesizedExpression,


It's a little weird that you're using parenthesized expression as the filter here. I think this should actually be ts.isConditionalExpression based on the check below.

getSymbolOfTemplateExpression does something similar - it looks for any node with the given span, then unwraps any parenthesized expression. I'm relying on the fact that all ternaries are parenthesized.

kyliau · 2020-12-04T00:03:03Z

packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts

@@ -28,6 +29,9 @@ export class CompletionEngine {
   */
  private globalCompletionCache = new Map<TmplAstTemplate|null, GlobalCompletion>();

+  private expressionCompletionCache =


Do you have any concerns about memory "leak" since we are not invalidating cache entries?

No, because the entire CompletionEngine is thrown away on a template change.

kyliau · 2020-12-04T00:10:22Z

packages/language-service/ivy/completions.ts

+  /**
+   * Analogue for `ts.LanguageService.getCompletionEntrySymbol`.
+   */
+  getCompletionEntrySymbol(name: string): ts.Symbol|undefined {


I think we can omit this method. I looked at the TypeScript language service,getCompletionEntryDetails is the main API for completions. Besides, I don't think we have a use case for the ts.Symbol right now. The symbol will be discarded at the LSP boundary.

I'm going to leave it in here, but can do a separate PR to remove all the getCompletionSymbol operations.

packages/language-service/ivy/completions.ts

packages/language-service/ivy/language_service.ts

packages/language-service/ivy/completions.ts

kyliau · 2020-12-07T21:24:46Z

packages/language-service/ivy/test/completions_spec.ts

+      const {ngLS, fileName, cursor} =
+          setup(`{{name.f¦}}`, `name!: {first: string; last: string;};`);
+      const completions = ngLS.getCompletionsAtPosition(fileName, cursor, /* options */ undefined);
+      expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['first']);


We should have a test that lists out all the results.
In the past, we've had unexpected completion results because we only check what's expected to be there, but not results that are not supposed to be there.

Done - switched to expectAll.

This commit adds support to the Language Service for autocompletion within expression contexts. Specifically, this is auto completion of property reads and method calls, both in normal and safe-navigational forms.

mhevery · 2020-12-09T00:26:28Z

presubmit

…gular#39727) This commit adds support to the Language Service for autocompletion within expression contexts. Specifically, this is auto completion of property reads and method calls, both in normal and safe-navigational forms. PR Close angular#39727

angular-automatic-lock-bot · 2021-01-10T16:58:03Z

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.}

google-cla bot added the cla: yes label Nov 17, 2020

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch 4 times, most recently from 04691c8 to 199194e Compare November 18, 2020 20:27

jessicajaniuk added the area: compiler Issues related to `ngc`, Angular's template compiler label Dec 1, 2020

ngbot bot added this to the needsTriage milestone Dec 1, 2020

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch 2 times, most recently from fc65434 to 62f9518 Compare December 3, 2020 22:01

alxhub marked this pull request as ready for review December 3, 2020 22:34

pullapprove bot requested a review from kyliau December 3, 2020 22:34

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch from 62f9518 to ea24631 Compare December 4, 2020 21:41

alxhub added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Dec 4, 2020

ngbot bot modified the milestones: needsTriage, Backlog Dec 4, 2020

alxhub requested review from atscott and removed request for kyliau December 7, 2020 18:23

pullapprove bot requested a review from kyliau December 7, 2020 18:23

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch from ea24631 to a8c7c87 Compare December 7, 2020 20:28

atscott approved these changes Dec 7, 2020

View reviewed changes

kyliau approved these changes Dec 7, 2020

View reviewed changes

kyliau added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 7, 2020

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch 3 times, most recently from 52eb6a2 to defa59b Compare December 8, 2020 20:33

alxhub removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Dec 8, 2020

alxhub added the action: merge The PR is ready for merge by the caretaker label Dec 8, 2020

feat(language-service): autocompletion within expression contexts

7fc2616

This commit adds support to the Language Service for autocompletion within expression contexts. Specifically, this is auto completion of property reads and method calls, both in normal and safe-navigational forms.

alxhub force-pushed the ngtsc/ls/autocomplete-6 branch from defa59b to 7fc2616 Compare December 8, 2020 20:57

pullapprove bot removed the area: compiler Issues related to `ngc`, Angular's template compiler label Dec 9, 2020

ngbot bot removed this from the Backlog milestone Dec 9, 2020

alxhub closed this in 93a8326 Dec 10, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autocompletion in expression contexts #39727

Autocompletion in expression contexts #39727

alxhub commented Nov 17, 2020

atscott Dec 7, 2020

alxhub Dec 8, 2020

alxhub Dec 8, 2020

atscott Dec 7, 2020

alxhub Dec 8, 2020

atscott Dec 7, 2020

alxhub Dec 8, 2020

kyliau Dec 4, 2020

alxhub Dec 8, 2020

kyliau Dec 4, 2020

alxhub Dec 8, 2020

kyliau Dec 7, 2020

alxhub Dec 8, 2020

mhevery commented Dec 9, 2020

angular-automatic-lock-bot bot commented Jan 10, 2021

Autocompletion in expression contexts #39727

Autocompletion in expression contexts #39727

Conversation

alxhub commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhevery commented Dec 9, 2020

angular-automatic-lock-bot bot commented Jan 10, 2021