Object comprehensions via loops in braces

[peey]

Work-in-progress to address the "object comprehension" part of issue #439.

Implementation rationale is that whenever the following is seen:

{ 
  for identifier of expression 
    [a] : b
    [c]:  d
    //...
}

Then it should be recognized as object comprehension (instead of being recognized as a block containing a ForOf statement).

This works because for identifier of expression being followed by a computed property key-value pair is invalid syntax in js.

I need some guidance:

1

My rules recognize the syntax but it isn't prioritized over "block containing a for-of" rule when the [a] : b are indented. This results in the following input-output pair:

{ for x of [1, 2, 3]
  [x]:  2 * x
  [x * 2]: 4 * x
}
// Becomes:
 for (const x of [1, 2, 3]) {
  ({[x]:  2 * x,
  [x * 2]: 4 * x})
}
}

This is also a test (last one) which fails. If I remove the leading whitespace, then it works fine

{ for x of [1, 2, 3]
  [x]:  2 * x
  [x * 2]: 4 * x
}
// Becomes:
(()=>{const _results = {};
  for (const x of [1, 2, 3]) {
    Object.assign(_results, {
      [x]:  2 * x,
      [x * 2]: 4 * x
  })}; 
  return _results
})()

Which provides the output { '1': 2, '2': 4, '3': 6, '4': 8, '6': 12 } if you run it. Note that the indentation in this output has been done by me manually for readability.

2

How should I deal with the indentation in output? I'm introduce an IIFE, a for loop, and an Object.assign. That's 3 extra levels of nesting. Should I just add two spaces for each level?

3

The second test case transpiles but it's incorrect because the inner _results shadows the outer _results and I need to use the variable I introduce in the inner scope. How do I create a variable that's guaranteed to not conflict with the inner scope?

4

I've noticed that my code differs from the existing code in following ways:

1. Most rules return an object with type
2. There's some way to deal with source locations so that it's useful for source mapping

I'll address these in future commits. Tell me if there are other conventions that I should learn about!

[edemaine]

Good start!

Instead of making a separate rule ObjectComprehension, I'd suggest adding for loops to ObjectLiteralContent. This would allow multiple for loops, mixing non-for content, etc. For example:

{
  name: "Upgrade"
  for key, value in object
    [key.toLowerCase()]: value + 1
}

Admittedly, this will be harder to compile. I'm imagining this:

{
  name: "Upgrade",
  ...(() => {const results=[]; for (key in object) {const value = object[key];
    Object.assign(results, {[key.toLowerCase()]: value + 1})
  })()
}

The code above might look complicated, but it's mostly what we already get from the for loop currently: the only difference is results.push( vs. Object.assign(results,.

You might check out the recent PR #1509 that provided alternatives to results.push, such as results += when using for sum. In the parser, I think you could just allow arbitrary for loops, and attach an extra property like object: true to denote that they're an object comprehension. Then, in iterationDeclaration where this code gets generated, you could use Object.assign when the object property is true.

I think this approach might also fix your first issue about priority of blocks vs. object literals, because object comprehensions will now be considered an object literal. (I'm not totally sure, but generally we favor object literals.)

How should I deal with the indentation in output? I'm introduce an IIFE, a for loop, and an Object.assign. That's 3 extra levels of nesting. Should I just add two spaces for each level?

No, try to preserve the indentation in the original input. Also try not to add any extra lines (except what for already does — we aim to remove those eventually, but obviously not this PR). So just one-line whatever you need to add.

The second test case transpiles but it's incorrect because the inner _results shadows the outer _results and I need to use the variable I introduce in the inner scope. How do I create a variable that's guaranteed to not conflict with the inner scope?

Use makeRef("results") to create a variable named results or results1 or results2 or ..., whatever avoids conflict with existing variable names.

2. There's some way to deal with source locations so that it's useful for source mapping

To help with this, use tokens as much as possible. For example, instead of using "for", use For, which attaches $loc data for source mapping. That said, I don't think you need for at all; ideally, you'd use ForStatement directly (though you may need to manually wrap this in an IterationExpression node so that it gets expressionized) and support the many many for loops that Civet already supports.

Hope this helps!

[peey]

Thanks, very helpful! I did not even realize that Civet already has these for loops. The fact that they are expression-like (they evaluate to their results) is quite interesting and changes the way I was thinking about incorporating loops into Object Comprehensions.

I'll study these a bit and then proceed.

[peey]

Hey @edemaine, look at this! I think it's exactly how you wanted to implement it?

When a loop (IterationExpression) is found where you'd normally expect a property definition, we now expressionize it and spread it into the containing object.

A few notes:

1. We can limit it to ForStatement, but since the loop is expressionized and loop's value is what's Object.assign-ed to the result, so in principle it doesn't matter. I'm yet to investigate utility of other loops and add tests for them.
2. Initially I wanted to restrict the loop body to be only computed-key computed-value pairs, but I now believe that's both unnecessary (see the last test case) and less clean to implement
3. Conceptually, this is actually a "reduction" much like the other reductions: we have an accumulator {}, an update operation, and we return the accumulated value. But in code, statement.reduction corresponds to a particular parsing and transpiling logic belonging to the for reduction loops (for some/every/count/sum/product/min/max reduction loops, empty for loop default behavior, fix unwrapping multiple loops in a row #1509).

Technically this is a "support loops inside object expressions" implementation which is rather clean, and as a special and most-commonly used case, it provides for syntax such as in #439.

As a consequence, here's a nice self-describing program

console.log({ 
  name: 'Square table generator'
  description: 'This table contains numeric properties mapped to their squares'
  for x of [1, 2, 3]
    [x]:  x * x
    [x * 2]: 4 * x * x
})

That would produce:

{
  name: 'Square table generator',
  description: 'This table contains numeric properties mapped to their squares',
  '1': 1,
  '2': 4,
  '3': 9,
  '4': 16,
  '6': 36
}

What do you think?

[edemaine]

This looks great!! Awesome how simple the code is now.

I like your idea of allowing arbitrary iteration expressions. I can imagine while making sense, and especially do being helpful (to allow for intermediate computations). Adding some tests for these would be good.

Actually, it'd be cool to allow if expressions too. (But feel free to leave that for another PR.)

Does this PR work with postfixed iteration, such as {[i]: i*2} for i of [1,2,3]? I'm guessing not. I'm not really sure whether it's a good idea... (i: i*2 for i of [1,2,3] would be neat but that already has a different meaning: i: (i*2 for i of [1,2,3]).)

[peey]

Thanks for your review!

I currently do not support postfixed iteration, I have not thought about it much yet.

EDIT: In an earlier version of comment I had misunderstood what you meant by postfixed iteration.

I'll also leave the if expressions out of the scope of this PR, but I agree that it could be interesting to have values conditionally slurped into the object, e.g.

{ 
  a: 1
  if x
    b: 2
}

Currently, the way to do that is not very friendly:

{
  a: 1,
  ...(x? {b : 2} : {})
}

I also added some docs!

[peey]

Ready to merge from my end. Let me know if anything else is required!

[edemaine]

LGTM, except for some overdue work on PropertyGlob (they probably shouldn't allow {} foo, bar either...??).

Possible TODO: if there's just a single loop and nothing else in the object literal, then we don't need a { ...(comprehension) } wrapper so should avoid emitting one. Also postfix loops remains an interesting TODO.

@STRd6 do you approve of this extension? Sadly we won't be able to do the same for array literals, as loops naturally make nested arrays unless you explicitly add ..., but I think it's pretty natural for objects to behave this way and it unlocks a new "reduction" for combining object literals (that you couldn't get with just ...).

[edemaine]

Oh, I didn't realize that it's property globs that are failing. This is annoying; this is valid syntax that we'd lose. But fixing it will require some work on PropertyGlob to not use BracedObjectLiteral, but rather just use a subset of those rules. I can work on this in another PR.

[peey]

Thanks for clarifying. I do not know much about property globs so I'll safely leave it with you.

[edemaine]

I was able to fix this without messing with globs; just forbidding implicit blocks with do and loop.

[peey]

Possible TODO: if there's just a single loop and nothing else in the object literal, then we don't need a { ...(comprehension) } wrapper so should avoid emitting one. Also postfix loops remains an interesting TODO.

I also wish to leave this optimization for later. I could definitely investigate both of these myself, but only later and not now.

[STRd6]

Thanks @peey for all the work on this and @edemaine for the feedback and review! This is a great extension to the language.

I think we may want to use direct assignment of the result rather than Object.assign but that can be done later as it is pretty minor.

Also added a note about a debugger statement that snuck in.