Skip to content

Commit

Permalink
fix(compiler-cli): suppress unnecessary optional chain for indexed ac…
Browse files Browse the repository at this point in the history
…cesses

TypeScript's type system does not include `undefined` in the type of an indexed
access, whereas array index accesses may be expected to be out-of-bounds.
This commit updates the extended diagnostic for unnecessary optional chains to
ignore indexed accesses.

Closes angular#46918
  • Loading branch information
JoostK committed Aug 22, 2022
1 parent a44b4bf commit 189a6ab
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
Expand Up @@ -27,9 +27,12 @@ class OptionalChainNotNullableCheck extends
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))
if (!(node instanceof SafeCall) && !(node instanceof SafePropertyRead)) {
// Note: `SafeKeyedRead` is also ignored here because TypeScript does not include `undefined`
// in the type of an indexed access, leading to potentially false positives for out-of-bounds
// accesses of arrays.
return [];
}

const symbolLeft = ctx.templateTypeChecker.getSymbolOfNode(node.receiver, component);
if (symbolLeft === null || symbolLeft.kind !== SymbolKind.Expression) {
Expand Down
Expand Up @@ -68,7 +68,10 @@ runInEachFileSystem(() => {
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`bar`);
});

it('should produce optional chain warning for indexed access', () => {
// Using indexed access syntax is not reported as a warning as TypeScript's type system
// does not include `undefined` in the type of an indexed access, whereas array index
// accesses may be expected to be out-of-bounds.
it('should not produce optional chain warning for indexed access', () => {
const fileName = absoluteFrom('/main.ts');
const {program, templateTypeChecker} = setup([{
fileName,
Expand All @@ -83,11 +86,7 @@ runInEachFileSystem(() => {
templateTypeChecker, program.getTypeChecker(), [optionalChainNotNullableFactory],
{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.OPTIONAL_CHAIN_NOT_NULLABLE));
expect(diags[0].messageText).toContain(`the '?.' operator can be safely removed`);
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`var1?.['bar']`);
expect(diags.length).toBe(0);
});

it('should produce optional chain warning for method call', () => {
Expand Down

0 comments on commit 189a6ab

Please sign in to comment.