Repository: source-academy/js-slang
Pull request #181 (2019-02-25 14:19:15)

Refactor rttc to use node instead of context
Native would like to reuse rttc, so passing in a node instead of the context would be beneficial.

Also, some tests to clarify any doubts on where the current interpreter throws errors for code like 

```js
if ( // error is on this line, start of if statement
'not boolean'
) {
  1;
} else {}
```

and 

```js
123 // error is on this line, start of the binary operation
*
'not a number';
```

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

* **9** of **9**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.08%**) to **91.2%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/21831261/badge)](https://coveralls.io/builds/21831261) |
| :-- | --: |
| Change from base [Build 696](https://coveralls.io/builds/21814562): |  0.08% |
| Covered Lines: | 1408 |
| Relevant Lines: | 1543 |

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


Patch:
diff --git a/src/interpreter.ts b/src/interpreter.ts
index c3015fd0..ce648470 100644
--- a/src/interpreter.ts
+++ b/src/interpreter.ts
@@ -217,7 +217,7 @@ function transformLogicalExpression(node: es.LogicalExpression): es.ConditionalE
 function* reduceIf(node: es.IfStatement | es.ConditionalExpression, context: Context) {
   const test = yield* evaluate(node.test, context)
 
-  const error = rttc.checkIfStatement(context, test)
+  const error = rttc.checkIfStatement(node, test)
   if (error) {
     return handleRuntimeError(context, error)
   }
@@ -299,7 +299,7 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
   *UnaryExpression(node: es.UnaryExpression, context: Context) {
     const value = yield* evaluate(node.argument, context)
 
-    const error = rttc.checkUnaryExpression(context, node.operator, value)
+    const error = rttc.checkUnaryExpression(node, node.operator, value)
     if (error) {
       return handleRuntimeError(context, error)
     }
@@ -317,7 +317,7 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
     const left = yield* evaluate(node.left, context)
     const right = yield* evaluate(node.right, context)
 
-    const error = rttc.checkBinaryExpression(context, node.operator, left, right)
+    const error = rttc.checkBinaryExpression(node, node.operator, left, right)
     if (error) {
       return handleRuntimeError(context, error)
     }
@@ -451,7 +451,7 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
       prop = (node.property as es.Identifier).name
     }
 
-    const error = rttc.checkMemberAccess(context, obj, prop)
+    const error = rttc.checkMemberAccess(node, obj, prop)
     if (error) {
       return handleRuntimeError(context, error)
     }
@@ -482,7 +482,7 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
         prop = (left.property as es.Identifier).name
       }
 
-      const error = rttc.checkMemberAccess(context, obj, prop)
+      const error = rttc.checkMemberAccess(node, obj, prop)
       if (error) {
         return handleRuntimeError(context, error)
       }
diff --git a/src/utils/rttc.ts b/src/utils/rttc.ts
index be0b2fad..b90735f8 100644
--- a/src/utils/rttc.ts
+++ b/src/utils/rttc.ts
@@ -1,6 +1,6 @@
 import * as es from 'estree'
 import { RuntimeSourceError } from '../interpreter-errors'
-import { Context, ErrorSeverity, ErrorType, Value } from '../types'
+import { ErrorSeverity, ErrorType, Value } from '../types'
 
 const LHS = ' on left hand side of operation'
 const RHS = ' on right hand side of operation'
@@ -40,12 +40,7 @@ const isBool = (v: Value) => typeOf(v) === 'boolean'
 const isObject = (v: Value) => typeOf(v) === 'object'
 const isArray = (v: Value) => typeOf(v) === 'array'
 
-export const checkUnaryExpression = (
-  context: Context,
-  operator: es.UnaryOperator,
-  value: Value
-) => {
-  const node = context.runtime.nodes[0]
+export const checkUnaryExpression = (node: es.Node, operator: es.UnaryOperator, value: Value) => {
   if ((operator === '+' || operator === '-') && !isNumber(value)) {
     return new TypeError(node, '', 'number', typeOf(value))
   } else if (operator === '!' && !isBool(value)) {
@@ -56,12 +51,11 @@ export const checkUnaryExpression = (
 }
 
 export const checkBinaryExpression = (
-  context: Context,
+  node: es.Node,
   operator: es.BinaryOperator,
   left: Value,
   right: Value
 ) => {
-  const node = context.runtime.nodes[0]
   switch (operator) {
     case '-':
     case '*':
@@ -93,13 +87,11 @@ export const checkBinaryExpression = (
   }
 }
 
-export const checkIfStatement = (context: Context, test: Value) => {
-  const node = context.runtime.nodes[0]
+export const checkIfStatement = (node: es.Node, test: Value) => {
   return isBool(test) ? undefined : new TypeError(node, ' as condition', 'boolean', typeOf(test))
 }
 
-export const checkMemberAccess = (context: Context, obj: Value, prop: Value) => {
-  const node = context.runtime.nodes[0]
+export const checkMemberAccess = (node: es.Node, obj: Value, prop: Value) => {
   if (isObject(obj)) {
     return isString(prop) ? undefined : new TypeError(node, ' as prop', 'string', typeOf(prop))
   } else if (isArray(obj)) {


Test patch:
diff --git a/src/__tests__/__snapshots__/interpreter-errors.ts.snap b/src/__tests__/__snapshots__/interpreter-errors.ts.snap
index 9c7c7469..51ee1065 100644
--- a/src/__tests__/__snapshots__/interpreter-errors.ts.snap
+++ b/src/__tests__/__snapshots__/interpreter-errors.ts.snap
@@ -967,6 +967,74 @@ Object {
 }
 `;
 
+exports[`Type error with <number> * <nonnumber>, error line at <number>, not <nonnumber>: expectParsedError 1`] = `
+Object {
+  "alertResult": Array [],
+  "code": "12
+*
+'string';",
+  "displayResult": Array [],
+  "errors": Array [
+    TypeError {
+      "expected": "number",
+      "got": "string",
+      "location": SourceLocation {
+        "end": Position {
+          "column": 8,
+          "line": 3,
+        },
+        "start": Position {
+          "column": 0,
+          "line": 1,
+        },
+      },
+      "severity": "Error",
+      "side": " on right hand side of operation",
+      "type": "Runtime",
+    },
+  ],
+  "parsedErrors": "Line 1: Expected number on right hand side of operation, got string.",
+  "result": undefined,
+  "resultStatus": "error",
+  "visualiseListResult": Array [],
+}
+`;
+
+exports[`Type error with non boolean in if statement, error line at if statement, not at 1: expectParsedError 1`] = `
+Object {
+  "alertResult": Array [],
+  "code": "if (
+1
+) {
+  2;
+} else {}",
+  "displayResult": Array [],
+  "errors": Array [
+    TypeError {
+      "expected": "boolean",
+      "got": "number",
+      "location": SourceLocation {
+        "end": Position {
+          "column": 9,
+          "line": 5,
+        },
+        "start": Position {
+          "column": 0,
+          "line": 1,
+        },
+      },
+      "severity": "Error",
+      "side": " as condition",
+      "type": "Runtime",
+    },
+  ],
+  "parsedErrors": "Line 1: Expected boolean as condition, got number.",
+  "result": undefined,
+  "resultStatus": "error",
+  "visualiseListResult": Array [],
+}
+`;
+
 exports[`Undefined variable error is thrown: expectParsedError 1`] = `
 Object {
   "alertResult": Array [],
diff --git a/src/__tests__/interpreter-errors.ts b/src/__tests__/interpreter-errors.ts
index df0ec243..1854eba1 100644
--- a/src/__tests__/interpreter-errors.ts
+++ b/src/__tests__/interpreter-errors.ts
@@ -623,3 +623,27 @@ test('Type error when assigning property of function', () => {
     { chapter: 100 }
   ).toMatchInlineSnapshot(`"Line 4: Expected object or array, got function."`)
 })
+
+test('Type error with non boolean in if statement, error line at if statement, not at 1', () => {
+  return expectParsedError(
+    stripIndent`
+    if (
+    1
+    ) {
+      2;
+    } else {}
+    `,
+    { chapter: 1 }
+  ).toMatchInlineSnapshot(`"Line 1: Expected boolean as condition, got number."`)
+})
+
+test('Type error with <number> * <nonnumber>, error line at <number>, not <nonnumber>', () => {
+  return expectParsedError(
+    stripIndent`
+    12
+    *
+    'string';
+    `,
+    { chapter: 1 }
+  ).toMatchInlineSnapshot(`"Line 1: Expected number on right hand side of operation, got string."`)
+})
diff --git a/src/utils/__tests__/rttc.ts b/src/utils/__tests__/rttc.ts
index 7404835e..0ec4cfc7 100644
--- a/src/utils/__tests__/rttc.ts
+++ b/src/utils/__tests__/rttc.ts
@@ -29,7 +29,8 @@ describe('Unary type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([operator, value]: [UnaryOperator, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkUnaryExpression(context, operator, value)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkUnaryExpression(node, operator, value)
       expect(error).toBeUndefined()
     })
   })
@@ -37,7 +38,8 @@ describe('Unary type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([operator, value]: [UnaryOperator, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkUnaryExpression(context, operator, value)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkUnaryExpression(node, operator, value)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         operator,
@@ -72,7 +74,8 @@ describe('Binary + type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeUndefined()
     })
   })
@@ -80,7 +83,8 @@ describe('Binary + type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         operator,
@@ -121,7 +125,8 @@ describe('Binary (-|*|/|%) type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeUndefined()
     })
   })
@@ -129,7 +134,8 @@ describe('Binary (-|*|/|%) type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         operator,
@@ -159,7 +165,8 @@ describe('Binary (===|!==) type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeUndefined()
     })
   })
@@ -167,7 +174,8 @@ describe('Binary (===|!==) type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         operator,
@@ -212,7 +220,8 @@ describe('Binary (<|>|<=|>=) type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeUndefined()
     })
   })
@@ -220,7 +229,8 @@ describe('Binary (<|>|<=|>=) type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([operator, left, right]: [BinaryOperator, Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkBinaryExpression(context, operator, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkBinaryExpression(node, operator, left, right)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         operator,
@@ -246,7 +256,8 @@ describe('Ternary/if test expression type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach((value: Value) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkIfStatement(context, value)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkIfStatement(node, value)
       expect(error).toBeUndefined()
     })
   })
@@ -254,7 +265,8 @@ describe('Ternary/if test expression type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach((value: Value) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkIfStatement(context, value)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkIfStatement(node, value)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         value,
@@ -280,7 +292,8 @@ describe('Member expression type combinations:', () => {
   test('Valid type combinations are OK', () => {
     valid.forEach(([left, right]: [Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkMemberAccess(context, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkMemberAccess(node, left, right)
       expect(error).toBeUndefined()
     })
   })
@@ -288,7 +301,8 @@ describe('Member expression type combinations:', () => {
   test('Invalid type combinations return TypeError', () => {
     invalid.forEach(([left, right]: [Value, Value]) => {
       const context = mockRuntimeContext()
-      const error = rttc.checkMemberAccess(context, left, right)
+      const node = context.runtime.nodes[0]
+      const error = rttc.checkMemberAccess(node, left, right)
       expect(error).toBeInstanceOf(rttc.TypeError)
       expect({
         left,