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.

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

Strict bind, call, and apply methods on functions #27028

Merged
merged 21 commits into from Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 27 additions & 13 deletions src/compiler/checker.ts
Expand Up @@ -77,6 +77,7 @@ namespace ts {
const allowSyntheticDefaultImports = getAllowSyntheticDefaultImports(compilerOptions);
const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
const strictFunctionTypes = getStrictOptionValue(compilerOptions, "strictFunctionTypes");
const strictBindCallApply = getStrictOptionValue(compilerOptions, "strictBindCallApply");
const strictPropertyInitialization = getStrictOptionValue(compilerOptions, "strictPropertyInitialization");
const noImplicitAny = getStrictOptionValue(compilerOptions, "noImplicitAny");
const noImplicitThis = getStrictOptionValue(compilerOptions, "noImplicitThis");
Expand Down Expand Up @@ -463,6 +464,8 @@ namespace ts {

let globalObjectType: ObjectType;
let globalFunctionType: ObjectType;
let globalCallableFunctionType: ObjectType;
let globalNewableFunctionType: ObjectType;
let globalArrayType: GenericType;
let globalReadonlyArrayType: GenericType;
let globalStringType: ObjectType;
Expand Down Expand Up @@ -7366,8 +7369,12 @@ namespace ts {
if (symbol && symbolIsValue(symbol)) {
return symbol;
}
if (resolved === anyFunctionType || resolved.callSignatures.length || resolved.constructSignatures.length) {
const symbol = getPropertyOfObjectType(globalFunctionType, name);
const functionType = resolved === anyFunctionType ? globalFunctionType :
Copy link
Member

Choose a reason for hiding this comment

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

This should be put into a function and anyFunctionType’s declaration needs a comment saying “Use this function instead”.

resolved.callSignatures.length ? globalCallableFunctionType :
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we prefer callable to newable if a given type is both?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I have a convincing argument for either preference. All I know is that it is very rare to have both and the two sets of signatures presumably align such that calling without new just causes the function to allocate an instance (as is the case with Array<T>, for example). I also know that having a combined list of overloads causes error reporting to be poor for one side because we always use the last arity-matching signature as the error exemplar.

Copy link
Member

Choose a reason for hiding this comment

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

Can we not use both sets of overloads for resolution, and if both fail, simply prefer one set or the other just when reporting errors?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not easily. We assume that all overloads are in a single list of signatures, so we'd somehow have to merge them or declare a third type (CallableNewableFunction) in lib.d.ts that has the combined sets of overloads.

resolved.constructSignatures.length ? globalNewableFunctionType :
undefined;
if (functionType) {
const symbol = getPropertyOfObjectType(functionType, name);
if (symbol) {
return symbol;
}
Expand Down Expand Up @@ -13752,15 +13759,17 @@ namespace ts {
if (!inferredType) {
const signature = context.signature;
if (signature) {
if (inference.contraCandidates && (!inference.candidates || inference.candidates.length === 1 && inference.candidates[0].flags & TypeFlags.Never)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The variance change is there to ensure we infer the most specific type possible when we have both co- and contra-variant candidates. Furthermore, knowing that an error will result when the co-variant inference is not a subtype of the contra-variant inference, we now prefer the contra-variant inference because it is likely to have come from an explicit type annotation on a function. This improves our error reporting.

// If we have contravariant inferences, but no covariant inferences or a single
// covariant inference of 'never', we find the best common subtype and treat that
// as a single covariant candidate.
inference.candidates = [getContravariantInference(inference)];
inference.contraCandidates = undefined;
}
if (inference.candidates) {
inferredType = getCovariantInference(inference, signature);
const inferredCovariantType = inference.candidates ? getCovariantInference(inference, signature) : undefined;
if (inference.contraCandidates) {
const inferredContravariantType = getContravariantInference(inference);
// If we have both co- and contra-variant inferences, we prefer the contra-variant inference
// unless the co-variant inference is a subtype and not 'never'.
inferredType = inferredCovariantType && !(inferredCovariantType.flags & TypeFlags.Never) &&
isTypeSubtypeOf(inferredCovariantType, inferredContravariantType) ?
inferredCovariantType : inferredContravariantType;
}
else if (inferredCovariantType) {
inferredType = inferredCovariantType;
}
else if (context.flags & InferenceFlags.NoDefault) {
// We use silentNeverType as the wildcard that signals no inferences.
Expand Down Expand Up @@ -27766,8 +27775,11 @@ namespace ts {
function getAugmentedPropertiesOfType(type: Type): Symbol[] {
type = getApparentType(type);
const propsByName = createSymbolTable(getPropertiesOfType(type));
if (typeHasCallOrConstructSignatures(type)) {
forEach(getPropertiesOfType(globalFunctionType), p => {
const functionType = getSignaturesOfType(type, SignatureKind.Call).length ? globalCallableFunctionType :
getSignaturesOfType(type, SignatureKind.Construct).length ? globalNewableFunctionType :
undefined;
if (functionType) {
forEach(getPropertiesOfType(functionType), p => {
if (!propsByName.has(p.escapedName)) {
propsByName.set(p.escapedName, p);
}
Expand Down Expand Up @@ -28543,6 +28555,8 @@ namespace ts {
globalArrayType = getGlobalType("Array" as __String, /*arity*/ 1, /*reportErrors*/ true);
globalObjectType = getGlobalType("Object" as __String, /*arity*/ 0, /*reportErrors*/ true);
globalFunctionType = getGlobalType("Function" as __String, /*arity*/ 0, /*reportErrors*/ true);
globalCallableFunctionType = strictBindCallApply && getGlobalType("CallableFunction" as __String, /*arity*/ 0, /*reportErrors*/ true) || globalFunctionType;
Copy link
Member

Choose a reason for hiding this comment

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

Comment here is also a good idea.

globalNewableFunctionType = strictBindCallApply && getGlobalType("NewableFunction" as __String, /*arity*/ 0, /*reportErrors*/ true) || globalFunctionType;
globalStringType = getGlobalType("String" as __String, /*arity*/ 0, /*reportErrors*/ true);
globalNumberType = getGlobalType("Number" as __String, /*arity*/ 0, /*reportErrors*/ true);
globalBooleanType = getGlobalType("Boolean" as __String, /*arity*/ 0, /*reportErrors*/ true);
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/commandLineParser.ts
Expand Up @@ -344,6 +344,14 @@ namespace ts {
category: Diagnostics.Strict_Type_Checking_Options,
description: Diagnostics.Enable_strict_checking_of_function_types
},
{
name: "strictBindCallApply",
type: "boolean",
strictFlag: true,
showInSimplifiedHelpView: true,
category: Diagnostics.Strict_Type_Checking_Options,
description: Diagnostics.Enable_strict_bind_call_and_apply_methods_on_functions
},
{
name: "strictPropertyInitialization",
type: "boolean",
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Expand Up @@ -3716,6 +3716,10 @@
"category": "Message",
"code": 6211
},
"Enable strict 'bind', 'call', and 'apply' methods on functions.": {
"category": "Message",
"code": 6212
},

"Projects to reference": {
"category": "Message",
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Expand Up @@ -4402,6 +4402,7 @@ namespace ts {
sourceRoot?: string;
strict?: boolean;
strictFunctionTypes?: boolean; // Always combine with strict property
strictBindCallApply?: boolean; // Always combine with strict property
strictNullChecks?: boolean; // Always combine with strict property
strictPropertyInitialization?: boolean; // Always combine with strict property
stripInternal?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Expand Up @@ -7094,7 +7094,7 @@ namespace ts {
return !!(compilerOptions.declaration || compilerOptions.composite);
}

export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "strictPropertyInitialization" | "alwaysStrict";
export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "strictBindCallApply" | "strictPropertyInitialization" | "alwaysStrict";

export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {
return compilerOptions[flag] === undefined ? !!compilerOptions.strict : !!compilerOptions[flag];
Expand Down
2 changes: 2 additions & 0 deletions src/harness/virtualFileSystemWithWatch.ts
Expand Up @@ -4,6 +4,8 @@ namespace ts.TestFSWithWatch {
content: `/// <reference no-default-lib="true"/>
interface Boolean {}
interface Function {}
interface CallableFunction {}
interface NewableFunction {}
interface IArguments {}
interface Number { toExponential: any; }
interface Object {}
Expand Down
58 changes: 58 additions & 0 deletions src/lib/es5.d.ts
Expand Up @@ -295,6 +295,64 @@ interface FunctionConstructor {

declare const Function: FunctionConstructor;

interface CallableFunction extends Function {
/**
* Calls the function with the specified object as the this value and the elements of specified array as the arguments.
* @param thisArg The object to be used as the this object.
* @param args An array of argument values to be passed to the function.
*/
apply<T, R>(this: (this: T) => R, thisArg: T): R;
apply<T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T, args: A): R;

/**
* Calls the function with the specified object as the this value and the specified rest arguments as the arguments.
* @param thisArg The object to be used as the this object.
* @param args Argument values to be passed to the function.
*/
call<T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T, ...args: A): R;

/**
* For a given function, creates a bound function that has the same body as the original function.
* The this object of the bound function is associated with the specified object, and has the specified initial parameters.
* @param thisArg The object to be used as the this object.
* @param args Arguments to bind to the parameters of the function.
*/
bind<T, A extends any[], R>(this: (this: T, ...args: A) => R, thisArg: T): (...args: A) => R;
bind<T, A0, A extends any[], R>(this: (this: T, arg0: A0, ...args: A) => R, thisArg: T, arg0: A0): (...args: A) => R;
bind<T, A0, A1, A extends any[], R>(this: (this: T, arg0: A0, arg1: A1, ...args: A) => R, thisArg: T, arg0: A0, arg1: A1): (...args: A) => R;
bind<T, A0, A1, A2, A extends any[], R>(this: (this: T, arg0: A0, arg1: A1, arg2: A2, ...args: A) => R, thisArg: T, arg0: A0, arg1: A1, arg2: A2): (...args: A) => R;
bind<T, A0, A1, A2, A3, A extends any[], R>(this: (this: T, arg0: A0, arg1: A1, arg2: A2, arg3: A3, ...args: A) => R, thisArg: T, arg0: A0, arg1: A1, arg2: A2, arg3: A3): (...args: A) => R;
}

interface NewableFunction extends Function {
/**
* Calls the function with the specified object as the this value and the elements of specified array as the arguments.
* @param thisArg The object to be used as the this object.
* @param args An array of argument values to be passed to the function.
*/
apply<T>(this: new () => T, thisArg: T): void;
apply<T, A extends any[]>(this: new (...args: A) => T, thisArg: T, args: A): void;

/**
* Calls the function with the specified object as the this value and the specified rest arguments as the arguments.
* @param thisArg The object to be used as the this object.
* @param args Argument values to be passed to the function.
*/
call<T, A extends any[]>(this: new (...args: A) => T, thisArg: T, ...args: A): void;

/**
* For a given function, creates a bound function that has the same body as the original function.
* The this object of the bound function is associated with the specified object, and has the specified initial parameters.
* @param thisArg The object to be used as the this object.
* @param args Arguments to bind to the parameters of the function.
*/
bind<A extends any[], R>(this: new (...args: A) => R, thisArg: any): new (...args: A) => R;
bind<A0, A extends any[], R>(this: new (arg0: A0, ...args: A) => R, thisArg: any, arg0: A0): new (...args: A) => R;
bind<A0, A1, A extends any[], R>(this: new (arg0: A0, arg1: A1, ...args: A) => R, thisArg: any, arg0: A0, arg1: A1): new (...args: A) => R;
bind<A0, A1, A2, A extends any[], R>(this: new (arg0: A0, arg1: A1, arg2: A2, ...args: A) => R, thisArg: any, arg0: A0, arg1: A1, arg2: A2): new (...args: A) => R;
bind<A0, A1, A2, A3, A extends any[], R>(this: new (arg0: A0, arg1: A1, arg2: A2, arg3: A3, ...args: A) => R, thisArg: any, arg0: A0, arg1: A1, arg2: A2, arg3: A3): new (...args: A) => R;
}

interface IArguments {
[index: number]: any;
length: number;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Expand Up @@ -2493,6 +2493,7 @@ declare namespace ts {
sourceRoot?: string;
strict?: boolean;
strictFunctionTypes?: boolean;
strictBindCallApply?: boolean;
strictNullChecks?: boolean;
strictPropertyInitialization?: boolean;
stripInternal?: boolean;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
Expand Up @@ -2493,6 +2493,7 @@ declare namespace ts {
sourceRoot?: string;
strict?: boolean;
strictFunctionTypes?: boolean;
strictBindCallApply?: boolean;
strictNullChecks?: boolean;
strictPropertyInitialization?: boolean;
stripInternal?: boolean;
Expand Down
@@ -0,0 +1,31 @@
error TS2318: Cannot find global type 'CallableFunction'.
error TS2318: Cannot find global type 'NewableFunction'.


!!! error TS2318: Cannot find global type 'CallableFunction'.
Copy link
Member

Choose a reason for hiding this comment

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

Probably shouldn’t get an error here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to update the private lib.d.ts used by the tests as I suspect that'll cause some really noisy baseline changes.

Copy link
Member

Choose a reason for hiding this comment

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

OK, just wanted to make sure this error wouldn't show up in some public-facing scenario.

!!! error TS2318: Cannot find global type 'NewableFunction'.
==== tests/cases/compiler/booleanLiteralsContextuallyTypedFromUnion.tsx (0 errors) ====
interface A { isIt: true; text: string; }
interface B { isIt: false; value: number; }
type C = A | B;
const isIt = Math.random() > 0.5;
const c: C = isIt ? { isIt, text: 'hey' } : { isIt, value: 123 };
const cc: C = isIt ? { isIt: isIt, text: 'hey' } : { isIt: isIt, value: 123 };

type ComponentProps =
| {
optionalBool: true;
mandatoryFn: () => void;
}
| {
optionalBool: false;
};

let Funk = (_props: ComponentProps) => <div>Hello</div>;

let Fail1 = () => <Funk mandatoryFn={() => { }} optionalBool={true} />
let Fail2 = () => <Funk mandatoryFn={() => { }} optionalBool={true as true} />
let True = true as true;
let Fail3 = () => <Funk mandatoryFn={() => { }} optionalBool={True} />
let attrs2 = { optionalBool: true as true, mandatoryFn: () => { } }
let Success = () => <Funk {...attrs2} />
Expand Up @@ -43,7 +43,7 @@ tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration4.ts(
a1(...array2); // Error parameter type is (number|string)[]
~~~~~~
!!! error TS2552: Cannot find name 'array2'. Did you mean 'Array'?
!!! related TS2728 /.ts/lib.es5.d.ts:1298:15: 'Array' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:1356:15: 'Array' is declared here.
a5([1, 2, "string", false, true]); // Error, parameter type is [any, any, [[any]]]
~~~~~~~~
!!! error TS2322: Type 'string' is not assignable to type '[[any]]'.
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/externModule.errors.txt
Expand Up @@ -69,20 +69,20 @@ tests/cases/compiler/externModule.ts(37,3): error TS2552: Cannot find name 'XDat
var d=new XDate();
~~~~~
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
!!! related TS2728 /.ts/lib.es5.d.ts:837:15: 'Date' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:895:15: 'Date' is declared here.
d.getDay();
d=new XDate(1978,2);
~~~~~
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
!!! related TS2728 /.ts/lib.es5.d.ts:837:15: 'Date' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:895:15: 'Date' is declared here.
d.getXDate();
var n=XDate.parse("3/2/2004");
~~~~~
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
!!! related TS2728 /.ts/lib.es5.d.ts:837:15: 'Date' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:895:15: 'Date' is declared here.
n=XDate.UTC(1964,2,1);
~~~~~
!!! error TS2552: Cannot find name 'XDate'. Did you mean 'Date'?
!!! related TS2728 /.ts/lib.es5.d.ts:837:15: 'Date' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:895:15: 'Date' is declared here.


12 changes: 8 additions & 4 deletions tests/baselines/reference/genericRestArityStrict.errors.txt
@@ -1,15 +1,19 @@
tests/cases/conformance/types/rest/genericRestArityStrict.ts(7,6): error TS2345: Argument of type '(x: number, y: number) => number' is not assignable to parameter of type '() => void'.
tests/cases/conformance/types/rest/genericRestArityStrict.ts(7,1): error TS2554: Expected 3 arguments, but got 1.
tests/cases/conformance/types/rest/genericRestArityStrict.ts(8,1): error TS2554: Expected 3 arguments, but got 8.


==== tests/cases/conformance/types/rest/genericRestArityStrict.ts (1 errors) ====
==== tests/cases/conformance/types/rest/genericRestArityStrict.ts (2 errors) ====
// Repro from #25559

declare function call<TS extends unknown[]>(
handler: (...args: TS) => void,
...args: TS): void;

call((x: number, y: number) => x + y);
Copy link
Member

Choose a reason for hiding this comment

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

Is this improved error because of the variance inference change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2345: Argument of type '(x: number, y: number) => number' is not assignable to parameter of type '() => void'.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2554: Expected 3 arguments, but got 1.
!!! related TS6210 tests/cases/conformance/types/rest/genericRestArityStrict.ts:5:5: An argument for 'args' was not provided.
call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2554: Expected 3 arguments, but got 8.

2 changes: 1 addition & 1 deletion tests/baselines/reference/genericRestArityStrict.types
Expand Up @@ -22,7 +22,7 @@ call((x: number, y: number) => x + y);
>y : number

call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7);
>call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7) : void
>call((x: number, y: number) => x + y, 1, 2, 3, 4, 5, 6, 7) : any
>call : <TS extends unknown[]>(handler: (...args: TS) => void, ...args: TS) => void
>(x: number, y: number) => x + y : (x: number, y: number) => number
>x : number
Expand Down
Expand Up @@ -41,7 +41,7 @@ tests/cases/compiler/modularizeLibrary_ErrorFromUsingES6FeaturesWithOnlyES5Lib.t
Math.sign(1);
~~~~
!!! error TS2551: Property 'sign' does not exist on type 'Math'. Did you mean 'sin'?
!!! related TS2728 /.ts/lib.es5.d.ts:643:5: 'sin' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:701:5: 'sin' is declared here.

// Using ES6 object
var o = {
Expand Down
Expand Up @@ -24,7 +24,7 @@ tests/cases/conformance/types/any/narrowExceptionVariableInCatchClause.ts(16,17)
err.massage; // ERROR: Property 'massage' does not exist on type 'Error'
~~~~~~~
!!! error TS2551: Property 'massage' does not exist on type 'Error'. Did you mean 'message'?
!!! related TS2728 /.ts/lib.es5.d.ts:904:5: 'message' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:962:5: 'message' is declared here.
}

else {
Expand Down
Expand Up @@ -22,14 +22,14 @@ tests/cases/conformance/types/any/narrowFromAnyWithInstanceof.ts(22,7): error TS
x.mesage;
~~~~~~
!!! error TS2551: Property 'mesage' does not exist on type 'Error'. Did you mean 'message'?
!!! related TS2728 /.ts/lib.es5.d.ts:904:5: 'message' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:962:5: 'message' is declared here.
}

if (x instanceof Date) {
x.getDate();
x.getHuors();
~~~~~~~~
!!! error TS2551: Property 'getHuors' does not exist on type 'Date'. Did you mean 'getHours'?
!!! related TS2728 /.ts/lib.es5.d.ts:693:5: 'getHours' is declared here.
!!! related TS2728 /.ts/lib.es5.d.ts:751:5: 'getHours' is declared here.
}