-
Notifications
You must be signed in to change notification settings - Fork 13k
added fullTypeCheckFlag to TypeChecker #450
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
Conversation
@@ -3945,7 +3944,7 @@ module ts { | |||
var typeArgNode = typeArguments[i]; | |||
var typeArgument = getTypeFromTypeNode(typeArgNode); | |||
var constraint = getConstraintOfTypeParameter(typeParameters[i]); | |||
if (constraint) { | |||
if (constraint && fullTypeCheck) { | |||
checkTypeAssignableTo(typeArgument, constraint, typeArgNode, Diagnostics.Type_0_does_not_satisfy_the_constraint_1_Colon, Diagnostics.Type_0_does_not_satisfy_the_constraint_1); |
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.
should we instead just augment checkTypeAssignableTo to check the fullTypeCheck flag?
@@ -1403,7 +1406,8 @@ module ts { | |||
|
|||
// Now create a new compiler | |||
program = createProgram(hostfilenames, compilationSettings, createCompilerHost()); | |||
typeChecker = program.getTypeChecker(); | |||
typeInfoResolver = program.getTypeChecker(/*fullTypeCheckMode*/ false); | |||
fullTypeCheckChecker = program.getTypeChecker(/*fullTypeCheckMode*/ true); |
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.
do not create it here. create it on demand. this will do a full bind, and unless the current operation is getSemanticDiagnositcs, this is wasted.
var typeInfoResolver: TypeChecker; | ||
// the sole purpose of this checkes is to reutrn semantic diagnostics | ||
// creation is deferred - use getFullTypeCheckChecker to get instance | ||
var fullTypeCheckChecker_doNotAccessDirectly: TypeChecker; |
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.
i do not really care for the name that much. and this is used in only one place, so the extra layer of abstraction, is not really useful, i would just put it in getSemanticDiagnostics, to recreate if it does not exist and forget about it.
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.
My preference in avoiding errors is to leave only one way that will do things right. Since accessing variable directly might fail without prior check - then this only was should be "always use function, do not touch the variable"
👍 |
added fullTypeCheckFlag to TypeChecker
LS now uses 2 instances of typechecker:
typeInfoResolver
- created viagetTypeChecker(/*fullTypeCheck*/ false)
- used by quickinfo\dot completion etc..fullTypeCheckChecker
- created viagetTypeChecker(/*fullTypeCheck*/ true)
- used to get semantic diagnostics.fullTypeCheck
parameter in thegetTypeChecker
controls if typechecker can shortcut and answer more quickly (producing less error messages but they are still ignored whenfullTypeCheck === true
)