From 7983a20bda3fb6b90bf30c2a74d93fae2a58993f Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Fri, 29 Jul 2022 09:03:01 -0400 Subject: [PATCH 1/7] test: add failing test for #19577 --- .../reference/nonNullFullInference.js | 33 +++++++++++++++++ .../reference/nonNullFullInference.symbols | 33 +++++++++++++++++ .../reference/nonNullFullInference.types | 37 +++++++++++++++++++ tests/cases/compiler/nonNullFullInference.ts | 17 +++++++++ 4 files changed, 120 insertions(+) create mode 100644 tests/baselines/reference/nonNullFullInference.js create mode 100644 tests/baselines/reference/nonNullFullInference.symbols create mode 100644 tests/baselines/reference/nonNullFullInference.types create mode 100644 tests/cases/compiler/nonNullFullInference.ts diff --git a/tests/baselines/reference/nonNullFullInference.js b/tests/baselines/reference/nonNullFullInference.js new file mode 100644 index 0000000000000..506d2cb610253 --- /dev/null +++ b/tests/baselines/reference/nonNullFullInference.js @@ -0,0 +1,33 @@ +//// [nonNullFullInference.ts] +// https://github.com/microsoft/TypeScript/issues/19577 + +function testNonNullInference(numbers: number[]) { + let last; + + for (const n of numbers) { + if (n % 2) { + return n; + } + + last = n; + } + + last; // number | undefined + last!; // number +} + + +//// [nonNullFullInference.js] +// https://github.com/microsoft/TypeScript/issues/19577 +function testNonNullInference(numbers) { + var last; + for (var _i = 0, numbers_1 = numbers; _i < numbers_1.length; _i++) { + var n = numbers_1[_i]; + if (n % 2) { + return n; + } + last = n; + } + last; // number | undefined + last; // number +} diff --git a/tests/baselines/reference/nonNullFullInference.symbols b/tests/baselines/reference/nonNullFullInference.symbols new file mode 100644 index 0000000000000..a18ed53aa0707 --- /dev/null +++ b/tests/baselines/reference/nonNullFullInference.symbols @@ -0,0 +1,33 @@ +=== tests/cases/compiler/nonNullFullInference.ts === +// https://github.com/microsoft/TypeScript/issues/19577 + +function testNonNullInference(numbers: number[]) { +>testNonNullInference : Symbol(testNonNullInference, Decl(nonNullFullInference.ts, 0, 0)) +>numbers : Symbol(numbers, Decl(nonNullFullInference.ts, 2, 30)) + + let last; +>last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) + + for (const n of numbers) { +>n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) +>numbers : Symbol(numbers, Decl(nonNullFullInference.ts, 2, 30)) + + if (n % 2) { +>n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) + + return n; +>n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) + } + + last = n; +>last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) +>n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) + } + + last; // number | undefined +>last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) + + last!; // number +>last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) +} + diff --git a/tests/baselines/reference/nonNullFullInference.types b/tests/baselines/reference/nonNullFullInference.types new file mode 100644 index 0000000000000..188f8b707d713 --- /dev/null +++ b/tests/baselines/reference/nonNullFullInference.types @@ -0,0 +1,37 @@ +=== tests/cases/compiler/nonNullFullInference.ts === +// https://github.com/microsoft/TypeScript/issues/19577 + +function testNonNullInference(numbers: number[]) { +>testNonNullInference : (numbers: number[]) => number +>numbers : number[] + + let last; +>last : any + + for (const n of numbers) { +>n : number +>numbers : number[] + + if (n % 2) { +>n % 2 : number +>n : number +>2 : 2 + + return n; +>n : number + } + + last = n; +>last = n : number +>last : any +>n : number + } + + last; // number | undefined +>last : number | undefined + + last!; // number +>last! : number +>last : number | undefined +} + diff --git a/tests/cases/compiler/nonNullFullInference.ts b/tests/cases/compiler/nonNullFullInference.ts new file mode 100644 index 0000000000000..2979c9110036b --- /dev/null +++ b/tests/cases/compiler/nonNullFullInference.ts @@ -0,0 +1,17 @@ +// @noImplicitAny: true +// https://github.com/microsoft/TypeScript/issues/19577 + +function testNonNullInference(numbers: number[]) { + let last; + + for (const n of numbers) { + if (n % 2) { + return n; + } + + last = n; + } + + last; // number | undefined + last!; // number +} From ccc89f2418b4f10d4a002b4ccdc4e3366927aeeb Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Fri, 29 Jul 2022 13:28:19 -0400 Subject: [PATCH 2/7] fix: create an ugly fix this fix is just easier for me to understand i'll refactor the fix to fit into the repo style in a later commit --- src/compiler/checker.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8556832569b76..2f580770e0891 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25986,10 +25986,23 @@ namespace ts { node.parent.kind === SyntaxKind.NonNullExpression || declaration.kind === SyntaxKind.VariableDeclaration && (declaration as VariableDeclaration).exclamationToken || declaration.flags & NodeFlags.Ambient; - const initialType = assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : - type === autoType || type === autoArrayType ? undefinedType : - getOptionalType(type); - const flowType = getFlowTypeOfReference(node, type, initialType, flowContainer); + + // TODO: refactor this + // when non null and an automatic type are involved, we should get the flow type and then remove optionality + const createFlowType = (initialType: Type) => getFlowTypeOfReference(node, type, initialType, flowContainer) + let flowType: Type | undefined; + if (node.parent.kind === SyntaxKind.NonNullExpression && (type === autoType || type === autoArrayType)) { + const initialType = undefinedType; + flowType = createFlowType(initialType); + flowType = getNonNullableType(flowType); + } + else { + const initialType = assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : + type === autoType || type === autoArrayType ? undefinedType : + getOptionalType(type); + flowType = createFlowType(initialType); + } + // A variable is considered uninitialized when it is possible to analyze the entire control flow graph // from declaration to use, and when the variable's declared type doesn't include undefined but the // control flow based type does include undefined. From 893d818ca76e8254a208a7f5183e07dd75e80777 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Fri, 29 Jul 2022 13:28:38 -0400 Subject: [PATCH 3/7] test: update regression test --- .../reference/nonNullFullInference.js | 33 +++++++++---------- .../reference/nonNullFullInference.symbols | 3 +- .../reference/nonNullFullInference.types | 7 ++-- tests/cases/compiler/nonNullFullInference.ts | 4 +-- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/tests/baselines/reference/nonNullFullInference.js b/tests/baselines/reference/nonNullFullInference.js index 506d2cb610253..8923cfd639f94 100644 --- a/tests/baselines/reference/nonNullFullInference.js +++ b/tests/baselines/reference/nonNullFullInference.js @@ -1,22 +1,21 @@ //// [nonNullFullInference.ts] -// https://github.com/microsoft/TypeScript/issues/19577 - -function testNonNullInference(numbers: number[]) { - let last; - - for (const n of numbers) { - if (n % 2) { - return n; - } - - last = n; - } - - last; // number | undefined - last!; // number +// https://github.com/microsoft/TypeScript/issues/19577 + +function testNonNullInference(numbers: number[]) { + let last; + + for (const n of numbers) { + if (n % 2) { + return n; + } + + last = n; + } + + last; // number + last!; // number } - //// [nonNullFullInference.js] // https://github.com/microsoft/TypeScript/issues/19577 function testNonNullInference(numbers) { @@ -28,6 +27,6 @@ function testNonNullInference(numbers) { } last = n; } - last; // number | undefined + last; // number last; // number } diff --git a/tests/baselines/reference/nonNullFullInference.symbols b/tests/baselines/reference/nonNullFullInference.symbols index a18ed53aa0707..11d33180a3264 100644 --- a/tests/baselines/reference/nonNullFullInference.symbols +++ b/tests/baselines/reference/nonNullFullInference.symbols @@ -24,10 +24,9 @@ function testNonNullInference(numbers: number[]) { >n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) } - last; // number | undefined + last; // number >last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) last!; // number >last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) } - diff --git a/tests/baselines/reference/nonNullFullInference.types b/tests/baselines/reference/nonNullFullInference.types index 188f8b707d713..8355009fb8ed9 100644 --- a/tests/baselines/reference/nonNullFullInference.types +++ b/tests/baselines/reference/nonNullFullInference.types @@ -27,11 +27,10 @@ function testNonNullInference(numbers: number[]) { >n : number } - last; // number | undefined ->last : number | undefined + last; // number +>last : number last!; // number >last! : number ->last : number | undefined +>last : number } - diff --git a/tests/cases/compiler/nonNullFullInference.ts b/tests/cases/compiler/nonNullFullInference.ts index 2979c9110036b..1ac3b7cadd9ae 100644 --- a/tests/cases/compiler/nonNullFullInference.ts +++ b/tests/cases/compiler/nonNullFullInference.ts @@ -12,6 +12,6 @@ function testNonNullInference(numbers: number[]) { last = n; } - last; // number | undefined + last; // number last!; // number -} +} \ No newline at end of file From 77f5f8121d7abc9e3c0e28bd2c48bda7495743f1 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Fri, 29 Jul 2022 14:42:34 -0400 Subject: [PATCH 4/7] chore: add semicolon to pass linting --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 2f580770e0891..54f96c09ca5dc 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25989,7 +25989,7 @@ namespace ts { // TODO: refactor this // when non null and an automatic type are involved, we should get the flow type and then remove optionality - const createFlowType = (initialType: Type) => getFlowTypeOfReference(node, type, initialType, flowContainer) + const createFlowType = (initialType: Type) => getFlowTypeOfReference(node, type, initialType, flowContainer); let flowType: Type | undefined; if (node.parent.kind === SyntaxKind.NonNullExpression && (type === autoType || type === autoArrayType)) { const initialType = undefinedType; From a5189b1913d25495aea28df57e4bf5adb216635b Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Fri, 29 Jul 2022 16:08:12 -0400 Subject: [PATCH 5/7] refactor: refactor the fix --- src/compiler/checker.ts | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 54f96c09ca5dc..ad921142268cb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -25969,6 +25969,8 @@ namespace ts { const isOuterVariable = flowContainer !== declarationContainer; const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent); const isModuleExports = symbol.flags & SymbolFlags.ModuleExports; + const typeIsAutomatic = type === autoType || type === autoArrayType; + const isAutomaticTypeInNonNull = typeIsAutomatic && node.parent.kind === SyntaxKind.NonNullExpression; // When the control flow originates in a function expression or arrow function and we are referencing // a const variable or parameter from an outer function, we extend the origin of the control flow // analysis to include the immediately enclosing function. @@ -25986,27 +25988,15 @@ namespace ts { node.parent.kind === SyntaxKind.NonNullExpression || declaration.kind === SyntaxKind.VariableDeclaration && (declaration as VariableDeclaration).exclamationToken || declaration.flags & NodeFlags.Ambient; - - // TODO: refactor this - // when non null and an automatic type are involved, we should get the flow type and then remove optionality - const createFlowType = (initialType: Type) => getFlowTypeOfReference(node, type, initialType, flowContainer); - let flowType: Type | undefined; - if (node.parent.kind === SyntaxKind.NonNullExpression && (type === autoType || type === autoArrayType)) { - const initialType = undefinedType; - flowType = createFlowType(initialType); - flowType = getNonNullableType(flowType); - } - else { - const initialType = assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : - type === autoType || type === autoArrayType ? undefinedType : - getOptionalType(type); - flowType = createFlowType(initialType); - } - + const initialType = isAutomaticTypeInNonNull ? undefinedType : + assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : + typeIsAutomatic ? undefinedType : getOptionalType(type); + const flowType = isAutomaticTypeInNonNull ? getNonNullableType(getFlowTypeOfReference(node, type, initialType, flowContainer)) : + getFlowTypeOfReference(node, type, initialType, flowContainer); // A variable is considered uninitialized when it is possible to analyze the entire control flow graph // from declaration to use, and when the variable's declared type doesn't include undefined but the // control flow based type does include undefined. - if (!isEvolvingArrayOperationTarget(node) && (type === autoType || type === autoArrayType)) { + if (!isEvolvingArrayOperationTarget(node) && typeIsAutomatic) { if (flowType === autoType || flowType === autoArrayType) { if (noImplicitAny) { error(getNameOfDeclaration(declaration), Diagnostics.Variable_0_implicitly_has_type_1_in_some_locations_where_its_type_cannot_be_determined, symbolToString(symbol), typeToString(flowType)); From 4a50f11ca50f34c4ebdcd83c10a5ecfa9fcec932 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Sun, 31 Jul 2022 09:43:29 -0400 Subject: [PATCH 6/7] test: add a test for array non null inference thanks to @Andarist for providing this test! --- .../reference/nonNullFullInference.js | 39 +++++++++++++-- .../reference/nonNullFullInference.symbols | 43 +++++++++++++++- .../reference/nonNullFullInference.types | 50 ++++++++++++++++++- tests/cases/compiler/nonNullFullInference.ts | 21 +++++++- 4 files changed, 143 insertions(+), 10 deletions(-) diff --git a/tests/baselines/reference/nonNullFullInference.js b/tests/baselines/reference/nonNullFullInference.js index 8923cfd639f94..0acc9c7f35f9d 100644 --- a/tests/baselines/reference/nonNullFullInference.js +++ b/tests/baselines/reference/nonNullFullInference.js @@ -12,8 +12,25 @@ function testNonNullInference(numbers: number[]) { last = n; } - last; // number - last!; // number + last; + last!; +} + +function testNonNullInferenceWithArrays(numbers: number[]) { + let result; + const arr = []; + + for (const n of numbers) { + if (n % 2) { + return [n]; + } + + arr.push(n); + result = arr; + } + + result; + result!; } //// [nonNullFullInference.js] @@ -27,6 +44,20 @@ function testNonNullInference(numbers) { } last = n; } - last; // number - last; // number + last; + last; +} +function testNonNullInferenceWithArrays(numbers) { + var result; + var arr = []; + for (var _i = 0, numbers_2 = numbers; _i < numbers_2.length; _i++) { + var n = numbers_2[_i]; + if (n % 2) { + return [n]; + } + arr.push(n); + result = arr; + } + result; + result; } diff --git a/tests/baselines/reference/nonNullFullInference.symbols b/tests/baselines/reference/nonNullFullInference.symbols index 11d33180a3264..a377529bad2fc 100644 --- a/tests/baselines/reference/nonNullFullInference.symbols +++ b/tests/baselines/reference/nonNullFullInference.symbols @@ -24,9 +24,48 @@ function testNonNullInference(numbers: number[]) { >n : Symbol(n, Decl(nonNullFullInference.ts, 5, 14)) } - last; // number + last; >last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) - last!; // number + last!; >last : Symbol(last, Decl(nonNullFullInference.ts, 3, 7)) } + +function testNonNullInferenceWithArrays(numbers: number[]) { +>testNonNullInferenceWithArrays : Symbol(testNonNullInferenceWithArrays, Decl(nonNullFullInference.ts, 15, 1)) +>numbers : Symbol(numbers, Decl(nonNullFullInference.ts, 17, 40)) + + let result; +>result : Symbol(result, Decl(nonNullFullInference.ts, 18, 7)) + + const arr = []; +>arr : Symbol(arr, Decl(nonNullFullInference.ts, 19, 9)) + + for (const n of numbers) { +>n : Symbol(n, Decl(nonNullFullInference.ts, 21, 14)) +>numbers : Symbol(numbers, Decl(nonNullFullInference.ts, 17, 40)) + + if (n % 2) { +>n : Symbol(n, Decl(nonNullFullInference.ts, 21, 14)) + + return [n]; +>n : Symbol(n, Decl(nonNullFullInference.ts, 21, 14)) + } + + arr.push(n); +>arr.push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --)) +>arr : Symbol(arr, Decl(nonNullFullInference.ts, 19, 9)) +>push : Symbol(Array.push, Decl(lib.es5.d.ts, --, --)) +>n : Symbol(n, Decl(nonNullFullInference.ts, 21, 14)) + + result = arr; +>result : Symbol(result, Decl(nonNullFullInference.ts, 18, 7)) +>arr : Symbol(arr, Decl(nonNullFullInference.ts, 19, 9)) + } + + result; +>result : Symbol(result, Decl(nonNullFullInference.ts, 18, 7)) + + result!; +>result : Symbol(result, Decl(nonNullFullInference.ts, 18, 7)) +} diff --git a/tests/baselines/reference/nonNullFullInference.types b/tests/baselines/reference/nonNullFullInference.types index 8355009fb8ed9..b12dd82259a24 100644 --- a/tests/baselines/reference/nonNullFullInference.types +++ b/tests/baselines/reference/nonNullFullInference.types @@ -27,10 +27,56 @@ function testNonNullInference(numbers: number[]) { >n : number } - last; // number + last; >last : number - last!; // number + last!; >last! : number >last : number } + +function testNonNullInferenceWithArrays(numbers: number[]) { +>testNonNullInferenceWithArrays : (numbers: number[]) => number[] +>numbers : number[] + + let result; +>result : any + + const arr = []; +>arr : any[] +>[] : undefined[] + + for (const n of numbers) { +>n : number +>numbers : number[] + + if (n % 2) { +>n % 2 : number +>n : number +>2 : 2 + + return [n]; +>[n] : number[] +>n : number + } + + arr.push(n); +>arr.push(n) : number +>arr.push : (...items: any[]) => number +>arr : any[] +>push : (...items: any[]) => number +>n : number + + result = arr; +>result = arr : number[] +>result : any +>arr : number[] + } + + result; +>result : number[] + + result!; +>result! : number[] +>result : number[] +} diff --git a/tests/cases/compiler/nonNullFullInference.ts b/tests/cases/compiler/nonNullFullInference.ts index 1ac3b7cadd9ae..1efda6f1374d7 100644 --- a/tests/cases/compiler/nonNullFullInference.ts +++ b/tests/cases/compiler/nonNullFullInference.ts @@ -12,6 +12,23 @@ function testNonNullInference(numbers: number[]) { last = n; } - last; // number - last!; // number + last; + last!; +} + +function testNonNullInferenceWithArrays(numbers: number[]) { + let result; + const arr = []; + + for (const n of numbers) { + if (n % 2) { + return [n]; + } + + arr.push(n); + result = arr; + } + + result; + result!; } \ No newline at end of file From d33b656f799b59355523c5672ea9eda2ac8edb37 Mon Sep 17 00:00:00 2001 From: Nicolas Rodriguez Date: Mon, 10 Apr 2023 15:42:31 +0000 Subject: [PATCH 7/7] rebuilt changes in checker --- src/compiler/checker.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 92a989a2bc707..89946fa40660a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27275,6 +27275,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const isOuterVariable = flowContainer !== declarationContainer; const isSpreadDestructuringAssignmentTarget = node.parent && node.parent.parent && isSpreadAssignment(node.parent) && isDestructuringAssignmentTarget(node.parent.parent); const isModuleExports = symbol.flags & SymbolFlags.ModuleExports; + const typeIsAutomatic = type === autoType || type === autoArrayType; + const isAutomaticTypeInNonNull = typeIsAutomatic && node.parent.kind === SyntaxKind.NonNullExpression; // When the control flow originates in a function expression or arrow function and we are referencing // a const variable or parameter from an outer function, we extend the origin of the control flow // analysis to include the immediately enclosing function. @@ -27292,10 +27294,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { node.parent.kind === SyntaxKind.NonNullExpression || declaration.kind === SyntaxKind.VariableDeclaration && (declaration as VariableDeclaration).exclamationToken || declaration.flags & NodeFlags.Ambient; - const initialType = assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : - type === autoType || type === autoArrayType ? undefinedType : - getOptionalType(type); - const flowType = getFlowTypeOfReference(node, type, initialType, flowContainer); + const initialType = isAutomaticTypeInNonNull ? undefinedType : + assumeInitialized ? (isParameter ? removeOptionalityFromDeclaredType(type, declaration as VariableLikeDeclaration) : type) : + typeIsAutomatic ? undefinedType : getOptionalType(type); + const flowType = isAutomaticTypeInNonNull ? getNonNullableType(getFlowTypeOfReference(node, type, initialType, flowContainer)) : + getFlowTypeOfReference(node, type, initialType, flowContainer); // A variable is considered uninitialized when it is possible to analyze the entire control flow graph // from declaration to use, and when the variable's declared type doesn't include undefined but the // control flow based type does include undefined.