Skip to content

Commit

Permalink
Fix macro-require from REPL and relative paths
Browse files Browse the repository at this point in the history
This change fixes 2 classes of bugs relating to using `require` to load
macros, and adds unit-tests for them:

 1. Importing eslisp into a Node.js REPL and using that eslisp instance
    to run code that calls `require` would fail due to an assumption
    that `require.main` would always be defined, which it's not when
    running an interactive REPL.  It should not fail.
 2. When an eslisp file called `require` with a relative path, it was
    handled relative to the present working directory.  It should
    instead be relative to that file.

Closes anko#48.  I went with a more in-depth fix than proposed there by
@ynohtna, after discovering the second issue while writing tests.
  • Loading branch information
anko committed Feb 9, 2017
1 parent 8f423aa commit 83b4002
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 42 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"bugs": "https://github.com/anko/eslisp/issues",
"license": "ISC",
"devDependencies": {
"concat-stream": "^1.6.0",
"livescript": "^1.4.0",
"rimraf": "^2.4.1",
"source-map": "^0.5.3",
Expand Down
54 changes: 35 additions & 19 deletions src/built-in-macros.ls
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ statementify = require \./es-statementify
import-compilerspace-macro
multiple-statements
} = require \./import-macro
Module = require \module
require! \path

chained-binary-expr = (type, operator) ->
macro = ->
Expand Down Expand Up @@ -401,25 +403,39 @@ contents =

compile-as-macro = (es-ast) ->

# This hack around require makes loading macros from relative paths work.
#
# It was guided by LiveScript's implementation
# https://github.com/gkz/LiveScript/blob/a7525ce6fe7d4906f5d401edf94f15fe5a6b471e/src/node.ls#L10-L18
# which originally derives from the Coco language.
#
# The gist of it is to use the main module's `require` method, such that
# the current working directory is the root relative to which packages
# are searched.

{main} = require
dirname = "."
main
..paths = main.constructor._node-module-paths process.cwd!
..filename = dirname

root-require = main.require.bind main

let require = root-require
[ lookup-filename, displayed-filename ] = do ->
# If we know we are compiling a particular file, have `require` look up
# relative paths relative to that file. This makes macro `require`s
# with relative paths work as expected.
| env.filename =>
p = path.resolve that
[ p, p ]
# If we are compiling without a filename (that is, code from stdin or
# interactively in a REPL), have `require` resolve relative to the
# current working directory.
| otherwise => [ process.cwd!, null ]

new-module = new Module "eslisp-internal:#displayed-filename" null
..paths = Module._node-module-paths lookup-filename
..filename = displayed-filename
require-substitute = new-module.require.bind new-module

/*
filename = env.filename
if filename
fname = path.resolve env.filename
new-module = new Module "eslisp-internal:#fname" null
..paths = Module._node-module-paths fname
..filename = path.resolve fname
require-substitute = new-module.require.bind new-module
else
new-module = new Module "eslisp-internal:." null
..paths = Module._node-module-paths process.cwd!
..filename = null
require-substitute = new-module.require.bind new-module
*/

let require = require-substitute
eval "(#{env.compile-to-js es-ast})"

switch &length
Expand Down
6 changes: 3 additions & 3 deletions src/cli.ls
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ compiler-opts = {}
if parsed-options.transform
compiler-opts.transform-macros = that .map require

compile-and-show = (code, filename) ->
compile-and-show = (code, filename=null) ->
code .= to-string!
try

Expand All @@ -84,9 +84,9 @@ compile-and-show = (code, filename) ->

var js-code, js-map

if opt-map-out or opt-map-embed
compiler-opts.filename = filename

compiler-opts.filename = filename
if opt-map-out or opt-map-embed

# Receive both code and source map from the compiler.
{ code, map } = esl.with-source-map code, compiler-opts
Expand Down
7 changes: 5 additions & 2 deletions src/env.ls
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class env
# implements macro scope; macros defined in the new environment aren't
# visible in the outer one.

env @macro-table, { @import-target-macro-tables }
env @macro-table, { @import-target-macro-tables, @filename }

derive-flattened : ~>

Expand Down Expand Up @@ -131,7 +131,10 @@ class env

env do
table-to-read-from
{ import-target-macro-tables : tables-to-import-into }
{
import-target-macro-tables : tables-to-import-into
@filename
}

derive-root : ~>
root-table = find-root @macro-table
Expand Down
120 changes: 102 additions & 18 deletions test.ls
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
#!/usr/bin/env lsc
test = (name, test-func) ->
(require \tape) name, (t) ->
test-func.call t # Make `this` refer to tape's asserts
t.end! # Automatically end tests
consume-map = require \source-map .SourceMapConsumer .bind!
{ spawn } = require \child_process
{ unique } = require \prelude-ls
require! <[ tape tmp fs uuid rimraf path ]>
concat = require \concat-stream

esl = require \./src/index.ls

consume-map = require \source-map .SourceMapConsumer .bind!

{ unique } = require \prelude-ls
require! <[ tmp fs uuid rimraf path ]>
test = (name, test-func) ->
tape name, (t) ->
test-func.call t # Make `this` refer to tape's asserts
t.end! # Automatically end tests
test-async = (name, test-func) ->
tape name, (t) ->
test-func.call t
# Don't end automatically

test "nothing" ->
esl ""
Expand Down Expand Up @@ -878,33 +883,58 @@ test "macro returning atom with empty or null name fails" ->
"""
Error

test "macros can be required relative to root directory" ->
test "require in macros is relative to the eslisp file" ->

{ exec-sync } = require \child_process

{ name : dir-name, fd } = tmp.dir-sync!
{ name : root-dir, fd } = tmp.dir-sync!

# Create dummy temporary file
# Create simple module in the root to import as a macro
module-basename = "#{uuid.v4!}.js"
module-path = path.join dir-name, module-basename
module-path = path.join root-dir, module-basename
module-fd = fs.open-sync module-path, \a+
fs.write-sync module-fd, "module.exports = function() { }"

fs.write-sync module-fd, '''
module.exports = function() {
return this.string("BOOM SHAKALAKA")
}
'''

# Create an eslisp file in the root that requires the root directory module
# as a macro
main-basename = "#{uuid.v4!}.js"
main-path = path.join dir-name, main-basename
main-path = path.join root-dir, main-basename
main-fd = fs.open-sync main-path, \a+
fs.write-sync main-fd, """
(macro (object x (require "./#module-basename")))
(x)
"""
# Create a subdirectory
subdir-basename = "subdir"
subdir-path = path.join root-dir, subdir-basename
fs.mkdir-sync subdir-path

# Create an eslisp file in the sub-directory that requires the module in the
# root directory (its parent directory) as a macro
main2-basename = "#{uuid.v4!}.js"
main2-path = path.join subdir-path, main2-basename
main2-fd = fs.open-sync main2-path, \a+
fs.write-sync main2-fd, """
(macro (object x (require "../#module-basename")))
(x)
"""

eslc-path = path.join process.cwd!, "bin/eslc"
try
exec-sync "#eslc-path #main-basename", cwd : dir-name
..to-string! `@equals` "\n"
# Call the eslisp compiler with current working directory set to the root
# directory, with both eslisp files in turn, and expect neither to error.
exec-sync "#eslc-path #main-basename", cwd : root-dir
..to-string! `@equals` "'BOOM SHAKALAKA';\n"
exec-sync "#eslc-path #main2-path", cwd : root-dir
..to-string! `@equals` "'BOOM SHAKALAKA';\n"
# This second one will only succeed if `require` within macros works
# relative to the eslisp file being compiled.
finally
e <~ rimraf dir-name
e <~ rimraf root-dir
@equals e, null

test "macros can be required from node_modules relative to root directory" ->
Expand Down Expand Up @@ -1309,3 +1339,57 @@ test "macro return source map (across multiple lines)" ->
..line `@equals` 3
..column `@equals` 0
..name `@equals` \b

test-async "macros can be defined when eslisp is used from the Node REPL" ->
@plan 2
# Spawn a new Node.js REPL process
spawn "node"
# Feed it some input:
..stdin
# Require the eslisp module in it
..write "eslisp = require('.')\n"
# Create a stateful eslisp compiler instance
..write "x = eslisp.stateful()\n"
# Define a macro in it
..write "x('(macro x (lambda () (return \\'42)))')\n"
# Call that macro, and log the resulting JavaScript code
..write "console.log(x('(x)'))\n"
..end!
..stdout.pipe concat (.to-string! `@equals` '42;\n')
..stderr.pipe concat (.to-string! `@equals` '')
..on \close ~> @end!

test-async "macros can be required from eslisp in Node REPL relative to REPL cwd" ->

{ name : dir-name, fd } = tmp.dir-sync!

# Create dummy temporary file
module-basename = "#{uuid.v4!}.js"
module-path = path.join dir-name, module-basename
module-fd = fs.open-sync module-path, \a+
fs.write-sync module-fd, '''
module.exports = function() {
return this.atom(42)
}
'''

@plan 4

spawn "node" cwd : dir-name
# Feed it some input:
..stdin
# Require the eslisp module in it
..write "eslisp = require('#{process.cwd!}')\n"
# Create a stateful eslisp compiler instance
..write "x = eslisp.stateful()\n"
# Define a macro in it
..write "x('(macro x (require \"./#module-basename\"))')\n"
# Call that macro, and log the resulting JavaScript code
..write "console.log(x('(x)'))\n"
..end!
..stdout.pipe concat (.to-string! `@equals` '42;\n')
..stderr.pipe concat (.to-string! `@equals` '')
..on \close ~>
@pass "Node.js process exited"
e <~ rimraf dir-name
@equals e, null, "Temporary directory removed"

0 comments on commit 83b4002

Please sign in to comment.