From 2a2744721b429cbce7fd9dabc2963c1c679b67bd Mon Sep 17 00:00:00 2001 From: JoostK Date: Thu, 30 Jul 2020 23:08:27 +0200 Subject: [PATCH] fix(compiler-cli): ensure literal types are retained when `strictNullInputTypes` is disabled (#38305) Consider the `NgModel` directive which has the `ngModelOptions` input: ```ts class NgModel { @Input() ngModelOptions: { updateOn: 'blur'|'change'|'submit' }; } ``` In a template this may be set using an object literal as follows: ```html ``` This assignment should be accepted, as the object's type aligns with the `ngModelOptions` input in `NgModel`. However, if the `strictNullInputTypes` option is disabled this assignment would inadvertently produce an error: ``` Type '{ updateOn: string; }' is not assignable to type '{ updateOn: "blur"|"change"|"submit"; }'. Types of property 'updateOn' are incompatible. Type 'string' is not assignable to type '"blur"|"change"|"submit"' ``` This is due to the `'blur'` value being inferred to be of type `string` instead of retaining its literal type. The non-null assertion operator that is automatically inserted for input binding assignments when `strictNullInputTypes` is disabled inhibits TypeScript from inferring the string value as its literal type. This commit fixes the issue by omitting the insertion of the non-null operator for object literals and array literals. PR Close #38305 --- .../ngtsc/typecheck/src/type_check_block.ts | 58 +++++++++---------- .../ngtsc/typecheck/test/diagnostics_spec.ts | 40 +++++++++++++ .../src/ngtsc/typecheck/testing/index.ts | 7 ++- 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 69cee59c7db..044e3a9f2f6 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -701,16 +701,7 @@ class TcbDirectiveInputsOp extends TcbOp { const inputs = getBoundInputs(this.dir, this.node, this.tcb); for (const input of inputs) { // For bound inputs, the property is assigned the binding expression. - let expr = translateInput(input.attribute, this.tcb, this.scope); - if (!this.tcb.env.config.checkTypeOfInputBindings) { - // If checking the type of bindings is disabled, cast the resulting expression to 'any' - // before the assignment. - expr = tsCastToAny(expr); - } else if (!this.tcb.env.config.strictNullInputBindings) { - // If strict null checks are disabled, erase `null` and `undefined` from the type by - // wrapping the expression in a non-null assertion. - expr = ts.createNonNullExpression(expr); - } + const expr = widenBinding(translateInput(input.attribute, this.tcb, this.scope), this.tcb); let assignment: ts.Expression = wrapForDiagnostics(expr); @@ -922,16 +913,7 @@ class TcbUnclaimedInputsOp extends TcbOp { continue; } - let expr = tcbExpression(binding.value, this.tcb, this.scope); - if (!this.tcb.env.config.checkTypeOfInputBindings) { - // If checking the type of bindings is disabled, cast the resulting expression to 'any' - // before the assignment. - expr = tsCastToAny(expr); - } else if (!this.tcb.env.config.strictNullInputBindings) { - // If strict null checks are disabled, erase `null` and `undefined` from the type by - // wrapping the expression in a non-null assertion. - expr = ts.createNonNullExpression(expr); - } + const expr = widenBinding(tcbExpression(binding.value, this.tcb, this.scope), this.tcb); if (this.tcb.env.config.checkTypeOfDomBindings && binding.type === BindingType.Property) { if (binding.name !== 'style' && binding.name !== 'class') { @@ -1811,16 +1793,7 @@ function tcbCallTypeCtor( if (input.type === 'binding') { // For bound inputs, the property is assigned the binding expression. - let expr = input.expression; - if (!tcb.env.config.checkTypeOfInputBindings) { - // If checking the type of bindings is disabled, cast the resulting expression to 'any' - // before the assignment. - expr = tsCastToAny(expr); - } else if (!tcb.env.config.strictNullInputBindings) { - // If strict null checks are disabled, erase `null` and `undefined` from the type by - // wrapping the expression in a non-null assertion. - expr = ts.createNonNullExpression(expr); - } + const expr = widenBinding(input.expression, tcb); const assignment = ts.createPropertyAssignment(propertyName, wrapForDiagnostics(expr)); addParseSpanInfo(assignment, input.sourceSpan); @@ -1883,6 +1856,31 @@ function translateInput( } } +/** + * Potentially widens the type of `expr` according to the type-checking configuration. + */ +function widenBinding(expr: ts.Expression, tcb: Context): ts.Expression { + if (!tcb.env.config.checkTypeOfInputBindings) { + // If checking the type of bindings is disabled, cast the resulting expression to 'any' + // before the assignment. + return tsCastToAny(expr); + } else if (!tcb.env.config.strictNullInputBindings) { + if (ts.isObjectLiteralExpression(expr) || ts.isArrayLiteralExpression(expr)) { + // Object literals and array literals should not be wrapped in non-null assertions as that + // would cause literals to be prematurely widened, resulting in type errors when assigning + // into a literal type. + return expr; + } else { + // If strict null checks are disabled, erase `null` and `undefined` from the type by + // wrapping the expression in a non-null assertion. + return ts.createNonNullExpression(expr); + } + } else { + // No widening is requested, use the expression as is. + return expr; + } +} + /** * An input binding that corresponds with a field of a directive. */ diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index fc061f4dd99..bc4a55a2ea5 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -277,6 +277,46 @@ runInEachFileSystem(() => { ]); }); + it('should retain literal types in object literals together if strictNullInputBindings is disabled', + () => { + const messages = diagnose( + `
`, ` + class Dir { + ngModelOptions: { updateOn: 'change'|'blur' }; + } + + class TestComponent {}`, + [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'ngModelOptions': 'ngModelOptions'}, + }], + [], {strictNullInputBindings: false}); + + expect(messages).toEqual([]); + }); + + it('should retain literal types in array literals together if strictNullInputBindings is disabled', + () => { + const messages = diagnose( + `
`, ` + class Dir { + options!: Array<'literal'>; + } + + class TestComponent {}`, + [{ + type: 'directive', + name: 'Dir', + selector: '[dir]', + inputs: {'options': 'options'}, + }], + [], {strictNullInputBindings: false}); + + expect(messages).toEqual([]); + }); + it('does not produce diagnostics for user code', () => { const messages = diagnose(`{{ person.name }}`, ` class TestComponent { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 76fa391fdfc..3e57890a933 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -46,6 +46,7 @@ export function typescriptLibDts(): TestFile { [index: number]: T; length: number; } + declare interface Iterable {} declare interface String { length: number; } @@ -63,7 +64,7 @@ export function typescriptLibDts(): TestFile { } declare interface HTMLElement { addEventListener(type: K, listener: (this: HTMLElement, ev: HTMLElementEventMap[K]) => any): void; - addEventListener(type: string, listener: (evt: Event): void;): void; + addEventListener(type: string, listener: (evt: Event) => void): void; } declare interface HTMLDivElement extends HTMLElement {} declare interface HTMLImageElement extends HTMLElement { @@ -94,8 +95,8 @@ export function angularCoreDts(): TestFile { name: absoluteFrom('/node_modules/@angular/core/index.d.ts'), contents: ` export declare class TemplateRef { - abstract readonly elementRef: unknown; - abstract createEmbeddedView(context: C): unknown; + readonly elementRef: unknown; + createEmbeddedView(context: C): unknown; } export declare class EventEmitter {