Repository: source-academy/js-slang
Pull request #96 (2018-10-19 12:32:22)

Disable function expressions
Right now, something like this is permitted(!!):
```
  map(function a(x) {
    return x + 1;
  }, list(1));
```
It's undocumented and messes up the environment model (no binding for `a` is created in the calling frame). 
Happens because estree is converting it to a function expression (which we include as an undocumented feature) if it sees a function declaration where it expects an expression.

To make js-slang align with the spec, this PR disables both named and unnamed function definition expressions. Only arrow functions are allowed as an expression now.

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

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.04%**) to **60.433%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/19614842/badge)](https://coveralls.io/builds/19614842) |
| :-- | --: |
| Change from base [Build 178](https://coveralls.io/builds/19512872): |  0.04% |
| Covered Lines: | 787 |
| Relevant Lines: | 1231 |

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


Patch:
diff --git a/package.json b/package.json
index 09683ee8..9f97227e 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "js-slang",
-  "version": "0.1.9",
+  "version": "0.1.10",
   "description": "Javascript-based interpreter for slang, written in Typescript",
   "author": {
     "name": "Source Academy",
diff --git a/src/interpreter.ts b/src/interpreter.ts
index 4df480e3..6a76a2a1 100644
--- a/src/interpreter.ts
+++ b/src/interpreter.ts
@@ -175,9 +175,6 @@ export const evaluators: { [nodeType: string]: Evaluator<es.Node> } = {
     }
     return res
   },
-  FunctionExpression: function*(node: es.FunctionExpression, context: Context) {
-    return new Closure(node, currentFrame(context), context)
-  },
   ArrowFunctionExpression: function*(node: es.Function, context: Context) {
     return new ArrowClosure(node, currentFrame(context), context)
   },
diff --git a/src/syntaxTypes.ts b/src/syntaxTypes.ts
index 134f3e40..357c7861 100644
--- a/src/syntaxTypes.ts
+++ b/src/syntaxTypes.ts
@@ -11,7 +11,6 @@ const syntaxTypes: { [nodeName: string]: number } = {
   BinaryExpression: 1,
   LogicalExpression: 1,
   ConditionalExpression: 1,
-  FunctionExpression: 1,
   ArrowFunctionExpression: 1,
   Identifier: 1,
   Literal: 1,
@@ -45,7 +44,9 @@ const syntaxTypes: { [nodeName: string]: number } = {
   CatchClause: Infinity,
   DoWhileStatement: Infinity,
   ForInStatement: Infinity,
-  SequenceExpression: Infinity
+  SequenceExpression: Infinity,
+  FunctionExpression: Infinity,
+
 }
 
 export default syntaxTypes


Test patch:
diff --git a/src/__tests__/__snapshots__/disallowed_syntax.ts.snap b/src/__tests__/__snapshots__/disallowed_syntax.ts.snap
index f1d6c96e..9fedeab8 100644
--- a/src/__tests__/__snapshots__/disallowed_syntax.ts.snap
+++ b/src/__tests__/__snapshots__/disallowed_syntax.ts.snap
@@ -8,4 +8,8 @@ exports[`Cannot leave blank expressions in for loops 3`] = `"Line 2: For stateme
 
 exports[`Cannot leave blank expressions in for loops 4`] = `"Line 2: For statement cannot have empty init,test,update expressions."`;
 
+exports[`Cannot use js function expressions 1`] = `"Line 2: Function expressions are not allowed"`;
+
 exports[`Cannot use update expressions 1`] = `"Line 3: Update expressions are not allowed"`;
+
+exports[`DEFINITELY CANNOT use named function declarations as expressions 1`] = `"Line 2: Function expressions are not allowed"`;
diff --git a/src/__tests__/disallowed_syntax.ts b/src/__tests__/disallowed_syntax.ts
index c6260d20..15550c0e 100644
--- a/src/__tests__/disallowed_syntax.ts
+++ b/src/__tests__/disallowed_syntax.ts
@@ -57,3 +57,32 @@ test("Cannot use update expressions", () => {
   });
 });
 
+test("Cannot use js function expressions", () => {
+  const code = `
+  map(function(x) {
+    return x + 1;
+  }, list(1));
+  `;
+  const context = mockContext(3);
+  const promise = runInContext(code, context, { scheduler: "preemptive" });
+  return promise.then(obj => {
+    expect(obj.status).toBe("error");
+    const errors = parseError(context.errors);
+    expect(errors).toMatchSnapshot();
+  });
+});
+
+test("DEFINITELY CANNOT use named function declarations as expressions", () => {
+  const code = `
+  map(function a(x) {
+    return x + 1;
+  }, list(1));
+  `;
+  const context = mockContext(3);
+  const promise = runInContext(code, context, { scheduler: "preemptive" });
+  return promise.then(obj => {
+    expect(obj.status).toBe("error");
+    const errors = parseError(context.errors);
+    expect(errors).toMatchSnapshot();
+  });
+});