indented property access bug

[glebm]

Took me hours to debug a problem that turned out to be a coffeescript bug:
Found a very evil nesting bug:

jQuery ($) ->
  foo()
    .bind ->
      bar()
        .baz 'click', ->
          foobar()
        .bar()
  THIS_IS_COMPILED_OUTSIDE_JQUERY$("Oh noes!")

coffee v1.1.1 output:

(function() {
  jQuery(function($) {
    return foo().bind(function() {
      return bar().baz('click', function() {
        return foobar();
      }).bar();
    });
  });
  THIS_IS_COMPILED_OUTSIDE_JQUERY$("Oh noes!");
}).call(this);

[michaelficarra]

Can you come up with a more minimal test case so that it's easier to reason about this? Or is this already a minimal case? Also, more-than-2 space indentation would be easier to read. But that's just my preference.

[glebm]

I was able to reduce it by 1 call.
Removing any call as it is right now invalidates the test case.

So does moving foobar() to the same line as .baz 'click', ->
I think this might point to where the issue is

[michaelficarra]

It looks like you're indenting the property accesses when there's no need to do so. Try this instead:

jQuery ($) ->
  foo()
  .bind ->
    bar()
    .baz 'click', ->
      foobar()
    .bar()
  THIS_IS_COMPILED_INSIDE_JQUERY$("Oh yes!")

[TrevorBurnham]

Hmm. As Michael says, the stylistically and semantically correct thing to do is to unindent .bind() and .baz. Interestingly, unindenting either one will fix the problem behavior in this case.

Another interesting observation: If, in the original example, you indent the last line with 6 spaces, you'll get the desired behavior. Any fewer than that, and you get the given bug. If you indent it with 8 spaces, then the line will be in the .bind callback (which was indented with only 4 spaces). Any more than 8 spaces, and you get Unexpected 'INDENT'.

This is pretty alarming, and gives us a simpler problem case:

a()
  .b()
  c()

compiles to

a().b();
c();

Seems to me that this should be an Unexpected 'INDENT', the same way

a()
  c()

is.

[glebm]

I disagree with this being stylistically correct.
As style is something subjective, I will give a few examples:

From jQuery documentation: http://api.jquery.com/end/

$('ul.first').find('.foo')
  .css('background-color', 'red')
.end().find('.bar')
  .css('background-color', 'green')
.end();

From Underscore.js documentation: http://documentcloud.github.com/underscore/

_(lyrics).chain()
  .map(function(line) { return line.words.split(' '); })
  .flatten()
  .reduce(function(counts, word) {
    counts[word] = (counts[word] || 0) + 1;
    return counts;
}, {}).value();

From SproutCore style guide: http://guides.sproutcore.com/style_guide.html

  return this.get('content')
             .getEach('prefix','firstName','middleName',
                      'lastName','suffix')
             .join(' ');

There are quite a few reasons on how and why this became a standard in JavaScript.

First, nesting lets you clearly indicate which object you are working on:

$("#elem1")
  .show()
  .find(".tab")
    .addClass(".active")
    .find(".closeButton")
      .show()
vs
$("#elem1")
.show()
.find(".tab")
.addClass(".active")
.find(".closeButton")
.show()  

Second, nesting prevents confusion of this sort mainly:

ab()
  .b()
  .c()
aa()

vs

ab()
.b()
.c()
aa()

I am sure one can come up with more reasons if you still don't feel convinced.

[satyr]

Formattings of this kind appear in test/function_invocation--they are expected to work.

[glebm]

Excerpt from https://github.com/jashkenas/coffee-script/blob/1.1.1/test/function_invocation.coffee#L198

test "Chained blocks, with proper indentation levels:", ->

  counter =
    results: []
    tick: (func) ->
      @results.push func()
      this
  counter
    .tick ->
      3
    .tick ->
      2
    .tick ->
      1
  arrayEq [3,2,1], counter.results