Skip to content

Commit 1f1fe0c

Browse files
Revert catastrophically bad fix and investigate root cause of last tuple element issue
Co-authored-by: RyanCavanaugh <[email protected]>
1 parent 2d1cf66 commit 1f1fe0c

3 files changed

Lines changed: 43 additions & 46 deletions

File tree

src/compiler/checker.ts

Lines changed: 24 additions & 46 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);
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.
@@ -36069,11 +36069,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3606936069
// If one or more arguments are still excluded (as indicated by CheckMode.SkipContextSensitive),
3607036070
// we obtain the regular type of any object literal arguments because we may not have inferred complete
3607136071
// parameter types yet and therefore excess property checks may yield false positives (see #17041).
36072-
// Also skip fresh literal checking when the call is in certain destructuring contexts that can cause
36073-
// incorrect excess property errors (see #41548).
36074-
const shouldSkipFreshness = (checkMode & CheckMode.SkipContextSensitive) ||
36075-
(isCallExpression(node) && isCallInProblematicDestructuringContext(node));
36076-
const checkArgType = shouldSkipFreshness ? getRegularTypeOfObjectLiteral(argType) : argType;
36072+
const checkArgType = checkMode & CheckMode.SkipContextSensitive ? getRegularTypeOfObjectLiteral(argType) : argType;
3607736073
const effectiveCheckArgumentNode = getEffectiveCheckNode(arg);
3607836074
if (!checkTypeRelatedToAndOptionallyElaborate(checkArgType, paramType, relation, reportErrors ? effectiveCheckArgumentNode : undefined, effectiveCheckArgumentNode, headMessage, containingMessageChain, errorOutputContainer)) {
3607936075
Debug.assert(!reportErrors || !!errorOutputContainer.errors, "parameter should have errors when reporting errors");
@@ -36421,24 +36417,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
3642136417
return createDiagnosticForNodeArray(getSourceFileOfNode(node), typeArguments, Diagnostics.Expected_0_type_arguments_but_got_1, belowArgCount === -Infinity ? aboveArgCount : belowArgCount, argCount);
3642236418
}
3642336419

36424-
function isCallInProblematicDestructuringContext(node: CallLikeExpression): boolean {
36425-
// Check if this call expression is used as the initializer in a variable declaration with a destructuring pattern
36426-
const parent = node.parent;
36427-
if (parent && isVariableDeclaration(parent) && parent.initializer === node) {
36428-
if (isArrayBindingPattern(parent.name)) {
36429-
// Only apply this fix for the specific known problematic case:
36430-
// destructuring where the third position (index 2) is accessed
36431-
const elements = parent.name.elements;
36432-
return elements.length === 3 &&
36433-
isOmittedExpression(elements[0]) &&
36434-
isOmittedExpression(elements[1]) &&
36435-
!isOmittedExpression(elements[2]);
36436-
}
36437-
}
36438-
36439-
return false;
36440-
}
36441-
3644236420
function resolveCall(node: CallLikeExpression, signatures: readonly Signature[], candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, callChainFlags: SignatureFlags, headMessage?: DiagnosticMessage): Signature {
3644336421
const isTaggedTemplate = node.kind === SyntaxKind.TaggedTemplateExpression;
3644436422
const isDecorator = node.kind === SyntaxKind.Decorator;
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Test for fixing excess property checking when accessing last tuple element in destructuring
2+
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
3+
4+
// This should NOT error after fix
5+
const [, , last] = foo({ dataType: 'a', day: 0 });
6+
7+
// This already works (doesn't access last element)
8+
const [, mid, ] = foo({ dataType: 'a', day: 0 });
9+
10+
// Also test that legitimate errors are still caught
11+
const [, , last2] = foo({ dataType: 'c' }); // Should still error

tests/debug_test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Test case to check the fix
2+
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
3+
4+
// Error case - accessing last element
5+
const [, , last] = foo({ dataType: 'a', day: 0 });
6+
7+
// Working case - not accessing last element
8+
const [, mid, ] = foo({ dataType: 'a', day: 0 });

0 commit comments

Comments
 (0)