Skip to content

Commit 31666c2

Browse files
committed
Fix macros' this.evaluate for object args
This supercedes my stupidity in be77679. as [raised in chat][1] by @terinjokes, passing a list `(object a 1)` to a macro and attempting to `this.evaluate` it caused unexpected results, and passing `(object a 1 b 2)` failed altogether. As mentioned, this was due to `eval` parsing "{ a: 1 }" as a block statement containing a labelled literal. [More detail on SO here][2]. The fix is to wrap the JS generated from ObjectExpression AST nodes in parentheses before evaluating it. As explained in the comments, we still want this.evaluate to be able to evaluate statements, so I added an if-branch for that and a test to cover it. [1]: https://gitter.im/anko/eslisp?at=566a7415cffd648a0554eeeb [2]: http://stackoverflow.com/questions/3731802
1 parent b2e760c commit 31666c2

File tree

2 files changed

+48
-7
lines changed

2 files changed

+48
-7
lines changed

src/env.ls

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,33 @@ class env
7171
compile-to-js : -> es-generate it
7272

7373
evaluate : ~>
74-
it |> @compile |> @compile-to-js |> eval
74+
ast = it |> @compile
75+
js = ast |> @compile-to-js
76+
77+
if ast.type is \ObjectExpression
78+
#
79+
# Because object expressions generated by escodegen (e.g. "{ a: 1 }") are
80+
# liable to be read by `eval` as block statements ("{}") containing a
81+
# labelled ("a:") literal ("1"), we guard against that here with the
82+
# [classic trick][1] of first wrapping object expressions in parentheses
83+
# before evaluating them.
84+
#
85+
# [1]: http://stackoverflow.com/questions/3360356
86+
#
87+
js |> (-> "(#it)") |> eval
88+
else
89+
#
90+
# With everything else, we just straight-up eval. This needs to stay the
91+
# default case because we also want to be able to evaluate statements.
92+
#
93+
# Always wrapping everything in parentheses before eval would only work
94+
# for expressions. For statements, it's nonsensical. For example,
95+
#
96+
# (if (a) {})
97+
#
98+
# gives "SyntaxError: Unexpected token if".
99+
#
100+
js |> eval
75101

76102
multi : multiple-statements
77103

test.ls

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -656,14 +656,29 @@ test "macros can evaluate number arguments to JS and convert them back again" ->
656656
"""
657657
..`@equals` "6 * 2;"
658658

659-
test "macros can evaluate macro-calling arguments to objects" ->
659+
test "macros can evaluate object arguments" ->
660+
# This macro uses this.evaluate to compile and evaluate a list that expands
661+
# to an object, then stringifies it.
660662
esl """
661-
(macro printObject (lambda (objDefinition)
662-
(= obj ((. this evaluate) objDefinition))
663-
(return ((. this atom) ((. obj toString))))))
664-
(printObject (object a 1))
663+
(macro objectAsString (lambda (input)
664+
(= obj ((. this evaluate) input))
665+
(return ((. this string) ((. JSON stringify) obj)))))
666+
(objectAsString (object a 1))
665667
"""
666-
..`@equals` "1;"
668+
..`@equals` "'{\"a\":1}';"
669+
670+
test "macros can evaluate statements" ->
671+
# This macro uses this.evaluate to compile and run an if-statement. A
672+
# statement does not evaluate to a value, so we check for undefined.
673+
esl """
674+
(macro evalThis (lambda (input)
675+
(= obj ((. this evaluate) input))
676+
(if (=== obj undefined)
677+
(return ((. this atom) "yep"))
678+
(return ((. this atom) "nope")))))
679+
(evalThis (if 1 (block) (block)))
680+
"""
681+
..`@equals` "yep;"
667682

668683
test "macros can unquote arrays into quasiquoted lists (non-splicing)" ->
669684
esl "(macro what (lambda (x)

0 commit comments

Comments
 (0)