Skip to content

Fix Callback argument type not inferred for union of interfaces #59309 #59672

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
MichalMarsalek wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Fix Callback argument type not inferred for union of interfaces #59309 #59672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
25b5914
contextual type
MichalMarsalek Aug 18, 2024
effe3a0
Merge branch 'main' into contextual-type
MichalMarsalek Aug 18, 2024
ed2bfa4
fix getContextualSignature
MichalMarsalek Aug 18, 2024
2e530af
format
MichalMarsalek Aug 18, 2024
ac64f64
simplify getContextualSignature
MichalMarsalek Aug 19, 2024
6eaf5e3
sort signatures by getMinArgumentCount
MichalMarsalek Aug 19, 2024
f69d6e1
use test file options
MichalMarsalek Aug 19, 2024
8b0b855
special case async function
MichalMarsalek Aug 19, 2024
8a3b095
fix logic for async functions
MichalMarsalek Aug 19, 2024
dcb8f5d
special case generators
MichalMarsalek Aug 19, 2024
4fd6bbd
update tests
MichalMarsalek Aug 19, 2024
381e4e0
update tests
MichalMarsalek Aug 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
39 changes: 19 additions & 20 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32422,29 +32422,28 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (!(type.flags & TypeFlags.Union)) {
return getContextualCallSignature(type, node);
}
let signatureList: Signature[] | undefined;
const types = (type as UnionType).types;
for (const current of types) {
const signature = getContextualCallSignature(current, node);
if (signature) {
if (!signatureList) {
// This signature will contribute to contextual union signature
signatureList = [signature];
}
else if (!compareSignaturesIdentical(signatureList[0], signature, /*partialMatch*/ false, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true, compareTypesIdentical)) {
// Signatures aren't identical, do not use
return undefined;
}
else {
// Use this signature for contextual union signature
signatureList.push(signature);
}

let signatures = mapDefined((type as UnionType).types, t => getContextualCallSignature(t, node));
const functionFlags = getFunctionFlags(node);
if (functionFlags & FunctionFlags.Generator) {
signatures = filter(signatures, s => checkGeneratorInstantiationAssignabilityToReturnType(getReturnTypeOfSignature(s), functionFlags, /*errorNode*/ undefined));
Copy link
Contributor

@Andarist Andarist Aug 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You need to account for rest parameters here so it would be more appropriate to use getParameterCount here over directly checking the .parameters.length. Please also consider cases like

((a: string, b?: string) => void) | ((a: string, ...rest: string[]) => void)

Both of those signatures have the same parameter count but perhaps you'd like to specifically sort one of them as the first one.

Copy link
Contributor Author

@MichalMarsalek MichalMarsalek Aug 19, 2024

Choose a reason for hiding this comment

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

Thank you. I think that actually what I need is getMinArgumentCount.
For the case ((a: string, b?: string) => void) | ((a: string, ...rest: string[]) => void) I don't think it matters which one is first.

Copy link
Contributor

@Andarist Andarist Aug 19, 2024

Choose a reason for hiding this comment

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

I'm not sure if it will make any difference but to exercise even more funky test cases you could add test cases with signatures of this shape:

((a: string, ...rest: [string, number?] | [number, boolean?]) => void) | ((a: string, ...rest: [string, number] | [number, boolean]) => void)

Note that I didn't think through what types you should use in those rest parameters, just that they can be unions of tuples and that one of the signatures might have some of the elements there required and some might have them optional etc.

}
else if (functionFlags & FunctionFlags.Async) {
signatures = filter(signatures, s => !!getAwaitedTypeOfPromise(getReturnTypeOfSignature(s)));
}
signatures.sort((s1, s2) => getMinArgumentCount(s1) - getMinArgumentCount(s2));
if (!signatures.length) {
return;
Copy link
Contributor

@Andarist Andarist Aug 18, 2024

Choose a reason for hiding this comment

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

If you are loosening here how parameter types are meant to be compared I'd probably expect here:

Suggested change
else if (!compareSignaturesIdentical(signatureList[0], signature, /*partialMatch*/ true, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true, compareTypesSubtypeOf)) {
else if (!compareSignaturesIdentical(signatureList[0], signature, /*partialMatch*/ true, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true, compareTypesAssignable)) {

Unless there is already a reason why the subtype relationship was used here.

Copy link
Contributor Author

@MichalMarsalek MichalMarsalek Aug 19, 2024

Choose a reason for hiding this comment

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

I'm not really sure when each relation should be used. I used compareTypesSubtypeOf because that's what is used in a call to compareSignaturesIdentical with partialMatch === true in the function findMatchingSignatures.

Copy link
Contributor

@Andarist Andarist Aug 19, 2024

Choose a reason for hiding this comment

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

findMatchingSignatures gets only called by getUnionSignatures so I agree that this probably makes sense

}

for (let i = 1; i < signatures.length; i++) {
if (!compareSignaturesIdentical(signatures[0], signatures[i], /*partialMatch*/ true, /*ignoreThisTypes*/ true, /*ignoreReturnTypes*/ true, compareTypesSubtypeOf)) {
// Signatures aren't identical, do not use
return undefined;
}
}
// Result is union of signatures collected (return type is union of return types of this signature set)
if (signatureList) {
return signatureList.length === 1 ? signatureList[0] : createUnionSignature(signatureList[0], signatureList);
}
return signatures.length === 1 ? signatures[0] : createUnionSignature(signatures[0], signatures);
}

function checkGrammarRegularExpressionLiteral(node: RegularExpressionLiteral) {
Expand Down
229 changes: 229 additions & 0 deletions tests/baselines/reference/contextualSignatureWithExtraParameters.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
//// [tests/cases/compiler/contextualSignatureWithExtraParameters.ts] ////

=== contextualSignatureWithExtraParameters.ts ===
// https://github.com/microsoft/TypeScript/issues/59309
function f1(
>f1 : Symbol(f1, Decl(contextualSignatureWithExtraParameters.ts, 0, 0))

cb: ((item: number) => void) | ((item: number, extra: string) => void),
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 1, 12))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 2, 10))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 2, 37))
>extra : Symbol(extra, Decl(contextualSignatureWithExtraParameters.ts, 2, 50))

) {}

f1((item) => {});
>f1 : Symbol(f1, Decl(contextualSignatureWithExtraParameters.ts, 0, 0))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 5, 4))

function f2<T>(
>f2 : Symbol(f2, Decl(contextualSignatureWithExtraParameters.ts, 5, 17))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 7, 12))

arr: T[],
>arr : Symbol(arr, Decl(contextualSignatureWithExtraParameters.ts, 7, 15))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 7, 12))

cb: ((item: T) => void) | ((item: T, extra: unknown) => void),
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 8, 13))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 9, 10))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 7, 12))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 9, 32))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 7, 12))
>extra : Symbol(extra, Decl(contextualSignatureWithExtraParameters.ts, 9, 40))

) {}

f2([1, 2, 3], (item) => {});
>f2 : Symbol(f2, Decl(contextualSignatureWithExtraParameters.ts, 5, 17))
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 12, 15))

export interface AsyncResultCallback<T, E = Error> {
>AsyncResultCallback : Symbol(AsyncResultCallback, Decl(contextualSignatureWithExtraParameters.ts, 12, 28))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 14, 37))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 14, 39))
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2022.error.d.ts, --, --))

(err?: E | null, result?: T): void;
>err : Symbol(err, Decl(contextualSignatureWithExtraParameters.ts, 15, 5))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 14, 39))
>result : Symbol(result, Decl(contextualSignatureWithExtraParameters.ts, 15, 20))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 14, 37))
}

export interface AsyncResultIterator<T, R, E = Error> {
>AsyncResultIterator : Symbol(AsyncResultIterator, Decl(contextualSignatureWithExtraParameters.ts, 16, 1))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 18, 37))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 18, 39))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 18, 42))
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2022.error.d.ts, --, --))

(item: T, callback: AsyncResultCallback<R, E>): void;
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 19, 5))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 18, 37))
>callback : Symbol(callback, Decl(contextualSignatureWithExtraParameters.ts, 19, 13))
>AsyncResultCallback : Symbol(AsyncResultCallback, Decl(contextualSignatureWithExtraParameters.ts, 12, 28))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 18, 39))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 18, 42))
}
export interface AsyncResultIteratorPromise<T, R> {
>AsyncResultIteratorPromise : Symbol(AsyncResultIteratorPromise, Decl(contextualSignatureWithExtraParameters.ts, 20, 1))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 21, 44))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 21, 46))

(item: T): Promise<R>;
>item : Symbol(item, Decl(contextualSignatureWithExtraParameters.ts, 22, 5))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 21, 44))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 21, 46))
}

declare function mapLimit<T, R, E = Error>(
>mapLimit : Symbol(mapLimit, Decl(contextualSignatureWithExtraParameters.ts, 23, 1))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 25, 26))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 25, 28))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 25, 31))
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es2022.error.d.ts, --, --))

arr: T[],
>arr : Symbol(arr, Decl(contextualSignatureWithExtraParameters.ts, 25, 43))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 25, 26))

limit: number,
>limit : Symbol(limit, Decl(contextualSignatureWithExtraParameters.ts, 26, 13))

iterator: AsyncResultIteratorPromise<T, R> | AsyncResultIterator<T, R, E>,
>iterator : Symbol(iterator, Decl(contextualSignatureWithExtraParameters.ts, 27, 18))
>AsyncResultIteratorPromise : Symbol(AsyncResultIteratorPromise, Decl(contextualSignatureWithExtraParameters.ts, 20, 1))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 25, 26))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 25, 28))
>AsyncResultIterator : Symbol(AsyncResultIterator, Decl(contextualSignatureWithExtraParameters.ts, 16, 1))
>T : Symbol(T, Decl(contextualSignatureWithExtraParameters.ts, 25, 26))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 25, 28))
>E : Symbol(E, Decl(contextualSignatureWithExtraParameters.ts, 25, 31))

): Promise<R[]>;
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))
>R : Symbol(R, Decl(contextualSignatureWithExtraParameters.ts, 25, 28))

mapLimit([1,2,3], 3, async (n) => {
>mapLimit : Symbol(mapLimit, Decl(contextualSignatureWithExtraParameters.ts, 23, 1))
>n : Symbol(n, Decl(contextualSignatureWithExtraParameters.ts, 31, 28))

return n ** 2;
>n : Symbol(n, Decl(contextualSignatureWithExtraParameters.ts, 31, 28))

});

function f3(
>f3 : Symbol(f3, Decl(contextualSignatureWithExtraParameters.ts, 33, 3))

cb: ((a: string, b?: string) => void) | ((a: string, ...rest: string[]) => void)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 35, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 36, 10))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 36, 20))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 36, 46))
>rest : Symbol(rest, Decl(contextualSignatureWithExtraParameters.ts, 36, 56))

) {}

f3((a) => {});
>f3 : Symbol(f3, Decl(contextualSignatureWithExtraParameters.ts, 33, 3))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 39, 4))

f3((a, b) => {});
>f3 : Symbol(f3, Decl(contextualSignatureWithExtraParameters.ts, 33, 3))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 40, 4))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 40, 6))

function f4(
>f4 : Symbol(f4, Decl(contextualSignatureWithExtraParameters.ts, 40, 17))

cb: ((a: string, b: string) => void) | ((a: string, ...rest: string[]) => void)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 42, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 43, 10))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 43, 20))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 43, 45))
>rest : Symbol(rest, Decl(contextualSignatureWithExtraParameters.ts, 43, 55))

) {}

f4((a) => {});
>f4 : Symbol(f4, Decl(contextualSignatureWithExtraParameters.ts, 40, 17))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 46, 4))

f4((a, b) => {});
>f4 : Symbol(f4, Decl(contextualSignatureWithExtraParameters.ts, 40, 17))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 47, 4))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 47, 6))

function f5(
>f5 : Symbol(f5, Decl(contextualSignatureWithExtraParameters.ts, 47, 17))

cb: ((a: string, b: string) => void) | ((...rest: string[]) => void)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 49, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 50, 10))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 50, 20))
>rest : Symbol(rest, Decl(contextualSignatureWithExtraParameters.ts, 50, 45))

) {}

f5((a) => {});
>f5 : Symbol(f5, Decl(contextualSignatureWithExtraParameters.ts, 47, 17))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 53, 4))

f5((a, b) => {});
>f5 : Symbol(f5, Decl(contextualSignatureWithExtraParameters.ts, 47, 17))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 54, 4))
>b : Symbol(b, Decl(contextualSignatureWithExtraParameters.ts, 54, 6))

function f6(
>f6 : Symbol(f6, Decl(contextualSignatureWithExtraParameters.ts, 54, 17))

cb: ((a: string) => string) | ((a: "a" | "b") => Promise<"a" | "b">)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 56, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 57, 8))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 57, 34))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.promise.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.es2018.promise.d.ts, --, --))

) {}

f6(async (a) => a);
>f6 : Symbol(f6, Decl(contextualSignatureWithExtraParameters.ts, 54, 17))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 60, 10))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 60, 10))

function f7(
>f7 : Symbol(f7, Decl(contextualSignatureWithExtraParameters.ts, 60, 19))

cb: ((a: string) => string) | ((a: number) => Generator<number, void, unknown>)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 62, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 63, 8))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 63, 34))
>Generator : Symbol(Generator, Decl(lib.es2015.generator.d.ts, --, --))

) {}

f7(function* generator(a) {yield 0});
>f7 : Symbol(f7, Decl(contextualSignatureWithExtraParameters.ts, 60, 19))
>generator : Symbol(generator, Decl(contextualSignatureWithExtraParameters.ts, 66, 3))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 66, 23))

function f8(
>f8 : Symbol(f8, Decl(contextualSignatureWithExtraParameters.ts, 66, 37))

cb: ((a: string) => string) | ((a: number) => AsyncGenerator<number, void, unknown>)
>cb : Symbol(cb, Decl(contextualSignatureWithExtraParameters.ts, 68, 12))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 69, 8))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 69, 34))
>AsyncGenerator : Symbol(AsyncGenerator, Decl(lib.es2018.asyncgenerator.d.ts, --, --))

) {}

f8(async function* generator(a) {yield 0});
>f8 : Symbol(f8, Decl(contextualSignatureWithExtraParameters.ts, 66, 37))
>generator : Symbol(generator, Decl(contextualSignatureWithExtraParameters.ts, 72, 3))
>a : Symbol(a, Decl(contextualSignatureWithExtraParameters.ts, 72, 29))


Loading