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 19 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 37 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
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 @@ -7369,8 +7372,12 @@ namespace ts {
if (symbol && symbolIsValue(symbol)) {
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”.

return symbol;
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.

}
if (resolved === anyFunctionType || resolved.callSignatures.length || resolved.constructSignatures.length) {
const symbol = getPropertyOfObjectType(globalFunctionType, name);
const functionType = resolved === anyFunctionType ? globalFunctionType :
resolved.callSignatures.length ? globalCallableFunctionType :
resolved.constructSignatures.length ? globalNewableFunctionType :
undefined;
if (functionType) {
const symbol = getPropertyOfObjectType(functionType, name);
if (symbol) {
return symbol;
}
Expand Down Expand Up @@ -13148,10 +13155,8 @@ namespace ts {
const targetCount = getParameterCount(target);
const sourceRestType = getEffectiveRestType(source);
const targetRestType = getEffectiveRestType(target);
const paramCount = targetRestType ? Math.min(targetCount - 1, sourceCount) :
sourceRestType ? targetCount :
Math.min(sourceCount, targetCount);

const targetNonRestCount = targetRestType ? targetCount - 1 : targetCount;
const paramCount = sourceRestType ? targetNonRestCount : Math.min(sourceCount, targetNonRestCount);
const sourceThisType = getThisTypeOfSignature(source);
if (sourceThisType) {
const targetThisType = getThisTypeOfSignature(target);
Expand Down Expand Up @@ -13779,15 +13784,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)) {
// 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 @@ -19503,7 +19510,10 @@ namespace ts {
checkCandidate = candidate;
}
if (!checkApplicableSignature(node, args, checkCandidate, relation, excludeArgument, /*reportErrors*/ false)) {
candidateForArgumentError = checkCandidate;
// Give preference to error candidates that have no rest parameters (as they are more specific)
if (!candidateForArgumentError || getEffectiveRestType(candidateForArgumentError) || !getEffectiveRestType(checkCandidate)) {
candidateForArgumentError = checkCandidate;
}
continue;
}
if (excludeArgument) {
Expand All @@ -19516,7 +19526,10 @@ namespace ts {
checkCandidate = getSignatureInstantiation(candidate, typeArgumentTypes, isInJSFile(candidate.declaration));
}
if (!checkApplicableSignature(node, args, checkCandidate, relation, excludeArgument, /*reportErrors*/ false)) {
candidateForArgumentError = checkCandidate;
// Give preference to error candidates that have no rest parameters (as they are more specific)
if (!candidateForArgumentError || getEffectiveRestType(candidateForArgumentError) || !getEffectiveRestType(checkCandidate)) {
candidateForArgumentError = checkCandidate;
}
continue;
}
}
Expand Down Expand Up @@ -27831,8 +27844,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 @@ -28608,6 +28624,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;
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -3752,6 +3752,10 @@
"category": "Message",
"code": 6213
},
"Enable strict 'bind', 'call', and 'apply' methods on functions.": {
"category": "Message",
"code": 6214
},

"Projects to reference": {
"category": "Message",
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
60 changes: 60 additions & 0 deletions src/lib/es5.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,66 @@ 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;
bind<T, AX, R>(this: (this: T, ...args: AX[]) => R, thisArg: T, ...args: AX[]): (...args: AX[]) => 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;
bind<AX, R>(this: new (...args: AX[]) => R, thisArg: any, ...args: AX[]): new (...args: AX[]) => R;
}

interface IArguments {
[index: number]: any;
length: number;
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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} />
Original file line number Diff line number Diff line change
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:1358: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
Original file line number Diff line number Diff line change
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:897: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:897: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:897: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:897:15: 'Date' is declared here.


Loading