Skip to content
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

Improve signature assignability for argument lists of different lengths #49218

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

Improve signature assignability for argument lists of different lengths #49218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1819397
Fixed an issue with shorter param list not being assignable to rest p…
Andarist May 23, 2022
67892c3
Merge branch 'main' into fix/rest-tuple-union-shorter-contextual-params
Andarist Sep 25, 2022
bd712ab
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Dec 28, 2022
88027d5
Fixed the contextual params assignability with target params declared…
Andarist Dec 28, 2022
6c5383d
Fill the shorter target tuple with undefined when comparing signatures
Andarist Dec 28, 2022
46f2e57
Add an additional test case for mixed length tuples in the target's rest
Andarist Dec 28, 2022
0de8923
Add tests for mixed-length tuples used as rest
Andarist Dec 28, 2022
beb241d
add tests from #45972
Andarist Dec 28, 2022
388ec89
Allow rest in source
Andarist Dec 29, 2022
8753248
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Jan 15, 2023
e357450
Fixed cases involving generics and add comments
Andarist Jan 16, 2023
29979e2
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Jun 13, 2023
fd52884
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Jul 10, 2023
336288a
fix extra cases
Andarist Jul 10, 2023
92d0267
Fixed tupel structure matching in the signature-related codepath
Andarist Jul 10, 2023
1c24035
use conditional undefined instead of any
Andarist Jul 10, 2023
608f7c3
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Jan 6, 2024
61d2612
Merge remote-tracking branch 'origin/main' into fix/rest-tuple-union-…
Andarist Feb 19, 2024
919fc0e
add extra test case
Andarist Feb 19, 2024
b920e9a
Merge branch 'main' into pr-49218
jakebailey Apr 19, 2024
a8ddfc3
Update baselines
jakebailey Apr 19, 2024
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
5 changes: 4 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17967,7 +17967,10 @@ namespace ts {

for (let i = 0; i < paramCount; i++) {
const sourceType = i === restIndex ? getRestTypeAtPosition(source, i) : tryGetTypeAtPosition(source, i);
const targetType = i === restIndex ? getRestTypeAtPosition(target, i) : tryGetTypeAtPosition(target, i);
let targetType = i === restIndex ? getRestTypeAtPosition(target, i) : tryGetTypeAtPosition(target, i);
if (i === restIndex && targetType && sourceType && isTupleType(sourceType) && !sourceType.target.hasRestElement) {
Andarist marked this conversation as resolved.
Show resolved Hide resolved
targetType = mapType(targetType, t => isTupleType(t) && !t.target.hasRestElement ? sliceTupleType(t, 0, t.target.fixedLength - sourceType.target.fixedLength) : t);
Andarist marked this conversation as resolved.
Show resolved Hide resolved
}
if (sourceType && targetType) {
// In order to ensure that any generic type Foo<T> is at least co-variant with respect to T no matter
// how Foo uses T, we need to relate parameters bi-variantly (given that parameters are input positions,
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/genericRestParameters3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ tests/cases/conformance/types/rest/genericRestParameters3.ts(18,1): error TS2345
Source has 0 element(s) but target requires 2.
tests/cases/conformance/types/rest/genericRestParameters3.ts(23,1): error TS2322: Type '(x: string, y: string) => void' is not assignable to type '(x: string, ...args: [string] | [number, boolean]) => void'.
Types of parameters 'y' and 'args' are incompatible.
Type '[string] | [number, boolean]' is not assignable to type '[y: string]'.
Type '[number, boolean]' is not assignable to type '[y: string]'.
Source has 2 element(s) but target allows only 1.
Type '[string] | [number]' is not assignable to type '[y: string]'.
Type '[number]' is not assignable to type '[y: string]'.
Type 'number' is not assignable to type 'string'.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To the best of my understanding, this change is correct and welcome under the implemented changes.

tests/cases/conformance/types/rest/genericRestParameters3.ts(24,1): error TS2322: Type '(x: string, y: number, z: boolean) => void' is not assignable to type '(x: string, ...args: [string] | [number, boolean]) => void'.
Types of parameters 'y' and 'args' are incompatible.
Type '[string] | [number, boolean]' is not assignable to type '[y: number, z: boolean]'.
Expand Down Expand Up @@ -69,9 +69,9 @@ tests/cases/conformance/types/rest/genericRestParameters3.ts(59,5): error TS2345
~~
!!! error TS2322: Type '(x: string, y: string) => void' is not assignable to type '(x: string, ...args: [string] | [number, boolean]) => void'.
!!! error TS2322: Types of parameters 'y' and 'args' are incompatible.
!!! error TS2322: Type '[string] | [number, boolean]' is not assignable to type '[y: string]'.
!!! error TS2322: Type '[number, boolean]' is not assignable to type '[y: string]'.
!!! error TS2322: Source has 2 element(s) but target allows only 1.
!!! error TS2322: Type '[string] | [number]' is not assignable to type '[y: string]'.
!!! error TS2322: Type '[number]' is not assignable to type '[y: string]'.
!!! error TS2322: Type 'number' is not assignable to type 'string'.
Comment on lines +73 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a change of the error for this case:

declare let f1: (x: string, ...args: [string] | [number, boolean]) => void;
declare let f2: (x: string, y: string) => void;

f1 = f2

With the current changes in this PR, I create a slice from the target tuple and I cap it to the source's length~. I do that for similar reasons that I've outlined in the other comment. The source signature can freely ignore the "extra" arguments so we don't even need to check them here (we'd have to ignore them through some other mechanism anyway)

f1 = f3; // Error
~~
!!! error TS2322: Type '(x: string, y: number, z: boolean) => void' is not assignable to type '(x: string, ...args: [string] | [number, boolean]) => void'.
Expand Down
31 changes: 31 additions & 0 deletions tests/baselines/reference/restTupleUnionShorterContextualParams.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
=== tests/cases/compiler/restTupleUnionShorterContextualParams.ts ===
// repro #48663

// showcase how those transitive assignments are OK
const f1: (x: string | number) => void = x => {};
>f1 : Symbol(f1, Decl(restTupleUnionShorterContextualParams.ts, 3, 5))
>x : Symbol(x, Decl(restTupleUnionShorterContextualParams.ts, 3, 11))
>x : Symbol(x, Decl(restTupleUnionShorterContextualParams.ts, 3, 40))

const f2: (x: string | number, y: string | number) => void = f1;
>f2 : Symbol(f2, Decl(restTupleUnionShorterContextualParams.ts, 4, 5))
>x : Symbol(x, Decl(restTupleUnionShorterContextualParams.ts, 4, 11))
>y : Symbol(y, Decl(restTupleUnionShorterContextualParams.ts, 4, 30))
>f1 : Symbol(f1, Decl(restTupleUnionShorterContextualParams.ts, 3, 5))

const f3: (...args: [number, string] | [string, number]) => void = f2;
>f3 : Symbol(f3, Decl(restTupleUnionShorterContextualParams.ts, 5, 5))
>args : Symbol(args, Decl(restTupleUnionShorterContextualParams.ts, 5, 11))
>f2 : Symbol(f2, Decl(restTupleUnionShorterContextualParams.ts, 4, 5))

// by extension those should be OK too
const f4: (...args: [number, string] | [string, number]) => void = (item) => {}
>f4 : Symbol(f4, Decl(restTupleUnionShorterContextualParams.ts, 8, 5))
>args : Symbol(args, Decl(restTupleUnionShorterContextualParams.ts, 8, 11))
>item : Symbol(item, Decl(restTupleUnionShorterContextualParams.ts, 8, 68))

const f5: (...args: [number, string] | [string, number]) => void = (item: number | string) => {}
>f5 : Symbol(f5, Decl(restTupleUnionShorterContextualParams.ts, 9, 5))
>args : Symbol(args, Decl(restTupleUnionShorterContextualParams.ts, 9, 11))
>item : Symbol(item, Decl(restTupleUnionShorterContextualParams.ts, 9, 68))

34 changes: 34 additions & 0 deletions tests/baselines/reference/restTupleUnionShorterContextualParams.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
=== tests/cases/compiler/restTupleUnionShorterContextualParams.ts ===
// repro #48663

// showcase how those transitive assignments are OK
const f1: (x: string | number) => void = x => {};
>f1 : (x: string | number) => void
>x : string | number
>x => {} : (x: string | number) => void
>x : string | number

const f2: (x: string | number, y: string | number) => void = f1;
>f2 : (x: string | number, y: string | number) => void
>x : string | number
>y : string | number
>f1 : (x: string | number) => void

const f3: (...args: [number, string] | [string, number]) => void = f2;
>f3 : (...args: [number, string] | [string, number]) => void
>args : [number, string] | [string, number]
>f2 : (x: string | number, y: string | number) => void

// by extension those should be OK too
const f4: (...args: [number, string] | [string, number]) => void = (item) => {}
>f4 : (...args: [number, string] | [string, number]) => void
>args : [number, string] | [string, number]
>(item) => {} : (item: string | number) => void
>item : string | number

const f5: (...args: [number, string] | [string, number]) => void = (item: number | string) => {}
>f5 : (...args: [number, string] | [string, number]) => void
>args : [number, string] | [string, number]
>(item: number | string) => {} : (item: number | string) => void
>item : string | number

12 changes: 12 additions & 0 deletions tests/cases/compiler/restTupleUnionShorterContextualParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// @noEmit: true

// repro #48663

// showcase how those transitive assignments are OK
const f1: (x: string | number) => void = x => {};
const f2: (x: string | number, y: string | number) => void = f1;
const f3: (...args: [number, string] | [string, number]) => void = f2;

// by extension those should be OK too
const f4: (...args: [number, string] | [string, number]) => void = (item) => {}
const f5: (...args: [number, string] | [string, number]) => void = (item: number | string) => {}