From 0802b59a7bfb1f30d8fd17307a50926aa44ef1e6 Mon Sep 17 00:00:00 2001 From: Daniel Trevino <23410540+danieltre23@users.noreply.github.com> Date: Fri, 13 Aug 2021 19:46:00 +0000 Subject: [PATCH] refactor(compiler-cli): add `NullishCoalescingNotNullableCheck` (#43232) Add a template check that returns diagnostics if the left side of a nullish coalescing operation is not nullable. Refs #42966 PR Close #43232 --- goldens/public-api/compiler-cli/error_code.md | 1 + .../compiler-cli/src/ngtsc/core/BUILD.bazel | 1 + .../src/ngtsc/core/src/compiler.ts | 8 ++- .../src/ngtsc/diagnostics/src/error_code.ts | 10 ++++ .../BUILD.bazel | 14 +++++ .../nullish_coalescing_not_nullable/index.ts | 51 +++++++++++++++++++ 6 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/BUILD.bazel create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/index.ts diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index 7134a7b404b..52aba50642c 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -45,6 +45,7 @@ export enum ErrorCode { NGMODULE_MODULE_WITH_PROVIDERS_MISSING_GENERIC = 6005, NGMODULE_REEXPORT_NAME_COLLISION = 6006, NGMODULE_VE_DEPENDENCY_ON_IVY_LIB = 6999, + NULLISH_COALESCING_NOT_NULLABLE = 8102, // (undocumented) PARAM_MISSING_TOKEN = 2003, // (undocumented) diff --git a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel index ba86ed7a8f1..28e3501ed2e 100644 --- a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel @@ -40,6 +40,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck/extended", "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable", "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/xi18n", "@npm//typescript", diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 4e4e4e7cb34..bb4724e6f09 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -31,8 +31,9 @@ import {aliasTransformFactory, CompilationMode, declarationTransformFactory, Dec import {TemplateTypeCheckerImpl} from '../../typecheck'; import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api'; import {ExtendedTemplateCheckerImpl} from '../../typecheck/extended'; -import {ExtendedTemplateChecker} from '../../typecheck/extended/api'; +import {ExtendedTemplateChecker, TemplateCheck} from '../../typecheck/extended/api'; import {InvalidBananaInBoxCheck} from '../../typecheck/extended/checks/invalid_banana_in_box'; +import {NullishCoalescingNotNullableCheck} from '../../typecheck/extended/checks/nullish_coalescing_not_nullable'; import {getSourceFileOrNull, isDtsPath, resolveModuleName, toUnredirectedSourceFile} from '../../util/src/typescript'; import {Xi18nContext} from '../../xi18n'; import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api'; @@ -1109,7 +1110,10 @@ export class NgCompiler { reflector, this.adapter, this.incrementalCompilation, scopeRegistry, typeCheckScopeRegistry, this.delegatingPerfRecorder); - const templateChecks = [new InvalidBananaInBoxCheck()]; + const templateChecks: TemplateCheck[] = [new InvalidBananaInBoxCheck()]; + if (this.options.strictNullChecks) { + templateChecks.push(new NullishCoalescingNotNullableCheck()); + } const extendedTemplateChecker = new ExtendedTemplateCheckerImpl(templateTypeChecker, checker, templateChecks); diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 26c5ffd5873..0adb4704db2 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -182,6 +182,16 @@ export enum ErrorCode { */ INVALID_BANANA_IN_BOX = 8101, + /** + * The left side of a nullish coalescing operation is not nullable. + * + * ``` + * {{ foo ?? bar }} + * ``` + * When the type of foo doesn't include `null` or `undefined`. + */ + NULLISH_COALESCING_NOT_NULLABLE = 8102, + /** * 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. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/BUILD.bazel new file mode 100644 index 00000000000..b687a466cff --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/BUILD.bazel @@ -0,0 +1,14 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "nullish_coalescing_not_nullable", + srcs = ["index.ts"], + visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck/api", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", + "@npm//typescript", + ], +) 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 new file mode 100644 index 00000000000..0e7415ab3ad --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable/index.ts @@ -0,0 +1,51 @@ +/** + * @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, Binary, TmplAstNode} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {ErrorCode} from '../../../../diagnostics'; +import {NgTemplateDiagnostic, SymbolKind} from '../../../api'; +import {TemplateCheckWithVisitor, TemplateContext} from '../../api'; + +/** + * Ensures the left side of a nullish coalescing 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. + */ +export class NullishCoalescingNotNullableCheck extends + TemplateCheckWithVisitor { + override code = ErrorCode.NULLISH_COALESCING_NOT_NULLABLE as const; + + override visitNode(ctx: TemplateContext, component: ts.ClassDeclaration, node: TmplAstNode|AST): + NgTemplateDiagnostic[] { + if (!(node instanceof Binary) || node.operation !== '??') return []; + + const symbolLeft = ctx.templateTypeChecker.getSymbolOfNode(node.left, component)!; + if (symbolLeft.kind !== SymbolKind.Expression) { + return []; + } + const typeLeft = symbolLeft.tsType; + // 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 span = + ctx.templateTypeChecker.getTemplateMappingAtShimLocation(symbol.shimLocation)!.span; + const diagnostic = ctx.templateTypeChecker.makeTemplateDiagnostic( + component, span, ts.DiagnosticCategory.Warning, ErrorCode.NULLISH_COALESCING_NOT_NULLABLE, + `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]; + } +}