Repository: source-academy/js-slang (ts)

Issue:
"implementation hidden" feature not working when adding function to a string
You can print the implementation of any function just by adding it to an empty string `""`.
For instance:
```JavaScript
//input
make_binary_tree_node+"";
//output
"function make_binary_tree_node(left, value, right) {
    if (!is_binary_tree(left)) {
        throw new Error("Left subtree is not a valid binary tree");
    } else if (!is_binary_tree(right)) {
        throw new Error("Right subtree is not a valid binary tree");
    }
    return list(left, value, right);
}"
```
This can be avoided by checking the type of operand before performing addition operation to strings. (Although in the Source language description it says any type can be added to a string, but a function added to a string should be censored.)

Pull request:
Concat string with (functions and lists) enhancement
Used Source ```toString``` method instead of native ```toString``` during ```BinaryExpression```.

Should resolve #69 and #56

Patch:
diff --git a/src/interpreter.ts b/src/interpreter.ts
index 2ad5e3b8..4df480e3 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 } from './interop'
+import { toJS, 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'
@@ -110,7 +110,6 @@ const getVariable = (context: Context, name: string) => {
   }
   handleError(context, new errors.UndefinedVariable(name, context.runtime.nodes[0]))
 }
-
 const setVariable = (context: Context, name: string, value: any) => {
   let frame: Frame | null = context.runtime.frames[0]
   while (frame) {
@@ -229,18 +228,24 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
     }
   },
   BinaryExpression: function*(node: es.BinaryExpression, context: Context) {
-    const left = yield* evaluate(node.left, context)
-    const right = yield* evaluate(node.right, context)
+    let left = yield* evaluate(node.left, context)
+    let right = yield* evaluate(node.right, context)
 
     const error = rttc.checkBinaryExpression(context, node.operator, left, right)
     if (error) {
       handleError(context, error)
       return undefined
     }
-
     let result
     switch (node.operator) {
       case '+':
+        let isLeftString = typeof left === 'string'
+        let isRightString = typeof right === 'string';
+        if (isLeftString && !isRightString) {
+          right = toString(right)
+        } else if (isRightString && !isLeftString) {
+          left = toString(left)
+        }
         result = left + right
         break
       case '-':


Test patch:
diff --git a/src/__tests__/__snapshots__/index.ts.snap b/src/__tests__/__snapshots__/index.ts.snap
index f0924c2a..aa32396b 100644
--- a/src/__tests__/__snapshots__/index.ts.snap
+++ b/src/__tests__/__snapshots__/index.ts.snap
@@ -1,5 +1,14 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`__SOURCE__ functions now uses Source toString instead of native when +ed with another string 1`] = `
+Object {
+  "status": "finished",
+  "value": "function pair(x, xs) {
+	[implementation hidden]
+}123",
+}
+`;
+
 exports[`Arrow function definition returns itself 1`] = `
 Object {
   "status": "finished",
@@ -254,4 +263,11 @@ Object {
 }
 `;
 
+exports[`list now uses Source toString instead of native when +ed with another string 1`] = `
+Object {
+  "status": "finished",
+  "value": "123[4, [5, [6, []]]]",
+}
+`;
+
 exports[`parseError for missing semicolon 1`] = `"Line 1: Missing semicolon at the end of statement"`;
diff --git a/src/__tests__/block_scoping.ts b/src/__tests__/block_scoping.ts
index d59aa942..6de17710 100644
--- a/src/__tests__/block_scoping.ts
+++ b/src/__tests__/block_scoping.ts
@@ -80,12 +80,13 @@ test("for loops use block scoping instead of function scoping", () => {
     }
     test();
   `;
-  const context = mockContext(4);
+  const context = mockContext(3);
   const promise = runInContext(code, context, { scheduler: "preemptive" });
   return promise.then(obj => {
     expect(obj.status).toBe("finished");
     expect(obj).toMatchSnapshot();
     expect((obj as Finished).value).toBe(true);
+
   });
 });
 
diff --git a/src/__tests__/index.ts b/src/__tests__/index.ts
index e3f4bf28..a68a9aba 100644
--- a/src/__tests__/index.ts
+++ b/src/__tests__/index.ts
@@ -58,12 +58,33 @@ test('Arrow function definition returns itself', () => {
   })
 })
 
+test('list now uses Source toString instead of native when +ed with another string', () => {
+  const code = '"123" + list(4, 5, 6);'
+  const context = mockContext(2)
+  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("123[4, [5, [6, []]]]")
+  })
+})
+
+test('__SOURCE__ functions now uses Source toString instead of native when +ed with another string', () => {
+  const code = ' pair + "123";'
+  const context = mockContext(2)
+  const promise = runInContext(code, context, { scheduler: 'preemptive' })
+  return promise.then(obj => {
+    expect(obj).toMatchSnapshot()
+    expect(obj.status).toBe('finished')
+  })
+})
+
 test('Factorial arrow function', () => {
   const code = stripIndent`
     const fac = (i) => i === 1 ? 1 : i * fac(i-1);
     fac(5);
   `
-  const context = mockContext()
+  const context = mockContext(4)
   const promise = runInContext(code, context, { scheduler: 'preemptive' })
   return promise.then(obj => {
     expect(obj).toMatchSnapshot()