Repository: source-academy/js-slang
Pull request #92 (2018-10-13 04:49:11)

Fix functions equality
#88 

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

* **15** of **18**   **(83.33%)**  changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**1.5%**) to **61.876%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [src/types.ts](https://coveralls.io/builds/19501622/source?filename=src%2Ftypes.ts#L176) | 12 | 15 | 80.0%
<!-- | **Total:** | **15** | **18** | **83.33%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/19501622/badge)](https://coveralls.io/builds/19501622) |
| :-- | --: |
| Change from base [Build 153](https://coveralls.io/builds/19346361): |  1.5% |
| Covered Lines: | 804 |
| Relevant Lines: | 1235 |

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

thomastanck: Might want to update this PR by resolving the merge conflicts.

Patch:
diff --git a/src/createContext.ts b/src/createContext.ts
index 50678b0c..12255e04 100644
--- a/src/createContext.ts
+++ b/src/createContext.ts
@@ -48,7 +48,7 @@ export const ensureGlobalEnvironmentExist = (context: Context) => {
   }
 }
 
-const defineSymbol = (context: Context, name: string, value: Value) => {
+export const defineSymbol = (context: Context, name: string, value: Value) => {
   const globalFrame = context.runtime.frames[0]
   Object.defineProperty(
     globalFrame.environment,
diff --git a/src/interop.ts b/src/interop.ts
index aae9a6a1..e19ab6b4 100644
--- a/src/interop.ts
+++ b/src/interop.ts
@@ -24,19 +24,16 @@ export const closureToJS = (value: Value, context: Context, klass: string) => {
       DummyClass.prototype.constructor = DummyClass
     }
   })
+  DummyClass.toString = function() {
+    return toString(value)
+  }
   DummyClass.call = (thisArg: Value, ...args: Value[]) => {
     return DummyClass.apply(thisArg, args)
   }
   return DummyClass
 }
 
-export const toJS = (value: Value, context: Context, klass?: string) => {
-  if (value instanceof Closure || value instanceof ArrowClosure) {
-    return value.fun
-  } else {
-    return value
-  }
-}
+
 
 const stripBody = (body: string) => {
   const lines = body.split(/\n/)
diff --git a/src/interpreter.ts b/src/interpreter.ts
index b68bc50d..519f38e1 100644
--- a/src/interpreter.ts
+++ b/src/interpreter.ts
@@ -2,7 +2,7 @@
 /* tslint:disable: object-literal-shorthand*/
 import * as es from 'estree'
 import * as constants from './constants'
-import { toJS, toString } from './interop'
+import { toString } from './interop'
 import * as errors from './interpreter-errors'
 import { ArrowClosure, Closure, Context, ErrorSeverity, Frame, SourceError, Value, Environment } from './types'
 import { createNode } from './utils/node'
@@ -26,7 +26,7 @@ const createFrame = (
   callExpression?: es.CallExpression
 ): Frame => {
   const frame: Frame = {
-    name: closure.name, // TODO: Change this
+    name: closure.functionName, // TODO: Change this
     parent: closure.frame,
     environment: {}
   }
@@ -607,8 +607,7 @@ export function* apply(
       result = new ReturnValue(yield* evaluate(fun.node.body, context))
     } else if (typeof fun === 'function') {
       try {
-        const as = args.map(a => toJS(a, context))
-        result = fun.apply(thisContext, as)
+        result = fun.apply(thisContext, args)
         break
       } catch (e) {
         // Recover from exception
diff --git a/src/types.ts b/src/types.ts
index 6b4fd8fc..723ac417 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -1,4 +1,4 @@
-/* tslint:disable:interface-name max-classes-per-file */
+/* tslint:disable:interface-functionName max-classes-per-file */
 
 import { SourceLocation } from 'acorn'
 import * as es from 'estree'
@@ -153,50 +153,65 @@ export interface Frame {
   thisContext?: Value
 }
 
+class Callable extends Function {
+  constructor(f: any) {
+    super();
+    return Object.setPrototypeOf(f, new.target.prototype);
+  }
+}
+
 /**
  * Models function value in the interpreter environment.
  */
-export class Closure {
+export class Closure extends Callable {
   /** Keep track how many lambdas are created */
   private static lambdaCtr = 0
 
   /** Unique ID defined for anonymous closure */
-  public name: string
+  public functionName: string
 
   /** Fake closure function */
   // tslint:disable-next-line:ban-types
   public fun: Function
 
   constructor(public node: es.FunctionExpression, public frame: Frame, context: Context) {
+    super(function (this: any, ...args: any[]) {
+      return funJS.apply(this, args)
+    })
     this.node = node
     try {
       if (this.node.id) {
-        this.name = this.node.id.name
+        this.functionName = this.node.id.name
       }
     } catch (e) {
-      this.name = `Anonymous${++Closure.lambdaCtr}`
+      this.functionName = `Anonymous${++Closure.lambdaCtr}`
     }
-    this.fun = closureToJS(this, context, this.name)
+    const funJS = closureToJS(this, context, this.functionName)
+    this.fun = funJS
   }
 }
 
 /**
  * Modified from class Closure, for construction of arrow functions.
  */
-export class ArrowClosure {
+export class ArrowClosure extends Callable {
   /** Keep track how many lambdas are created */
   private static arrowCtr = 0
 
   /** Unique ID defined for anonymous closure */
-  public name: string
+  public functionName: string
 
   /** Fake closure function */
   // tslint:disable-next-line:ban-types
   public fun: Function
 
   constructor(public node: es.Function, public frame: Frame, context: Context) {
-    this.name = `Anonymous${++ArrowClosure.arrowCtr}`
-    this.fun = closureToJS(this, context, this.name)
+    super(function (this: any, ...args: any[]) {
+      return funJS.apply(this, args)
+    })
+    this.functionName = `Anonymous${++ArrowClosure.arrowCtr}`
+    const funJS = closureToJS(this, context, this.functionName)
+    this.fun = funJS
   }
 }
 


Test patch:
diff --git a/src/__tests__/__snapshots__/index.ts.snap b/src/__tests__/__snapshots__/index.ts.snap
index bebf1b23..f6c199b8 100644
--- a/src/__tests__/__snapshots__/index.ts.snap
+++ b/src/__tests__/__snapshots__/index.ts.snap
@@ -12,209 +12,11 @@ Object {
 exports[`Arrow function definition returns itself 1`] = `
 Object {
   "status": "finished",
-  "value": ArrowClosure {
-    "frame": Object {
-      "environment": Object {
-        "Infinity": Infinity,
-        "NaN": NaN,
-        "display": [Function],
-        "error": [Function],
-        "math_E": 2.718281828459045,
-        "math_LN10": 2.302585092994046,
-        "math_LN2": 0.6931471805599453,
-        "math_LOG10E": 0.4342944819032518,
-        "math_LOG2E": 1.4426950408889634,
-        "math_PI": 3.141592653589793,
-        "math_SQRT1_2": 0.7071067811865476,
-        "math_SQRT2": 1.4142135623730951,
-        "math_abs": [Function],
-        "math_acos": [Function],
-        "math_acosh": [Function],
-        "math_asin": [Function],
-        "math_asinh": [Function],
-        "math_atan": [Function],
-        "math_atan2": [Function],
-        "math_atanh": [Function],
-        "math_cbrt": [Function],
-        "math_ceil": [Function],
-        "math_clz32": [Function],
-        "math_cos": [Function],
-        "math_cosh": [Function],
-        "math_exp": [Function],
-        "math_expm1": [Function],
-        "math_floor": [Function],
-        "math_fround": [Function],
-        "math_hypot": [Function],
-        "math_imul": [Function],
-        "math_log": [Function],
-        "math_log10": [Function],
-        "math_log1p": [Function],
-        "math_log2": [Function],
-        "math_max": [Function],
-        "math_min": [Function],
-        "math_pow": [Function],
-        "math_random": [Function],
-        "math_round": [Function],
-        "math_sign": [Function],
-        "math_sin": [Function],
-        "math_sinh": [Function],
-        "math_sqrt": [Function],
-        "math_tan": [Function],
-        "math_tanh": [Function],
-        "math_trunc": [Function],
-        "parse_int": [Function],
-        "prompt": [Function],
-        "runtime": [Function],
-        "undefined": undefined,
-      },
-      "name": "global",
-      "parent": null,
-    },
-    "fun": [Function],
-    "name": "Anonymous1",
-    "node": Node {
-      "__id": "node_12",
-      "body": Node {
-        "__id": "node_11",
-        "end": 8,
-        "loc": SourceLocation {
-          "end": Position {
-            "column": 8,
-            "line": 1,
-          },
-          "start": Position {
-            "column": 6,
-            "line": 1,
-          },
-        },
-        "raw": "42",
-        "start": 6,
-        "type": "Literal",
-        "value": 42,
-      },
-      "end": 8,
-      "expression": true,
-      "generator": false,
-      "id": null,
-      "loc": SourceLocation {
-        "end": Position {
-          "column": 8,
-          "line": 1,
-        },
-        "start": Position {
-          "column": 0,
-          "line": 1,
-        },
-      },
-      "params": Array [],
-      "start": 0,
-      "type": "ArrowFunctionExpression",
-    },
-  },
+  "value": [Function],
 }
 `;
 
-exports[`Arrow function definition returns itself 2`] = `
-ArrowClosure {
-  "frame": Object {
-    "environment": Object {
-      "Infinity": Infinity,
-      "NaN": NaN,
-      "display": [Function],
-      "error": [Function],
-      "math_E": 2.718281828459045,
-      "math_LN10": 2.302585092994046,
-      "math_LN2": 0.6931471805599453,
-      "math_LOG10E": 0.4342944819032518,
-      "math_LOG2E": 1.4426950408889634,
-      "math_PI": 3.141592653589793,
-      "math_SQRT1_2": 0.7071067811865476,
-      "math_SQRT2": 1.4142135623730951,
-      "math_abs": [Function],
-      "math_acos": [Function],
-      "math_acosh": [Function],
-      "math_asin": [Function],
-      "math_asinh": [Function],
-      "math_atan": [Function],
-      "math_atan2": [Function],
-      "math_atanh": [Function],
-      "math_cbrt": [Function],
-      "math_ceil": [Function],
-      "math_clz32": [Function],
-      "math_cos": [Function],
-      "math_cosh": [Function],
-      "math_exp": [Function],
-      "math_expm1": [Function],
-      "math_floor": [Function],
-      "math_fround": [Function],
-      "math_hypot": [Function],
-      "math_imul": [Function],
-      "math_log": [Function],
-      "math_log10": [Function],
-      "math_log1p": [Function],
-      "math_log2": [Function],
-      "math_max": [Function],
-      "math_min": [Function],
-      "math_pow": [Function],
-      "math_random": [Function],
-      "math_round": [Function],
-      "math_sign": [Function],
-      "math_sin": [Function],
-      "math_sinh": [Function],
-      "math_sqrt": [Function],
-      "math_tan": [Function],
-      "math_tanh": [Function],
-      "math_trunc": [Function],
-      "parse_int": [Function],
-      "prompt": [Function],
-      "runtime": [Function],
-      "undefined": undefined,
-    },
-    "name": "global",
-    "parent": null,
-  },
-  "fun": [Function],
-  "name": "Anonymous1",
-  "node": Node {
-    "__id": "node_12",
-    "body": Node {
-      "__id": "node_11",
-      "end": 8,
-      "loc": SourceLocation {
-        "end": Position {
-          "column": 8,
-          "line": 1,
-        },
-        "start": Position {
-          "column": 6,
-          "line": 1,
-        },
-      },
-      "raw": "42",
-      "start": 6,
-      "type": "Literal",
-      "value": 42,
-    },
-    "end": 8,
-    "expression": true,
-    "generator": false,
-    "id": null,
-    "loc": SourceLocation {
-      "end": Position {
-        "column": 8,
-        "line": 1,
-      },
-      "start": Position {
-        "column": 0,
-        "line": 1,
-      },
-    },
-    "params": Array [],
-    "start": 0,
-    "type": "ArrowFunctionExpression",
-  },
-}
-`;
+exports[`Arrow function definition returns itself 2`] = `[Function]`;
 
 exports[`Cannot overwrite consts even when assignment is allowed 1`] = `"Line 4: Cannot assign new value to constant constant"`;
 
@@ -265,6 +67,13 @@ Object {
 }
 `;
 
+exports[`Multi-dimensional arrays display properly 2`] = `
+Object {
+  "status": "finished",
+  "value": "[1, function a() {}, 3, [() => 1, 5]]",
+}
+`;
+
 exports[`Simple inifinite recursion represents CallExpression well 1`] = `
 "Line 1: Infinite recursion
   x(x => x(x))..  x(x => x(x))..  x(x => x(x)).."
diff --git a/src/__tests__/index.ts b/src/__tests__/index.ts
index d0e1aa81..41fa201a 100644
--- a/src/__tests__/index.ts
+++ b/src/__tests__/index.ts
@@ -2,6 +2,7 @@ import { stripIndent } from 'common-tags'
 import { mockContext } from '../mocks/context'
 import { parseError, runInContext } from '../index'
 import { Finished } from '../types'
+import { defineSymbol } from '../createContext'
 
 test('Empty code returns undefined', () => {
   const code = ''
@@ -25,6 +26,20 @@ test('Single string self-evaluates to itself', () => {
   })
 })
 
+test('Multi-dimensional arrays display properly', () => {
+  const code = `
+    function a() {} 
+    ""+[1, a, 3, [() => 1, 5]];
+   `;
+  const context = mockContext(4)
+  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('[1, function a() {}, 3, [() => 1, 5]]')
+  })
+})
+
 test('Single number self-evaluates to itself', () => {
   const code = '42;'
   const context = mockContext()
@@ -189,6 +204,22 @@ test(
   30000
 )
 
+test("Functions passed into non-source functions remain equal", () => {
+  const code = `
+  function t(x, y, z) {
+    return x + y + z;
+  }
+  identity(t) === t && t(1, 2, 3) === 6;
+  `;
+  const context = mockContext(4);
+  defineSymbol(context, "identity", (x: any) => x)
+  const promise = runInContext(code, context, { scheduler: "preemptive" })
+  return promise.then(obj => {
+    expect(obj.status).toBe('finished')
+    expect((obj as Finished).value).toBe(true)
+  });
+});
+
 test('Metacircular Interpreter parses Arrow Function Expressions properly', () => {
   const code = 'stringify(parse("x => x + 1;"));'
   const context = mockContext(4)
@@ -254,4 +285,3 @@ test('Deep object assignment and retrieval', () => {
     expect((obj as Finished).value).toBe("string")
   })
 })
-