fix(compiler-cli): handle inline type-check blocks in nullish coalescing extended check (#45454)

JoostK · dylhunn · commit 06050ac2b493 · 2022-03-29T14:09:33.000-07:00
This commit fixes an inconsistency where a type check location for an inline type check block would be interpreted to occur in a type-checking shim instead. This resulted in a missing template mapping, causing a crash due to an unsafe non-null assertion operator. In the prior commit the `TcbLocation` has been extended with an `isShimFile` field that is now being used to look for the template mapping in the correct location. Additionally, the non-null assertion operator is refactored such that a missing template mapping will now ignore the warning instead of crashing the compiler. Fixes #45413 PR Close #45454
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/index.ts
@@ -50,9 +50,13 @@ class NullishCoalescingNotNullableCheck extends
     if (symbol.kind !== SymbolKind.Expression) {
       return [];
     }
-    const span = ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation)!.span;
+    const templateMapping =
+        ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation);
+    if (templateMapping === null) {
+      return [];
+    }
     const diagnostic = ctx.makeTemplateDiagnostic(
-        span,
+        templateMapping.span,
         `The left side of this nullish coalescing operation does not include 'null' or 'undefined' in its type, therefore the '??' operator can be safely removed.`);
     return [diagnostic];
   }
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/nullish_coalescing_not_nullable/nullish_coalescing_not_nullable_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/nullish_coalescing_not_nullable/nullish_coalescing_not_nullable_spec.ts
@@ -67,6 +67,29 @@ runInEachFileSystem(() => {
       expect(getSourceCodeForDiagnostic(diags[0])).toBe(`var1 ?? 'foo'`);
     });
 
+    it('should produce nullish coalescing warning for classes with inline TCBs', () => {
+      const fileName = absoluteFrom('/main.ts');
+      const {program, templateTypeChecker} = setup(
+          [{
+            fileName,
+            templates: {
+              'TestCmp': `{{ var1 ?? 'foo' }}`,
+            },
+            source: 'class TestCmp { var1: string = "text"; }'
+          }],
+          {inlining: true});
+      const sf = getSourceFileOrError(program, fileName);
+      const component = getClass(sf, 'TestCmp');
+      const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(
+          templateTypeChecker, program.getTypeChecker(), [nullishCoalescingNotNullableFactory],
+          {strictNullChecks: true} /* options */);
+      const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
+      expect(diags.length).toBe(1);
+      expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
+      expect(diags[0].code).toBe(ngErrorCode(ErrorCode.NULLISH_COALESCING_NOT_NULLABLE));
+      expect(getSourceCodeForDiagnostic(diags[0])).toBe(`var1 ?? 'foo'`);
+    });
+
     it('should not produce nullish coalescing warning for a nullable type', () => {
       const fileName = absoluteFrom('/main.ts');
       const {program, templateTypeChecker} = setup([{
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
@@ -146,6 +146,28 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     return this.getFileAndShimRecordsForPath(filePath) !== null;
   }
 
+  private getFileRecordForTcbLocation({tcbPath, isShimFile}: TcbLocation): FileTypeCheckingData
+      |null {
+    if (!isShimFile) {
+      // The location is not within a shim file but corresponds with an inline TCB in an original
+      // source file; we can obtain the record directly by its path.
+      if (this.state.has(tcbPath)) {
+        return this.state.get(tcbPath)!;
+      } else {
+        return null;
+      }
+    }
+
+    // The location is within a type-checking shim file; find the type-checking data that owns this
+    // shim path.
+    const records = this.getFileAndShimRecordsForPath(tcbPath);
+    if (records !== null) {
+      return records.fileRecord;
+    } else {
+      return null;
+    }
+  }
+
   private getFileAndShimRecordsForPath(shimPath: AbsoluteFsPath):
       {fileRecord: FileTypeCheckingData, shimRecord: ShimTypeCheckingData}|null {
     for (const fileRecord of this.state.values()) {
@@ -156,20 +178,19 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     return null;
   }
 
-  getTemplateMappingAtTcbLocation({tcbPath, positionInFile}: TcbLocation): FullTemplateMapping
-      |null {
-    const records = this.getFileAndShimRecordsForPath(absoluteFrom(tcbPath));
-    if (records === null) {
+  getTemplateMappingAtTcbLocation(tcbLocation: TcbLocation): FullTemplateMapping|null {
+    const fileRecord = this.getFileRecordForTcbLocation(tcbLocation);
+    if (fileRecord === null) {
       return null;
     }
-    const {fileRecord} = records;
 
-    const shimSf = this.programDriver.getProgram().getSourceFile(absoluteFrom(tcbPath));
+    const shimSf = this.programDriver.getProgram().getSourceFile(tcbLocation.tcbPath);
     if (shimSf === undefined) {
       return null;
     }
     return getTemplateMapping(
-        shimSf, positionInFile, fileRecord.sourceManager, /*isDiagnosticsRequest*/ false);
+        shimSf, tcbLocation.positionInFile, fileRecord.sourceManager,
+        /*isDiagnosticsRequest*/ false);
   }
 
   generateAllTypeCheckBlocks() {
