Skip to content

Commit af36878

Browse files
AndaristAndarist
authored
Avoid dependent parameters narrowings if any declared symbol of the parameter is assigned to (#56313)
1 parent 973b0e6 commit af36878

10 files changed

Lines changed: 352 additions & 3 deletions

‎src/compiler/checker.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28680,6 +28680,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2868028680
return symbol.isAssigned || false;
2868128681
}
2868228682

28683+
// Check if a parameter or catch variable (or their bindings elements) is assigned anywhere
28684+
function isSomeSymbolAssigned(rootDeclaration: Node) {
28685+
Debug.assert(isVariableDeclaration(rootDeclaration) || isParameter(rootDeclaration));
28686+
return isSomeSymbolAssignedWorker(rootDeclaration.name);
28687+
}
28688+
28689+
function isSomeSymbolAssignedWorker(node: BindingName): boolean {
28690+
if (node.kind === SyntaxKind.Identifier) {
28691+
return isSymbolAssigned(getSymbolOfDeclaration(node.parent as Declaration));
28692+
}
28693+
28694+
return some(node.elements, e => e.kind !== SyntaxKind.OmittedExpression && isSomeSymbolAssignedWorker(e.name));
28695+
}
28696+
2868328697
function hasParentWithAssignmentsMarked(node: Node) {
2868428698
return !!findAncestor(node.parent, node => (isFunctionLike(node) || isCatchClause(node)) && !!(getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked));
2868528699
}
@@ -28863,7 +28877,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2886328877
const parentType = getTypeForBindingElementParent(parent, CheckMode.Normal);
2886428878
const parentTypeConstraint = parentType && mapType(parentType, getBaseConstraintOrType);
2886528879
links.flags &= ~NodeCheckFlags.InCheckIdentifier;
28866-
if (parentTypeConstraint && parentTypeConstraint.flags & TypeFlags.Union && !(rootDeclaration.kind === SyntaxKind.Parameter && isSymbolAssigned(symbol))) {
28880+
if (parentTypeConstraint && parentTypeConstraint.flags & TypeFlags.Union && !(rootDeclaration.kind === SyntaxKind.Parameter && isSomeSymbolAssigned(rootDeclaration))) {
2886728881
const pattern = declaration.parent;
2886828882
const narrowedType = getFlowTypeOfReference(pattern, parentTypeConstraint, parentTypeConstraint, /*flowContainer*/ undefined, location.flowNode);
2886928883
if (narrowedType.flags & TypeFlags.Never) {
@@ -28903,7 +28917,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2890328917
const contextualSignature = getContextualSignature(func);
2890428918
if (contextualSignature && contextualSignature.parameters.length === 1 && signatureHasRestParameter(contextualSignature)) {
2890528919
const restType = getReducedApparentType(instantiateType(getTypeOfSymbol(contextualSignature.parameters[0]), getInferenceContext(func)?.nonFixingMapper));
28906-
if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !isSymbolAssigned(symbol)) {
28920+
if (restType.flags & TypeFlags.Union && everyType(restType, isTupleType) && !some(func.parameters, isSomeSymbolAssigned)) {
2890728921
const narrowedType = getFlowTypeOfReference(func, restType, restType, /*flowContainer*/ undefined, location.flowNode);
2890828922
const index = func.parameters.indexOf(declaration) - (getThisParameter(func) ? 1 : 0);
2890928923
return getIndexedAccessType(narrowedType, getNumberLiteralType(index));

‎src/compiler/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5824,7 +5824,7 @@ export interface Symbol {
58245824
/** @internal */ constEnumOnlyModule: boolean | undefined; // True if module contains only const enums or other modules with only const enums
58255825
/** @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.
58265826
/** @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?
5827-
/** @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments
5827+
/** @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments
58285828
/** @internal */ assignmentDeclarationMembers?: Map<number, Declaration>; // detected late-bound assignment declarations associated with the symbol
58295829
}
58305830

‎tests/baselines/reference/dependentDestructuredVariables.errors.txt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,4 +443,35 @@ dependentDestructuredVariables.ts(431,15): error TS2322: Type 'number' is not as
443443
!!! error TS2322: Type 'number' is not assignable to type 'never'.
444444
}
445445
}
446+
447+
// https://github.com/microsoft/TypeScript/issues/56312
448+
449+
function parameterReassigned1([x, y]: [1, 2] | [3, 4]) {
450+
if (Math.random()) {
451+
x = 1;
452+
}
453+
if (y === 2) {
454+
x; // 1 | 3
455+
}
456+
}
457+
458+
function parameterReassigned2([x, y]: [1, 2] | [3, 4]) {
459+
if (Math.random()) {
460+
y = 2;
461+
}
462+
if (y === 2) {
463+
x; // 1 | 3
464+
}
465+
}
466+
467+
// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490
468+
469+
const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => {
470+
if (Math.random()) {
471+
y = 2;
472+
}
473+
if (y === 2) {
474+
x; // 1 | 3
475+
}
476+
}
446477

‎tests/baselines/reference/dependentDestructuredVariables.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,37 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) {
434434
const shouldNotBeOk: never = x; // Error
435435
}
436436
}
437+
438+
// https://github.com/microsoft/TypeScript/issues/56312
439+
440+
function parameterReassigned1([x, y]: [1, 2] | [3, 4]) {
441+
if (Math.random()) {
442+
x = 1;
443+
}
444+
if (y === 2) {
445+
x; // 1 | 3
446+
}
447+
}
448+
449+
function parameterReassigned2([x, y]: [1, 2] | [3, 4]) {
450+
if (Math.random()) {
451+
y = 2;
452+
}
453+
if (y === 2) {
454+
x; // 1 | 3
455+
}
456+
}
457+
458+
// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490
459+
460+
const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => {
461+
if (Math.random()) {
462+
y = 2;
463+
}
464+
if (y === 2) {
465+
x; // 1 | 3
466+
}
467+
}
437468

438469

439470
//// [dependentDestructuredVariables.js]
@@ -766,6 +797,32 @@ function tooNarrow([x, y]) {
766797
const shouldNotBeOk = x; // Error
767798
}
768799
}
800+
// https://github.com/microsoft/TypeScript/issues/56312
801+
function parameterReassigned1([x, y]) {
802+
if (Math.random()) {
803+
x = 1;
804+
}
805+
if (y === 2) {
806+
x; // 1 | 3
807+
}
808+
}
809+
function parameterReassigned2([x, y]) {
810+
if (Math.random()) {
811+
y = 2;
812+
}
813+
if (y === 2) {
814+
x; // 1 | 3
815+
}
816+
}
817+
// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490
818+
const parameterReassignedContextualRest1 = (x, y) => {
819+
if (Math.random()) {
820+
y = 2;
821+
}
822+
if (y === 2) {
823+
x; // 1 | 3
824+
}
825+
};
769826

770827

771828
//// [dependentDestructuredVariables.d.ts]
@@ -916,3 +973,6 @@ declare class Client {
916973
declare const bot: Client;
917974
declare function fz1([x, y]: [1, 2] | [3, 4] | [5]): void;
918975
declare function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]): void;
976+
declare function parameterReassigned1([x, y]: [1, 2] | [3, 4]): void;
977+
declare function parameterReassigned2([x, y]: [1, 2] | [3, 4]): void;
978+
declare const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void;

‎tests/baselines/reference/dependentDestructuredVariables.symbols‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,3 +1100,71 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) {
11001100
}
11011101
}
11021102

1103+
// https://github.com/microsoft/TypeScript/issues/56312
1104+
1105+
function parameterReassigned1([x, y]: [1, 2] | [3, 4]) {
1106+
>parameterReassigned1 : Symbol(parameterReassigned1, Decl(dependentDestructuredVariables.ts, 432, 1))
1107+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31))
1108+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 436, 33))
1109+
1110+
if (Math.random()) {
1111+
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1112+
>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, --, --))
1113+
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1114+
1115+
x = 1;
1116+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31))
1117+
}
1118+
if (y === 2) {
1119+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 436, 33))
1120+
1121+
x; // 1 | 3
1122+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 436, 31))
1123+
}
1124+
}
1125+
1126+
function parameterReassigned2([x, y]: [1, 2] | [3, 4]) {
1127+
>parameterReassigned2 : Symbol(parameterReassigned2, Decl(dependentDestructuredVariables.ts, 443, 1))
1128+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 445, 31))
1129+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33))
1130+
1131+
if (Math.random()) {
1132+
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1133+
>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, --, --))
1134+
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1135+
1136+
y = 2;
1137+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33))
1138+
}
1139+
if (y === 2) {
1140+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 445, 33))
1141+
1142+
x; // 1 | 3
1143+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 445, 31))
1144+
}
1145+
}
1146+
1147+
// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490
1148+
1149+
const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => {
1150+
>parameterReassignedContextualRest1 : Symbol(parameterReassignedContextualRest1, Decl(dependentDestructuredVariables.ts, 456, 5))
1151+
>args : Symbol(args, Decl(dependentDestructuredVariables.ts, 456, 43))
1152+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 456, 80))
1153+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82))
1154+
1155+
if (Math.random()) {
1156+
>Math.random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1157+
>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, --, --))
1158+
>random : Symbol(Math.random, Decl(lib.es5.d.ts, --, --))
1159+
1160+
y = 2;
1161+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82))
1162+
}
1163+
if (y === 2) {
1164+
>y : Symbol(y, Decl(dependentDestructuredVariables.ts, 456, 82))
1165+
1166+
x; // 1 | 3
1167+
>x : Symbol(x, Decl(dependentDestructuredVariables.ts, 456, 80))
1168+
}
1169+
}
1170+

‎tests/baselines/reference/dependentDestructuredVariables.types‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,3 +1263,87 @@ function tooNarrow([x, y]: [1, 1] | [1, 2] | [1]) {
12631263
}
12641264
}
12651265

1266+
// https://github.com/microsoft/TypeScript/issues/56312
1267+
1268+
function parameterReassigned1([x, y]: [1, 2] | [3, 4]) {
1269+
>parameterReassigned1 : ([x, y]: [1, 2] | [3, 4]) => void
1270+
>x : 1 | 3
1271+
>y : 2 | 4
1272+
1273+
if (Math.random()) {
1274+
>Math.random() : number
1275+
>Math.random : () => number
1276+
>Math : Math
1277+
>random : () => number
1278+
1279+
x = 1;
1280+
>x = 1 : 1
1281+
>x : 1 | 3
1282+
>1 : 1
1283+
}
1284+
if (y === 2) {
1285+
>y === 2 : boolean
1286+
>y : 2 | 4
1287+
>2 : 2
1288+
1289+
x; // 1 | 3
1290+
>x : 1 | 3
1291+
}
1292+
}
1293+
1294+
function parameterReassigned2([x, y]: [1, 2] | [3, 4]) {
1295+
>parameterReassigned2 : ([x, y]: [1, 2] | [3, 4]) => void
1296+
>x : 1 | 3
1297+
>y : 2 | 4
1298+
1299+
if (Math.random()) {
1300+
>Math.random() : number
1301+
>Math.random : () => number
1302+
>Math : Math
1303+
>random : () => number
1304+
1305+
y = 2;
1306+
>y = 2 : 2
1307+
>y : 2 | 4
1308+
>2 : 2
1309+
}
1310+
if (y === 2) {
1311+
>y === 2 : boolean
1312+
>y : 2 | 4
1313+
>2 : 2
1314+
1315+
x; // 1 | 3
1316+
>x : 1 | 3
1317+
}
1318+
}
1319+
1320+
// https://github.com/microsoft/TypeScript/pull/56313#discussion_r1416482490
1321+
1322+
const parameterReassignedContextualRest1: (...args: [1, 2] | [3, 4]) => void = (x, y) => {
1323+
>parameterReassignedContextualRest1 : (...args: [1, 2] | [3, 4]) => void
1324+
>args : [1, 2] | [3, 4]
1325+
>(x, y) => { if (Math.random()) { y = 2; } if (y === 2) { x; // 1 | 3 }} : (x: 1 | 3, y: 2 | 4) => void
1326+
>x : 1 | 3
1327+
>y : 2 | 4
1328+
1329+
if (Math.random()) {
1330+
>Math.random() : number
1331+
>Math.random : () => number
1332+
>Math : Math
1333+
>random : () => number
1334+
1335+
y = 2;
1336+
>y = 2 : 2
1337+
>y : 2 | 4
1338+
>2 : 2
1339+
}
1340+
if (y === 2) {
1341+
>y === 2 : boolean
1342+
>y : 2 | 4
1343+
>2 : 2
1344+
1345+
x; // 1 | 3
1346+
>x : 1 | 3
1347+
}
1348+
}
1349+

‎tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.symbols‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//// [tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts] ////
2+
3+
=== narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts ===
4+
function ff({ a, b }: { a: string | undefined, b: () => void }) {
5+
>ff : Symbol(ff, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 0))
6+
>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13))
7+
>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 16))
8+
>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 23))
9+
>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 46))
10+
11+
if (a !== undefined) {
12+
>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13))
13+
>undefined : Symbol(undefined)
14+
15+
b = () => {
16+
>b : Symbol(b, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 16))
17+
18+
const x: string = a;
19+
>x : Symbol(x, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 3, 11))
20+
>a : Symbol(a, Decl(narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts, 0, 13))
21+
}
22+
}
23+
}
24+

‎tests/baselines/reference/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.types‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//// [tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts] ////
2+
3+
=== narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts ===
4+
function ff({ a, b }: { a: string | undefined, b: () => void }) {
5+
>ff : ({ a, b }: { a: string | undefined; b: () => void;}) => void
6+
>a : string | undefined
7+
>b : () => void
8+
>a : string | undefined
9+
>b : () => void
10+
11+
if (a !== undefined) {
12+
>a !== undefined : boolean
13+
>a : string | undefined
14+
>undefined : undefined
15+
16+
b = () => {
17+
>b = () => { const x: string = a; } : () => void
18+
>b : () => void
19+
>() => { const x: string = a; } : () => void
20+
21+
const x: string = a;
22+
>x : string
23+
>a : string
24+
}
25+
}
26+
}
27+

‎tests/cases/compiler/narrowRefinedConstLikeParameterBIndingElementNameInInnerScope.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// @strict: true
2+
// @noEmit: true
3+
4+
function ff({ a, b }: { a: string | undefined, b: () => void }) {
5+
if (a !== undefined) {
6+
b = () => {
7+
const x: string = a;
8+
}
9+
}
10+
}

0 commit comments

Comments
 (0)