From 982f99acd7d9ec4f3ff29bceb84a19ae242fa278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Sun, 5 Nov 2023 14:13:18 +0100 Subject: [PATCH 1/6] Avoid dependent parameters narrowings if any declared symbols of the parameter are assigned to --- src/compiler/checker.ts | 30 +++++++---- src/compiler/types.ts | 1 + .../dependentDestructuredVariables.errors.txt | 20 +++++++ .../dependentDestructuredVariables.js | 39 ++++++++++++++ .../dependentDestructuredVariables.symbols | 44 +++++++++++++++ .../dependentDestructuredVariables.types | 54 +++++++++++++++++++ .../dependentDestructuredVariables.ts | 20 +++++++ 7 files changed, 198 insertions(+), 10 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 43cf8f622c68b..8d185816959c8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -28472,32 +28472,42 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (!symbol.valueDeclaration) { return false; } - const parent = getRootDeclaration(symbol.valueDeclaration).parent; + markNodeAssignments(getRootDeclaration(symbol.valueDeclaration)); + return symbol.isAssigned || false; + } + + function isSomeSymbolAssigned(rootDeclaration: Node) { + markNodeAssignments(rootDeclaration); + return getNodeLinks(rootDeclaration).someSymbolAssigned; + } + + function hasParentWithAssignmentsMarked(node: Node) { + return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked)); + } + + function markNodeAssignments(rootDeclaration: Node) { + const parent = rootDeclaration.parent; const links = getNodeLinks(parent); if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) { links.flags |= NodeCheckFlags.AssignmentsMarked; if (!hasParentWithAssignmentsMarked(parent)) { - markNodeAssignments(parent); + markNodeAssignmentsWorker(parent); } } - return symbol.isAssigned || false; - } - - function hasParentWithAssignmentsMarked(node: Node) { - return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked)); } - function markNodeAssignments(node: Node) { + function markNodeAssignmentsWorker(node: Node) { if (node.kind === SyntaxKind.Identifier) { if (isAssignmentTarget(node)) { const symbol = getResolvedSymbol(node as Identifier); if (isParameterOrCatchClauseVariable(symbol)) { + getNodeLinks(getRootDeclaration(symbol.valueDeclaration!)).someSymbolAssigned = true; symbol.isAssigned = true; } } } else { - forEachChild(node, markNodeAssignments); + forEachChild(node, markNodeAssignmentsWorker); } } @@ -28665,7 +28675,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const parentType = getTypeForBindingElementParent(parent, CheckMode.Normal); const parentTypeConstraint = parentType && mapType(parentType, getBaseConstraintOrType); links.flags &= ~NodeCheckFlags.InCheckIdentifier; - if (parentTypeConstraint && parentTypeConstraint.flags & TypeFlags.Union && !(parent.kind === SyntaxKind.Parameter && isSymbolAssigned(symbol))) { + if (parentTypeConstraint && parentTypeConstraint.flags & TypeFlags.Union && !(parent.kind === SyntaxKind.Parameter && isSomeSymbolAssigned(parent))) { const pattern = declaration.parent; const narrowedType = getFlowTypeOfReference(pattern, parentTypeConstraint, parentTypeConstraint, /*flowContainer*/ undefined, location.flowNode); if (narrowedType.flags & TypeFlags.Never) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 927720e580da8..c396ec9b367a4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -6049,6 +6049,7 @@ export interface NodeLinks { parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined". fakeScopeForSignatureDeclaration?: boolean; // True if this is a fake scope injected into an enclosing declaration chain. assertionExpressionType?: Type; // Cached type of the expression of a type assertion + someSymbolAssigned?: boolean; // True if any symbol declared in a parameter or catch clause is being assigned to } /** @internal */ diff --git a/tests/baselines/reference/dependentDestructuredVariables.errors.txt b/tests/baselines/reference/dependentDestructuredVariables.errors.txt index 58fda3e63d75f..7bf012c7e542d 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.errors.txt +++ b/tests/baselines/reference/dependentDestructuredVariables.errors.txt @@ -443,4 +443,24 @@ dependentDestructuredVariables.ts(431,15): error TS2322: Type 'number' is not as !!! error TS2322: Type 'number' is not assignable to type 'never'. } } + + // https://github.com/microsoft/TypeScript/issues/56312 + + function parameterReassigned1([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + x = 1; + } + if (y === 2) { + x; // 1 | 3 + } + } + + function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } + } \ No newline at end of file diff --git a/tests/baselines/reference/dependentDestructuredVariables.js b/tests/baselines/reference/dependentDestructuredVariables.js index ed1a7307e58b9..347413b9485f2 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.js +++ b/tests/baselines/reference/dependentDestructuredVariables.js @@ -434,6 +434,26 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { const shouldNotBeOk: never = x; // Error } } + +// https://github.com/microsoft/TypeScript/issues/56312 + +function parameterReassigned1([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + x = 1; + } + if (y === 2) { + x; // 1 | 3 + } +} + +function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +} //// [dependentDestructuredVariables.js] @@ -766,6 +786,23 @@ function tooNarrow([x, y]) { const shouldNotBeOk = x; // Error } } +// https://github.com/microsoft/TypeScript/issues/56312 +function parameterReassigned1([x, y]) { + if (Math.random()) { + x = 1; + } + if (y === 2) { + x; // 1 | 3 + } +} +function parameterReassigned2([x, y]) { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +} //// [dependentDestructuredVariables.d.ts] @@ -916,3 +953,5 @@ declare class Client { 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; +declare function parameterReassigned1([x, y]: [1, 2] | [3, 4]): void; +declare function parameterReassigned2([x, y]: [1, 2] | [3, 4]): void; diff --git a/tests/baselines/reference/dependentDestructuredVariables.symbols b/tests/baselines/reference/dependentDestructuredVariables.symbols index 8d4298cdb12c9..ce8462c092a2a 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.symbols +++ b/tests/baselines/reference/dependentDestructuredVariables.symbols @@ -1100,3 +1100,47 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { } } +// https://github.com/microsoft/TypeScript/issues/56312 + +function parameterReassigned1([x, y]: [1, 2] | [3, 4]) { +>parameterReassigned1 : Symbol(parameterReassigned1, Decl(dependentDestructuredVariables.ts, 432, 1)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31)) +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 436, 33)) + + if (Math.random()) { +>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) +>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --)) +>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) + + x = 1; +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31)) + } + if (y === 2) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 436, 33)) + + x; // 1 | 3 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31)) + } +} + +function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { +>parameterReassigned2 : Symbol(parameterReassigned2, Decl(dependentDestructuredVariables.ts, 443, 1)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 445, 31)) +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33)) + + if (Math.random()) { +>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) +>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --)) +>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) + + y = 2; +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33)) + } + if (y === 2) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33)) + + x; // 1 | 3 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 445, 31)) + } +} + diff --git a/tests/baselines/reference/dependentDestructuredVariables.types b/tests/baselines/reference/dependentDestructuredVariables.types index 3b89a4048d89c..37296af008bd8 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.types +++ b/tests/baselines/reference/dependentDestructuredVariables.types @@ -1263,3 +1263,57 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { } } +// https://github.com/microsoft/TypeScript/issues/56312 + +function parameterReassigned1([x, y]: [1, 2] | [3, 4]) { +>parameterReassigned1 : ([x, y]: [1, 2] | [3, 4]) => void +>x : 1 | 3 +>y : 2 | 4 + + if (Math.random()) { +>Math.random() : number +>Math.random : () => number +>Math : Math +>random : () => number + + x = 1; +>x = 1 : 1 +>x : 1 | 3 +>1 : 1 + } + if (y === 2) { +>y === 2 : boolean +>y : 2 | 4 +>2 : 2 + + x; // 1 | 3 +>x : 1 | 3 + } +} + +function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { +>parameterReassigned2 : ([x, y]: [1, 2] | [3, 4]) => void +>x : 1 | 3 +>y : 2 | 4 + + if (Math.random()) { +>Math.random() : number +>Math.random : () => number +>Math : Math +>random : () => number + + y = 2; +>y = 2 : 2 +>y : 2 | 4 +>2 : 2 + } + if (y === 2) { +>y === 2 : boolean +>y : 2 | 4 +>2 : 2 + + x; // 1 | 3 +>x : 1 | 3 + } +} + diff --git a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts index 7b18bec96103b..924cdd17c8e33 100644 --- a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts +++ b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts @@ -436,3 +436,23 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) { const shouldNotBeOk: never = x; // Error } } + +// https://github.com/microsoft/TypeScript/issues/56312 + +function parameterReassigned1([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + x = 1; + } + if (y === 2) { + x; // 1 | 3 + } +} + +function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +} From 72b50044b155bac050d862aea3497543c5c8288e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Sun, 5 Nov 2023 14:28:55 +0100 Subject: [PATCH 2/6] Remove `isSymbolAssigned` --- src/compiler/checker.ts | 41 +++++++++++++-------------------------- src/compiler/types.ts | 3 +-- src/compiler/utilities.ts | 1 - 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8d185816959c8..9fb37bdad798a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27203,7 +27203,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { case SyntaxKind.Identifier: if (!isThisInTypeQuery(node)) { const symbol = getResolvedSymbol(node as Identifier); - return isConstantVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSymbolAssigned(symbol); + return isConstantVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSomeSymbolAssigned(getRootDeclaration(symbol.valueDeclaration!)); } break; case SyntaxKind.PropertyAccessExpression: @@ -28467,47 +28467,33 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { node.kind === SyntaxKind.PropertyDeclaration)!; } - // Check if a parameter or catch variable is assigned anywhere - function isSymbolAssigned(symbol: Symbol) { - if (!symbol.valueDeclaration) { - return false; - } - markNodeAssignments(getRootDeclaration(symbol.valueDeclaration)); - return symbol.isAssigned || false; - } - function isSomeSymbolAssigned(rootDeclaration: Node) { - markNodeAssignments(rootDeclaration); - return getNodeLinks(rootDeclaration).someSymbolAssigned; - } - - function hasParentWithAssignmentsMarked(node: Node) { - return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked)); - } - - function markNodeAssignments(rootDeclaration: Node) { const parent = rootDeclaration.parent; const links = getNodeLinks(parent); if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) { links.flags |= NodeCheckFlags.AssignmentsMarked; if (!hasParentWithAssignmentsMarked(parent)) { - markNodeAssignmentsWorker(parent); + markNodeAssignments(parent); } } + return getNodeLinks(rootDeclaration).someSymbolAssigned; + } + + function hasParentWithAssignmentsMarked(node: Node) { + return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked)); } - function markNodeAssignmentsWorker(node: Node) { + function markNodeAssignments(node: Node): true | undefined { if (node.kind === SyntaxKind.Identifier) { if (isAssignmentTarget(node)) { const symbol = getResolvedSymbol(node as Identifier); if (isParameterOrCatchClauseVariable(symbol)) { - getNodeLinks(getRootDeclaration(symbol.valueDeclaration!)).someSymbolAssigned = true; - symbol.isAssigned = true; + return getNodeLinks(getRootDeclaration(symbol.valueDeclaration!)).someSymbolAssigned = true; } } } else { - forEachChild(node, markNodeAssignmentsWorker); + return forEachChild(node, markNodeAssignments); } } @@ -28715,7 +28701,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const contextualSignature = getContextualSignature(func); if (contextualSignature && contextualSignature.parameters.length === 1 && signatureHasRestParameter(contextualSignature)) { const restType = getReducedApparentType(instantiateType(getTypeOfSymbol(contextualSignature.parameters[0]), getInferenceContext(func)?.nonFixingMapper)); - if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !isSymbolAssigned(symbol)) { + if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !isSomeSymbolAssigned(declaration)) { const narrowedType = getFlowTypeOfReference(func, restType, restType, /*flowContainer*/ undefined, location.flowNode); const index = func.parameters.indexOf(declaration) - (getThisParameter(func) ? 1 : 0); return getIndexedAccessType(narrowedType, getNumberLiteralType(index)); @@ -28856,7 +28842,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { // The declaration container is the innermost function that encloses the declaration of the variable // or parameter. The flow container is the innermost function starting with which we analyze the control // flow graph to determine the control flow based type. - const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter; + const rootDeclaration = getRootDeclaration(declaration); + const isParameter = rootDeclaration.kind === SyntaxKind.Parameter; const declarationContainer = getControlFlowContainer(declaration); let flowContainer = getControlFlowContainer(node); const isOuterVariable = flowContainer !== declarationContainer; @@ -28870,7 +28857,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { while ( flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression || flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) && - (isConstantVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSymbolAssigned(localOrExportSymbol)) + (isConstantVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSomeSymbolAssigned(rootDeclaration)) ) { flowContainer = getControlFlowContainer(flowContainer); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index c396ec9b367a4..1612d06f97d34 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -858,7 +858,7 @@ export const enum ModifierFlags { Protected = 1 << 2, // Property/Method Readonly = 1 << 3, // Property/Method Override = 1 << 4, // Override method. - + // Syntactic-only modifiers Export = 1 << 5, // Declarations Abstract = 1 << 6, // Class/Method/ConstructSignature @@ -5811,7 +5811,6 @@ export interface Symbol { /** @internal */ constEnumOnlyModule: boolean | undefined; // True if module contains only const enums or other modules with only const enums /** @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter. /** @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol? - /** @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments /** @internal */ assignmentDeclarationMembers?: Map; // detected late-bound assignment declarations associated with the symbol } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index ac4e6cb95d575..1d63445a496cd 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -8130,7 +8130,6 @@ function Symbol(this: Symbol, flags: SymbolFlags, name: __String) { this.exportSymbol = undefined; this.constEnumOnlyModule = undefined; this.isReferenced = undefined; - this.isAssigned = undefined; (this as any).links = undefined; // used by TransientSymbol } From a02be27daffbed390ce0c0380219b56207f660d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Wed, 6 Dec 2023 09:40:15 +0100 Subject: [PATCH 3/6] add a comment back --- src/compiler/checker.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 036a99e6fdbd1..bbc672e36c1df 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -28668,6 +28668,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { node.kind === SyntaxKind.PropertyDeclaration)!; } + // Check if a parameter or catch variable (or their bindings elements) is assigned anywhere function isSomeSymbolAssigned(rootDeclaration: Node) { const parent = rootDeclaration.parent; const links = getNodeLinks(parent); From fc368df60d5c2b2c5825ceae6ce20e0735227ecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 7 Dec 2023 10:13:26 +0100 Subject: [PATCH 4/6] Fixed the dependent parameters narrowing invalidation with contextual rest --- src/compiler/checker.ts | 4 +-- .../dependentDestructuredVariables.errors.txt | 11 +++++++ .../dependentDestructuredVariables.js | 21 +++++++++++++ .../dependentDestructuredVariables.symbols | 24 +++++++++++++++ .../dependentDestructuredVariables.types | 30 +++++++++++++++++++ .../dependentDestructuredVariables.ts | 11 +++++++ 6 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index bbc672e36c1df..23b4951b7d858 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -28678,7 +28678,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { markNodeAssignments(parent); } } - return getNodeLinks(rootDeclaration).someSymbolAssigned; + return !!getNodeLinks(rootDeclaration).someSymbolAssigned; } function hasParentWithAssignmentsMarked(node: Node) { @@ -28904,7 +28904,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const contextualSignature = getContextualSignature(func); if (contextualSignature && contextualSignature.parameters.length === 1 && signatureHasRestParameter(contextualSignature)) { const restType = getReducedApparentType(instantiateType(getTypeOfSymbol(contextualSignature.parameters[0]), getInferenceContext(func)?.nonFixingMapper)); - if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !isSomeSymbolAssigned(declaration)) { + if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !some(func.parameters, isSomeSymbolAssigned)) { const narrowedType = getFlowTypeOfReference(func, restType, restType, /*flowContainer*/ undefined, location.flowNode); const index = func.parameters.indexOf(declaration) - (getThisParameter(func) ? 1 : 0); return getIndexedAccessType(narrowedType, getNumberLiteralType(index)); diff --git a/tests/baselines/reference/dependentDestructuredVariables.errors.txt b/tests/baselines/reference/dependentDestructuredVariables.errors.txt index 7bf012c7e542d..f6e2dd3d42745 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.errors.txt +++ b/tests/baselines/reference/dependentDestructuredVariables.errors.txt @@ -463,4 +463,15 @@ dependentDestructuredVariables.ts(431,15): error TS2322: Type 'number' is not as x; // 1 | 3 } } + + // https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 + + const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } + } \ No newline at end of file diff --git a/tests/baselines/reference/dependentDestructuredVariables.js b/tests/baselines/reference/dependentDestructuredVariables.js index 347413b9485f2..7d32808909bf7 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.js +++ b/tests/baselines/reference/dependentDestructuredVariables.js @@ -454,6 +454,17 @@ function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { x; // 1 | 3 } } + +// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 + +const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +} //// [dependentDestructuredVariables.js] @@ -803,6 +814,15 @@ function parameterReassigned2([x, y]) { x; // 1 | 3 } } +// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 +const parameterReassignedContextualRest1 = (x, y) => { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +}; //// [dependentDestructuredVariables.d.ts] @@ -955,3 +975,4 @@ declare function fz1([x, y]: [1, 2] | [3, 4] | [5]): void; declare function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]): void; declare function parameterReassigned1([x, y]: [1, 2] | [3, 4]): void; declare function parameterReassigned2([x, y]: [1, 2] | [3, 4]): void; +declare const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void; diff --git a/tests/baselines/reference/dependentDestructuredVariables.symbols b/tests/baselines/reference/dependentDestructuredVariables.symbols index ce8462c092a2a..ae71b255d1a88 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.symbols +++ b/tests/baselines/reference/dependentDestructuredVariables.symbols @@ -1144,3 +1144,27 @@ function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { } } +// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 + +const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => { +>parameterReassignedContextualRest1 : Symbol(parameterReassignedContextualRest1, Decl(dependentDestructuredVariables.ts, 456, 5)) +>args : Symbol(args, Decl(dependentDestructuredVariables.ts, 456, 43)) +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 456, 80)) +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82)) + + if (Math.random()) { +>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) +>Math : Symbol(Math, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.core.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --)) +>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --)) + + y = 2; +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82)) + } + if (y === 2) { +>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82)) + + x; // 1 | 3 +>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 456, 80)) + } +} + diff --git a/tests/baselines/reference/dependentDestructuredVariables.types b/tests/baselines/reference/dependentDestructuredVariables.types index 1801c08d3829b..0480bb1631c62 100644 --- a/tests/baselines/reference/dependentDestructuredVariables.types +++ b/tests/baselines/reference/dependentDestructuredVariables.types @@ -1317,3 +1317,33 @@ function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { } } +// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 + +const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => { +>parameterReassignedContextualRest1 : (...args: [1, 2] | [3, 4]) => void +>args : [1, 2] | [3, 4] +>(x, y) => { if (Math.random()) { y = 2; } if (y === 2) { x; // 1 | 3 }} : (x: 1 | 3, y: 2 | 4) => void +>x : 1 | 3 +>y : 2 | 4 + + if (Math.random()) { +>Math.random() : number +>Math.random : () => number +>Math : Math +>random : () => number + + y = 2; +>y = 2 : 2 +>y : 2 | 4 +>2 : 2 + } + if (y === 2) { +>y === 2 : boolean +>y : 2 | 4 +>2 : 2 + + x; // 1 | 3 +>x : 1 | 3 + } +} + diff --git a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts index 924cdd17c8e33..67bce5559e9cf 100644 --- a/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts +++ b/tests/cases/conformance/controlFlow/dependentDestructuredVariables.ts @@ -456,3 +456,14 @@ function parameterReassigned2([x, y]: [1, 2] | [3, 4]) { x; // 1 | 3 } } + +// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490 + +const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => { + if (Math.random()) { + y = 2; + } + if (y === 2) { + x; // 1 | 3 + } +} From bbda1ed6c775d85f7e98ff8e504a7b34cf4f6247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 7 Dec 2023 23:45:28 +0100 Subject: [PATCH 5/6] Keep narrowing non-reassigned names coming from parameters --- src/compiler/checker.ts | 38 +++++++++++++------ src/compiler/types.ts | 2 +- src/compiler/utilities.ts | 3 +- ...eterBIndingElementNameInInnerScope.symbols | 24 ++++++++++++ ...ameterBIndingElementNameInInnerScope.types | 27 +++++++++++++ ...ParameterBIndingElementNameInInnerScope.ts | 10 +++++ 6 files changed, 91 insertions(+), 13 deletions(-) create mode 100644 tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.symbols create mode 100644 tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.types create mode 100644 tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 23b4951b7d858..29846a33ec976 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -27393,7 +27393,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { case SyntaxKind.Identifier: if (!isThisInTypeQuery(node)) { const symbol = getResolvedSymbol(node as Identifier); - return isConstantVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSomeSymbolAssigned(getRootDeclaration(symbol.valueDeclaration!)); + return isConstantVariable(symbol) || isParameterOrCatchClauseVariable(symbol) && !isSymbolAssigned(symbol); } break; case SyntaxKind.PropertyAccessExpression: @@ -28668,9 +28668,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { node.kind === SyntaxKind.PropertyDeclaration)!; } - // Check if a parameter or catch variable (or their bindings elements) is assigned anywhere - function isSomeSymbolAssigned(rootDeclaration: Node) { - const parent = rootDeclaration.parent; + // Check if a parameter or catch variable is assigned anywhere + function isSymbolAssigned(symbol: Symbol) { + if (!symbol.valueDeclaration) { + return false; + } + const parent = getRootDeclaration(symbol.valueDeclaration).parent; const links = getNodeLinks(parent); if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) { links.flags |= NodeCheckFlags.AssignmentsMarked; @@ -28678,24 +28681,38 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { markNodeAssignments(parent); } } - return !!getNodeLinks(rootDeclaration).someSymbolAssigned; + return symbol.isAssigned || false; + } + + // Check if a parameter or catch variable (or their bindings elements) is assigned anywhere + function isSomeSymbolAssigned(rootDeclaration: Node) { + Debug.assert(isVariableDeclaration(rootDeclaration) || isParameter(rootDeclaration)); + return isSomeSymbolAssignedWorker(rootDeclaration.name); + } + + function isSomeSymbolAssignedWorker(node: BindingName): boolean { + if (node.kind === SyntaxKind.Identifier) { + return isSymbolAssigned(getSymbolOfDeclaration(node.parent as Declaration)); + } + + return some(node.elements, e => e.kind !== SyntaxKind.OmittedExpression && isSomeSymbolAssignedWorker(e.name)); } function hasParentWithAssignmentsMarked(node: Node) { return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked)); } - function markNodeAssignments(node: Node): true | undefined { + function markNodeAssignments(node: Node) { if (node.kind === SyntaxKind.Identifier) { if (isAssignmentTarget(node)) { const symbol = getResolvedSymbol(node as Identifier); if (isParameterOrCatchClauseVariable(symbol)) { - return getNodeLinks(getRootDeclaration(symbol.valueDeclaration!)).someSymbolAssigned = true; + symbol.isAssigned = true; } } } else { - return forEachChild(node, markNodeAssignments); + forEachChild(node, markNodeAssignments); } } @@ -29045,8 +29062,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { // The declaration container is the innermost function that encloses the declaration of the variable // or parameter. The flow container is the innermost function starting with which we analyze the control // flow graph to determine the control flow based type. - const rootDeclaration = getRootDeclaration(declaration); - const isParameter = rootDeclaration.kind === SyntaxKind.Parameter; + const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter; const declarationContainer = getControlFlowContainer(declaration); let flowContainer = getControlFlowContainer(node); const isOuterVariable = flowContainer !== declarationContainer; @@ -29060,7 +29076,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { while ( flowContainer !== declarationContainer && (flowContainer.kind === SyntaxKind.FunctionExpression || flowContainer.kind === SyntaxKind.ArrowFunction || isObjectLiteralOrClassExpressionMethodOrAccessor(flowContainer)) && - (isConstantVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSomeSymbolAssigned(rootDeclaration)) + (isConstantVariable(localOrExportSymbol) && type !== autoArrayType || isParameter && !isSymbolAssigned(localOrExportSymbol)) ) { flowContainer = getControlFlowContainer(flowContainer); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index f29967a03961e..9078270ee340d 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5811,6 +5811,7 @@ export interface Symbol { /** @internal */ constEnumOnlyModule: boolean | undefined; // True if module contains only const enums or other modules with only const enums /** @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter. /** @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol? + /** @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments /** @internal */ assignmentDeclarationMembers?: Map; // detected late-bound assignment declarations associated with the symbol } @@ -6049,7 +6050,6 @@ export interface NodeLinks { parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined". fakeScopeForSignatureDeclaration?: "params" | "typeParams"; // If present, this is a fake scope injected into an enclosing declaration chain. assertionExpressionType?: Type; // Cached type of the expression of a type assertion - someSymbolAssigned?: boolean; // True if any symbol declared in a parameter or catch clause is being assigned to } /** @internal */ diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index da81f37ffddd3..197ba446543bc 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -5278,7 +5278,7 @@ export function isPushOrUnshiftIdentifier(node: Identifier) { * * @internal */ -export function isParameterDeclaration(node: Declaration): boolean { +export function isParameterDeclaration(node: Declaration): node is ParameterDeclaration { const root = getRootDeclaration(node); return root.kind === SyntaxKind.Parameter; } @@ -8155,6 +8155,7 @@ function Symbol(this: Symbol, flags: SymbolFlags, name: __String) { this.exportSymbol = undefined; this.constEnumOnlyModule = undefined; this.isReferenced = undefined; + this.isAssigned = undefined; (this as any).links = undefined; // used by TransientSymbol } diff --git a/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.symbols b/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.symbols new file mode 100644 index 0000000000000..7c22602f09ac9 --- /dev/null +++ b/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.symbols @@ -0,0 +1,24 @@ +//// [tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts] //// + +=== narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts === +function ff({ a, b }: { a: string | undefined, b: () => void }) { +>ff : Symbol(ff, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 0)) +>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13)) +>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 16)) +>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 23)) +>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 46)) + + if (a !== undefined) { +>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13)) +>undefined : Symbol(undefined) + + b = () => { +>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 16)) + + const x: string = a; +>x : Symbol(x, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 3, 11)) +>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13)) + } + } +} + diff --git a/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.types b/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.types new file mode 100644 index 0000000000000..16ddc0c8ad1ab --- /dev/null +++ b/tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.types @@ -0,0 +1,27 @@ +//// [tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts] //// + +=== narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts === +function ff({ a, b }: { a: string | undefined, b: () => void }) { +>ff : ({ a, b }: { a: string | undefined; b: () => void;}) => void +>a : string | undefined +>b : () => void +>a : string | undefined +>b : () => void + + if (a !== undefined) { +>a !== undefined : boolean +>a : string | undefined +>undefined : undefined + + b = () => { +>b = () => { const x: string = a; } : () => void +>b : () => void +>() => { const x: string = a; } : () => void + + const x: string = a; +>x : string +>a : string + } + } +} + diff --git a/tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts b/tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts new file mode 100644 index 0000000000000..3f88e085c31cc --- /dev/null +++ b/tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts @@ -0,0 +1,10 @@ +// @strict: true +// @noEmit: true + +function ff({ a, b }: { a: string | undefined, b: () => void }) { + if (a !== undefined) { + b = () => { + const x: string = a; + } + } +} From f82fbb4e7c3f0980f988a5f0fdef2d94d0632fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Mon, 11 Dec 2023 21:16:27 +0100 Subject: [PATCH 6/6] Revert `isParameterDeclaration` change --- src/compiler/utilities.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 197ba446543bc..738538d24520e 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -5278,7 +5278,7 @@ export function isPushOrUnshiftIdentifier(node: Identifier) { * * @internal */ -export function isParameterDeclaration(node: Declaration): node is ParameterDeclaration { +export function isParameterDeclaration(node: Declaration): boolean { const root = getRootDeclaration(node); return root.kind === SyntaxKind.Parameter; }