Skip to content

Fixed an issue with spreading generic types with tuple constraints into calls #53615

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.

Sign up for GitHub

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

Open
Andarist wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fixed an issue with spreading generic types with tuple constraints into calls #53615

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
533f7de
Fixed an issue with spreading generic types with tuple constraints in…
Andarist Mar 31, 2023
e35d71f
simplify the fix
Andarist Mar 31, 2023
bca909d
Create deferred indexed access type
Andarist Apr 1, 2023
2774d57
Update comments containing "expected" results in `genericRestParamete…
Andarist Apr 1, 2023
874f1cc
Merge remote-tracking branch 'origin/main' into fix/generic-spread-args
Andarist Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32711,6 +32711,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const arg = args[i];
// We can call checkExpressionCached because spread expressions never have a contextual type.
const spreadType = arg.kind === SyntaxKind.SpreadElement && (flowLoopCount ? checkExpression((arg as SpreadElement).expression) : checkExpressionCached((arg as SpreadElement).expression));
const constraint = spreadType && getConstraintOfType(spreadType);
if (spreadType && isTupleType(spreadType)) {
forEach(getTypeArguments(spreadType), (t, i) => {
const flags = spreadType.target.elementFlags[i];
Expand All @@ -32719,6 +32720,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
effectiveArgs.push(syntheticArg);
});
}
else if (constraint && isTupleType(constraint)) {
forEach(getTypeArguments(constraint), (_, i) => {
const isVariable = !!(constraint.target.elementFlags[i] & ElementFlags.Variable);
const syntheticArg = createSyntheticExpression(arg, !isVariable ? getTypeOfPropertyOfType(spreadType, "" + i as __String)! : spreadType,
Copy link
Contributor Author

@Andarist Andarist Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this won't help with variable elements at all - there is no way to express a "deferred" type that would later skip some elements from it so a variable element gets synthesized as the whole spreadType. It isn't ideal but it also isn't worse than the current baseline.

An example code that ideally should work but it doesn't:

interface HasMethod {
  method(second?: number, ...rest: boolean[]): void;
  method2(first?: string, second?: number, ...rest: boolean[]): void;
}

function fn<HasMethodLike extends HasMethod>(
  instance: HasMethodLike,
  ...args: Parameters<HasMethodLike["method"]>
) {
  instance.method2('', ...args);
}

Copy link
Contributor Author

@Andarist Andarist Mar 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best idea I have for fixing that is to add more information to synthetic expressions (like .skipElementCount) and then use that information in checkSyntheticExpression. This is how currently checkSyntheticExpression looks like:

function checkSyntheticExpression(node: SyntheticExpression): Type {
    return node.isSpread ? getIndexedAccessType(node.type, numberType) : node.type;
}

/*isSpread*/ isVariable, constraint.target.labeledElementDeclarations?.[i]);
effectiveArgs.push(syntheticArg);
});
}
else {
effectiveArgs.push(arg);
}
Expand Down
32 changes: 32 additions & 0 deletions tests/baselines/reference/arraySpreadInCall.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,36 @@ tests/cases/conformance/es6/spread/arraySpreadInCall.ts(21,12): error TS2554: Ex
~~~~~~~~~~~~~~~
!!! error TS2554: Expected 0-1 arguments, but got 2.

// repro from #53541
class T {
fn(name?: string, sex?: number): void {}
}

class M<X extends T> {
constructor(public m: X) {}

fn(...args: Parameters<X["fn"]>) {
this.m.fn(...args);
}
}

// repro from #53541#issuecomment-1487859044
interface HasMethod {
method(first?: string, second?: number): void;
method2(...args: [name: string, sex?: number] | [other: number]): void;
}

function fn21<HasMethodLike extends HasMethod>(
instance: HasMethodLike,
...args: Parameters<HasMethodLike["method"]>
) {
instance.method(...args);
}

function fn22<HasMethodLike extends HasMethod>(
instance: HasMethodLike,
...args: Parameters<HasMethodLike["method2"]>
) {
instance.method2(...args);
}

92 changes: 92 additions & 0 deletions tests/baselines/reference/arraySpreadInCall.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,96 @@ action.run(...[100, 'foo']) // error
>action : Symbol(action, Decl(arraySpreadInCall.ts, 19, 13))
>run : Symbol(IAction.run, Decl(arraySpreadInCall.ts, 16, 19))

// repro from #53541
class T {
>T : Symbol(T, Decl(arraySpreadInCall.ts, 20, 27))

fn(name?: string, sex?: number): void {}
>fn : Symbol(T.fn, Decl(arraySpreadInCall.ts, 23, 9))
>name : Symbol(name, Decl(arraySpreadInCall.ts, 24, 5))
>sex : Symbol(sex, Decl(arraySpreadInCall.ts, 24, 19))
}

class M<X extends T> {
>M : Symbol(M, Decl(arraySpreadInCall.ts, 25, 1))
>X : Symbol(X, Decl(arraySpreadInCall.ts, 27, 8))
>T : Symbol(T, Decl(arraySpreadInCall.ts, 20, 27))

constructor(public m: X) {}
>m : Symbol(M.m, Decl(arraySpreadInCall.ts, 28, 14))
>X : Symbol(X, Decl(arraySpreadInCall.ts, 27, 8))

fn(...args: Parameters<X["fn"]>) {
>fn : Symbol(M.fn, Decl(arraySpreadInCall.ts, 28, 29))
>args : Symbol(args, Decl(arraySpreadInCall.ts, 30, 5))
>Parameters : Symbol(Parameters, Decl(lib.es5.d.ts, --, --))
>X : Symbol(X, Decl(arraySpreadInCall.ts, 27, 8))

this.m.fn(...args);
>this.m.fn : Symbol(T.fn, Decl(arraySpreadInCall.ts, 23, 9))
>this.m : Symbol(M.m, Decl(arraySpreadInCall.ts, 28, 14))
>this : Symbol(M, Decl(arraySpreadInCall.ts, 25, 1))
>m : Symbol(M.m, Decl(arraySpreadInCall.ts, 28, 14))
>fn : Symbol(T.fn, Decl(arraySpreadInCall.ts, 23, 9))
>args : Symbol(args, Decl(arraySpreadInCall.ts, 30, 5))
}
}

// repro from #53541#issuecomment-1487859044
interface HasMethod {
>HasMethod : Symbol(HasMethod, Decl(arraySpreadInCall.ts, 33, 1))

method(first?: string, second?: number): void;
>method : Symbol(HasMethod.method, Decl(arraySpreadInCall.ts, 36, 21))
>first : Symbol(first, Decl(arraySpreadInCall.ts, 37, 9))
>second : Symbol(second, Decl(arraySpreadInCall.ts, 37, 24))

method2(...args: [name: string, sex?: number] | [other: number]): void;
>method2 : Symbol(HasMethod.method2, Decl(arraySpreadInCall.ts, 37, 48))
>args : Symbol(args, Decl(arraySpreadInCall.ts, 38, 10))
}

function fn21<HasMethodLike extends HasMethod>(
>fn21 : Symbol(fn21, Decl(arraySpreadInCall.ts, 39, 1))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 41, 14))
>HasMethod : Symbol(HasMethod, Decl(arraySpreadInCall.ts, 33, 1))

instance: HasMethodLike,
>instance : Symbol(instance, Decl(arraySpreadInCall.ts, 41, 47))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 41, 14))

...args: Parameters<HasMethodLike["method"]>
>args : Symbol(args, Decl(arraySpreadInCall.ts, 42, 26))
>Parameters : Symbol(Parameters, Decl(lib.es5.d.ts, --, --))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 41, 14))

) {
instance.method(...args);
>instance.method : Symbol(HasMethod.method, Decl(arraySpreadInCall.ts, 36, 21))
>instance : Symbol(instance, Decl(arraySpreadInCall.ts, 41, 47))
>method : Symbol(HasMethod.method, Decl(arraySpreadInCall.ts, 36, 21))
>args : Symbol(args, Decl(arraySpreadInCall.ts, 42, 26))
}

function fn22<HasMethodLike extends HasMethod>(
>fn22 : Symbol(fn22, Decl(arraySpreadInCall.ts, 46, 1))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 48, 14))
>HasMethod : Symbol(HasMethod, Decl(arraySpreadInCall.ts, 33, 1))

instance: HasMethodLike,
>instance : Symbol(instance, Decl(arraySpreadInCall.ts, 48, 47))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 48, 14))

...args: Parameters<HasMethodLike["method2"]>
>args : Symbol(args, Decl(arraySpreadInCall.ts, 49, 26))
>Parameters : Symbol(Parameters, Decl(lib.es5.d.ts, --, --))
>HasMethodLike : Symbol(HasMethodLike, Decl(arraySpreadInCall.ts, 48, 14))

) {
instance.method2(...args);
>instance.method2 : Symbol(HasMethod.method2, Decl(arraySpreadInCall.ts, 37, 48))
>instance : Symbol(instance, Decl(arraySpreadInCall.ts, 48, 47))
>method2 : Symbol(HasMethod.method2, Decl(arraySpreadInCall.ts, 37, 48))
>args : Symbol(args, Decl(arraySpreadInCall.ts, 49, 26))
}

81 changes: 81 additions & 0 deletions tests/baselines/reference/arraySpreadInCall.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,85 @@ action.run(...[100, 'foo']) // error
>100 : 100
>'foo' : "foo"

// repro from #53541
class T {
>T : T

fn(name?: string, sex?: number): void {}
>fn : (name?: string, sex?: number) => void
>name : string | undefined
>sex : number | undefined
}

class M<X extends T> {
>M : M<X>

constructor(public m: X) {}
>m : X

fn(...args: Parameters<X["fn"]>) {
>fn : (...args: Parameters<X["fn"]>) => void
>args : Parameters<X["fn"]>

this.m.fn(...args);
>this.m.fn(...args) : void
>this.m.fn : (name?: string | undefined, sex?: number | undefined) => void
>this.m : X
>this : this
>m : X
>fn : (name?: string | undefined, sex?: number | undefined) => void
>...args : string | number | undefined
>args : Parameters<X["fn"]>
}
}

// repro from #53541#issuecomment-1487859044
interface HasMethod {
method(first?: string, second?: number): void;
>method : (first?: string, second?: number) => void
>first : string | undefined
>second : number | undefined

method2(...args: [name: string, sex?: number] | [other: number]): void;
>method2 : (...args: [name: string, sex?: number] | [other: number]) => void
>args : [name: string, sex?: number | undefined] | [other: number]
}

function fn21<HasMethodLike extends HasMethod>(
>fn21 : <HasMethodLike extends HasMethod>(instance: HasMethodLike, ...args: Parameters<HasMethodLike["method"]>) => void

instance: HasMethodLike,
>instance : HasMethodLike

...args: Parameters<HasMethodLike["method"]>
>args : Parameters<HasMethodLike["method"]>

) {
instance.method(...args);
>instance.method(...args) : void
>instance.method : (first?: string | undefined, second?: number | undefined) => void
>instance : HasMethodLike
>method : (first?: string | undefined, second?: number | undefined) => void
>...args : string | number | undefined
>args : Parameters<HasMethodLike["method"]>
}

function fn22<HasMethodLike extends HasMethod>(
>fn22 : <HasMethodLike extends HasMethod>(instance: HasMethodLike, ...args: Parameters<HasMethodLike["method2"]>) => void

instance: HasMethodLike,
>instance : HasMethodLike

...args: Parameters<HasMethodLike["method2"]>
>args : Parameters<HasMethodLike["method2"]>

) {
instance.method2(...args);
>instance.method2(...args) : void
>instance.method2 : (...args: [name: string, sex?: number | undefined] | [other: number]) => void
>instance : HasMethodLike
>method2 : (...args: [name: string, sex?: number | undefined] | [other: number]) => void
>...args : string | number | undefined
>args : Parameters<HasMethodLike["method2"]>
}

32 changes: 32 additions & 0 deletions tests/cases/conformance/es6/spread/arraySpreadInCall.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,35 @@ interface IAction {
declare const action: IAction
action.run(...[100, 'foo']) // error

// repro from #53541
class T {
fn(name?: string, sex?: number): void {}
}

class M<X extends T> {
constructor(public m: X) {}

fn(...args: Parameters<X["fn"]>) {
this.m.fn(...args);
}
}

// repro from #53541#issuecomment-1487859044
interface HasMethod {
method(first?: string, second?: number): void;
method2(...args: [name: string, sex?: number] | [other: number]): void;
}

function fn21<HasMethodLike extends HasMethod>(
instance: HasMethodLike,
...args: Parameters<HasMethodLike["method"]>
) {
instance.method(...args);
}

function fn22<HasMethodLike extends HasMethod>(
instance: HasMethodLike,
...args: Parameters<HasMethodLike["method2"]>
) {
instance.method2(...args);
}