feat(compiler-cli): add extended diagnostic for non-nullable optional chains (#46686)

This commit adds an extended diagnostics check that is similar to the nullish
coalescing check, but targeting optional chains. If the receiver expression
of the optional chain is non-nullable, then the extended diagnostic can report
an error or warning that can be fixed by changing the optional chain into a
regular access.

Closes #44870

PR Close #46686

Author: JoostK

goldens/public-api/compiler-cli/error_code.md

@@ -60,6 +60,7 @@ export enum ErrorCode {
     NGMODULE_REEXPORT_NAME_COLLISION = 6006,
     NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999,
     NULLISH_COALESCING_NOT_NULLABLE = 8102,
+    OPTIONAL_CHAIN_NOT_NULLABLE = 8107,
     // (undocumented)
     PARAM_MISSING_TOKEN = 2003,
     // (undocumented)

goldens/public-api/compiler-cli/extended_template_diagnostic_name.md

@@ -15,6 +15,8 @@ export enum ExtendedTemplateDiagnosticName {
     // (undocumented)
     NULLISH_COALESCING_NOT_NULLABLE = "nullishCoalescingNotNullable",
     // (undocumented)
+    OPTIONAL_CHAIN_NOT_NULLABLE = "optionalChainNotNullable",
+    // (undocumented)
     SUFFIX_NOT_SUPPORTED = "suffixNotSupported",
     // (undocumented)
     TEXT_ATTRIBUTE_NOT_BINDING = "textAttributeNotBinding"

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -278,6 +278,18 @@ export enum ErrorCode {
    */
   SUFFIX_NOT_SUPPORTED = 8106,
 
+  /**
+   * The left side of an optional chain operation is not nullable.
+   *
+   * ```
+   * {{ foo?.bar }}
+   * {{ foo?.['bar'] }}
+   * {{ foo?.() }}
+   * ```
+   * When the type of foo doesn't include `null` or `undefined`.
+   */
+  OPTIONAL_CHAIN_NOT_NULLABLE = 8107,
+
   /**
    * The template type-checking engine would need to generate an inline type check block for a
    * component, but the current type-checking environment doesn't support it.

packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts

@@ -18,6 +18,7 @@
 export enum ExtendedTemplateDiagnosticName {
   INVALID_BANANA_IN_BOX = 'invalidBananaInBox',
   NULLISH_COALESCING_NOT_NULLABLE = 'nullishCoalescingNotNullable',
+  OPTIONAL_CHAIN_NOT_NULLABLE = 'optionalChainNotNullable',
   MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective',
   TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding',
   MISSING_NGFOROF_LET = 'missingNgForOfLet',

packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel

@@ -16,6 +16,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive",
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let",
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable",
+        "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable",
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/suffix_not_supported",
         "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/text_attribute_not_binding",
         "@npm//typescript",

packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "optional_chain_not_nullable",
+    srcs = ["index.ts"],
+    visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"],
+    deps = [
+        "//packages/compiler",
+        "//packages/compiler-cli/src/ngtsc/core:api",
+        "//packages/compiler-cli/src/ngtsc/diagnostics",
+        "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
+        "@npm//typescript",
+    ],
+)

packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable/index.ts

@@ -0,0 +1,85 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {AST, SafeCall, SafeKeyedRead, SafePropertyRead, TmplAstNode} from '@angular/compiler';
+import ts from 'typescript';
+
+import {NgCompilerOptions} from '../../../../core/api';
+import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
+import {NgTemplateDiagnostic, SymbolKind} from '../../../api';
+import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
+
+/**
+ * Ensures the left side of an optional chain operation is nullable.
+ * Returns diagnostics for the cases where the operator is useless.
+ * This check should only be use if `strictNullChecks` is enabled,
+ * otherwise it would produce inaccurate results.
+ */
+class OptionalChainNotNullableCheck extends
+    TemplateCheckWithVisitor<ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE> {
+  override code = ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE as const;
+
+  override visitNode(
+      ctx: TemplateContext<ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE>, component: ts.ClassDeclaration,
+      node: TmplAstNode|AST): NgTemplateDiagnostic<ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE>[] {
+    if (!(node instanceof SafeCall) && !(node instanceof SafePropertyRead) &&
+        !(node instanceof SafeKeyedRead))
+      return [];
+
+    const symbolLeft = ctx.templateTypeChecker.getSymbolOfNode(node.receiver, component);
+    if (symbolLeft === null || symbolLeft.kind !== SymbolKind.Expression) {
+      return [];
+    }
+    const typeLeft = symbolLeft.tsType;
+    if (typeLeft.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
+      // We should not make assumptions about the any and unknown types; using a nullish coalescing
+      // operator is acceptable for those.
+      return [];
+    }
+
+    // If the left operand's type is different from its non-nullable self, then it must
+    // contain a null or undefined so this nullish coalescing operator is useful. No diagnostic to
+    // report.
+    if (typeLeft.getNonNullableType() !== typeLeft) return [];
+
+    const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component)!;
+    if (symbol.kind !== SymbolKind.Expression) {
+      return [];
+    }
+    const templateMapping =
+        ctx.templateTypeChecker.getTemplateMappingAtTcbLocation(symbol.tcbLocation);
+    if (templateMapping === null) {
+      return [];
+    }
+    const advice = node instanceof SafePropertyRead ?
+        `the '?.' operator can be replaced with the '.' operator` :
+        `the '?.' operator can be safely removed`;
+    const diagnostic = ctx.makeTemplateDiagnostic(
+        templateMapping.span,
+        `The left side of this optional chain operation does not include 'null' or 'undefined' in its type, therefore ${
+            advice}.`);
+    return [diagnostic];
+  }
+}
+
+export const factory: TemplateCheckFactory<
+    ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE,
+    ExtendedTemplateDiagnosticName.OPTIONAL_CHAIN_NOT_NULLABLE> = {
+  code: ErrorCode.OPTIONAL_CHAIN_NOT_NULLABLE,
+  name: ExtendedTemplateDiagnosticName.OPTIONAL_CHAIN_NOT_NULLABLE,
+  create: (options: NgCompilerOptions) => {
+    // Require `strictNullChecks` to be enabled.
+    const strictNullChecks =
+        options.strictNullChecks === undefined ? !!options.strict : !!options.strictNullChecks;
+    if (!strictNullChecks) {
+      return null;
+    }
+
+    return new OptionalChainNotNullableCheck();
+  },
+};

packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts

@@ -13,6 +13,7 @@ import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_b
 import {factory as missingControlFlowDirectiveFactory} from './checks/missing_control_flow_directive';
 import {factory as missingNgForOfLetFactory} from './checks/missing_ngforof_let';
 import {factory as nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable';
+import {factory as optionalChainNotNullableFactory} from './checks/optional_chain_not_nullable';
 import {factory as suffixNotSupportedFactory} from './checks/suffix_not_supported';
 import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding';
 
@@ -22,6 +23,7 @@ export const ALL_DIAGNOSTIC_FACTORIES:
     readonly TemplateCheckFactory<ErrorCode, ExtendedTemplateDiagnosticName>[] = [
       invalidBananaInBoxFactory,
       nullishCoalescingNotNullableFactory,
+      optionalChainNotNullableFactory,
       missingControlFlowDirectiveFactory,
       textAttributeNotBindingFactory,
       missingNgForOfLetFactory,

packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/optional_chain_not_nullable/BUILD.bazel

@@ -0,0 +1,27 @@
+load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
+
+ts_library(
+    name = "test_lib",
+    testonly = True,
+    srcs = ["optional_chain_not_nullable_spec.ts"],
+    deps = [
+        "//packages/compiler",
+        "//packages/compiler-cli/src/ngtsc/core:api",
+        "//packages/compiler-cli/src/ngtsc/diagnostics",
+        "//packages/compiler-cli/src/ngtsc/file_system",
+        "//packages/compiler-cli/src/ngtsc/file_system/testing",
+        "//packages/compiler-cli/src/ngtsc/testing",
+        "//packages/compiler-cli/src/ngtsc/typecheck/extended",
+        "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable",
+        "//packages/compiler-cli/src/ngtsc/typecheck/testing",
+        "@npm//typescript",
+    ],
+)
+
+jasmine_node_test(
+    name = "test",
+    bootstrap = ["//tools/testing:node_no_angular_es2015"],
+    deps = [
+        ":test_lib",
+    ],
+)