From 269a775287069eaa30d8c99812631672ae4aef61 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Tue, 17 Nov 2020 13:00:22 -0800
Subject: [PATCH] refactor(compiler-cli): produce binding access when
 checkTypeOfOutputEvents is false (#39515)

When `checkTypeOfOutputEvents` is `false`, we still need to produce the access
to the `EventEmitter` so the Language Service can still get the
type information about the field. That is, in a template `<div
(output)="handle($event)"`, we still want to be able to grab information
when the cursor is inside the "output" parens. The flag is intended only
to affect whether the compiler produces diagnostics for the inferred
type of the `$event`.

PR Close #39515
---
 .../typecheck/src/template_symbol_builder.ts  |  4 +++-
 .../ngtsc/typecheck/src/type_check_block.ts   | 20 +++++++++++--------
 .../typecheck/test/type_check_block_spec.ts   |  2 +-
 ...ecker__get_symbol_of_template_node_spec.ts | 13 ++++++++----
 .../ivy/test/quick_info_spec.ts               | 11 ++++++++++
 5 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
index 00a1b32988..86cb34e882 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts
@@ -165,9 +165,11 @@ export class SymbolBuilder {
   }
 
   private getSymbolOfBoundEvent(eventBinding: TmplAstBoundEvent): OutputBindingSymbol|null {
-    // Outputs are a `ts.CallExpression` that look like one of the two:
+    // Outputs in the TCB look like one of the two:
     // * _outputHelper(_t1["outputField"]).subscribe(handler);
     // * _t1.addEventListener(handler);
+    // Even with strict null checks disabled, we still produce the access as a separate statement
+    // so that it can be found here.
     const outputFieldAccess = findFirstMatchingNode(
         this.typeCheckBlock, {withSpan: eventBinding.keySpan, filter: isAccessExpression});
     if (outputFieldAccess === null) {
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
index 55713439b9..ba3a23d083 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -865,6 +865,11 @@ export class TcbDirectiveOutputsOp extends TcbOp {
       // TODO(alxhub): consider supporting multiple fields with the same property name for outputs.
       const field = outputs.getByBindingPropertyName(output.name)![0].classPropertyName;
 
+      if (dirId === null) {
+        dirId = this.scope.resolve(this.node, this.dir);
+      }
+      const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
+      addParseSpanInfo(outputField, output.keySpan);
       if (this.tcb.env.config.checkTypeOfOutputEvents) {
         // For strict checking of directive events, generate a call to the `subscribe` method
         // on the directive's output field to let type information flow into the handler function's
@@ -877,12 +882,6 @@ export class TcbDirectiveOutputsOp extends TcbOp {
         // specially crafted set of signatures, to effectively cast `EventEmitter<T>` to something
         // that has a `subscribe` method that properly carries the `T` into the handler function.
         const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);
-
-        if (dirId === null) {
-          dirId = this.scope.resolve(this.node, this.dir);
-        }
-        const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
-        addParseSpanInfo(outputField, output.keySpan);
         const outputHelper =
             ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]);
         const subscribeFn = ts.createPropertyAccess(outputHelper, 'subscribe');
@@ -890,8 +889,13 @@ export class TcbDirectiveOutputsOp extends TcbOp {
         addParseSpanInfo(call, output.sourceSpan);
         this.scope.addStatement(ts.createExpressionStatement(call));
       } else {
-        // If strict checking of directive events is disabled, emit a handler function where the
-        // `$event` parameter has an explicit `any` type.
+        // If strict checking of directive events is disabled:
+        //
+        // * We still generate the access to the output field as a statement in the TCB so consumers
+        //   of the `TemplateTypeChecker` can still find the node for the class member for the
+        //   output.
+        // * Emit a handler function where the `$event` parameter has an explicit `any` type.
+        this.scope.addStatement(ts.createExpressionStatement(outputField));
         const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Any);
         this.scope.addStatement(ts.createExpressionStatement(handler));
       }
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
index 82fc1d0583..8e2b580d7a 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
@@ -839,7 +839,7 @@ describe('type check blocks', () => {
         expect(block).toContain('function ($event: any): any { (ctx).foo($event); }');
         // Note that DOM events are still checked, that is controlled by `checkTypeOfDomEvents`
         expect(block).toContain(
-            '_t1.addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
+            'addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
       });
     });
 
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts
index d39489714a..23bb5912e7 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts
@@ -1271,7 +1271,7 @@ runInEachFileSystem(() => {
            assertExpressionSymbol(eventSymbol);
          });
 
-      it('returns empty list when checkTypeOfOutputEvents is false', () => {
+      it('still returns binding when checkTypeOfOutputEvents is false', () => {
         const fileName = absoluteFrom('/main.ts');
         const dirFile = absoluteFrom('/dir.ts');
         const {program, templateTypeChecker} = setup(
@@ -1302,9 +1302,14 @@ runInEachFileSystem(() => {
         const nodes = templateTypeChecker.getTemplate(cmp)!;
 
         const outputABinding = (nodes[0] as TmplAstElement).outputs[0];
-        const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp);
-        // TODO(atscott): should type checker still generate the subscription in this case?
-        expect(symbol).toBeNull();
+        const symbol = templateTypeChecker.getSymbolOfNode(outputABinding, cmp)!;
+        assertOutputBindingSymbol(symbol);
+        expect(
+            (symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration).name.getText())
+            .toEqual('outputA');
+        expect((symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration)
+                   .parent.name?.text)
+            .toEqual('TestDir');
       });
     });
 
diff --git a/packages/language-service/ivy/test/quick_info_spec.ts b/packages/language-service/ivy/test/quick_info_spec.ts
index 0a863ecd09..b6c91786b1 100644
--- a/packages/language-service/ivy/test/quick_info_spec.ts
+++ b/packages/language-service/ivy/test/quick_info_spec.ts
@@ -496,6 +496,17 @@ describe('quick info', () => {
         expectedDisplayString: '(property) TestComponent.name: string'
       });
     });
+
+    it('can still get quick info when strictOutputEventTypes is false', () => {
+      initMockFileSystem('Native');
+      env = LanguageServiceTestEnvironment.setup(
+          quickInfoSkeleton(), {strictOutputEventTypes: false});
+      expectQuickInfo({
+        templateOverride: `<test-comp (te¦st)="myClick($event)"></test-comp>`,
+        expectedSpanText: 'test',
+        expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter<string>'
+      });
+    });
   });
 
   function expectQuickInfo(