-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Changes from 19 commits
46bd405
df83784
22a384d
55b6513
a85b896
b687d90
1e3625c
a5dece3
d069da2
e9679f0
8a41f5f
91123fc
b67a261
0226b66
52293ed
2e17deb
df1e33a
339f310
9414fbe
5510e07
b6e66c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
|
@@ -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; | ||
|
@@ -7369,8 +7372,12 @@ namespace ts { | |
if (symbol && symbolIsValue(symbol)) { | ||
return symbol; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( |
||
} | ||
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; | ||
} | ||
|
@@ -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); | ||
|
@@ -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. | ||
|
@@ -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) { | ||
|
@@ -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; | ||
} | ||
} | ||
|
@@ -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); | ||
} | ||
|
@@ -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); | ||
|
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'. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably shouldn’t get an error here. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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} /> |
There was a problem hiding this comment.
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”.