Improve error messages, fix naming bug and assignment bug in native

package.json

@@ -1,6 +1,6 @@
 {
   "name": "js-slang",
-  "version": "0.3.3",
+  "version": "0.3.5",
   "description": "Javascript-based interpreter for slang, written in Typescript",
   "author": {
     "name": "Source Academy",

src/__tests__/__snapshots__/interpreter-errors.ts.snap

@@ -67,6 +67,195 @@ map(f, list(1, 2));",
 }
 `;
 
+exports[`Cascading js errors work properly 1: expectParsedError 1`] = `
+Object {
+  "alertResult": Array [],
+  "code": "function make_alternating_stream(stream) {
+  return pair(head(stream), () => make_alternating_stream(
+                                    negate_whole_stream(
+                                        stream_tail(stream))));
+}
+
+function negate_whole_stream(stream) {
+    return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));
+}
+
+const ones = pair(1, () => ones);
+eval_stream(make_alternating_stream(enum_stream(1, 9)), 9);",
+  "displayResult": Array [],
+  "errors": Array [
+    ExceptionError {
+      "error": [Error: head(xs) expects a pair as argument xs, but encountered null],
+      "location": SourceLocation {
+        "end": Position {
+          "column": 29,
+          "line": 8,
+        },
+        "start": Position {
+          "column": 17,
+          "line": 8,
+        },
+      },
+      "severity": "Error",
+      "type": "Runtime",
+    },
+  ],
+  "parsedErrors": "Line 8: Error: head(xs) expects a pair as argument xs, but encountered null",
+  "result": undefined,
+  "resultStatus": "error",
+  "transpiled": "const native = $$NATIVE_STORAGE;
+const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
+const boolOrErr = native.operators.get(\\"boolOrErr\\");
+const wrap = native.operators.get(\\"wrap\\");
+const unaryOp = native.operators.get(\\"unaryOp\\");
+const binaryOp = native.operators.get(\\"binaryOp\\");
+const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
+const setProp = native.operators.get(\\"setProp\\");
+const getProp = native.operators.get(\\"getProp\\");
+let lastStatementResult = undefined;
+const globals = $NATIVE_STORAGE.globals;
+(( <globals redacted> ) => {
+  return (() => {
+    {
+      {
+        const make_alternating_stream = wrap(stream => {
+          return {
+            isTail: true,
+            function: pair,
+            functionName: \\"pair\\",
+            arguments: [callIfFuncAndRightArgs(head, 2, 14, stream), wrap(() => ({
+              isTail: true,
+              function: make_alternating_stream,
+              functionName: \\"make_alternating_stream\\",
+              arguments: [callIfFuncAndRightArgs(negate_whole_stream, 3, 36, callIfFuncAndRightArgs(stream_tail, 4, 40, stream))],
+              line: 2,
+              column: 34
+            }), \\"() => make_alternating_stream(negate_whole_stream(stream_tail(stream)))\\")],
+            line: 2,
+            column: 9
+          };
+        }, \\"function make_alternating_stream(stream) {\\\\n  return pair(head(stream), () => make_alternating_stream(negate_whole_stream(stream_tail(stream))));\\\\n}\\");
+        const negate_whole_stream = wrap(stream => {
+          return {
+            isTail: true,
+            function: pair,
+            functionName: \\"pair\\",
+            arguments: [unaryOp(\\"-\\", callIfFuncAndRightArgs(head, 8, 17, stream), 8, 16), wrap(() => ({
+              isTail: true,
+              function: negate_whole_stream,
+              functionName: \\"negate_whole_stream\\",
+              arguments: [callIfFuncAndRightArgs(stream_tail, 8, 57, stream)],
+              line: 8,
+              column: 37
+            }), \\"() => negate_whole_stream(stream_tail(stream))\\")],
+            line: 8,
+            column: 11
+          };
+        }, \\"function negate_whole_stream(stream) {\\\\n  return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));\\\\n}\\");
+        const ones = callIfFuncAndRightArgs(pair, 11, 13, 1, wrap(() => ({
+          isTail: false,
+          value: ones
+        }), \\"() => ones\\"));
+        lastStatementResult = eval(\\"callIfFuncAndRightArgs(eval_stream, 12, 0, callIfFuncAndRightArgs(make_alternating_stream, 12, 12, callIfFuncAndRightArgs(enum_stream, 12, 36, 1, 9)), 9);\\");
+        globals.variables.set(\\"make_alternating_stream\\", {
+          kind: \\"const\\",
+          getValue: () => {
+            return make_alternating_stream;
+          }
+        });
+        globals.variables.set(\\"negate_whole_stream\\", {
+          kind: \\"const\\",
+          getValue: () => {
+            return negate_whole_stream;
+          }
+        });
+        globals.variables.set(\\"ones\\", {
+          kind: \\"const\\",
+          getValue: () => {
+            return ones;
+          }
+        });
+      }
+    }
+    return lastStatementResult;
+  })();
+})();
+",
+  "visualiseListResult": Array [],
+}
+`;
+
+exports[`Cascading js errors work properly: expectParsedError 1`] = `
+Object {
+  "alertResult": Array [],
+  "code": "function h(p) {
+  return head(p);
+}
+
+h(null);",
+  "displayResult": Array [],
+  "errors": Array [
+    ExceptionError {
+      "error": [Error: head(xs) expects a pair as argument xs, but encountered null],
+      "location": SourceLocation {
+        "end": Position {
+          "column": 16,
+          "line": 2,
+        },
+        "start": Position {
+          "column": 9,
+          "line": 2,
+        },
+      },
+      "severity": "Error",
+      "type": "Runtime",
+    },
+  ],
+  "parsedErrors": "Line 2: Error: head(xs) expects a pair as argument xs, but encountered null",
+  "result": undefined,
+  "resultStatus": "error",
+  "transpiled": "const native = $$NATIVE_STORAGE;
+const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
+const boolOrErr = native.operators.get(\\"boolOrErr\\");
+const wrap = native.operators.get(\\"wrap\\");
+const unaryOp = native.operators.get(\\"unaryOp\\");
+const binaryOp = native.operators.get(\\"binaryOp\\");
+const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
+const setProp = native.operators.get(\\"setProp\\");
+const getProp = native.operators.get(\\"getProp\\");
+let lastStatementResult = undefined;
+const globals = $NATIVE_STORAGE.globals;
+(( <globals redacted> ) => {
+  return (() => {
+    {
+      {
+        const h = wrap(p => {
+          return {
+            isTail: true,
+            function: head,
+            functionName: \\"head\\",
+            arguments: [p],
+            line: 2,
+            column: 9
+          };
+        }, \\"function h(p) {\\\\n  return head(p);\\\\n}\\");
+        lastStatementResult = eval(\\"callIfFuncAndRightArgs(h, 5, 0, null);\\");
+        globals.variables.set(\\"h\\", {
+          kind: \\"const\\",
+          getValue: () => {
+            return h;
+          }
+        });
+      }
+    }
+    return lastStatementResult;
+  })();
+})();
+",
+  "visualiseListResult": Array [],
+}
+`;
+
 exports[`Error when accessing inherited property of object: expectParsedError 1`] = `
 Object {
   "alertResult": Array [],
@@ -3097,6 +3286,28 @@ Object {
   "parsedErrors": "Line 1: Expected number on right hand side of operation, got string.",
   "result": undefined,
   "resultStatus": "error",
+  "transpiled": "const native = $$NATIVE_STORAGE;
+const callIfFuncAndRightArgs = native.operators.get(\\"callIfFuncAndRightArgs\\");
+const boolOrErr = native.operators.get(\\"boolOrErr\\");
+const wrap = native.operators.get(\\"wrap\\");
+const unaryOp = native.operators.get(\\"unaryOp\\");
+const binaryOp = native.operators.get(\\"binaryOp\\");
+const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");
+const setProp = native.operators.get(\\"setProp\\");
+const getProp = native.operators.get(\\"getProp\\");
+let lastStatementResult = undefined;
+const globals = $NATIVE_STORAGE.globals;
+(( <globals redacted> ) => {
+  return (() => {
+    {
+      {
+        lastStatementResult = eval(\\"binaryOp(\\\\\\"*\\\\\\", 12, 'string', 1, 0);\\");
+      }
+    }
+    return lastStatementResult;
+  })();
+})();
+",
   "visualiseListResult": Array [],
 }
 `;

src/__tests__/__snapshots__/transpiled-code.ts.snap

@@ -2,9 +2,9 @@
 
 exports[`Ensure no name clashes 1`] = `
 "const native0 = $$NATIVE_STORAGE;
-const callIfFuncAndRightArgs0 = native.operators.get(\\"callIfFuncAndRightArgs0\\");
-const boolOrErr0 = native.operators.get(\\"boolOrErr0\\");
-const wrap90 = native.operators.get(\\"wrap90\\");
+const callIfFuncAndRightArgs0 = native.operators.get(\\"callIfFuncAndRightArgs\\");
+const boolOrErr0 = native.operators.get(\\"boolOrErr\\");
+const wrap90 = native.operators.get(\\"wrap\\");
 const unaryOp = native.operators.get(\\"unaryOp\\");
 const binaryOp = native.operators.get(\\"binaryOp\\");
 const throwIfTimeout = native.operators.get(\\"throwIfTimeout\\");

src/__tests__/interpreter-errors.ts

@@ -963,6 +963,43 @@ test('Type error with <number> * <nonnumber>, error line at <number>, not <nonnu
     *
     'string';
     `,
-    { chapter: 1 }
+    { chapter: 1, native: true }
   ).toMatchInlineSnapshot(`"Line 1: Expected number on right hand side of operation, got string."`)
 })
+
+test('Cascading js errors work properly 1', () => {
+  return expectParsedError(
+    stripIndent`
+    function make_alternating_stream(stream) {
+      return pair(head(stream), () => make_alternating_stream(
+                                        negate_whole_stream(
+                                            stream_tail(stream))));
+    }
+
+    function negate_whole_stream(stream) {
+        return pair(-head(stream), () => negate_whole_stream(stream_tail(stream)));
+    }
+
+    const ones = pair(1, () => ones);
+    eval_stream(make_alternating_stream(enum_stream(1, 9)), 9);
+    `,
+    { chapter: 3, native: true }
+  ).toMatchInlineSnapshot(
+    `"Line 8: Error: head(xs) expects a pair as argument xs, but encountered null"`
+  )
+})
+
+test('Cascading js errors work properly', () => {
+  return expectParsedError(
+    stripIndent`
+    function h(p) {
+      return head(p);
+    }
+
+    h(null);
+    `,
+    { chapter: 2, native: true }
+  ).toMatchInlineSnapshot(
+    `"Line 2: Error: head(xs) expects a pair as argument xs, but encountered null"`
+  )
+})

src/__tests__/native.ts

@@ -81,3 +81,33 @@ test('test proper setting of variables in an outer scope', async () => {
   expect(result.status).toBe('finished')
   expect((result as Finished).value).toBe('new')
 })
+
+test('using internal names still work', async () => {
+  const context = mockContext(3)
+  const result = await runInContext(
+    stripIndent`
+    const boolOrErr = 1;
+    function wrap() {
+      return boolOrErr;
+    }
+    wrap();
+  `,
+    context
+  )
+  expect(result.status).toBe('finished')
+  expect((result as Finished).value).toBe(1)
+})
+
+test('assigning a = b where b was from a previous program call works', async () => {
+  const context = mockContext(3)
+  const result = await runInContext(
+    stripIndent`
+    let b = null;
+    b = pair;
+    b = 1;
+  `,
+    context
+  )
+  expect(result.status).toBe('finished')
+  expect((result as Finished).value).toBe(1)
+})

src/index.ts

@@ -201,10 +201,19 @@ export async function runInContext(
         value: sandboxedEval(transpiled)
       } as Result)
     } catch (error) {
-      if (error instanceof RuntimeSourceError || error instanceof ExceptionError) {
+      if (error instanceof RuntimeSourceError) {
         context.errors.push(error)
         return resolvedErrorPromise
       }
+      if (error instanceof ExceptionError) {
+        // if we know the location of the error, just throw it
+        if (error.location.start.line !== -1) {
+          context.errors.push(error)
+          return resolvedErrorPromise
+        } else {
+          error = error.error // else we try to get the location from source map
+        }
+      }
       const errorStack = error.stack
       const match = /<anonymous>:(\d+):(\d+)/.exec(errorStack)
       if (match === null) {

src/interpreter.ts

@@ -619,7 +619,7 @@ export function* apply(
         )
 
         const loc = node ? node.loc! : constants.UNKNOWN_LOCATION
-        if (!(e instanceof errors.RuntimeSourceError)) {
+        if (!(e instanceof errors.RuntimeSourceError || e instanceof errors.ExceptionError)) {
           // The error could've arisen when the builtin called a source function which errored.
           // If the cause was a source error, we don't want to include the error.
           // However if the error came from the builtin itself, we need to handle it.

src/stdlib/__tests__/__snapshots__/list.ts.snap

@@ -22,8 +22,7 @@ Object {
       "type": "Runtime",
     },
   ],
-  "parsedErrors": "native:\\"Line -1: Error: head(xs) expects a pair as argument xs, but encountered null\\"
-interpreted:\\"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null\\"",
+  "parsedErrors": "Line 147: Error: head(xs) expects a pair as argument xs, but encountered null",
   "result": undefined,
   "resultStatus": "error",
   "transpiled": "const native = $$NATIVE_STORAGE;

src/stdlib/__tests__/list.ts

@@ -524,10 +524,9 @@ describe('These tests are reporting weird line numbers, as list functions are no
     list_ref(list(1, 2, 3), 3);
   `,
       { chapter: 2, native: true }
-    ).toMatchInlineSnapshot(`
-"native:\\"Line -1: Error: head(xs) expects a pair as argument xs, but encountered null\\"
-interpreted:\\"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null\\""
-`)
+    ).toMatchInlineSnapshot(
+      `"Line 147: Error: head(xs) expects a pair as argument xs, but encountered null"`
+    )
   })
 
   test('bad index error list_ref', () => {

src/transpiler.ts

@@ -364,7 +364,7 @@ export function checkForUndefinedVariablesAndTransformAssignmentsToPropagateBack
       if (previousVariablesToAst.has(name)) {
         const lastAncestor: es.Node = ancestors[ancestors.length - 2]
         const { isConstant, variableLocationId } = previousVariablesToAst.get(name)!
-        if (lastAncestor.type === 'AssignmentExpression') {
+        if (lastAncestor.type === 'AssignmentExpression' && lastAncestor.left === identifier) {
           // if this is an assignment expression, we want to propagate back the change
           if (isConstant) {
             throw new ConstAssignment(identifier, name)
@@ -717,7 +717,7 @@ function getDeclarationsToAccessTranspilerInternals(): es.VariableDeclaration[]
           ),
           'get'
         ),
-        [create.literal(name)]
+        [create.literal(key)]
       )
     }
     return create.declaration(name, kind, value)