From 56d5ff2a8990981c5fd9add3c8ad49885c240d5c Mon Sep 17 00:00:00 2001 From: JoostK Date: Wed, 2 Sep 2020 20:59:57 +0200 Subject: [PATCH] fix(compiler-cli): ensure that a declaration is available in type-to-value conversion (#38684) The type-to-value conversion could previously crash if a symbol was resolved that does not have any declarations, e.g. because it's imported from a missing module. This would typically result in a semantic TypeScript diagnostic and halt further compilation, therefore not reaching the type-to-value conversion logic. In Bazel however, it turns out that Angular semantic diagnostics are requested even if there are semantic TypeScript errors in the program, so it would then reach the type-to-value conversation and crash. This commit fixes the unsafe access and adds a test that ignores the TypeScript semantic error, effectively replicating the situation as experienced under Bazel. Fixes #38670 PR Close #38684 --- .../src/ngtsc/annotations/src/util.ts | 4 ++- .../src/ngtsc/reflection/src/host.ts | 2 +- .../src/ngtsc/reflection/src/type_to_value.ts | 8 +++-- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 29 +++++++++++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 6db327724cac5..4d52eb6b805c3 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -215,8 +215,10 @@ function createUnsuitableInjectionTokenError( makeRelatedInformation( reason.typeNode, 'This type does not have a value, so it cannot be used as injection token.'), - makeRelatedInformation(reason.decl, 'The type is declared here.'), ]; + if (reason.decl !== null) { + hints.push(makeRelatedInformation(reason.decl, 'The type is declared here.')); + } break; case ValueUnavailableKind.TYPE_ONLY_IMPORT: chainMessage = diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index 3d28a12d31940..8ad56ce32c75b 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -336,7 +336,7 @@ export interface UnsupportedType { export interface NoValueDeclaration { kind: ValueUnavailableKind.NO_VALUE_DECLARATION; typeNode: ts.TypeNode; - decl: ts.Declaration; + decl: ts.Declaration|null; } export interface TypeOnlyImport { diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts index eea33c9bfe5c7..bfacb8987e6ae 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts @@ -38,7 +38,11 @@ export function typeToValue( // has a value declaration associated with it. Note that const enums are an exception, // because while they do have a value declaration, they don't exist at runtime. if (decl.valueDeclaration === undefined || decl.flags & ts.SymbolFlags.ConstEnum) { - return noValueDeclaration(typeNode, decl.declarations[0]); + let typeOnlyDecl: ts.Declaration|null = null; + if (decl.declarations !== undefined && decl.declarations.length > 0) { + typeOnlyDecl = decl.declarations[0]; + } + return noValueDeclaration(typeNode, typeOnlyDecl); } // The type points to a valid value declaration. Rewrite the TypeReference into an @@ -140,7 +144,7 @@ function unsupportedType(typeNode: ts.TypeNode): UnavailableTypeValueReference { } function noValueDeclaration( - typeNode: ts.TypeNode, decl: ts.Declaration): UnavailableTypeValueReference { + typeNode: ts.TypeNode, decl: ts.Declaration|null): UnavailableTypeValueReference { return { kind: TypeValueReferenceKind.UNAVAILABLE, reason: {kind: ValueUnavailableKind.NO_VALUE_DECLARATION, typeNode, decl}, diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 74b4fdc6014c6..3fc856389f421 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2214,6 +2214,35 @@ runInEachFileSystem(os => { expect(diags[0].relatedInformation![1].messageText).toBe('The type is declared here.'); }); + it('should report an error when using a missing type as injection token', () => { + // This test replicates the situation where a symbol does not have any declarations at + // all, e.g. because it's imported from a missing module. This would result in a + // semantic TypeScript diagnostic which we ignore in this test to verify that ngtsc's + // analysis is able to operate in this situation. + env.tsconfig({strictInjectionParameters: true}); + env.write(`test.ts`, ` + import {Injectable} from '@angular/core'; + // @ts-expect-error + import {Interface} from 'missing'; + + @Injectable() + export class MyService { + constructor(param: Interface) {} + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(ts.flattenDiagnosticMessageText(diags[0].messageText, '\n')) + .toBe( + `No suitable injection token for parameter 'param' of ` + + `class 'MyService'.\n` + + ` Consider using the @Inject decorator to specify an injection token.`); + expect(diags[0].relatedInformation!.length).toBe(1); + expect(diags[0].relatedInformation![0].messageText) + .toBe('This type does not have a value, so it cannot be used as injection token.'); + }); + it('should report an error when no type is present', () => { env.tsconfig({strictInjectionParameters: true, noImplicitAny: false}); env.write(`test.ts`, `