Handle forwarding bindings better; fixes #736 (#737)

Only add forwarding bindings to import names when:

- not the entrypoint or
- importing for phase > 0
- importing a compiletime binding

package.json

@@ -72,6 +72,7 @@
     "jquery": "3.1.1",
     "nyc": "^6.0.0",
     "prettier": "^1.5.3",
+    "rimraf": "^2.6.1",
     "source-map": "~0.5.3",
     "source-map-support": "^0.4.0",
     "webpack": "^1.13.1"

src/module-visitor.js

@@ -10,14 +10,24 @@ import SweetModule from './sweet-module';
 import { List } from 'immutable';
 import SweetToShiftReducer from './sweet-to-shift-reducer';
 import codegen, { FormattedCodeGen } from 'shift-codegen';
+import Syntax from './syntax';
 
 import type { Context } from './sweet-loader';
 
+export function isBoundToCompiletime(name: Syntax, store: Map<*, *>) {
+  let resolvedName = name.resolve(0);
+  if (store.has(resolvedName)) {
+    return store.get(resolvedName) instanceof CompiletimeTransform;
+  }
+  return false;
+}
+
 export function bindImports(
   impTerm: any,
   exModule: SweetModule,
   phase: any,
   context: Context,
+  isEntrypoint: boolean,
 ) {
   let names = [];
   let phaseToBind = impTerm.forSyntax ? phase + 1 : phase;
@@ -29,7 +39,14 @@ export function bindImports(
     if (exportName != null) {
       let newBinding = gensym('_default');
       let toForward = exportName.exportedName;
-      context.bindings.addForward(name, toForward, newBinding, phaseToBind);
+
+      if (
+        !isEntrypoint ||
+        isBoundToCompiletime(toForward, context.store) ||
+        impTerm.forSyntax
+      ) {
+        context.bindings.addForward(name, toForward, newBinding, phaseToBind);
+      }
       names.push(name);
     }
   }
@@ -44,7 +61,13 @@ export function bindImports(
         let toForward = exportName.name
           ? exportName.name
           : exportName.exportedName;
-        context.bindings.addForward(name, toForward, newBinding, phaseToBind);
+        if (
+          !isEntrypoint ||
+          isBoundToCompiletime(toForward, context.store) ||
+          impTerm.forSyntax
+        ) {
+          context.bindings.addForward(name, toForward, newBinding, phaseToBind);
+        }
         names.push(name);
       }
     });
@@ -92,7 +115,7 @@ export default class {
         );
         this.visit(mod, phase, store, mod.path);
       }
-      bindImports(imp, mod, phase, this.context);
+      bindImports(imp, mod, phase, this.context, false);
     });
     for (let term of mod.compiletimeItems()) {
       if (S.isSyntaxDeclarationStatement(term)) {
@@ -114,7 +137,7 @@ export default class {
           cwd,
         );
         this.invoke(mod, phase, store, mod.path);
-        bindImports(imp, mod, phase, this.context);
+        bindImports(imp, mod, phase, this.context, false);
       }
     });
     let items = mod.runtimeItems();

src/sweet-loader.js

@@ -154,10 +154,18 @@ export default class SweetLoader {
   // skip instantiate
   compile(
     entryPath: string,
-    refererName?: string,
-    enforceLangPragma?: boolean = true,
+    {
+      refererName,
+      enforceLangPragma,
+      isEntrypoint,
+    }: {
+      refererName?: string,
+      enforceLangPragma: boolean,
+      isEntrypoint: boolean,
+    },
   ) {
     let metadata = {
+      isEntrypoint,
       enforceLangPragma,
       entryPath,
     };
@@ -168,7 +176,11 @@ export default class SweetLoader {
   }
 
   get(entryPath: string, entryPhase: number, refererName?: string) {
-    return this.compile(`${entryPath}:${entryPhase}`, refererName);
+    return this.compile(`${entryPath}:${entryPhase}`, {
+      refererName,
+      enforceLangPragma: true,
+      isEntrypoint: false,
+    });
   }
 
   read(source: string): List<Term> {
@@ -197,6 +209,7 @@ export default class SweetLoader {
       _.merge(this.context, {
         currentScope: [outScope, inScope],
         cwd: path,
+        isEntrypoint: metadata.isEntrypoint,
       }),
     );
     return new SweetModule(

src/sweet.js

@@ -14,7 +14,11 @@ function compileModule(
   loader: SweetLoader,
   refererName?: string,
 ) {
-  return loader.compile(entryPath, refererName, false);
+  return loader.compile(entryPath, {
+    refererName,
+    enforceLangPragma: false,
+    isEntrypoint: true,
+  });
 }
 
 export function parse(

src/token-expander.js

@@ -14,15 +14,10 @@ import { ALL_PHASES } from './syntax';
 import ASTDispatcher from './ast-dispatcher';
 import Syntax from './syntax.js';
 import ScopeReducer from './scope-reducer';
-import ModuleVisitor, { bindImports } from './module-visitor';
-
-function isBoundToCompiletime(name, store) {
-  let resolvedName = name.resolve(0);
-  if (store.has(resolvedName)) {
-    return store.get(resolvedName) instanceof CompiletimeTransform;
-  }
-  return false;
-}
+import ModuleVisitor, {
+  bindImports,
+  isBoundToCompiletime,
+} from './module-visitor';
 
 // $FlowFixMe: flow doesn't know about the CloneReducer yet
 class RegisterBindingsReducer extends Term.CloneReducer {
@@ -162,7 +157,13 @@ export default class TokenExpander extends ASTDispatcher {
         mod.path,
       );
     }
-    bindImports(term, mod, this.context.phase, this.context);
+    bindImports(
+      term,
+      mod,
+      this.context.phase,
+      this.context,
+      this.context.isEntrypoint,
+    );
     let defaultBinding = null;
     let namedImports = List();
     if (term.defaultBinding != null) {

test/modules/_helpers.js

@@ -0,0 +1,31 @@
+import { writeFileSync, mkdtempSync } from 'fs';
+import { execSync } from 'child_process';
+import { compile } from '../../src/sweet';
+import NodeLoader from '../../src/node-loader';
+import { sync as rimraf } from 'rimraf';
+
+// write each module to a temp dir, compile them with sweet,
+// compile with babel to get commonjs modules, then assert on
+// the console log in main.js
+export function compileToCjs(t, inputModules, expected) {
+  let tmp = mkdtempSync(`./`);
+  let loader = new NodeLoader(tmp);
+  for (let fname of Object.keys(inputModules)) {
+    let path = `${tmp}/${fname}`;
+    writeFileSync(path, inputModules[fname], 'utf8');
+    let outfile = compile(path, loader);
+    writeFileSync(`${tmp}/${fname}.esm`, outfile.code, 'utf8');
+    execSync(
+      `babel --out-file ${tmp}/${fname} ${tmp}/${fname}.esm --no-babelrc --plugins=transform-es2015-modules-commonjs`,
+    );
+  }
+  let result = execSync(`node ${tmp}/main.js`).toString().trim();
+  t.is(result, expected);
+  rimraf(tmp);
+}
+compileToCjs.title = (title, inputStore, expected) =>
+  `${title}
+${Array.from(Object.entries(inputStore))
+    .map(([modName, modSrc]) => `${modName}\n----\n${modSrc}\n----`)
+    .join('\n')}
+> ${expected}`;

test/modules/test-expanding-cjs.js

@@ -0,0 +1,43 @@
+import { compileToCjs } from './_helpers';
+import test from 'ava';
+
+test(
+  'exporting and importing a variable works',
+  compileToCjs,
+  {
+    'mod.js': `
+  'lang sweet.js';
+  export var x = 'foo';
+  `,
+
+    'main.js': `
+  'lang sweet.js';
+  import { x } from './mod.js';
+  console.log(x);
+  `,
+  },
+  'foo',
+);
+
+test(
+  'exporting and importing a name passing through macro expansion works',
+  compileToCjs,
+  {
+    'mod.js': `
+  'lang sweet.js';
+  syntax m = ctx => {
+    let first = ctx.next().value;
+    let second = ctx.next().value;
+    return #\`var \${first} = \${second}\`;
+  }  
+  export m x 'foo'
+  `,
+
+    'main.js': `
+  'lang sweet.js';
+  import { x } from './mod.js';
+  console.log(x);
+  `,
+  },
+  'foo',
+);