Postfix conditionals in object literals

[couchand]

There seems to be some inconsistency in the handling of postfix conditionals within nested object literals. For instance, this compiles as one would expect:

# this compiles correctly
one =
  foo:
    baz: 1337
    bar: 42 if qux

However, this (apparently equivalent code) fails to compile at all (error: unexpected newline):

# this does not compile
two =
  foo:
    bar: 42 if qux
    baz: 1337

Additionally, adding a function call into the mix makes things even more confused. While this example compiles as expected:

# this compiles correctly
three =
  foo: call
    baz: 1337
    bar: 42 if qux

This one splits up what should be a single object literal:

# this compiles incorrectly
four =
  foo: call
    bar: 42 if qux
    baz: 1337

Compiling to:

  four = {
    foo: call(qux ? {
      bar: 42
    } : void 0, {
      baz: 1337
    })
  };

And this similar example:

five =
  foo: call
    bar: 1337
  baz: 42 if qux

Compiles to:

  five = qux ? {
    foo: call({
      bar: 1337
    }),
    baz: 42
  } : void 0;

And since reversing the order of foo and baz results in error: unexpected newline, there doesn't seem to be a right way to express this with postfix conditionals.

I can start looking into the lexing and parsing to sort this out if it's agreed that it's a bug and nobody has an immediate idea for a fix.

n.b. I believe this is the same issue as #3507, but since the title there wasn't great I decided to open a new one. Feel free to close or file as appropriate.

[connec]

As a workaround you can enclose the postfix'd expression with parentheses (... if a) to disambiguate.

It would be nice if all postfixes behaved more usefully by default in object literals, as you propose.

[couchand]

Ahh, extra parens, 👍 for that workaround. Simple, easy, only a little bit annoying. Thanks!

I guess I'm going to start digging around unless someone pipes up soon that this is intended behavior.

[nickfargo]

I do think this is intended behavior. Following the rules for parsing of postfix-if, where the consequent of the conditional expression is the entire preceding clause, we’re led to conclude that:

foo:
  bar: 42 if qux
  # does not suggest
  bar: (42 if qux)
  # but does suggest
  (bar: 42) if qux

I.e., the conditional (qux) determines not just the presence of the value (foo = {bar: 42} | foo = {bar: undefined}), but also the presence of the key (foo = {bar: 42} | foo = {}).

As an example, consider the analogous

class Foo
  bar: 42 if qux

which compiles to

var Foo;

Foo = (function() {
  function Foo() {}

  if (qux) {
    Foo.prototype.bar = 42;
  }

  return Foo;

})();

rather than

  Foo.prototype.bar = qux ? 42 : void 0;

[couchand]

@nickfargo, while I do think that's a related concern, it's not exactly what I meant. I'm completely in agreement about the conditional determining the presence of the key, that seems natural.

But it appears that the backtracking goes further than that. If one postfix implies this:

foo:
    (bar: 42) if qux

Then two ought to imply this:

foo:
    (bar: 42) if qux
    (baz: 1337) if wibble

But without parens that won't compile.

_EDIT:_ even with the parens that doesn't compile. With the parens the meaning is arguably ambiguous, but without them the meaning is obvious, so it seems the compiler should handle it in the intuitive way.

foo:
    bar: 42 if qux
    baz: 1337 if wibble

I guess the question is what that compiles to, since there's not an obvious expression syntax in JavaScript object literals that would do it correctly. The current compilation of a single conditional can't just be extended to multiples.

foo: qux ? {                                                        
    bar: 42                                                           
} : void 0

To keep the semantics mentioned by @nickfargo I think we'll need it to be in an IIFE.

[couchand]

@nickfargo if you really like the semantics you've described it sounds like we've got a second issue on our hands. Example:

me =
    foo: 42
    bar: 1337 if no

evaluates to:

me =
  foo: 42
  bar: undefined

[nickfargo]

Interesting. Well I’ve no opinion on whether postfixes inside an object literal should be expected to have special precedence rules relative to the :.

I suppose then it’s just a problem of the first-key postfix causing the rewriter to incorrectly apply implicit braces around the object literal. I note now that your first failing case is also fixed with explicit braces:

two =
  foo: {
    bar: 42 if qux
    baz: 1337
  }

In any case, that first-key problem does appear to be worth tagging as a bug, if it isn’t elsewhere already.

[phedny]

I'd like to add that the behaviour has changed between CoffeeScript versions. Below an example where the semantics between 1.6.3 and 1.8.0 differ. Our code is running with the expectation of the 1.6.3 behaviour, and upgrade to 1.8.0 broke stuff.

b1 = b2 = true
test = ->
    p1: 1 if b1
    p2: 2 if b2

// Generated by CoffeeScript 1.6.3
(function() {
  var b1, b2, test;

  b1 = b2 = true;

  test = function() {
    return {
      p1: b1 ? 1 : void 0,
      p2: b2 ? 2 : void 0
    };
  };

}).call(this);

// Generated by CoffeeScript 1.8.0
(function() {
  var b1, b2, test;

  b1 = b2 = true;

  test = function() {
    if (b1) {
      ({
        p1: 1
      });
    }
    if (b2) {
      return {
        p2: 2
      };
    }
  };

}).call(this);

[couchand]

@phedny Yep, that's changed, I believe it was part of the breaking changes made in 1.7.0. If you're upgrading past 1.7 make sure to take a look at the changelog for that release, since there were a number of things that might cause you trouble, and a lot of new things you'll want to take advantage of.

The behavior you're seeing is a different manifestation of the same thing. The issue I raised was in nested object literals, but seeing your example of the function return value makes me scratch my head. Now I can't decide whether the current behavior makes sense and I was wrong originally. I can see it both ways, it might be simpler to handle the postfix the current way. @nickfargo, perhaps you can weigh in on whether the rewriter could put implicit braces around a function body in the same way as you suggested for the nested objects?

I guess I still think that the meaning of my original code sample is unambiguous (and @phedny's too), so we should be able to find a way to parse it (unless there are some degenerate cases I'm not thinking of). The annoyance is that the only compilation I can come up with that matches @nickfargo's semantics is:

thing =
  foo:
    bar: 42 if qux
    baz: 1337 if quz

to...

var thing;

thing = {
  foo: (function(){
    var _a = {};
    if (qux) {
      _a.bar = 42;
    }
    if (quz) {
      _a.baz = 1337;
    }
    return _a;
  })()
};

Which is ugly and annoying. Then again, I'd rather the CoffeeScript compiler write that code than force me to do it, so maybe this is a valid case.

[vendethiel]

(This would need to be an expression though, so it'd probably use the comma operator instead)

[GeoffreyBooth]

Reading this thread, I’m not sure what the bug is, or what the expected output would be. Perhaps someone could open a new issue with a narrower focus.

[couchand]

@GeoffreyBooth, I think my original report was pretty complete, can you expand on what about that description is unclear?