Skip to content

Fix excess property checking in array destructuring contexts #62102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed
34 changes: 31 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36069,7 +36069,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// If one or more arguments are still excluded (as indicated by CheckMode.SkipContextSensitive),
// we obtain the regular type of any object literal arguments because we may not have inferred complete
// parameter types yet and therefore excess property checks may yield false positives (see #17041).
const checkArgType = checkMode & CheckMode.SkipContextSensitive ? getRegularTypeOfObjectLiteral(argType) : argType;
// Also skip fresh literal checking when the call is in a destructuring context to avoid inappropriate
// excess property checking (see #41548).
const shouldSkipFreshness = (checkMode & CheckMode.SkipContextSensitive) ||
(isCallExpression(node) && isCallInDestructuringContext(node));
const checkArgType = shouldSkipFreshness ? getRegularTypeOfObjectLiteral(argType) : argType;
const effectiveCheckArgumentNode = getEffectiveCheckNode(arg);
if (!checkTypeRelatedToAndOptionallyElaborate(checkArgType, paramType, relation, reportErrors ? effectiveCheckArgumentNode : undefined, effectiveCheckArgumentNode, headMessage, containingMessageChain, errorOutputContainer)) {
Debug.assert(!reportErrors || !!errorOutputContainer.errors, "parameter should have errors when reporting errors");
Expand Down Expand Up @@ -36417,6 +36421,21 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return createDiagnosticForNodeArray(getSourceFileOfNode(node), typeArguments, Diagnostics.Expected_0_type_arguments_but_got_1, belowArgCount === -Infinity ? aboveArgCount : belowArgCount, argCount);
}

function isCallInDestructuringContext(node: CallLikeExpression): boolean {
// Check if this call expression is used as the initializer in a variable declaration with a destructuring pattern
const parent = node.parent;
if (parent && isVariableDeclaration(parent) && parent.initializer === node) {
return isBindingPattern(parent.name);
}

// Check for assignment expressions: [a, b] = foo()
if (parent && isBinaryExpression(parent) && parent.operatorToken.kind === SyntaxKind.EqualsToken && parent.right === node) {
return isArrayLiteralExpression(parent.left) || isObjectLiteralExpression(parent.left);
}

return false;
}

function resolveCall(node: CallLikeExpression, signatures: readonly Signature[], candidatesOutArray: Signature[] | undefined, checkMode: CheckMode, callChainFlags: SignatureFlags, headMessage?: DiagnosticMessage): Signature {
const isTaggedTemplate = node.kind === SyntaxKind.TaggedTemplateExpression;
const isDecorator = node.kind === SyntaxKind.Decorator;
Expand Down Expand Up @@ -36489,7 +36508,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// For a decorator, no arguments are susceptible to contextual typing due to the fact
// decorators are applied to a declaration by the emitter, and not to an expression.
const isSingleNonGenericCandidate = candidates.length === 1 && !candidates[0].typeParameters;
if (!isDecorator && !isSingleNonGenericCandidate && some(args, isContextSensitive)) {
let shouldSkipContextSensitive = !isDecorator && !isSingleNonGenericCandidate && some(args, isContextSensitive);

// Also skip context sensitive checking when the call is used in a destructuring context
// to avoid inappropriate excess property checking on object literal arguments
const isInDestructuring = !isDecorator && isCallInDestructuringContext(node);
if (isInDestructuring && !shouldSkipContextSensitive) {
shouldSkipContextSensitive = true;
}

if (shouldSkipContextSensitive) {
argCheckMode = CheckMode.SkipContextSensitive;
}

Expand Down Expand Up @@ -36678,7 +36706,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (some(typeArguments) || !hasCorrectArity(node, args, candidate, signatureHelpTrailingComma)) {
return undefined;
}
if (getSignatureApplicabilityError(node, args, candidate, relation, CheckMode.Normal, /*reportErrors*/ false, /*containingMessageChain*/ undefined)) {
if (getSignatureApplicabilityError(node, args, candidate, relation, argCheckMode, /*reportErrors*/ false, /*containingMessageChain*/ undefined)) {
candidatesForArgumentError = [candidate];
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
excessPropertyCheckingInDestructuring.ts(14,30): error TS2322: Type '"invalid"' is not assignable to type '"a" | "b"'.
excessPropertyCheckingInDestructuring.ts(15,30): error TS2322: Type '"invalid"' is not assignable to type '"a" | "b"'.
excessPropertyCheckingInDestructuring.ts(19,38): error TS2353: Object literal may only specify known properties, and 'day' does not exist in type '{ dataType: "a" | "b"; }'.


==== excessPropertyCheckingInDestructuring.ts (3 errors) ====
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
declare function bar(template: { dataType: 'a' | 'b' }): [any, any, any];

// These should work without excess property errors - destructuring contexts
const [, ,] = foo({ dataType: 'a', day: 0 });
const [, , t] = foo({ dataType: 'a', day: 0 });
const [x, y, z] = foo({ dataType: 'a', day: 0 });

const [, ,] = bar({ dataType: 'a', day: 0 });
const [, , u] = bar({ dataType: 'a', day: 0 });
const [a, b, c] = bar({ dataType: 'a', day: 0 });

// These should still report legitimate type errors
const [, , invalid1] = foo({ dataType: 'invalid' });
~~~~~~~~
!!! error TS2322: Type '"invalid"' is not assignable to type '"a" | "b"'.
!!! related TS6500 excessPropertyCheckingInDestructuring.ts:1:34: The expected type comes from property 'dataType' which is declared here on type '{ dataType: "a" | "b"; }'
const [, , invalid2] = bar({ dataType: 'invalid' });
~~~~~~~~
!!! error TS2322: Type '"invalid"' is not assignable to type '"a" | "b"'.
!!! related TS6500 excessPropertyCheckingInDestructuring.ts:2:34: The expected type comes from property 'dataType' which is declared here on type '{ dataType: "a" | "b"; }'

// Non-destructuring cases - generic function should work, non-generic should error
const result1 = foo({ dataType: 'a', day: 0 }); // OK - generic function
const result2 = bar({ dataType: 'a', day: 0 }); // Error - non-generic with excess property
~~~
!!! error TS2353: Object literal may only specify known properties, and 'day' does not exist in type '{ dataType: "a" | "b"; }'.

// Assignment destructuring should also work
let d, e, f: any;
[d, e, f] = foo({ dataType: 'a', day: 0 });
46 changes: 46 additions & 0 deletions tests/baselines/reference/excessPropertyCheckingInDestructuring.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//// [tests/cases/compiler/excessPropertyCheckingInDestructuring.ts] ////

//// [excessPropertyCheckingInDestructuring.ts]
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
declare function bar(template: { dataType: 'a' | 'b' }): [any, any, any];

// These should work without excess property errors - destructuring contexts
const [, ,] = foo({ dataType: 'a', day: 0 });
const [, , t] = foo({ dataType: 'a', day: 0 });
const [x, y, z] = foo({ dataType: 'a', day: 0 });

const [, ,] = bar({ dataType: 'a', day: 0 });
const [, , u] = bar({ dataType: 'a', day: 0 });
const [a, b, c] = bar({ dataType: 'a', day: 0 });

// These should still report legitimate type errors
const [, , invalid1] = foo({ dataType: 'invalid' });
const [, , invalid2] = bar({ dataType: 'invalid' });

// Non-destructuring cases - generic function should work, non-generic should error
const result1 = foo({ dataType: 'a', day: 0 }); // OK - generic function
const result2 = bar({ dataType: 'a', day: 0 }); // Error - non-generic with excess property

// Assignment destructuring should also work
let d, e, f: any;
[d, e, f] = foo({ dataType: 'a', day: 0 });

//// [excessPropertyCheckingInDestructuring.js]
"use strict";
var _a;
// These should work without excess property errors - destructuring contexts
var _b = foo({ dataType: 'a', day: 0 });
var _c = foo({ dataType: 'a', day: 0 }), t = _c[2];
var _d = foo({ dataType: 'a', day: 0 }), x = _d[0], y = _d[1], z = _d[2];
var _e = bar({ dataType: 'a', day: 0 });
var _f = bar({ dataType: 'a', day: 0 }), u = _f[2];
var _g = bar({ dataType: 'a', day: 0 }), a = _g[0], b = _g[1], c = _g[2];
// These should still report legitimate type errors
var _h = foo({ dataType: 'invalid' }), invalid1 = _h[2];
var _j = bar({ dataType: 'invalid' }), invalid2 = _j[2];
// Non-destructuring cases - generic function should work, non-generic should error
var result1 = foo({ dataType: 'a', day: 0 }); // OK - generic function
var result2 = bar({ dataType: 'a', day: 0 }); // Error - non-generic with excess property
// Assignment destructuring should also work
var d, e, f;
_a = foo({ dataType: 'a', day: 0 }), d = _a[0], e = _a[1], f = _a[2];
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//// [tests/cases/compiler/excessPropertyCheckingInDestructuring.ts] ////

=== excessPropertyCheckingInDestructuring.ts ===
declare function foo<T extends { dataType: 'a' | 'b' }>(template: T): [T, any, any];
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>T : Symbol(T, Decl(excessPropertyCheckingInDestructuring.ts, 0, 21))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 0, 32))
>template : Symbol(template, Decl(excessPropertyCheckingInDestructuring.ts, 0, 56))
>T : Symbol(T, Decl(excessPropertyCheckingInDestructuring.ts, 0, 21))
>T : Symbol(T, Decl(excessPropertyCheckingInDestructuring.ts, 0, 21))

declare function bar(template: { dataType: 'a' | 'b' }): [any, any, any];
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>template : Symbol(template, Decl(excessPropertyCheckingInDestructuring.ts, 1, 21))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 1, 32))

// These should work without excess property errors - destructuring contexts
const [, ,] = foo({ dataType: 'a', day: 0 });
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 4, 19))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 4, 34))

const [, , t] = foo({ dataType: 'a', day: 0 });
>t : Symbol(t, Decl(excessPropertyCheckingInDestructuring.ts, 5, 10))
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 5, 21))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 5, 36))

const [x, y, z] = foo({ dataType: 'a', day: 0 });
>x : Symbol(x, Decl(excessPropertyCheckingInDestructuring.ts, 6, 7))
>y : Symbol(y, Decl(excessPropertyCheckingInDestructuring.ts, 6, 9))
>z : Symbol(z, Decl(excessPropertyCheckingInDestructuring.ts, 6, 12))
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 6, 23))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 6, 38))

const [, ,] = bar({ dataType: 'a', day: 0 });
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 8, 19))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 8, 34))

const [, , u] = bar({ dataType: 'a', day: 0 });
>u : Symbol(u, Decl(excessPropertyCheckingInDestructuring.ts, 9, 10))
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 9, 21))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 9, 36))

const [a, b, c] = bar({ dataType: 'a', day: 0 });
>a : Symbol(a, Decl(excessPropertyCheckingInDestructuring.ts, 10, 7))
>b : Symbol(b, Decl(excessPropertyCheckingInDestructuring.ts, 10, 9))
>c : Symbol(c, Decl(excessPropertyCheckingInDestructuring.ts, 10, 12))
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 10, 23))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 10, 38))

// These should still report legitimate type errors
const [, , invalid1] = foo({ dataType: 'invalid' });
>invalid1 : Symbol(invalid1, Decl(excessPropertyCheckingInDestructuring.ts, 13, 10))
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 13, 28))

const [, , invalid2] = bar({ dataType: 'invalid' });
>invalid2 : Symbol(invalid2, Decl(excessPropertyCheckingInDestructuring.ts, 14, 10))
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 14, 28))

// Non-destructuring cases - generic function should work, non-generic should error
const result1 = foo({ dataType: 'a', day: 0 }); // OK - generic function
>result1 : Symbol(result1, Decl(excessPropertyCheckingInDestructuring.ts, 17, 5))
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 17, 21))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 17, 36))

const result2 = bar({ dataType: 'a', day: 0 }); // Error - non-generic with excess property
>result2 : Symbol(result2, Decl(excessPropertyCheckingInDestructuring.ts, 18, 5))
>bar : Symbol(bar, Decl(excessPropertyCheckingInDestructuring.ts, 0, 84))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 18, 21))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 18, 36))

// Assignment destructuring should also work
let d, e, f: any;
>d : Symbol(d, Decl(excessPropertyCheckingInDestructuring.ts, 21, 3))
>e : Symbol(e, Decl(excessPropertyCheckingInDestructuring.ts, 21, 6))
>f : Symbol(f, Decl(excessPropertyCheckingInDestructuring.ts, 21, 9))

[d, e, f] = foo({ dataType: 'a', day: 0 });
>d : Symbol(d, Decl(excessPropertyCheckingInDestructuring.ts, 21, 3))
>e : Symbol(e, Decl(excessPropertyCheckingInDestructuring.ts, 21, 6))
>f : Symbol(f, Decl(excessPropertyCheckingInDestructuring.ts, 21, 9))
>foo : Symbol(foo, Decl(excessPropertyCheckingInDestructuring.ts, 0, 0))
>dataType : Symbol(dataType, Decl(excessPropertyCheckingInDestructuring.ts, 22, 17))
>day : Symbol(day, Decl(excessPropertyCheckingInDestructuring.ts, 22, 32))

Loading
Loading