Skip to content

Commit dd26507

Browse files
Fix excess property checking when accessing last tuple element in destructuring
Co-authored-by: RyanCavanaugh <[email protected]>
1 parent 1f1fe0c commit dd26507

7 files changed

Lines changed: 531 additions & 38 deletions

src/compiler/checker.ts

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31857,10 +31857,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3185731857
const parentType = getContextualTypeForVariableLikeDeclaration(parent, contextFlags) ||
3185831858
parent.kind !== SyntaxKind.BindingElement && parent.initializer && checkDeclarationInitializer(parent, declaration.dotDotDotToken ? CheckMode.RestBindingElement : CheckMode.Normal);
3185931859
if (!parentType || isBindingPattern(name) || isComputedNonLiteralName(name)) return undefined;
31860-
if (parent.name.kind === SyntaxKind.ArrayBindingPattern) {
31861-
const index = indexOfNode(declaration.parent.elements, declaration);
31862-
if (index < 0) return undefined;
31863-
return getContextualTypeForElementExpression(parentType, index, declaration.parent.elements.length);
31860+
if (parent.name.kind === SyntaxKind.ArrayBindingPattern) {
31861+
const index = indexOfNode(declaration.parent.elements, declaration);
31862+
if (index < 0) return undefined;
31863+
return getContextualTypeForElementExpression(parentType, index, declaration.parent.elements.length);
3186431864
}
3186531865
const nameType = getLiteralTypeFromPropertyName(name);
3186631866
if (isTypeUsableAsPropertyName(nameType)) {
@@ -32391,25 +32391,25 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3239132391
return { first, last };
3239232392
}
3239332393

32394-
function getContextualTypeForElementExpression(type: Type | undefined, index: number, length?: number, firstSpreadIndex?: number, lastSpreadIndex?: number): Type | undefined {
32395-
return type && mapType(type, t => {
32396-
if (isTupleType(t)) {
32397-
// If index is before any spread element and within the fixed part of the contextual tuple type, return
32398-
// the type of the contextual tuple element.
32399-
if ((firstSpreadIndex === undefined || index < firstSpreadIndex) && index < t.target.fixedLength) {
32400-
return removeMissingType(getTypeArguments(t)[index], !!(t.target.elementFlags[index] && ElementFlags.Optional));
32401-
}
32402-
// When the length is known and the index is after all spread elements we compute the offset from the element
32403-
// to the end and the number of ending fixed elements in the contextual tuple type.
32404-
const offset = length !== undefined && (lastSpreadIndex === undefined || index > lastSpreadIndex) ? length - index : 0;
32405-
const fixedEndLength = offset > 0 && (t.target.combinedFlags & ElementFlags.Variable) ? getEndElementCount(t.target, ElementFlags.Fixed) : 0;
32406-
// If the offset is within the ending fixed part of the contextual tuple type, return the type of the contextual
32407-
// tuple element.
32408-
if (offset > 0 && offset <= fixedEndLength) {
32409-
return getTypeArguments(t)[getTypeReferenceArity(t) - offset];
32410-
}
32411-
// Return a union of the possible contextual element types with no subtype reduction.
32412-
return getElementTypeOfSliceOfTupleType(t, firstSpreadIndex === undefined ? t.target.fixedLength : Math.min(t.target.fixedLength, firstSpreadIndex), length === undefined || lastSpreadIndex === undefined ? fixedEndLength : Math.min(fixedEndLength, length - lastSpreadIndex), /*writing*/ false, /*noReductions*/ true);
32394+
function getContextualTypeForElementExpression(type: Type | undefined, index: number, length?: number, firstSpreadIndex?: number, lastSpreadIndex?: number): Type | undefined {
32395+
return type && mapType(type, t => {
32396+
if (isTupleType(t)) {
32397+
// If index is before any spread element and within the fixed part of the contextual tuple type, return
32398+
// the type of the contextual tuple element.
32399+
if ((firstSpreadIndex === undefined || index < firstSpreadIndex) && index < t.target.fixedLength) {
32400+
return removeMissingType(getTypeArguments(t)[index], !!(t.target.elementFlags[index] && ElementFlags.Optional));
32401+
}
32402+
// When the length is known and the index is after all spread elements we compute the offset from the element
32403+
// to the end and the number of ending fixed elements in the contextual tuple type.
32404+
const offset = length !== undefined && (lastSpreadIndex === undefined || index > lastSpreadIndex) ? length - index : 0;
32405+
const fixedEndLength = offset > 0 && (t.target.combinedFlags & ElementFlags.Variable) ? getEndElementCount(t.target, ElementFlags.Fixed) : 0;
32406+
// If the offset is within the ending fixed part of the contextual tuple type, return the type of the contextual
32407+
// tuple element.
32408+
if (offset > 0 && offset <= fixedEndLength) {
32409+
return getTypeArguments(t)[getTypeReferenceArity(t) - offset];
32410+
}
32411+
// Return a union of the possible contextual element types with no subtype reduction.
32412+
return getElementTypeOfSliceOfTupleType(t, firstSpreadIndex === undefined ? t.target.fixedLength : Math.min(t.target.fixedLength, firstSpreadIndex), length === undefined || lastSpreadIndex === undefined ? fixedEndLength : Math.min(fixedEndLength, length - lastSpreadIndex), /*writing*/ false, /*noReductions*/ true);
3241332413
}
3241432414
// If element index is known and a contextual property with that name exists, return it. Otherwise return the
3241532415
// iterated or element type of the contextual type.
@@ -36027,6 +36027,23 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3602736027
return skipOuterExpressions(argument, flags);
3602836028
}
3602936029

36030+
function isCallInLastTupleElementDestructuring(node: CallLikeExpression): boolean {
36031+
// Check if this call expression is used as initializer in a variable declaration with array destructuring
36032+
const parent = node.parent;
36033+
if (parent && isVariableDeclaration(parent) && parent.initializer === node && isArrayBindingPattern(parent.name)) {
36034+
const elements = parent.name.elements;
36035+
// Check if the destructuring pattern accesses the last element
36036+
// (i.e., the last non-omitted element is at the end of the pattern)
36037+
for (let i = elements.length - 1; i >= 0; i--) {
36038+
if (!isOmittedExpression(elements[i])) {
36039+
// If the last non-omitted element is at the last position, it's accessing the last tuple element
36040+
return i === elements.length - 1;
36041+
}
36042+
}
36043+
}
36044+
return false;
36045+
}
36046+
3603036047
function getSignatureApplicabilityError(
3603136048
node: CallLikeExpression,
3603236049
args: readonly Expression[],
@@ -36069,7 +36086,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3606936086
// If one or more arguments are still excluded (as indicated by CheckMode.SkipContextSensitive),
3607036087
// we obtain the regular type of any object literal arguments because we may not have inferred complete
3607136088
// parameter types yet and therefore excess property checks may yield false positives (see #17041).
36072-
const checkArgType = checkMode & CheckMode.SkipContextSensitive ? getRegularTypeOfObjectLiteral(argType) : argType;
36089+
// Also skip fresh literal checking when the call is in last tuple element destructuring context
36090+
// to prevent incorrect excess property errors (see #41548).
36091+
const shouldSkipFreshness = (checkMode & CheckMode.SkipContextSensitive) ||
36092+
(isCallExpression(node) && isCallInLastTupleElementDestructuring(node));
36093+
const checkArgType = shouldSkipFreshness ? getRegularTypeOfObjectLiteral(argType) : argType;
3607336094
const effectiveCheckArgumentNode = getEffectiveCheckNode(arg);
3607436095
if (!checkTypeRelatedToAndOptionallyElaborate(checkArgType, paramType, relation, reportErrors ? effectiveCheckArgumentNode : undefined, effectiveCheckArgumentNode, headMessage, containingMessageChain, errorOutputContainer)) {
3607536096
Debug.assert(!reportErrors || !!errorOutputContainer.errors, "parameter should have errors when reporting errors");
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
lastTupleElementDestructuring.ts(18,27): error TS2322: Type '"c"' is not assignable to type '"a" | "b"'.
2+
lastTupleElementDestructuring.ts(19,24): error TS2345: Argument of type '{ notDataType: string; }' is not assignable to parameter of type '{ dataType: "a" | "b"; }'.
3+
Property 'dataType' is missing in type '{ notDataType: string; }' but required in type '{ dataType: "a" | "b"; }'.
4+
lastTupleElementDestructuring.ts(33,34): error TS2345: Argument of type '{ optional: number; }' is not assignable to parameter of type 'Config'.
5+
Property 'required' is missing in type '{ optional: number; }' but required in type 'Config'.
6+
7+
8+
==== lastTupleElementDestructuring.ts (3 errors) ====
9+
// Test for fixing excess property checking when accessing last tuple element in destructuring
10+
// Fixes https://github.com/microsoft/TypeScript/issues/41548
11+
12+
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
13+
declare function bar<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any, any];
14+
15+
// Cases that should NOT error after fix (accessing last element)
16+
const [, , last1] = foo({ dataType: 'a', day: 0 });
17+
const [,,last2] = foo({ dataType: 'a', day: 0 });
18+
const [,,,last3] = bar({ dataType: 'a', day: 0 });
19+
20+
// Cases that already worked (not accessing last element)
21+
const [, mid1, ] = foo({ dataType: 'a', day: 0 });
22+
const [first1, , ] = foo({ dataType: 'a', day: 0 });
23+
const [,,third,] = bar({ dataType: 'a', day: 0 });
24+
25+
// Legitimate errors should still be caught
26+
const [, , last4] = foo({ dataType: 'c' }); // Error: 'c' not assignable to 'a' | 'b'
27+
~~~~~~~~
28+
!!! error TS2322: Type '"c"' is not assignable to type '"a" | "b"'.
29+
!!! related TS6500 lastTupleElementDestructuring.ts:4:34: The expected type comes from property 'dataType' which is declared here on type '{ dataType: "a" | "b"; }'
30+
const [,,,last5] = bar({ notDataType: 'a' }); // Error: missing required property 'dataType'
31+
~~~~~~~~~~~~~~~~~~~~
32+
!!! error TS2345: Argument of type '{ notDataType: string; }' is not assignable to parameter of type '{ dataType: "a" | "b"; }'.
33+
!!! error TS2345: Property 'dataType' is missing in type '{ notDataType: string; }' but required in type '{ dataType: "a" | "b"; }'.
34+
!!! related TS2728 lastTupleElementDestructuring.ts:5:34: 'dataType' is declared here.
35+
36+
// Test with more complex object properties
37+
interface Config {
38+
required: string;
39+
optional?: number;
40+
}
41+
42+
declare function withConfig<T extends Config>(template: T): [T, string];
43+
44+
// Should work - accessing last element with extra property
45+
const [,configStr] = withConfig({ required: 'test', extra: 'should work' });
46+
47+
// Should still error - missing required property
48+
const [,configStr2] = withConfig({ optional: 42 }); // Error: missing 'required'
49+
~~~~~~~~~~~~~~~~
50+
!!! error TS2345: Argument of type '{ optional: number; }' is not assignable to parameter of type 'Config'.
51+
!!! error TS2345: Property 'required' is missing in type '{ optional: number; }' but required in type 'Config'.
52+
!!! related TS2728 lastTupleElementDestructuring.ts:23:5: 'required' is declared here.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//// [tests/cases/compiler/lastTupleElementDestructuring.ts] ////
2+
3+
//// [lastTupleElementDestructuring.ts]
4+
// Test for fixing excess property checking when accessing last tuple element in destructuring
5+
// Fixes https://github.com/microsoft/TypeScript/issues/41548
6+
7+
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
8+
declare function bar<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any, any];
9+
10+
// Cases that should NOT error after fix (accessing last element)
11+
const [, , last1] = foo({ dataType: 'a', day: 0 });
12+
const [,,last2] = foo({ dataType: 'a', day: 0 });
13+
const [,,,last3] = bar({ dataType: 'a', day: 0 });
14+
15+
// Cases that already worked (not accessing last element)
16+
const [, mid1, ] = foo({ dataType: 'a', day: 0 });
17+
const [first1, , ] = foo({ dataType: 'a', day: 0 });
18+
const [,,third,] = bar({ dataType: 'a', day: 0 });
19+
20+
// Legitimate errors should still be caught
21+
const [, , last4] = foo({ dataType: 'c' }); // Error: 'c' not assignable to 'a' | 'b'
22+
const [,,,last5] = bar({ notDataType: 'a' }); // Error: missing required property 'dataType'
23+
24+
// Test with more complex object properties
25+
interface Config {
26+
required: string;
27+
optional?: number;
28+
}
29+
30+
declare function withConfig<T extends Config>(template: T): [T, string];
31+
32+
// Should work - accessing last element with extra property
33+
const [,configStr] = withConfig({ required: 'test', extra: 'should work' });
34+
35+
// Should still error - missing required property
36+
const [,configStr2] = withConfig({ optional: 42 }); // Error: missing 'required'
37+
38+
//// [lastTupleElementDestructuring.js]
39+
// Test for fixing excess property checking when accessing last tuple element in destructuring
40+
// Fixes https://github.com/microsoft/TypeScript/issues/41548
41+
// Cases that should NOT error after fix (accessing last element)
42+
var _a = foo({ dataType: 'a', day: 0 }), last1 = _a[2];
43+
var _b = foo({ dataType: 'a', day: 0 }), last2 = _b[2];
44+
var _c = bar({ dataType: 'a', day: 0 }), last3 = _c[3];
45+
// Cases that already worked (not accessing last element)
46+
var _d = foo({ dataType: 'a', day: 0 }), mid1 = _d[1];
47+
var _e = foo({ dataType: 'a', day: 0 }), first1 = _e[0];
48+
var _f = bar({ dataType: 'a', day: 0 }), third = _f[2];
49+
// Legitimate errors should still be caught
50+
var _g = foo({ dataType: 'c' }), last4 = _g[2]; // Error: 'c' not assignable to 'a' | 'b'
51+
var _h = bar({ notDataType: 'a' }), last5 = _h[3]; // Error: missing required property 'dataType'
52+
// Should work - accessing last element with extra property
53+
var _j = withConfig({ required: 'test', extra: 'should work' }), configStr = _j[1];
54+
// Should still error - missing required property
55+
var _k = withConfig({ optional: 42 }), configStr2 = _k[1]; // Error: missing 'required'

0 commit comments

Comments
 (0)