Skip to content

Commit

Permalink
feat(compiler-cli): add extended diagnostic for non-nullable optional…
Browse files Browse the repository at this point in the history
… chains

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
  • Loading branch information
JoostK committed Jul 2, 2022
1 parent 96c6139 commit c2c5f36
Show file tree
Hide file tree
Showing 9 changed files with 411 additions and 0 deletions.
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.md
Expand Up @@ -59,6 +59,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 = 8105,
// (undocumented)
PARAM_MISSING_TOKEN = 2003,
// (undocumented)
Expand Down
12 changes: 12 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Expand Up @@ -256,6 +256,18 @@ export enum ErrorCode {
*/
TEXT_ATTRIBUTE_NOT_BINDING = 8104,

/**
* 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 = 8105,

/**
* 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.
Expand Down
Expand Up @@ -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',
}
Expand Up @@ -15,6 +15,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive",
"//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/text_attribute_not_binding",
"@npm//typescript",
],
Expand Down
@@ -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",
],
)
@@ -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();
},
};
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts
Expand Up @@ -12,6 +12,7 @@ import {TemplateCheckFactory} from './api';
import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_box';
import {factory as missingControlFlowDirectiveFactory} from './checks/missing_control_flow_directive';
import {factory as nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable';
import {factory as optionalChainNotNullableFactory} from './checks/optional_chain_not_nullable';
import {factory as textAttributeNotBindingFactory} from './checks/text_attribute_not_binding';

export {ExtendedTemplateCheckerImpl} from './src/extended_template_checker';
Expand All @@ -20,6 +21,7 @@ export const ALL_DIAGNOSTIC_FACTORIES:
readonly TemplateCheckFactory<ErrorCode, ExtendedTemplateDiagnosticName>[] = [
invalidBananaInBoxFactory,
nullishCoalescingNotNullableFactory,
optionalChainNotNullableFactory,
missingControlFlowDirectiveFactory,
textAttributeNotBindingFactory,
];
@@ -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",
],
)

0 comments on commit c2c5f36

Please sign in to comment.