Allow var statements to declare multiple symbols #9

imteekay · 2023-05-21T02:33:55Z

Add test to cover the new variable declaration list AST node
Update all tests that have the var statement
Create the comma token, parse the list AST node (with a list of declarations) — followed this example
Type check each statement in the var declaration list

imteekay · 2023-05-21T02:37:44Z

src/check.ts

@@ -42,6 +43,9 @@ export function check(module: Module) {
        return t;
      case Node.TypeAlias:
        return checkType(statement.typename);
+      case Node.VariableDeclarationList:
+        statement.declarations.forEach(checkStatement);
+        return anyType;


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.

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)

That makes sense. I just made a change handle var as not a statement. imteekay/mini-typescript@147c648

imteekay · 2023-05-21T02:45:50Z

README.md

@@ -42,7 +42,7 @@ npm run mtsc ./tests/singleVar.ts
 - [ ] Add let.
  - Then add use-before-declaration errors in the checker.
  - Finally, add an ES2015 -> ES5 transform that transforms `let` to `var`.
- [ ] Allow var to have multiple declarations.
+- [x] Allow var to have multiple declarations.
  - You'll need to convert a Symbol's declaration into a list.
  - Check that all declarations have the same type.


@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:

var var1: string = 'test', var2: number = 2;

whoops. I was unclear on this exercise. What I meant was that this is legal typescript:

var var1: string = 'test'; 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".

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.

imteekay/mini-typescript@232fa06

imteekay · 2023-05-21T02:47:06Z

cc @sandersn

…List

sandersn

Looks about right. For long-term correctness, I would make Var not a statement, but it actually works fine for now.

sandersn · 2023-07-05T23:23:16Z

README.md

@@ -42,7 +42,7 @@ npm run mtsc ./tests/singleVar.ts
 - [ ] Add let.
  - Then add use-before-declaration errors in the checker.
  - Finally, add an ES2015 -> ES5 transform that transforms `let` to `var`.
- [ ] Allow var to have multiple declarations.
+- [x] Allow var to have multiple declarations.
  - You'll need to convert a Symbol's declaration into a list.
  - Check that all declarations have the same type.


whoops. I was unclear on this exercise. What I meant was that this is legal typescript:

var var1: string = 'test'; 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".

sandersn · 2023-07-05T23:32:32Z

src/types.ts

-export type Statement = ExpressionStatement | Var | TypeAlias | EmptyStatement;
+export type Statement =
+  | ExpressionStatement
+  | Var


Var isn't really a statement anymore, it's a child of VariableStatement.

Just removed it imteekay/mini-typescript@147c648

sandersn · 2023-07-05T23:35:23Z

src/check.ts

@@ -42,6 +43,9 @@ export function check(module: Module) {
        return t;
      case Node.TypeAlias:
        return checkType(statement.typename);
+      case Node.VariableDeclarationList:
+        statement.declarations.forEach(checkStatement);
+        return anyType;


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)

imteekay added 3 commits May 20, 2023 19:49

Implement multiple declarations for var

f3e7a6d

Add variable declaration list test

eddfcb9

Fix baseline tests

9a023ae

imteekay changed the title ~~VariableDeclarationList: allow var to have multiple declarations~~ Allow var to have multiple declarations May 21, 2023

imteekay commented May 21, 2023

View reviewed changes

imteekay self-assigned this May 21, 2023

imteekay commented May 21, 2023

View reviewed changes

Check exercise

dcab74f

imteekay force-pushed the variable-declaration-list branch from 4060228 to dcab74f Compare May 21, 2023 14:10

imteekay added 3 commits June 27, 2023 08:27

Merge with main

10bb027

Fix emitted js for variable declaration list

19e46f7

Add VariableStatement as a parent node for the `VariableDeclaration…

02b919a

…List

sandersn reviewed Jul 5, 2023

View reviewed changes

imteekay added 8 commits July 6, 2023 08:55

Merge with master

f9ca23b

Add exercise

232fa06

Handle var not as a statement

147c648

Merge branch 'master' into variable-declaration-list

05990e8

Merge branch 'master' into variable-declaration-list

72f0b85

Variable declaration list with let declarations

c085f1e

Improve logic and make it easier to understand

bdd2a74

From flatMap to map

3bf3174

imteekay merged commit e5749be into master Jul 17, 2023

imteekay deleted the variable-declaration-list branch July 17, 2023 21:20

imteekay changed the title ~~Allow var to have multiple declarations~~ Allow var statements to declare multiple symbols Jul 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow var statements to declare multiple symbols #9

Allow var statements to declare multiple symbols #9

imteekay commented May 21, 2023 •

edited

Loading

imteekay May 21, 2023 •

edited

Loading

sandersn Jul 5, 2023

imteekay Jul 7, 2023

imteekay May 21, 2023 •

edited

Loading

sandersn Jul 5, 2023

imteekay Jul 6, 2023 •

edited

Loading

imteekay commented May 21, 2023

sandersn left a comment

sandersn Jul 5, 2023

sandersn Jul 5, 2023

imteekay Jul 7, 2023

sandersn Jul 5, 2023

Allow var statements to declare multiple symbols #9

Allow var statements to declare multiple symbols #9

Conversation

imteekay commented May 21, 2023 • edited Loading

imteekay May 21, 2023 • edited Loading

Choose a reason for hiding this comment

sandersn Jul 5, 2023

Choose a reason for hiding this comment

imteekay Jul 7, 2023

Choose a reason for hiding this comment

imteekay May 21, 2023 • edited Loading

Choose a reason for hiding this comment

sandersn Jul 5, 2023

Choose a reason for hiding this comment

imteekay Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

imteekay commented May 21, 2023

sandersn left a comment

Choose a reason for hiding this comment

sandersn Jul 5, 2023

Choose a reason for hiding this comment

sandersn Jul 5, 2023

Choose a reason for hiding this comment

imteekay Jul 7, 2023

Choose a reason for hiding this comment

sandersn Jul 5, 2023

Choose a reason for hiding this comment

imteekay commented May 21, 2023 •

edited

Loading

imteekay May 21, 2023 •

edited

Loading

imteekay May 21, 2023 •

edited

Loading

imteekay Jul 6, 2023 •

edited

Loading