Skip to content

Strictly check callback parameters #18976

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

Merged
merged 6 commits into from
Oct 6, 2017
Merged

Strictly check callback parameters #18976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c3d1b02
Don't use callback parameter code path when strictly checking functions
ahejlsberg Oct 5, 2017
0dc1c77
Accept new baselines
ahejlsberg Oct 5, 2017
aae7572
Add test
ahejlsberg Oct 5, 2017
70e259a
Always use callback parameter code path, but stricter if necessary
ahejlsberg Oct 6, 2017
c8d5260
Accept new baselines
ahejlsberg Oct 6, 2017
7fcf519
Add tests
ahejlsberg Oct 6, 2017
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
20 changes: 13 additions & 7 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,12 @@ namespace ts {
Inferential = 2, // Inferential typing
}

const enum CallbackCheck {
None,
Bivariant,
Strict,
}

const builtinGlobals = createSymbolTable();
builtinGlobals.set(undefinedSymbol.escapedName, undefinedSymbol);

Expand Down Expand Up @@ -8510,7 +8516,7 @@ namespace ts {
function isSignatureAssignableTo(source: Signature,
target: Signature,
ignoreReturnTypes: boolean): boolean {
return compareSignaturesRelated(source, target, /*checkAsCallback*/ false, ignoreReturnTypes, /*reportErrors*/ false,
return compareSignaturesRelated(source, target, CallbackCheck.None, ignoreReturnTypes, /*reportErrors*/ false,
/*errorReporter*/ undefined, compareTypesAssignable) !== Ternary.False;
}

Expand All @@ -8521,7 +8527,7 @@ namespace ts {
*/
function compareSignaturesRelated(source: Signature,
target: Signature,
checkAsCallback: boolean,
callbackCheck: CallbackCheck,
ignoreReturnTypes: boolean,
reportErrors: boolean,
errorReporter: ErrorReporter,
Expand All @@ -8540,7 +8546,7 @@ namespace ts {
}

const kind = target.declaration ? target.declaration.kind : SyntaxKind.Unknown;
const strictVariance = strictFunctionTypes && kind !== SyntaxKind.MethodDeclaration &&
const strictVariance = !callbackCheck && strictFunctionTypes && kind !== SyntaxKind.MethodDeclaration &&
kind !== SyntaxKind.MethodSignature && kind !== SyntaxKind.Constructor;
let result = Ternary.True;

Expand Down Expand Up @@ -8582,8 +8588,8 @@ namespace ts {
const callbacks = sourceSig && targetSig && !sourceSig.typePredicate && !targetSig.typePredicate &&
(getFalsyFlags(sourceType) & TypeFlags.Nullable) === (getFalsyFlags(targetType) & TypeFlags.Nullable);
const related = callbacks ?
compareSignaturesRelated(targetSig, sourceSig, /*checkAsCallback*/ true, /*ignoreReturnTypes*/ false, reportErrors, errorReporter, compareTypes) :
!checkAsCallback && !strictVariance && compareTypes(sourceType, targetType, /*reportErrors*/ false) || compareTypes(targetType, sourceType, reportErrors);
compareSignaturesRelated(targetSig, sourceSig, strictVariance ? CallbackCheck.Strict : CallbackCheck.Bivariant, /*ignoreReturnTypes*/ false, reportErrors, errorReporter, compareTypes) :
!callbackCheck && !strictVariance && compareTypes(sourceType, targetType, /*reportErrors*/ false) || compareTypes(targetType, sourceType, reportErrors);
if (!related) {
if (reportErrors) {
errorReporter(Diagnostics.Types_of_parameters_0_and_1_are_incompatible,
Expand Down Expand Up @@ -8618,7 +8624,7 @@ namespace ts {
// When relating callback signatures, we still need to relate return types bi-variantly as otherwise
// the containing type wouldn't be co-variant. For example, interface Foo<T> { add(cb: () => T): void }
// wouldn't be co-variant for T without this rule.
result &= checkAsCallback && compareTypes(targetReturnType, sourceReturnType, /*reportErrors*/ false) ||
result &= callbackCheck === CallbackCheck.Bivariant && compareTypes(targetReturnType, sourceReturnType, /*reportErrors*/ false) ||
compareTypes(sourceReturnType, targetReturnType, reportErrors);
}

Expand Down Expand Up @@ -9715,7 +9721,7 @@ namespace ts {
*/
function signatureRelatedTo(source: Signature, target: Signature, erase: boolean, reportErrors: boolean): Ternary {
return compareSignaturesRelated(erase ? getErasedSignature(source) : source, erase ? getErasedSignature(target) : target,
/*checkAsCallback*/ false, /*ignoreReturnTypes*/ false, reportErrors, reportError, isRelatedTo);
CallbackCheck.None, /*ignoreReturnTypes*/ false, reportErrors, reportError, isRelatedTo);
}

function signaturesIdenticalTo(source: Type, target: Type, kind: SignatureKind): Ternary {
Expand Down
66 changes: 64 additions & 2 deletions tests/baselines/reference/strictFunctionTypesErrors.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,24 @@ tests/cases/compiler/strictFunctionTypesErrors.ts(126,1): error TS2322: Type 'Cr
tests/cases/compiler/strictFunctionTypesErrors.ts(127,1): error TS2322: Type 'Crate<Animal>' is not assignable to type 'Crate<Dog>'.
Types of property 'item' are incompatible.
Type 'Animal' is not assignable to type 'Dog'.
tests/cases/compiler/strictFunctionTypesErrors.ts(133,1): error TS2322: Type '(f: (x: Dog) => Dog) => void' is not assignable to type '(f: (x: Animal) => Animal) => void'.
Types of parameters 'f' and 'f' are incompatible.
Type 'Animal' is not assignable to type 'Dog'.
tests/cases/compiler/strictFunctionTypesErrors.ts(134,1): error TS2322: Type '(f: (x: Animal) => Animal) => void' is not assignable to type '(f: (x: Dog) => Dog) => void'.
Types of parameters 'f' and 'f' are incompatible.
Types of parameters 'x' and 'x' are incompatible.
Type 'Animal' is not assignable to type 'Dog'.
tests/cases/compiler/strictFunctionTypesErrors.ts(147,5): error TS2322: Type '(cb: (x: Animal) => Animal) => void' is not assignable to type '(cb: (x: Dog) => Animal) => void'.
Types of parameters 'cb' and 'cb' are incompatible.
Types of parameters 'x' and 'x' are incompatible.
Type 'Animal' is not assignable to type 'Dog'.
tests/cases/compiler/strictFunctionTypesErrors.ts(155,5): error TS2322: Type '(cb: (x: Animal) => Animal) => void' is not assignable to type '(cb: (x: Dog) => Animal) => void'.
Types of parameters 'cb' and 'cb' are incompatible.
Types of parameters 'x' and 'x' are incompatible.
Type 'Animal' is not assignable to type 'Dog'.


==== tests/cases/compiler/strictFunctionTypesErrors.ts (31 errors) ====
==== tests/cases/compiler/strictFunctionTypesErrors.ts (35 errors) ====
export {}


Expand Down Expand Up @@ -337,4 +352,51 @@ tests/cases/compiler/strictFunctionTypesErrors.ts(127,1): error TS2322: Type 'Cr
!!! error TS2322: Type 'Crate<Animal>' is not assignable to type 'Crate<Dog>'.
!!! error TS2322: Types of property 'item' are incompatible.
!!! error TS2322: Type 'Animal' is not assignable to type 'Dog'.


// Verify that callback parameters are strictly checked

declare let fc1: (f: (x: Animal) => Animal) => void;
declare let fc2: (f: (x: Dog) => Dog) => void;
fc1 = fc2; // Error
~~~
!!! error TS2322: Type '(f: (x: Dog) => Dog) => void' is not assignable to type '(f: (x: Animal) => Animal) => void'.
!!! error TS2322: Types of parameters 'f' and 'f' are incompatible.
!!! error TS2322: Type 'Animal' is not assignable to type 'Dog'.
fc2 = fc1; // Error
~~~
!!! error TS2322: Type '(f: (x: Animal) => Animal) => void' is not assignable to type '(f: (x: Dog) => Dog) => void'.
!!! error TS2322: Types of parameters 'f' and 'f' are incompatible.
!!! error TS2322: Types of parameters 'x' and 'x' are incompatible.
!!! error TS2322: Type 'Animal' is not assignable to type 'Dog'.

// Verify that callback parameters aren't loosely checked when types
// originate in method declarations

namespace n1 {
class Foo {
static f1(x: Animal): Animal { throw "wat"; }
static f2(x: Dog): Animal { throw "wat"; };
}
declare let f1: (cb: typeof Foo.f1) => void;
declare let f2: (cb: typeof Foo.f2) => void;
f1 = f2;
f2 = f1; // Error
~~
!!! error TS2322: Type '(cb: (x: Animal) => Animal) => void' is not assignable to type '(cb: (x: Dog) => Animal) => void'.
!!! error TS2322: Types of parameters 'cb' and 'cb' are incompatible.
!!! error TS2322: Types of parameters 'x' and 'x' are incompatible.
!!! error TS2322: Type 'Animal' is not assignable to type 'Dog'.
}

namespace n2 {
type BivariantHack<Input, Output> = { foo(x: Input): Output }["foo"];
declare let f1: (cb: BivariantHack<Animal, Animal>) => void;
declare let f2: (cb: BivariantHack<Dog, Animal>) => void;
f1 = f2;
f2 = f1; // Error
~~
!!! error TS2322: Type '(cb: (x: Animal) => Animal) => void' is not assignable to type '(cb: (x: Dog) => Animal) => void'.
!!! error TS2322: Types of parameters 'cb' and 'cb' are incompatible.
!!! error TS2322: Types of parameters 'x' and 'x' are incompatible.
!!! error TS2322: Type 'Animal' is not assignable to type 'Dog'.
}
52 changes: 51 additions & 1 deletion tests/baselines/reference/strictFunctionTypesErrors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,35 @@ declare let dogCrate: Crate<Dog>;

animalCrate = dogCrate; // Error
dogCrate = animalCrate; // Error


// Verify that callback parameters are strictly checked

declare let fc1: (f: (x: Animal) => Animal) => void;
declare let fc2: (f: (x: Dog) => Dog) => void;
fc1 = fc2; // Error
fc2 = fc1; // Error

// Verify that callback parameters aren't loosely checked when types
// originate in method declarations

namespace n1 {
class Foo {
static f1(x: Animal): Animal { throw "wat"; }
static f2(x: Dog): Animal { throw "wat"; };
}
declare let f1: (cb: typeof Foo.f1) => void;
declare let f2: (cb: typeof Foo.f2) => void;
f1 = f2;
f2 = f1; // Error
}

namespace n2 {
type BivariantHack<Input, Output> = { foo(x: Input): Output }["foo"];
declare let f1: (cb: BivariantHack<Animal, Animal>) => void;
declare let f2: (cb: BivariantHack<Dog, Animal>) => void;
f1 = f2;
f2 = f1; // Error
}

//// [strictFunctionTypesErrors.js]
"use strict";
Expand Down Expand Up @@ -186,3 +214,25 @@ dogComparer2 = animalComparer2; // Ok
// Errors below should elaborate the reason for invariance
animalCrate = dogCrate; // Error
dogCrate = animalCrate; // Error
fc1 = fc2; // Error
fc2 = fc1; // Error
// Verify that callback parameters aren't loosely checked when types
// originate in method declarations
var n1;
(function (n1) {
var Foo = /** @class */ (function () {
function Foo() {
}
Foo.f1 = function (x) { throw "wat"; };
Foo.f2 = function (x) { throw "wat"; };
;
return Foo;
}());
f1 = f2;
f2 = f1; // Error
})(n1 || (n1 = {}));
var n2;
(function (n2) {
f1 = f2;
f2 = f1; // Error
})(n2 || (n2 = {}));
102 changes: 102 additions & 0 deletions tests/baselines/reference/strictFunctionTypesErrors.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,3 +400,105 @@ dogCrate = animalCrate; // Error
>dogCrate : Symbol(dogCrate, Decl(strictFunctionTypesErrors.ts, 121, 11))
>animalCrate : Symbol(animalCrate, Decl(strictFunctionTypesErrors.ts, 120, 11))

// Verify that callback parameters are strictly checked

declare let fc1: (f: (x: Animal) => Animal) => void;
>fc1 : Symbol(fc1, Decl(strictFunctionTypesErrors.ts, 130, 11))
>f : Symbol(f, Decl(strictFunctionTypesErrors.ts, 130, 18))
>x : Symbol(x, Decl(strictFunctionTypesErrors.ts, 130, 22))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))

declare let fc2: (f: (x: Dog) => Dog) => void;
>fc2 : Symbol(fc2, Decl(strictFunctionTypesErrors.ts, 131, 11))
>f : Symbol(f, Decl(strictFunctionTypesErrors.ts, 131, 18))
>x : Symbol(x, Decl(strictFunctionTypesErrors.ts, 131, 22))
>Dog : Symbol(Dog, Decl(strictFunctionTypesErrors.ts, 89, 33))
>Dog : Symbol(Dog, Decl(strictFunctionTypesErrors.ts, 89, 33))

fc1 = fc2; // Error
>fc1 : Symbol(fc1, Decl(strictFunctionTypesErrors.ts, 130, 11))
>fc2 : Symbol(fc2, Decl(strictFunctionTypesErrors.ts, 131, 11))

fc2 = fc1; // Error
>fc2 : Symbol(fc2, Decl(strictFunctionTypesErrors.ts, 131, 11))
>fc1 : Symbol(fc1, Decl(strictFunctionTypesErrors.ts, 130, 11))

// Verify that callback parameters aren't loosely checked when types
// originate in method declarations

namespace n1 {
>n1 : Symbol(n1, Decl(strictFunctionTypesErrors.ts, 133, 10))

class Foo {
>Foo : Symbol(Foo, Decl(strictFunctionTypesErrors.ts, 138, 14))

static f1(x: Animal): Animal { throw "wat"; }
>f1 : Symbol(Foo.f1, Decl(strictFunctionTypesErrors.ts, 139, 15))
>x : Symbol(x, Decl(strictFunctionTypesErrors.ts, 140, 18))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))

static f2(x: Dog): Animal { throw "wat"; };
>f2 : Symbol(Foo.f2, Decl(strictFunctionTypesErrors.ts, 140, 53))
>x : Symbol(x, Decl(strictFunctionTypesErrors.ts, 141, 18))
>Dog : Symbol(Dog, Decl(strictFunctionTypesErrors.ts, 89, 33))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))
}
declare let f1: (cb: typeof Foo.f1) => void;
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 143, 15))
>cb : Symbol(cb, Decl(strictFunctionTypesErrors.ts, 143, 21))
>Foo.f1 : Symbol(Foo.f1, Decl(strictFunctionTypesErrors.ts, 139, 15))
>Foo : Symbol(Foo, Decl(strictFunctionTypesErrors.ts, 138, 14))
>f1 : Symbol(Foo.f1, Decl(strictFunctionTypesErrors.ts, 139, 15))

declare let f2: (cb: typeof Foo.f2) => void;
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 144, 15))
>cb : Symbol(cb, Decl(strictFunctionTypesErrors.ts, 144, 21))
>Foo.f2 : Symbol(Foo.f2, Decl(strictFunctionTypesErrors.ts, 140, 53))
>Foo : Symbol(Foo, Decl(strictFunctionTypesErrors.ts, 138, 14))
>f2 : Symbol(Foo.f2, Decl(strictFunctionTypesErrors.ts, 140, 53))

f1 = f2;
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 143, 15))
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 144, 15))

f2 = f1; // Error
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 144, 15))
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 143, 15))
}

namespace n2 {
>n2 : Symbol(n2, Decl(strictFunctionTypesErrors.ts, 147, 1))

type BivariantHack<Input, Output> = { foo(x: Input): Output }["foo"];
>BivariantHack : Symbol(BivariantHack, Decl(strictFunctionTypesErrors.ts, 149, 14))
>Input : Symbol(Input, Decl(strictFunctionTypesErrors.ts, 150, 23))
>Output : Symbol(Output, Decl(strictFunctionTypesErrors.ts, 150, 29))
>foo : Symbol(foo, Decl(strictFunctionTypesErrors.ts, 150, 41))
>x : Symbol(x, Decl(strictFunctionTypesErrors.ts, 150, 46))
>Input : Symbol(Input, Decl(strictFunctionTypesErrors.ts, 150, 23))
>Output : Symbol(Output, Decl(strictFunctionTypesErrors.ts, 150, 29))

declare let f1: (cb: BivariantHack<Animal, Animal>) => void;
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 151, 15))
>cb : Symbol(cb, Decl(strictFunctionTypesErrors.ts, 151, 21))
>BivariantHack : Symbol(BivariantHack, Decl(strictFunctionTypesErrors.ts, 149, 14))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))

declare let f2: (cb: BivariantHack<Dog, Animal>) => void;
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 152, 15))
>cb : Symbol(cb, Decl(strictFunctionTypesErrors.ts, 152, 21))
>BivariantHack : Symbol(BivariantHack, Decl(strictFunctionTypesErrors.ts, 149, 14))
>Dog : Symbol(Dog, Decl(strictFunctionTypesErrors.ts, 89, 33))
>Animal : Symbol(Animal, Decl(strictFunctionTypesErrors.ts, 87, 8))

f1 = f2;
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 151, 15))
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 152, 15))

f2 = f1; // Error
>f2 : Symbol(f2, Decl(strictFunctionTypesErrors.ts, 152, 15))
>f1 : Symbol(f1, Decl(strictFunctionTypesErrors.ts, 151, 15))
}
Loading