Splat comprehensions

commit 4b4979e
Author: William C. Johnson <wcjohnson@oigroup.net>
Date:   Thu Sep 28 21:09:41 2017 -0400

    Changelog

commit d809a5e
Author: William C. Johnson <wcjohnson@oigroup.net>
Date:   Thu Sep 28 19:57:21 2017 -0400

    Throw on empty comprehensions

commit 8b37b95
Author: William C. Johnson <wcjohnson@oigroup.net>
Date:   Thu Sep 28 19:50:22 2017 -0400

    Fix: `LabeledStatement` bodies now included in tail transforms

commit dab8a33
Author: William C. Johnson <wcjohnson@oigroup.net>
Date:   Thu Sep 28 19:07:51 2017 -0400

    Enhanced comprehensions use `…` syntax

CHANGELOG.md

@@ -99,6 +99,55 @@ Here we are converging with the direction that JavaScript proper is headed in, a
 
 LightScript upstream has indicated they will be accepting this feature, so it is now on by default. `{bangCall: false}` can still be passed to disable it. The flag will be removed altogether when LightScript proper integrates the feature.
 
+## Comprehensions
+
+### Change
+
+When `{enhancedComprehension: true}` is enabled, comprehensions have a new syntax:
+
+```js
+x = [
+  // Comprehensions may include regular array elements, which are passed directly
+  // into the produced array
+  1
+  // `...for` introduces a loop comprehension: every iteration of the loop will
+  // produce one value which will be added to the array. Note the addition of the
+  // ellipsis `...` which was not required in the previous syntax.
+  ...for elem e in [2, 3, 4]: e
+  // `...if` introduces a conditional comprehension: if the test expression is
+  // truthy, the consequent expression is inserted into the array. If no alternate
+  // expression is provided, and the test is falsy, nothing is inserted into
+  // the array.
+  //
+  // This behavior differs from a standard `if` expression which would insert an
+  // `undefined` entry into the array in that circumstance.
+  ...if not skipFive: 5
+  // Comprehensions can be mixed in with regular items in any combination.
+  6
+  ...for elem e in [7, 8]: e
+]
+```
+
+Object comprehensions no longer use tuples to represent object elements. Instead, an object expression that is effectively merged into the underlying object is provided:
+
+```js
+reverse(obj) -> ({
+  // Object comprehensions now end with an object literal that will effectively
+  // be `Object.assign`ed to the object being assembled.
+  ...for key k, val v in obj: { [v]: k }
+})
+```
+
+The `case` keyword is no longer used.
+
+### Rationale
+
+The addition of `...` solves the serious grammar ambiguity at https://github.com/wcjohnson/lightscript/issues/25.
+
+`...if` should be more readable and clearer than the `case` syntax it replaced.
+
+Sequence expressions in object comprehensions have always been a bit unfortunate, as the overload violates the semantics of JS sequence expressions. The object expression, though more verbose, is ultimately a clearer syntax that doesn't introduce an edge case into the language.
+
 ## Object-block ambiguity
 
 ### Changes

src/comprehension.lsc

@@ -3,7 +3,7 @@ import { transformTails } from './tails'
 import { isa } from './is'
 import { toStatement } from './blocks'
 
-import { getLoc, placeAtLoc as atLoc, placeAtNode as atNode, getSurroundingLoc, span } from 'ast-loc-utils'
+import { getLoc, placeAtLoc as atLoc, placeAtNode as atNode, getSurroundingLoc, span, placeTreeAtLocWhenUnplaced as allAtLoc } from 'ast-loc-utils'
 
 validateComprehensionBody(path) ->
   path.traverse({
@@ -14,7 +14,7 @@ validateComprehensionBody(path) ->
     AwaitExpression(awaitPath) ->
       throw awaitPath.buildCodeFrameError(
         "`await` is not allowed within Comprehensions; " +
-        "instead, await the Comprehension (eg; `y <- [for x of xs: x]`)."
+        "instead, await the Comprehension."
       )
 
     YieldExpression(yieldPath) ->
@@ -37,60 +37,114 @@ iife(body, id, initializer) ->
     []
   )~atLoc(loc)
 
-retailObject(path, id, transformPathName, returnPathName) ->
+isSimpleObject(objExpr) ->
+  objExpr.properties?.length == 1 and
+  (not objExpr~isa("ObjectComprehension")) and
+  objExpr.properties[0].type == "ObjectProperty" and
+  (not objExpr.properties[0].decorators?.length)
+
+retailObject(info, path, id, transformPathName, returnPathName) ->
   transformPath = path.get(transformPathName)
   validateComprehensionBody(transformPath)
+  let foundOne = false
   transformTails(
     transformPath
     true
     false
-    (seqExpr, tailPath) ->
-      if (
-        seqExpr.type !== "SequenceExpression" or
-        seqExpr.expressions.length !== 2
-      ):
-        throw tailPath.buildCodeFrameError("Object comprehensions must end" +
-        " with a (key, value) pair.")
-
-      [ keyExpr, valExpr ] = seqExpr.expressions
-
-      t.assignmentExpression("=",
-        t.memberExpression(id, keyExpr, true)~atNode(seqExpr),
-        valExpr
-      )~atNode(seqExpr)
+    (expr, tailPath) ->
+      if info.isLegacy:
+        if (
+          expr.type !== "SequenceExpression" or
+          expr.expressions.length !== 2
+        ):
+          throw tailPath.buildCodeFrameError("Object comprehensions must end" +
+          " with a (key, value) pair.")
+
+        now foundOne = true
+        [ keyExpr, valExpr ] = expr.expressions
+
+        return t.assignmentExpression("=",
+          t.memberExpression(id, keyExpr, true)~atNode(expr),
+          valExpr
+        )~atNode(expr)
+
+      if not expr~isa("ObjectExpression"):
+        throw tailPath.buildCodeFrameError("Object comprehensions must end with an object expression.")
+
+      now foundOne = true
+
+      if expr~isSimpleObject():
+        // Simple object case: { [k]: v } --> obj[k] = v
+        { properties: [prop] } = expr
+        t.assignmentExpression("=",
+          t.memberExpression(id, prop.key, prop.computed)~atNode(expr),
+          prop.value
+        )~atNode(expr)
+      else:
+        // Complex object case: { ... } -> Object.assign(obj, { ... })
+        t.callExpression(
+          t.memberExpression(
+            t.identifier("Object")
+            t.identifier("assign")
+          )~allAtLoc(expr~getLoc())
+          [id, expr]
+        )~atNode(expr)
   )
 
+  if not foundOne:
+    throw path.buildCodeFrameError("Object comprehensions must end with an object expression.")
+
   path.get(returnPathName).node
 
-retailArray(path, id, transformPathName, returnPathName) ->
+retailArray(info, path, id, transformPathName, returnPathName) ->
   transformPath = path.get(transformPathName)
   validateComprehensionBody(transformPath)
+  let foundOne = false
   transformTails(
     transformPath
     true
     false
     (expr) ->
+      now foundOne = true
       t.callExpression(
         t.memberExpression(id, t.identifier("push")~atNode(expr))~atNode(expr)
         [expr]
       )~atNode(expr)
+
+      // XXX: below code is for allowing ArrayExpressions in tail position
+      // if not expr~isa("ArrayExpression"):
+      //   throw tailPath.buildCodeFrameError("Array comprehensions must end with an array expression.")
+
+      // t.callExpression(
+      //   t.memberExpression(id, t.identifier("push")~atNode(expr))~atNode(expr)
+      //   if expr.elements?.length == 1 and (not expr~isa("ArrayComprehension")):
+      //     // Shortcut for simple array exprs: just array.push the single entry.
+      //     [expr.elements[0]]
+      //   else:
+      //     // ES6-spread the tail array onto the base array
+      //     [t.spreadElement(expr)~atNode(expr)]
+      // )~atNode(expr)
   )
 
+  if not foundOne:
+    throw path.buildCodeFrameError("Array comprehensions must end with an expression.")
+
   path.get(returnPathName).node
 
-transformLoop(path, ref, isObject, stmts) ->
-  if isObject:
-    stmts.push(retailObject(path, ref, "loop.body", "loop"))
+transformLoop(info, path, ref, stmts) ->
+  if info.isObject:
+    stmts.push(retailObject(info, path, ref, "loop.body", "loop"))
   else:
-    stmts.push(retailArray(path, ref, "loop.body", "loop"))
+    stmts.push(retailArray(info, path, ref, "loop.body", "loop"))
 
-transformCase(path, ref, isObject, stmts) ->
-  if isObject:
-    stmts.push(retailObject(path, ref, "conditional", "conditional"))
+transformCase(info, path, ref, stmts) ->
+  if info.isObject:
+    stmts.push(retailObject(info, path, ref, "conditional", "conditional"))
   else:
-    stmts.push(retailArray(path, ref, "conditional", "conditional"))
+    stmts.push(retailArray(info, path, ref, "conditional", "conditional"))
 
-pushBundle(stmts, ref, bundle, isObject) ->
+pushBundle(info, stmts, ref, bundle) ->
+  { isObject } = info
   if isObject:
     // _ref.k1 = v1; _ref.k2 = v2; ...
     for elem property in bundle:
@@ -111,36 +165,39 @@ pushBundle(stmts, ref, bundle, isObject) ->
       )~atLoc(loc)~toStatement()
     )
 
-export transformComprehension(path, isObject) ->
+export transformComprehension(info) ->
+  { path, isObject } = info
   { node } = path
   elements = if isObject: node.properties else: node.elements
   nodeKey = if isObject: "properties" else: "elements"
   stmts = []
   id = path.scope.generateUidIdentifier(isObject ? "obj" : "arr")~t.clone()~atLoc(getLoc(node)~span(1))
 
   let i = 0, len = elements.length, bundle = [], first = true, initializer
+
   while i < len:
     element = elements[i]
     if element~isa("Comprehension"):
       if first:
         now initializer = bundle
         now first = false
       else:
-        if bundle.length > 0: stmts~pushBundle(id, bundle, isObject)
+        if bundle.length > 0: pushBundle(info, stmts, id, bundle)
       now bundle = []
 
       match element:
         | ~isa("LoopComprehension"):
-          path.get(`${nodeKey}.${i}`)~transformLoop(id, isObject, stmts)
+          info~transformLoop(path.get(`${nodeKey}.${i}`), id, stmts)
         | ~isa("CaseComprehension"):
-          path.get(`${nodeKey}.${i}`)~transformCase(id, isObject, stmts)
-        | else: throw new Error("Invalid comprehension node (this is an internal error)")
+          info~transformCase(path.get(`${nodeKey}.${i}`), id, stmts)
+        | else:
+          throw new Error("Invalid comprehension node (this is an internal error)")
     else:
       bundle.push(element)
 
     i++
 
-  if bundle.length > 0: stmts~pushBundle(id, bundle, isObject)
+  if bundle.length > 0: pushBundle(info, stmts, id, bundle)
 
   initializerLoc = if initializer.length == 0:
     getLoc(node)~span(1)
@@ -154,36 +211,31 @@ export transformComprehension(path, isObject) ->
 
   path.replaceWith(stmts~iife(id, finalInitializer))
 
+getComprehensionInfo(path, isObject, isLegacy) ->
+  { path, isObject, isLegacy }
+
 export transformArrayComprehension(path): void ->
-  transformComprehension(path, false)
+  getComprehensionInfo(path, false, false)~transformComprehension()
 
 export transformObjectComprehension(path): void ->
-  transformComprehension(path, true)
+  getComprehensionInfo(path, true, false)~transformComprehension()
 
-export transformPlainArrayComprehension(path): void ->
-  // Shim V1 onto V2
+export transformLegacyComprehension(path, isObject): void ->
+  // Shim legacy comprehensions onto new model
   { node } = path
   { loop } = node
   if loop:
     delete node.loop
     // TODO: fix babel patch so there's a builder for this...
-    node.elements = [ {
+    node[if isObject: "properties" else: "elements"] = [ {
       type: "LoopComprehension"
       loop
     }~atNode(node) ]
     path.replaceWith(node)
-    transformArrayComprehension(path)
+    getComprehensionInfo(path, isObject, true)~transformComprehension()
+
+export transformPlainArrayComprehension(path): void ->
+  transformLegacyComprehension(path, false)
 
 export transformPlainObjectComprehension(path): void ->
-  // Shim V1 onto V2
-  { node } = path
-  { loop } = node
-  if loop:
-    delete node.loop
-    // TODO: fix babel patch so there's a builder for this...
-    node.properties = [ {
-      type: "LoopComprehension"
-      loop
-    }~atNode(node) ]
-    path.replaceWith(node)
-    transformObjectComprehension(path)
+  transformLegacyComprehension(path, true)

src/config.lsc

@@ -94,7 +94,7 @@ export getParserOpts(pluginOpts, initialParserOpts) ->
   if pluginOpts?.bangCall != false: plugins.push("bangCall")
   if not pluginOpts?.noEnforcedSubscriptIndentation: plugins.push("enforceSubscriptIndentation")
   if pluginOpts?.flippedImports: plugins.push("flippedImports")
-  if pluginOpts?.enhancedComprehension: plugins.push("enhancedComprehension")
+  if pluginOpts?.enhancedComprehension: plugins.push("splatComprehension")
   if pluginOpts?.whiteblock: plugins.push("whiteblockOnly")
   if pluginOpts?.placeholderArgs: plugins.push("syntacticPlaceholder")
   if pluginOpts?.placeholder:

src/tails.lsc

@@ -17,6 +17,8 @@ export getTails(path, allowLoops) ->
     | ~isa("IfStatement"):
       add(path.get("consequent"))
       add(path.get("alternate"))
+    | ~isa("LabeledStatement"):
+      add(path.get("body"))
     | ~isa("DoExpression"):
       add(path.get("body"))
     | if allowLoops when ~isa("For"), ~isa("While"):

test/fixtures/comprehensions/await/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "`await` is not allowed within Comprehensions; instead, await the Comprehension (eg; `y <- [for x of xs: x]`)."
+  "throws": "`await` is not allowed within Comprehensions; instead, await the Comprehension."
 }

test/fixtures/enhanced-comprehensions/assignment-expr/actual.js

@@ -1,3 +1,3 @@
-[for idx i in Array(10):
+[...for idx i in Array(10):
   now x = f(i)
 ]

test/fixtures/enhanced-comprehensions/assignment-expr/exec.js

@@ -1,5 +1,5 @@
 let x = 0
-result = [for idx i in Array(10):
+result = [...for idx i in Array(10):
   now x = i
 ]
 assert.deepEqual(result, [0,1,2,3,4,5,6,7,8,9])

test/fixtures/enhanced-comprehensions/await/actual.js

@@ -1,4 +1,4 @@
 f() -/>
-  <- [for now x of arr:
+  <- [...for now x of arr:
     <- x
   ]

test/fixtures/enhanced-comprehensions/await/options.json

@@ -1,3 +1,3 @@
 {
-  "throws": "`await` is not allowed within Comprehensions; instead, await the Comprehension (eg; `y <- [for x of xs: x]`)."
+  "throws": "`await` is not allowed within Comprehensions; instead, await the Comprehension."
 }

test/fixtures/enhanced-comprehensions/basic/actual.js

@@ -1 +1 @@
-[for elem x in y: x]
+[...for elem x in y: x]

test/fixtures/enhanced-comprehensions/closure-nested-deep/actual.js

@@ -1,9 +1,9 @@
-[for idx i in Array(10):
+[...for idx i in Array(10):
   for idx j in a:
     if i < 5:
       f() ->
-        {for idx k in Array(10):
+        {...for idx k in Array(10):
           if k > 7:
-            (k, g() -> function h() { [i,j,k] })
+            {[k]: g() -> function h() { [i,j,k] }}
         }
 ]

test/fixtures/enhanced-comprehensions/closure-nested/actual.js

@@ -1 +1 @@
-[ for idx i in Array(10): f() -> g() -> i ]
+[ ...for idx i in Array(10): f() -> g() -> i ]

test/fixtures/enhanced-comprehensions/closure-nested/exec.js

@@ -0,0 +1,4 @@
+closures = [ ...for idx i in Array(10): f() -> g() -> i ]
+closureResults = [ ...for elem f in closures: f()() ]
+
+assert.deepEqual(closureResults, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9])

test/fixtures/enhanced-comprehensions/closure/actual.js

@@ -1 +1 @@
-[ for idx i in Array(10): f() -> i ]
+[ ...for idx i in Array(10): f() -> i ]

test/fixtures/enhanced-comprehensions/closure/exec.js

@@ -0,0 +1,4 @@
+closures = [ ...for idx i in Array(10): f() -> i ]
+closureResults = [ ...for elem f in closures: f() ]
+
+assert.deepEqual(closureResults, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9])