From b0a64446d18d1d901432a52d1177328bc19fe66e Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Tue, 10 Oct 2017 13:34:57 -0700 Subject: [PATCH 1/2] Track recursive calls to compareSignaturesRelated Previously, `compareSignaturesRelated` would try to relate (parameters of) callback parameters covariantly without checking whether the two signatures were already being checked. This led to an infinite recursion when checking recursive types with callbacks: ```ts interface Foo { (bar: Bar): void } type Bar = (foo: Foo) => Foo declare function foo(bar: Bar): void declare var bar: Bar<{}> bar = foo ``` The fix is to track which callbacks are already being compared. The mechanism is per-call to compareSignaturesRelated, so it only stops recursions on the callback-covariance code path. However, the maybe cache check in recursiveTypeRelatedTo will catch infinite recursions on the usual code path through isRelatedTo. Fixes #18277 --- src/compiler/checker.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 58ac3bc52c0bc..1ddb234c7dfcd 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8521,7 +8521,7 @@ namespace ts { target: Signature, ignoreReturnTypes: boolean): boolean { return compareSignaturesRelated(source, target, CallbackCheck.None, ignoreReturnTypes, /*reportErrors*/ false, - /*errorReporter*/ undefined, compareTypesAssignable) !== Ternary.False; + /*errorReporter*/ undefined, compareTypesAssignable, createMap()) !== Ternary.False; } type ErrorReporter = (message: DiagnosticMessage, arg0?: string, arg1?: string) => void; @@ -8535,7 +8535,8 @@ namespace ts { ignoreReturnTypes: boolean, reportErrors: boolean, errorReporter: ErrorReporter, - compareTypes: TypeComparer): Ternary { + compareTypes: TypeComparer, + previousCallbacks: Map): Ternary { // TODO (drosen): De-duplicate code between related functions. if (source === target) { return Ternary.True; @@ -8589,10 +8590,19 @@ namespace ts { // similar to return values, callback parameters are output positions. This means that a Promise, // where T is used only in callback parameter positions, will be co-variant (as opposed to bi-variant) // with respect to T. - const callbacks = sourceSig && targetSig && !sourceSig.typePredicate && !targetSig.typePredicate && + let callbacks = sourceSig && targetSig && !sourceSig.typePredicate && !targetSig.typePredicate && (getFalsyFlags(sourceType) & TypeFlags.Nullable) === (getFalsyFlags(targetType) & TypeFlags.Nullable); + if (callbacks) { + const key = getRelationKey(sourceType, targetType, /*relation*/ undefined); + if (previousCallbacks.has(key)) { + callbacks = false; + } + else { + previousCallbacks.set(key, true); + } + } const related = callbacks ? - compareSignaturesRelated(targetSig, sourceSig, strictVariance ? CallbackCheck.Strict : CallbackCheck.Bivariant, /*ignoreReturnTypes*/ false, reportErrors, errorReporter, compareTypes) : + compareSignaturesRelated(targetSig, sourceSig, strictVariance ? CallbackCheck.Strict : CallbackCheck.Bivariant, /*ignoreReturnTypes*/ false, reportErrors, errorReporter, compareTypes, previousCallbacks) : !callbackCheck && !strictVariance && compareTypes(sourceType, targetType, /*reportErrors*/ false) || compareTypes(targetType, sourceType, reportErrors); if (!related) { if (reportErrors) { @@ -9736,7 +9746,7 @@ namespace ts { */ function signatureRelatedTo(source: Signature, target: Signature, erase: boolean, reportErrors: boolean): Ternary { return compareSignaturesRelated(erase ? getErasedSignature(source) : source, erase ? getErasedSignature(target) : target, - CallbackCheck.None, /*ignoreReturnTypes*/ false, reportErrors, reportError, isRelatedTo); + CallbackCheck.None, /*ignoreReturnTypes*/ false, reportErrors, reportError, isRelatedTo, createMap()); } function signaturesIdenticalTo(source: Type, target: Type, kind: SignatureKind): Ternary { From d93542253fcfa27a4da1ba3a2d625bbd3dd0cab1 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Tue, 10 Oct 2017 13:59:00 -0700 Subject: [PATCH 2/2] Test fix #18277:mutually recursive callbacks --- .../mutuallyRecursiveCallbacks.errors.txt | 35 +++++++++++++++++++ .../reference/mutuallyRecursiveCallbacks.js | 12 +++++++ .../mutuallyRecursiveCallbacks.symbols | 33 +++++++++++++++++ .../mutuallyRecursiveCallbacks.types | 34 ++++++++++++++++++ .../compiler/mutuallyRecursiveCallbacks.ts | 6 ++++ 5 files changed, 120 insertions(+) create mode 100644 tests/baselines/reference/mutuallyRecursiveCallbacks.errors.txt create mode 100644 tests/baselines/reference/mutuallyRecursiveCallbacks.js create mode 100644 tests/baselines/reference/mutuallyRecursiveCallbacks.symbols create mode 100644 tests/baselines/reference/mutuallyRecursiveCallbacks.types create mode 100644 tests/cases/compiler/mutuallyRecursiveCallbacks.ts diff --git a/tests/baselines/reference/mutuallyRecursiveCallbacks.errors.txt b/tests/baselines/reference/mutuallyRecursiveCallbacks.errors.txt new file mode 100644 index 0000000000000..c9e7f651366c6 --- /dev/null +++ b/tests/baselines/reference/mutuallyRecursiveCallbacks.errors.txt @@ -0,0 +1,35 @@ +tests/cases/compiler/mutuallyRecursiveCallbacks.ts(6,1): error TS2322: Type '(bar: Bar) => void' is not assignable to type 'Bar<{}>'. + Types of parameters 'bar' and 'foo' are incompatible. + Types of parameters 'bar' and 'foo' are incompatible. + Types of parameters 'bar' and 'foo' are incompatible. + Type 'Foo<{}>' is not assignable to type 'Bar<{}>'. + Types of parameters 'bar' and 'foo' are incompatible. + Types of parameters 'bar' and 'foo' are incompatible. + Types of parameters 'bar' and 'foo' are incompatible. + Type 'Foo<{}>' is not assignable to type 'Bar<{}>'. + Types of parameters 'bar' and 'foo' are incompatible. + Types of parameters 'bar' and 'foo' are incompatible. + Type 'void' is not assignable to type 'Foo<{}>'. + + +==== tests/cases/compiler/mutuallyRecursiveCallbacks.ts (1 errors) ==== + // #18277 + interface Foo { (bar: Bar): void }; + type Bar = (foo: Foo) => Foo; + declare function foo(bar: Bar): void; + declare var bar: Bar<{}>; + bar = foo; + ~~~ +!!! error TS2322: Type '(bar: Bar) => void' is not assignable to type 'Bar<{}>'. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Type 'Foo<{}>' is not assignable to type 'Bar<{}>'. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Type 'Foo<{}>' is not assignable to type 'Bar<{}>'. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Types of parameters 'bar' and 'foo' are incompatible. +!!! error TS2322: Type 'void' is not assignable to type 'Foo<{}>'. + \ No newline at end of file diff --git a/tests/baselines/reference/mutuallyRecursiveCallbacks.js b/tests/baselines/reference/mutuallyRecursiveCallbacks.js new file mode 100644 index 0000000000000..dc581fde6e1bc --- /dev/null +++ b/tests/baselines/reference/mutuallyRecursiveCallbacks.js @@ -0,0 +1,12 @@ +//// [mutuallyRecursiveCallbacks.ts] +// #18277 +interface Foo { (bar: Bar): void }; +type Bar = (foo: Foo) => Foo; +declare function foo(bar: Bar): void; +declare var bar: Bar<{}>; +bar = foo; + + +//// [mutuallyRecursiveCallbacks.js] +; +bar = foo; diff --git a/tests/baselines/reference/mutuallyRecursiveCallbacks.symbols b/tests/baselines/reference/mutuallyRecursiveCallbacks.symbols new file mode 100644 index 0000000000000..fbf1db7d0aa74 --- /dev/null +++ b/tests/baselines/reference/mutuallyRecursiveCallbacks.symbols @@ -0,0 +1,33 @@ +=== tests/cases/compiler/mutuallyRecursiveCallbacks.ts === +// #18277 +interface Foo { (bar: Bar): void }; +>Foo : Symbol(Foo, Decl(mutuallyRecursiveCallbacks.ts, 0, 0)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 1, 14)) +>bar : Symbol(bar, Decl(mutuallyRecursiveCallbacks.ts, 1, 20)) +>Bar : Symbol(Bar, Decl(mutuallyRecursiveCallbacks.ts, 1, 41)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 1, 14)) + +type Bar = (foo: Foo) => Foo; +>Bar : Symbol(Bar, Decl(mutuallyRecursiveCallbacks.ts, 1, 41)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 2, 9)) +>foo : Symbol(foo, Decl(mutuallyRecursiveCallbacks.ts, 2, 15)) +>Foo : Symbol(Foo, Decl(mutuallyRecursiveCallbacks.ts, 0, 0)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 2, 9)) +>Foo : Symbol(Foo, Decl(mutuallyRecursiveCallbacks.ts, 0, 0)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 2, 9)) + +declare function foo(bar: Bar): void; +>foo : Symbol(foo, Decl(mutuallyRecursiveCallbacks.ts, 2, 38)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 3, 21)) +>bar : Symbol(bar, Decl(mutuallyRecursiveCallbacks.ts, 3, 24)) +>Bar : Symbol(Bar, Decl(mutuallyRecursiveCallbacks.ts, 1, 41)) +>T : Symbol(T, Decl(mutuallyRecursiveCallbacks.ts, 3, 21)) + +declare var bar: Bar<{}>; +>bar : Symbol(bar, Decl(mutuallyRecursiveCallbacks.ts, 4, 11)) +>Bar : Symbol(Bar, Decl(mutuallyRecursiveCallbacks.ts, 1, 41)) + +bar = foo; +>bar : Symbol(bar, Decl(mutuallyRecursiveCallbacks.ts, 4, 11)) +>foo : Symbol(foo, Decl(mutuallyRecursiveCallbacks.ts, 2, 38)) + diff --git a/tests/baselines/reference/mutuallyRecursiveCallbacks.types b/tests/baselines/reference/mutuallyRecursiveCallbacks.types new file mode 100644 index 0000000000000..978aa736216c7 --- /dev/null +++ b/tests/baselines/reference/mutuallyRecursiveCallbacks.types @@ -0,0 +1,34 @@ +=== tests/cases/compiler/mutuallyRecursiveCallbacks.ts === +// #18277 +interface Foo { (bar: Bar): void }; +>Foo : Foo +>T : T +>bar : Bar +>Bar : Bar +>T : T + +type Bar = (foo: Foo) => Foo; +>Bar : Bar +>T : T +>foo : Foo +>Foo : Foo +>T : T +>Foo : Foo +>T : T + +declare function foo(bar: Bar): void; +>foo : (bar: Bar) => void +>T : T +>bar : Bar +>Bar : Bar +>T : T + +declare var bar: Bar<{}>; +>bar : Bar<{}> +>Bar : Bar + +bar = foo; +>bar = foo : (bar: Bar) => void +>bar : Bar<{}> +>foo : (bar: Bar) => void + diff --git a/tests/cases/compiler/mutuallyRecursiveCallbacks.ts b/tests/cases/compiler/mutuallyRecursiveCallbacks.ts new file mode 100644 index 0000000000000..52ee68942b0ee --- /dev/null +++ b/tests/cases/compiler/mutuallyRecursiveCallbacks.ts @@ -0,0 +1,6 @@ +// #18277 +interface Foo { (bar: Bar): void }; +type Bar = (foo: Foo) => Foo; +declare function foo(bar: Bar): void; +declare var bar: Bar<{}>; +bar = foo;