added quite a few comments

.eslintrc

@@ -10,6 +10,8 @@
  },
  "globals": {
  "__DEV__": true,
+ "expect": true,
+ "jest": true,
  },
  "parser": "babel-eslint",
  "rules": {

package.json

@@ -4,29 +4,30 @@
  "description": "",
  "main": "index.js",
  "scripts": {
- "test": "echo \"Error: no test specified\" && exit 1"
+ "test": "jest"
  },
  "repository": {
  "type": "git",
  "url": "git+https://github.com/keyanzhang/trivial-js-interpreter.git"
  },
  "keywords": [],
  "author": "Keyan Zhang <[email protected]> (http:https://keya.nz)",
- "license": "ISC",
+ "license": "MIT",
  "bugs": {
  "url": "https://github.com/keyanzhang/trivial-js-interpreter/issues"
  },
  "homepage": "https://github.com/keyanzhang/trivial-js-interpreter#readme",
  "devDependencies": {
  "babel-core": "^6.7.6",
  "babel-eslint": "^6.0.2",
+ "babel-jest": "^11.0.0",
  "babel-plugin-transform-flow-strip-types": "^6.7.0",
+ "babel-polyfill": "^6.7.4",
  "babel-preset-es2015": "^6.6.0",
  "babel-preset-stage-0": "^6.5.0",
  "eslint": "^2.7.0",
  "eslint-config-airbnb": "^7.0.0",
  "eslint-plugin-babel": "^3.2.0",
- "expect": "^1.16.0",
  "jest": "^0.1.40",
  "rimraf": "^2.5.2"
  },

src/Closure.js

@@ -1,21 +1,41 @@
 import { batchExtendEnv } from './Environment';
 
+/*
+ * As we've said,
+ * a closure is just a combination of a function definition and an environment snapshot.
+ * Here we represent a closure simply as a plain object.
+ */
 const makeClosure = (args, body, defineTimeEnv) => ({
  args,
  body,
  env: defineTimeEnv,
 });
 
-const applyClosure = (interp, closure, vals, callTimeEnv, isLexical = true) => {
+/*
+ * `applyClosure` is where the function invocation (computation) happens.
+ * For demonstration purposes, we pass an additional `callTimeEnv` and an `isLexical` flag
+ * so you can toggle the behavior between lexical and dynamic bindings.
+ *
+ * As you can see, there's no black magic™. The resolving behavior is simply determined
+ * by which environment you want the interpreter to use.
+ *
+ * Some people call dynamic binding "late binding". That makes sense, cuz `callTimeEnv`
+ * is definitely fresher than `defineTimeEnv`. We just have too many names though.
+ */
+const applyClosure = (evaluator, closure, vals, callTimeEnv, isLexical = true) => {
  const { args, body, env: defineTimeEnv } = closure;
 
- if (isLexical) {
- const newEnv = batchExtendEnv(args, vals, defineTimeEnv);
- return interp(body, newEnv);
+ if (!isLexical) {
+ // Dynamic scope.
+ // `callTimeEnv` is the latest binding information.
+ const envForTheEvaluator = batchExtendEnv(args, vals, callTimeEnv);
+ return evaluator(body, envForTheEvaluator);
  }
 
- const newEnv = batchExtendEnv(args, vals, callTimeEnv);
- return interp(body, newEnv);
+ // Lexical closure yo.
+ // `defineTimeEnv` is the one that got extracted from the closure.
+ const envForTheEvaluator = batchExtendEnv(args, vals, defineTimeEnv);
+ return evaluator(body, envForTheEvaluator);
 };
 
 export {

src/Environment.js

@@ -1,26 +1,43 @@
 /*
  * An environment is just a mapping from names to values.
+ *
  * For instance, after we evaluate `const foo = 12;`,
  * the environment contains a mapping that looks like `{ foo: 12 }`.
- * Here we use the immutable map from Immutable.js (mostly because its api is pretty nice).
+ *
+ * Here we use the immutable map from Immutable.js. The immutable model fits with
+ * our implementation and its API is pretty nice.
  */
 import { Map as iMap } from 'immutable';
 
+/*
+ * An empty map to start with.
+ */
 const emptyEnv = iMap();
 
+/*
+ * get(name)
+ */
 const lookupEnv = (name, env) => {
  if (!env.has(name)) {
- throw new Error(`unbound variable ${name}`);
+ throw new Error(`unbound variable ${name}. environment snapshot: ${env.toString()}`);
  }
 
  return env.get(name);
 };
 
+/*
+ * set(name, value)
+ */
 const extendEnv = (name, val, env) => env.set(name, val);
 
+/*
+ * Batch set. Nothing fancy.
+ */
 const batchExtendEnv = (names, vals, env) => {
  if (names.length !== vals.length) {
- throw new Error(`unmatched parameter vs. argument count: ${names}, ${vals}`);
+ throw new Error(
+ `unmatched argument count: parameters are [${names}] and arguments are [${vals}]`,
+ );
  }
 
  return names.reduce(

src/Parser.js

@@ -3,7 +3,7 @@
  * Let's just borrow it from babel.
  */
 import { parse as bParse } from 'babylon';
-import { isObject } from './utils';
+import { isObject, mapFilterObject } from './utils';
 
 const defaultOptions = {
  sourceType: 'script',
@@ -21,24 +21,20 @@ const astStripList = [
 ];
 
 /*
- * We don't care about line numbers and source locations for now -- let's clean them up.
+ * We don't care about line numbers nor source locations for now -- let's clean them up.
  * The correct way to implement this AST traversal is to use the visitor pattern.
  * See https://github.com/thejameskyle/babel-handbook/blob/master/translations/en/plugin-handbook.md#traversal
  */
 const cleanupAst = (target) => {
  if (isObject(target)) {
- const fields = Object.keys(target);
-
- return fields.reduce(
- (res, fieldName) => {
- if (astStripList.indexOf(fieldName) === -1) {
- res[fieldName] = cleanupAst(target[fieldName]); // eslint-disable-line no-param-reassign
- }
-
- return res;
- },
- {},
- );
+ return mapFilterObject(target, (val, key) => {
+ if (astStripList.includes(key)) {
+ return false;
+ }
+
+ const newVal = cleanupAst(val);
+ return [ key, newVal ];
+ });
  } else if (Array.isArray(target)) {
  return target.map(cleanupAst);
  }

src/Reader.js

@@ -1,5 +1,8 @@
-import fs from 'fs';
+/*
+ * Sorry if this guy makes you disappointed. It's just a wrapper around `fs.readFileSync`.
+ */
+import { readFileSync } from 'fs';
 
-const readFile = (filename: string) => fs.readFileSync(filename, 'utf8');
+const readFile = (filename: string) => readFileSync(filename, 'utf8');
 
 export { readFile };

src/__tests__/Environment-test.js

@@ -0,0 +1,37 @@
+jest.unmock('../Environment').unmock('immutable');
+
+const {
+ emptyEnv,
+ lookupEnv,
+ extendEnv,
+ batchExtendEnv,
+} = require('../Environment');
+
+describe('Environment', () => {
+ it('has an empty env that contains nothing', () => {
+ expect(emptyEnv.size).toBe(0);
+ });
+
+ it('should extend and lookup stuff correctly', () => {
+ const env = extendEnv('foo', 999, emptyEnv);
+ expect(lookupEnv('foo', env)).toBe(999);
+ });
+
+ it('should batch extend stuff correctly', () => {
+ const env = extendEnv('foo', 999, emptyEnv);
+ const keys = [ 'foo', 'bar', 'james', 'huang' ];
+ const vals = [ 1, 2, 3, 4 ];
+ const extendedEnv = batchExtendEnv(keys, vals, env);
+
+ expect(extendedEnv.size).toBe(4);
+ expect(lookupEnv('james', extendedEnv)).toBe(3);
+ expect(lookupEnv('foo', extendedEnv)).toBe(1);
+ });
+
+ it('should throw when the argument count is not correct', () => {
+ const env = extendEnv('foo', 999, emptyEnv);
+ const keys = [ 'foo', 'bar', 'james', 'huang', 'yeah' ];
+ const vals = [ 1, 2, 3 ];
+ expect(() => { batchExtendEnv(keys, vals, env); }).toThrow();
+ });
+});

src/utils.js

@@ -1,5 +1,25 @@
 const isObject = (x) => x && typeof x === 'object' && !Array.isArray(x);
 
+/*
+ * fn acts as both a mapFn and a predicate
+ */
+const mapFilterObject = (obj, fn) => {
+ const keys = Object.keys(obj);
+ const result = {};
+
+ keys.forEach((key) => {
+ const val = obj[key];
+ const fnResult = fn(val, key); // `fn` return `false` for removal, or a tuple of `[ key, val ]`
+ if (fnResult) {
+ const [ newKey, newVal ] = fnResult;
+ result[newKey] = newVal;
+ }
+ });
+
+ return result;
+};
+
 export {
  isObject,
+ mapFilterObject,
 };