-
Notifications
You must be signed in to change notification settings - Fork 13k
Pull model for contextual types #330
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
New baselines reflect a couple of unrelated bug fixes.
Changed binder to record catch clause instead of catch variable as symbol declaration. Restructuring of getTypeOfVariableDeclaration and getTypeOfVariableOrParameterOrProperty methods. Restructuring of checkFunctionExpression method.
Previous commit fixes overly aggressive -noImplictAny reporting. In the test case the source of the error is the 'getAndSet' property that implicitly gets type any. The fact that the setter then gets type any isn't actually an error.
checkFunctionExpression only type checks function body if fullTypeCheck is true.
@@ -250,7 +250,7 @@ module ts { | |||
|
|||
function bindCatchVariableDeclaration(node: CatchBlock) { | |||
var symbol = createSymbol(SymbolFlags.Variable, node.variable.text || "__missing"); | |||
addDeclarationToSymbol(symbol, node.variable, SymbolFlags.Variable); | |||
addDeclarationToSymbol(symbol, node, SymbolFlags.Variable); |
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'm awfully confused - why are we binding on the catch block instead of the variable? It seems inherently wrong, and is otherwise undocumented.
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.
Symbols are supposed to be bound to the declaration node that contains their identifier, but a catch block was erroneously bound to the identifier itself.
checkImplicitAny now returns void.
} | ||
|
||
// Return contextual type of parameter or undefined if no contextual type is available | ||
function getContextuallyTypedParameterType(parameter: VariableDeclaration): Type { |
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 think parameter type should be ParameterDeclaration
to emphasize that node here should be parameter
👍 |
Pull model for contextual types
This pull request switches our contextual typing logic to use a “pull model” such that you can grab any expression node or symbol and ask for its type without having to first ensure types have been “pushed” into the right places. This includes contextually typed sub-expressions and contextually typed parameters of function expressions. We need this change to deliver correct results in the language service.
The key difference is a new
getContextualType
function that returns the contextual type associated with an arbitrary expression node with no requirement that the node or its containing function(s) have been fully type checked first. The methods works its way up the parent chain to compute the contextual type and binds contextually typed function expressions as necessary. Also, the types of contextually typed lambda parameters are computed on demand in a pull fashion so it is now safe to ask for the type of any symbol anywhere.