`Let` statements #8

imteekay · 2023-05-20T10:41:22Z

Add the Let token and AST node
Add a test for a single let statement
Add a test for redeclaring let after var and var after let with the same variable "name"
Add a test to handle the use-before-declaration error
bind: handle re-declaration of variables with the same name (let after var and the other way around)
bind: resolving symbols for let statements too (not only var)
typechecker: handle use-before-declaration error if an identifier expression is being called before the let variable declaration (used pos to handle this logic)
emitter: emit the let statement as a var statement in the final JS code

imteekay · 2023-05-20T10:45:39Z

sandersn

Looks pretty good. Mostly comments on how to make the code more general.

sandersn · 2023-05-31T19:40:32Z

src/emit.ts

@@ -30,6 +30,11 @@ function emitStatement(statement: Statement): string {
      return `var ${statement.name.text}${typestring} = ${emitExpression(
        statement.init,
      )}`;
+    case Node.Let:


let should be converted to var in a transform; otherwise there's no way to emit modern JS.

So, to clarify it. Maybe there are more details here but in general, can we say:

Transform: transform TS into JS (depending on the configuration - es2017, es2015, ...)

Emit: emit JS code (the transformed TS but without the types)

Almost: emit should actually even emit TS if the types are still in the tree. That is, emit should be able to emit anything that can be parsed, with 100% fidelity (in theory). The transformer is the thing solely responsible for turning TS into ES20xx.

sandersn · 2023-05-31T19:42:23Z

src/transform.ts

@@ -13,6 +13,7 @@ function typescript(statements: Statement[]) {
      case Node.ExpressionStatement:
        return [statement];
      case Node.Var:
+      case Node.Let:


a transform should convert let to var, but it shouldn't be the typescript one. Make a new function called es2015 that converts es2015 to es5. For now, the only thing it has to do is the let->var transformation.

(Also, while you're here, EmptyStatement shouldn't be dropped in a transform since it's a JS statement too. It should be emitted as empty in emit.ts)

a transform should convert let to var, but it shouldn't be the typescript one. Make a new function called es2015 that converts es2015 to es5. For now, the only thing it has to do is the let->var transformation.

I built a short version in a separate branch just to get feedback on it. https://github.com/imteekay/mini-typescript/compare/let...imteekay:mini-typescript:let-es5?expand=1

added compiler options with the target just to test it out

transforming es2015 when targeting es5

emitting let when it's a let ast node

(Also, while you're here, EmptyStatement shouldn't be dropped in a transform since it's a JS statement too. It should be emitted as empty in emit.ts)

Just opened a new PR to fix it. https://github.com/imteekay/mini-typescript/pull/10

Yep, that looks about right

Thanks, just merged both PRs.

https://github.com/imteekay/mini-typescript/pull/10

https://github.com/imteekay/mini-typescript/pull/11

sandersn · 2023-05-31T20:50:31Z

src/bind.ts

+          (d) =>
+            d.kind === statement.kind ||
+            (d.kind === Node.Var && statement.kind === Node.Let) ||
+            (d.kind === Node.Let && statement.kind === Node.Var),


now that you have let, you should allow redeclaration of var, since that's how real JS behaves.

Added the possibility to redeclare var imteekay/mini-typescript@03e83b2

sandersn · 2023-05-31T20:53:00Z

src/bind.ts

@@ -20,14 +27,16 @@ export function bind(m: Module) {
          );
        } else {
          symbol.declarations.push(statement);
-          if (statement.kind === Node.Var) {
+          if ([Node.Var, Node.Let].includes(statement.kind)) {
            symbol.valueDeclaration = statement;


this is fine, but consider what happens to valueDeclaration in the following program for this code versus the alternative, where you don't set valueDeclaration once it's been set once:

var x = 1; var x = 2;

This changes how goto-def behaves in Typescript, for example.

So in all cases, valueDeclaration should actually be a list of all variable declaration statements?
Meaning, I should rename it to valueDeclarations (plural) and push the statement to this list

e.g.

when redeclaring variables

if ([Node.Var, Node.Let].includes(statement.kind)) { - symbol.valueDeclaration = statement; + symbol.valueDeclarations.push(statement); }

set the first declaration symbol

locals.set(statement.name.text, { declarations: [statement], valueDeclaration: [Node.Var, Node.Let].includes(statement.kind) - ? statement + ? [statement] : undefined, });

So editors can make use of this and have a better goto-def, listing the definitions of the variable

Lmk if this is what you meant.

That's one way to solve it!

Basically, you can either

Have a list

Take the first

Take the last

Not even have valueDeclarations and search all of declarations every time.

Typescript chooses (2) because valueDeclaration is basically a shortcut that's good for 99% of declarations and it's OK if 1% of declarations have slightly incorrect behaviour with goto-def.

I brought this up because small decisions like this can have non-obvious ramifications for corner cases.

Thanks for the clarification. Following the second approach (Take the first), I changed the code using the logical or assignment operator (||=) to handle that.

imteekay/mini-typescript@e224a68

sandersn · 2023-05-31T20:54:50Z

src/bind.ts

@@ -37,7 +46,7 @@ export function bind(m: Module) {
 export function resolve(
  locals: Table,
  name: string,
-  meaning: Node.Var | Node.TypeAlias,
+  meaning: Node.Var | Node.TypeAlias | Node.Let,


now that you have two different kinds of value declarations, you need to abstract meaning into something like enum Meaning { Value, Type } -- and Meaning.Value should match declarations with kind Var or Let.

Just made this change imteekay/mini-typescript@5f853d7

Another approach would be adding an optional fourth parameter to the resolve function. So we could distinguish if it is a let or var and then I don't need to this kind of logic symbol?.valueDeclaration?.kind === Node.Let. It could just check if the symbol was found.

Which approach do you think is best for this scenario? There's any other?

What the full TS compiler does is have the binder save a meaning flag when binding a symbol. So: you declare enum Meaning { Value = 1 << 1; Type = 1 << 2 }, and then add a flags property on Symbol. Then when binding a let or var, if the symbol already exists, you set symbol.flags |= Meaning.Value. (If it doesn't you start withsymbol.flags = Meaning.Value)

That way you don't have to check the list of declarations at all when resolving.

Very interesting use of bitwise! Made the change, it became way simpler imteekay/mini-typescript@926bcd2

sandersn · 2023-05-31T20:55:51Z

src/check.ts

+          if (
+            letSymbol.valueDeclaration &&
+            letSymbol.valueDeclaration.pos < expression.pos
+          ) {
+            return checkStatement(letSymbol.valueDeclaration!);
+          }
+
+          error(
+            expression.pos,
+            `Block-scoped variable '${expression.text}' used before its declaration.`,
+          );


sandersn

Looks pretty good. Mostly comments on how to make the code more general.

`Let` -> `Var` targeting es5

sandersn · 2023-07-05T22:53:44Z

src/check.ts

+            `Block-scoped variable '${expression.text}' used before its declaration.`,
+          );
+
+          return errorType;


you should still return the type of the symbol, even if it's used before its declaration. Otherwise you'll get much worse intellisense in an editor scenario.

Now returning the actual statement type imteekay/mini-typescript@0c43054

imteekay added 7 commits May 18, 2023 22:23

Implement let statement

0aa5dd1

Check "Add let." exercise

bbf39b6

Emitting es5 var for let statements

12d9251

Add file to test let/var redeclare

c0a7c28

Add used-before-its-declaration error in the typechecker

e76ebf3

Check "Then add use-before-declaration errors in the checker."

3f4ea09

Handle var and let symbols separately in the type checker

2d548dd

imteekay self-assigned this May 20, 2023

Add PR link

65749cd

sandersn reviewed May 31, 2023

View reviewed changes

imteekay added 6 commits June 1, 2023 08:37

Let -> Var targeting es5

fbabf4a

Merge with master

7a5d077

Fix baselines

6f0a41a

Merge pull request #11 from imteekay/let-es5

7657d8d

`Let` -> `Var` targeting es5

Allow redeclare variables defined with var

03e83b2

Abstract meaning into the Meaning enum

5f853d7

imteekay requested a review from sandersn June 4, 2023 01:21

merge with main

f3baaee

imteekay force-pushed the let branch from 36110e5 to f3baaee Compare July 2, 2023 17:55

sandersn reviewed Jul 5, 2023

View reviewed changes

imteekay added 7 commits July 7, 2023 08:18

Merge branch 'master' into let

82957bf

Merge branch 'master' into let

d73b8b4

Merge branch 'master' into let

cb614e8

Take the first value statement

e224a68

Return actual statement type rather than error type

0c43054

Add flags to bind symbol

926bcd2

check exercise

e9c8328

imteekay merged commit 4237faa into master Jul 9, 2023

imteekay deleted the let branch July 9, 2023 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Let` statements #8

`Let` statements #8

imteekay commented May 20, 2023 •

edited

Loading

imteekay commented May 20, 2023

sandersn left a comment

sandersn May 31, 2023

imteekay Jun 1, 2023 •

edited

Loading

sandersn Jul 5, 2023

sandersn May 31, 2023

imteekay Jun 1, 2023

imteekay Jun 1, 2023

sandersn Jun 2, 2023

imteekay Jun 3, 2023 •

edited

Loading

sandersn May 31, 2023

imteekay Jun 3, 2023 •

edited

Loading

sandersn May 31, 2023

imteekay Jun 3, 2023

sandersn Jul 5, 2023

imteekay Jul 8, 2023

sandersn May 31, 2023

imteekay Jun 4, 2023 •

edited

Loading

sandersn Jul 5, 2023

imteekay Jul 9, 2023

sandersn May 31, 2023

sandersn left a comment

sandersn Jul 5, 2023

imteekay Jul 9, 2023

Let statements #8

Let statements #8

Conversation

imteekay commented May 20, 2023 • edited Loading

imteekay commented May 20, 2023

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imteekay Jun 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imteekay Jun 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imteekay Jun 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imteekay Jun 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`Let` statements #8

`Let` statements #8

imteekay commented May 20, 2023 •

edited

Loading

imteekay Jun 1, 2023 •

edited

Loading

imteekay Jun 3, 2023 •

edited

Loading

imteekay Jun 3, 2023 •

edited

Loading

imteekay Jun 4, 2023 •

edited

Loading