[CS2] Alternative destructuring

[connec]

Sorry @GeoffreyBooth, I'd started working on a destructuring branch with a slightly different approach! Maybe there are some things in it that might be useful.

For comparison, my goal was to reuse as much of the compilation of Arr and Obj as possible. The main change is making them isAssignable and adding an eachName method for extracting variable names.

Currently, the whole assignment will bail if any entry is a special CS assignable such as default arguments, non-final splats, or soaked values. If I keep going it would be to refactor compilePatternMatch to only deal with these special cases, and otherwise delegate back into Assign.

[GeoffreyBooth]

So . . . multiple people working in isolation is frustrating, to say the least. I started my branch over a month ago and I thought I had advertised it widely enough . . . anyway let’s try to avoid duplication of effort going forward?

I like your approach. It seems a lot cleaner than trying to rework compilePatternMatch to basically detect every conceivable destructuring scenario and re-output them as essentially the same input. I also like that you now output the shorthand object parameter syntax whenever possible. There’s a lot of great stuff here.

I think the next step would be to merge my branch into yours, or more precisely to merge into yours what few improvements I’ve made that you haven’t. Have you implemented default values yet? I’ve only implemented for the simplest cases in compilePatternMatch, as a new option on Assign, which I’m not sure is the way to go to mesh with your approach; is that how you would do it, if you haven’t yet?

I’ve also started to update the non-ES-output paths of compilePatternMatch to assign default values only when the variable is undefined, not falsy, to match the ES spec. That, at least, we should port over from my branch (and I can do that). I also updated some tests to correspond with that. Does your branch pass all the tests?

There’s also GeoffreyBooth#4, which tackles object spreads. So we have some merging to do. Do you mind if we move the work into my repo, since I’ve already given everyone access to it? And we can base it off of your branch if you think that makes more sense than refactoring my branch.

[connec]

@GeoffreyBooth sure, if you put the branch where you want it I can pick it up from there.

Sorry about the duplicated work. I had the idea for how to handle destructuring ages ago but didn't have time to act on it until recently, and I wasn't too aware of what else was going on. My bad!

What I've done doesn't cover defaults values, but I think they should be handle-able in the same way. The other case that would be nice to cover would be destructured parameters - again I think it should be possible to get the existing machinery to do most of the work.

There quite a few improvements that could be made to compilePatternMatch in my branch. The approach currently is to detect 'non assignable' targets and hand them off to the old logic. Ideally that logic should be updated to extract only the non assignable parts, and leave the rest intact.

[GeoffreyBooth]

I’ll move some branches around tonight when I get home. Do you mind taking a look at what I’ve done for default values? (Search for defaultValue, in class Assign). Does this mesh with what you’ve done, or would default values be implemented differently in your approach?

What are destructured parameters?

[GeoffreyBooth]

@connec I renamed my branch to destructuring-geoffrey and copied your branch into my repo’s destructuring. Do you mind working there going forward? And please no rebasing.

Closing this PR since now your branch is the source for #4478. Let’s continue the discussion over there.

[connec]

I'll take another look tonight.

The main approach I'm going for is to use as much of the existing machinery as possible. If you look at the nodes for destructured expressions they look like they should just about compile verbatim, and it's only guard logic in nodes.coffee that's preventing that. With some small tweaks we should be able to update the guard logic to be more permissive (e.g. here).

For compiling default values, I was planning to just leverage the existing node structure, e.g.

$ echo '{ a = 1 }' | bin/coffee -bns
Block
  Assign
    Value
      Obj
        Assign
          Value IdentifierLiteral: a
          Value NumberLiteral: 1
    Value IdentifierLiteral: b

We should be able to tweak Obj so that it will compile that inner assign verbatim, rather than doing special handling for Assign's whose context isn't object.

By destructured parameters I just mean e.g. ({a}) ->, which on my branch compiles to

(function(arg) {
  var a
  ({a} = arg);
});

...but could be compiled to

(function({a}) {
});

Again, I suspect there's some small logic tweak we could make to the parameter handling to make them compile more things in-line.