From 58650d97c5c290944da59f209f6c3ca6cfe5387e Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 14 Sep 2023 13:06:48 -0700 Subject: [PATCH] Fix narrowing of destructured tuples with different arities (#55744) --- src/compiler/checker.ts | 24 +++--- .../dependentDestructuredVariables.errors.txt | 36 ++++++++- .../dependentDestructuredVariables.js | 60 +++++++++++++++ .../dependentDestructuredVariables.symbols | 63 ++++++++++++++++ .../dependentDestructuredVariables.types | 75 +++++++++++++++++++ .../dependentDestructuredVariables.ts | 31 ++++++++ 6 files changed, 275 insertions(+), 14 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 64a27977950..65673177efe 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10513,14 +10513,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { return prop ? getTypeOfSymbol(prop) : undefined; } - function getTypeOfPropertyOrIndexSignature(type: Type, name: __String): Type { - return getTypeOfPropertyOfType(type, name) || getApplicableIndexInfoForName(type, name)?.type || unknownType; - } - /** - * Similar to `getTypeOfPropertyOrIndexSignature`, - * but returns `undefined` if there is no matching property or index signature, - * and adds optionality to index signature types. + * Return the type of the matching property or index signature in the given type, or undefined + * if no matching property or index signature exists. Add optionality to index signature types. */ function getTypeOfPropertyOrIndexSignatureOfType(type: Type, name: __String): Type | undefined { let propType; @@ -10681,10 +10676,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { function getTypeForBindingElement(declaration: BindingElement): Type | undefined { const checkMode = declaration.dotDotDotToken ? CheckMode.RestBindingElement : CheckMode.Normal; const parentType = getTypeForBindingElementParent(declaration.parent.parent, checkMode); - return parentType && getBindingElementTypeFromParentType(declaration, parentType); + return parentType && getBindingElementTypeFromParentType(declaration, parentType, /*noTupleBoundsCheck*/ false); } - function getBindingElementTypeFromParentType(declaration: BindingElement, parentType: Type): Type { + function getBindingElementTypeFromParentType(declaration: BindingElement, parentType: Type, noTupleBoundsCheck: boolean): Type { // If an any type was inferred for parent, infer that for the binding element if (isTypeAny(parentType)) { return parentType; @@ -10740,7 +10735,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } else if (isArrayLikeType(parentType)) { const indexType = getNumberLiteralType(index); - const accessFlags = AccessFlags.ExpressionPosition | (hasDefaultValue(declaration) ? AccessFlags.NoTupleBoundsCheck : 0); + const accessFlags = AccessFlags.ExpressionPosition | (noTupleBoundsCheck || hasDefaultValue(declaration) ? AccessFlags.NoTupleBoundsCheck : 0); const declaredType = getIndexedAccessTypeOrUndefined(parentType, indexType, accessFlags, declaration.name) || errorType; type = getFlowTypeOfDestructuring(declaration, declaredType); } @@ -27587,7 +27582,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { propType = removeNullable && optionalChain ? getOptionalType(propType) : propType; const narrowedPropType = narrowType(propType); return filterType(type, t => { - const discriminantType = getTypeOfPropertyOrIndexSignature(t, propName); + const discriminantType = getTypeOfPropertyOrIndexSignatureOfType(t, propName) || unknownType; return !(discriminantType.flags & TypeFlags.Never) && !(narrowedPropType.flags & TypeFlags.Never) && areTypesComparable(narrowedPropType, discriminantType); }); } @@ -28445,7 +28440,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (narrowedType.flags & TypeFlags.Never) { return neverType; } - return getBindingElementTypeFromParentType(declaration, narrowedType); + // Destructurings are validated against the parent type elsewhere. Here we disable tuple bounds + // checks because the narrowed type may have lower arity than the full parent type. For example, + // for the declaration [x, y]: [1, 2] | [3], we may have narrowed the parent type to just [3]. + return getBindingElementTypeFromParentType(declaration, narrowedType, /*noTupleBoundsCheck*/ true); } } } @@ -35745,7 +35743,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { function assignBindingElementTypes(pattern: BindingPattern, parentType: Type) { for (const element of pattern.elements) { if (!isOmittedExpression(element)) { - const type = getBindingElementTypeFromParentType(element, parentType); + const type = getBindingElementTypeFromParentType(element, parentType, /*noTupleBoundsCheck*/ false); if (element.name.kind === SyntaxKind.Identifier) { getSymbolLinks(getSymbolOfDeclaration(element)).type = type; } diff --git a/tests/baselines/reference/dependentDestructuredVariables.errors.txt b/tests/baselines/reference/dependentDestructuredVariables.errors.txt index 45ec4c2c74d..58fda3e63d7 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.errors.txt +++ b/tests/baselines/reference/dependentDestructuredVariables.errors.txt @@ -1,8 +1,9 @@ dependentDestructuredVariables.ts(334,5): error TS7022: 'value1' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer. dependentDestructuredVariables.ts(334,5): error TS7031: Binding element 'value1' implicitly has an 'any' type. +dependentDestructuredVariables.ts(431,15): error TS2322: Type 'number' is not assignable to type 'never'. -==== dependentDestructuredVariables.ts (2 errors) ==== +==== dependentDestructuredVariables.ts (3 errors) ==== type Action = | { kind: 'A', payload: number } | { kind: 'B', payload: string }; @@ -409,4 +410,37 @@ dependentDestructuredVariables.ts(334,5): error TS7031: Binding element 'value1' const bot = new Client(); bot.on("shardDisconnect", (event, shard) => console.log(`Shard ${shard} disconnected (${event.code},${event.wasClean}): ${event.reason}`)); bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} ${event.reason}`)); + + // Destructuring tuple types with different arities + + function fz1([x, y]: [1, 2] | [3, 4] | [5]) { + if (y === 2) { + x; // 1 + } + if (y === 4) { + x; // 3 + } + if (y === undefined) { + x; // 5 + } + if (x === 1) { + y; // 2 + } + if (x === 3) { + y; // 4 + } + if (x === 5) { + y; // undefined + } + } + + // Repro from #55661 + + function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { + if (y === undefined) { + const shouldNotBeOk: never = x; // Error + ~~~~~~~~~~~~~ +!!! error TS2322: Type 'number' is not assignable to type 'never'. + } + } \ No newline at end of file diff --git a/tests/baselines/reference/dependentDestructuredVariables.js b/tests/baselines/reference/dependentDestructuredVariables.js index 324a25a9369..f8d565cdba6 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.js +++ b/tests/baselines/reference/dependentDestructuredVariables.js @@ -403,6 +403,37 @@ declare class Client { const bot = new Client(); bot.on("shardDisconnect", (event, shard) => console.log(`Shard ${shard} disconnected (${event.code},${event.wasClean}): ${event.reason}`)); bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} ${event.reason}`)); + +// Destructuring tuple types with different arities + +function fz1([x, y]: [1, 2] | [3, 4] | [5]) { + if (y === 2) { + x; // 1 + } + if (y === 4) { + x; // 3 + } + if (y === undefined) { + x; // 5 + } + if (x === 1) { + y; // 2 + } + if (x === 3) { + y; // 4 + } + if (x === 5) { + y; // undefined + } +} + +// Repro from #55661 + +function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { + if (y === undefined) { + const shouldNotBeOk: never = x; // Error + } +} //// [dependentDestructuredVariables.js] @@ -707,6 +738,33 @@ const fa3 = (guard, value) => { const bot = new Client(); bot.on("shardDisconnect", (event, shard) => console.log(`Shard ${shard} disconnected (${event.code},${event.wasClean}): ${event.reason}`)); bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} ${event.reason}`)); +// Destructuring tuple types with different arities +function fz1([x, y]) { + if (y === 2) { + x; // 1 + } + if (y === 4) { + x; // 3 + } + if (y === undefined) { + x; // 5 + } + if (x === 1) { + y; // 2 + } + if (x === 3) { + y; // 4 + } + if (x === 5) { + y; // undefined + } +} +// Repro from #55661 +function tooNarrow([x, y]) { + if (y === undefined) { + const shouldNotBeOk = x; // Error + } +} //// [dependentDestructuredVariables.d.ts] @@ -855,3 +913,5 @@ declare class Client { on(event: K, listener: (...args: ClientEvents[K]) => void): void; } declare const bot: Client; +declare function fz1([x, y]: [1, 2] | [3, 4] | [5]): void; +declare function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]): void; diff --git a/tests/baselines/reference/dependentDestructuredVariables.symbols b/tests/baselines/reference/dependentDestructuredVariables.symbols index 35cc35469db..8d4298cdb12 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.symbols +++ b/tests/baselines/reference/dependentDestructuredVariables.symbols @@ -1037,3 +1037,66 @@ bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} >event : Symbol(event, Decl(dependentDestructuredVariables.ts, 401, 25)) >reason : Symbol(CloseEvent.reason, Decl(lib.dom.d.ts, --, --)) +// Destructuring tuple types with different arities + +function fz1([x, y]: [1, 2] | [3, 4] | [5]) { +>fz1 : Symbol(fz1, Decl(dependentDestructuredVariables.ts, 401, 99)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + + if (y === 2) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + + x; // 1 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + } + if (y === 4) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + + x; // 3 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + } + if (y === undefined) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) +>undefined : Symbol(undefined) + + x; // 5 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + } + if (x === 1) { +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + + y; // 2 +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + } + if (x === 3) { +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + + y; // 4 +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + } + if (x === 5) { +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 405, 14)) + + y; // undefined +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 405, 16)) + } +} + +// Repro from #55661 + +function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { +>tooNarrow : Symbol(tooNarrow, Decl(dependentDestructuredVariables.ts, 424, 1)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 428, 20)) +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 428, 22)) + + if (y === undefined) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 428, 22)) +>undefined : Symbol(undefined) + + const shouldNotBeOk: never = x; // Error +>shouldNotBeOk : Symbol(shouldNotBeOk, Decl(dependentDestructuredVariables.ts, 430, 13)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 428, 20)) + } +} + diff --git a/tests/baselines/reference/dependentDestructuredVariables.types b/tests/baselines/reference/dependentDestructuredVariables.types index c0842243275..3b89a4048d8 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.types +++ b/tests/baselines/reference/dependentDestructuredVariables.types @@ -1188,3 +1188,78 @@ bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} >event : CloseEvent >reason : string +// Destructuring tuple types with different arities + +function fz1([x, y]: [1, 2] | [3, 4] | [5]) { +>fz1 : ([x, y]: [1, 2] | [3, 4] | [5]) => void +>x : 1 | 3 | 5 +>y : 2 | 4 | undefined + + if (y === 2) { +>y === 2 : boolean +>y : 2 | 4 | undefined +>2 : 2 + + x; // 1 +>x : 1 + } + if (y === 4) { +>y === 4 : boolean +>y : 2 | 4 | undefined +>4 : 4 + + x; // 3 +>x : 3 + } + if (y === undefined) { +>y === undefined : boolean +>y : 2 | 4 | undefined +>undefined : undefined + + x; // 5 +>x : 5 + } + if (x === 1) { +>x === 1 : boolean +>x : 1 | 3 | 5 +>1 : 1 + + y; // 2 +>y : 2 + } + if (x === 3) { +>x === 3 : boolean +>x : 1 | 3 | 5 +>3 : 3 + + y; // 4 +>y : 4 + } + if (x === 5) { +>x === 5 : boolean +>x : 1 | 3 | 5 +>5 : 5 + + y; // undefined +>y : undefined + } +} + +// Repro from #55661 + +function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { +>tooNarrow : ([x, y]: [1, 1] | [1, 2] | [1]) => void +>x : 1 +>y : 2 | 1 | undefined + + if (y === undefined) { +>y === undefined : boolean +>y : 2 | 1 | undefined +>undefined : undefined + + const shouldNotBeOk: never = x; // Error +>shouldNotBeOk : never +>x : 1 + } +} + diff --git a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts index e967ffd1d77..7b18bec9610 100644 --- a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts +++ b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts @@ -405,3 +405,34 @@ declare class Client { const bot = new Client(); bot.on("shardDisconnect", (event, shard) => console.log(`Shard ${shard} disconnected (${event.code},${event.wasClean}): ${event.reason}`)); bot.on("shardDisconnect", event => console.log(`${event.code} ${event.wasClean} ${event.reason}`)); + +// Destructuring tuple types with different arities + +function fz1([x, y]: [1, 2] | [3, 4] | [5]) { + if (y === 2) { + x; // 1 + } + if (y === 4) { + x; // 3 + } + if (y === undefined) { + x; // 5 + } + if (x === 1) { + y; // 2 + } + if (x === 3) { + y; // 4 + } + if (x === 5) { + y; // undefined + } +} + +// Repro from #55661 + +function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { + if (y === undefined) { + const shouldNotBeOk: never = x; // Error + } +}