-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
- Add test to cover the new variable declaration list AST node
- Update all tests that have the
varstatement - Create the
commatoken, parse the list AST node (with a list of declarations) -- followed this example - Type check each statement in the var declaration list
VariableDeclarationList: allow var to have multiple declarationsvar to have multiple declarations
src/check.ts
Outdated
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.
Not sure which type I should return for VariableDeclarationList nodes.
But looking at this example on the TS AST, the type is a "any" 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.
That sounds about right for many nodes that don't have a sensible type. However, Var isn't really a statement anymore, so it should be handled in a different place from checkStatement. (this is true in the transformer and emitter too)
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.
That makes sense. I just made a change handle var as not a statement. imteekay/mini-typescript@147c648
README.md
Outdated
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.
@sandersn, what did you mean by all declarations with the same type? When defining multiple variables with a var, we can have different types, right?
For instance, this would work:
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.
whoops. I was unclear on this exercise. What I meant was that this is legal typescript:
var var1: string = 'what, another one?'
var var1 = 'no type annotation needed'
But you cannot have a conflicting type or type annotation like var var1 = 12
This PR implements var statements that declare multiple symbols, not symbols with multiple declarations, each from different statements. I see now that both are valid interpretations of "Allow var to have multiple declarations".
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.
Oooh, now I got it! Thanks for clarifying.
I created a new exercise "Allow var statements to declare multiple symbols" and this PR will handle that. So I can handle the "Allow var to have multiple declarations" in another PR.
4060228 to
dcab74f
Compare
sandersn
left a comment
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.
Looks about right. For long-term correctness, I would make Var not a statement, but it actually works fine for now.
README.md
Outdated
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.
whoops. I was unclear on this exercise. What I meant was that this is legal typescript:
var var1: string = 'what, another one?'
var var1 = 'no type annotation needed'
But you cannot have a conflicting type or type annotation like var var1 = 12
This PR implements var statements that declare multiple symbols, not symbols with multiple declarations, each from different statements. I see now that both are valid interpretations of "Allow var to have multiple declarations".
src/types.ts
Outdated
| export type Statement = ExpressionStatement | Var | TypeAlias | EmptyStatement; | ||
| export type Statement = | ||
| | ExpressionStatement | ||
| | Var |
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.
Var isn't really a statement anymore, it's a child of VariableStatement.
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.
Just removed it imteekay/mini-typescript@147c648
src/check.ts
Outdated
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.
That sounds about right for many nodes that don't have a sensible type. However, Var isn't really a statement anymore, so it should be handled in a different place from checkStatement. (this is true in the transformer and emitter too)
var to have multiple declarations