Repository: source-academy/js-slang
Pull request #44 (2018-08-28 04:03:38)

Add block scoping for conditional statements and expressions
`const` declarations are now block-scoped to if statements

adding block scoping to for loops will come later

http://13.250.115.8/playground#chap=2&ext=NONE&prgrm=MYewdgzgLgBAHjAvDKAnArgUwNwChcCWAZgBRpYCUMA3rjPTKJLAskQIYA2EOuAvjEzdMNOgybR4SGB2F4%2B%2BONiA

coveralls: ## Pull Request Test Coverage Report for [Build 75](https://coveralls.io/builds/18699801)

* **11** of **11**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**3.6%**) to **51.771%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/18699801/badge)](https://coveralls.io/builds/18699801) |
| :-- | --: |
| Change from base [Build 74](https://coveralls.io/builds/18699774): |  3.6% |
| Covered Lines: | 666 |
| Relevant Lines: | 1194 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/src/interpreter.ts b/src/interpreter.ts
index 1fb45006..65b348ab 100644
--- a/src/interpreter.ts
+++ b/src/interpreter.ts
@@ -43,6 +43,20 @@ const createFrame = (
   return frame
 }
 
+const createBlockFrame = (
+  context: Context,
+  vars: es.Identifier[],
+  args: Value[],
+): Frame => {
+  const frame: Frame = {
+    name: 'ifStatementBlock',
+    parent: currentFrame(context),
+    environment: {},
+    thisContext: context
+  }
+  return frame
+}
+
 const handleError = (context: Context, error: SourceError) => {
   context.errors.push(error)
   if (error.severity === ErrorSeverity.ERROR) {
@@ -369,10 +383,18 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
       return undefined
     }
 
+    // Create a new frame (block scoping)
+    const frame = createBlockFrame(context, [], [])
+    pushFrame(context, frame)
+
     if (test) {
-      return yield* evaluate(node.consequent, context)
+      const result = yield* evaluate(node.consequent, context)
+      popFrame(context)
+      return result
     } else if (node.alternate) {
-      return yield* evaluate(node.alternate, context)
+      const result = yield* evaluate(node.alternate, context)
+      popFrame(context)
+      return result
     } else {
       return undefined
     }


Test patch:
diff --git a/src/__tests__/__snapshots__/index.ts.snap b/src/__tests__/__snapshots__/index.ts.snap
index 7964c125..7173af9a 100644
--- a/src/__tests__/__snapshots__/index.ts.snap
+++ b/src/__tests__/__snapshots__/index.ts.snap
@@ -252,4 +252,11 @@ Object {
 }
 `;
 
+exports[`const uses block scoping instead of function scoping 1`] = `
+Object {
+  "status": "finished",
+  "value": true,
+}
+`;
+
 exports[`parseError for missing semicolon 1`] = `"Line 1: Missing semicolon at the end of statement"`;
diff --git a/src/__tests__/index.ts b/src/__tests__/index.ts
index 6092f48a..4e57548e 100644
--- a/src/__tests__/index.ts
+++ b/src/__tests__/index.ts
@@ -131,3 +131,26 @@ test(
   },
   30000
 )
+
+// This is bad practice. Don't do this!
+test('const uses block scoping instead of function scoping', () => {
+  const code = `
+    function test(){
+      const x = true;
+      if(true) {
+          const x = false;
+      } else {
+          const x = false;
+      }
+      return x;
+    }
+    test();
+  `;
+  const context = mockContext()
+  const promise = runInContext(code, context, { scheduler: 'preemptive' })
+  return promise.then(obj => {
+    expect(obj).toMatchSnapshot()
+    expect(obj.status).toBe('finished')
+    expect((obj as Finished).value).toBe(true)
+  })
+})