michaelficarra / CoffeeScriptRedux

:sweat: rewrite of the CoffeeScript compiler with proper compiler design principles and a focus on robustness and extensibility
https://michaelficarra.github.com/CoffeeScriptRedux/
BSD 3-Clause "New" or "Revised" License
1.84k stars 110 forks source link

newlines in function arguments or parenthesis enclosed blocks #325

Open arkarkark opened 10 years ago

arkarkark commented 10 years ago

the following code (bad.coffee) is parsed by the original coffee but not CoffeeScriptRedux

angular.module('App').controller('AppController',
['$location', '$scope', (
  $location,   $scope) ->

  @link = (field) ->
    ("http://example.com" +
     "/" + field)
  @
])

if I modify it to this (good.coffee) it works

angular.module('App').controller('AppController',['$location', '$scope', (  $location,   $scope) ->

  @link = (field) ->
    ("http://example.com" +     "/" + field)
  @
])

I can't tell if this is #119 or #142 related so apologies if it's a dupe.

I installed from npm

$ ./bin/coffee -v
CoffeeScript version 2.0.0-beta8
$ ./bin/coffee -js < ~/mysrc/third_party/bad.coffee
Syntax error on line 1, column 50: unexpected '\n' (\u000A)
1 : angular.module('App').controller('AppController',
^ :~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
2 : ['$location', '$scope', (
3 :   $location,   $scope) ->
4 :

and the original CoffeeScript

$ coffee -v
CoffeeScript version 1.7.1
$ coffee -p ~/mysrc/third_party/bad.coffee
(function() {
  angular.module('App').controller('AppController', [
    '$location', '$scope', function($location, $scope) {
      this.link = function(field) {
        return "http://example.com" + "/" + field;
      };
      return this;
    }
  ]);

}).call(this);

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/3560984-newlines-in-function-arguments-or-parenthesis-enclosed-blocks?utm_campaign=plugin&utm_content=tracker%2F33145&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F33145&utm_medium=issues&utm_source=github).
yumitsu commented 10 years ago

Your code looks bad for me too. Anyway, if I modify your code just for a bit:

angular.module('App').controller('AppController', [
  '$location', '$scope', (
    $location,   $scope) ->

    @link = (field) ->
      ("http://example.com" +
      "/" + field)
    @
])

CoffeeScriptRedux compiles it as expected:

// Generated by CoffeeScript 2.0.0-beta8
angular.module('App').controller('AppController', [
  '$location',
  '$scope',
  function ($location, $scope) {
    this.link = function (field) {
      return 'http://example.com' + '/' + field;
    };
    return this;
  }
]);
arkarkark commented 10 years ago

indeed, but if it's valid for the original coffee compiler, shouldn't your compiler like it too?

yumitsu commented 10 years ago

@arkarkark Honestly, I can't understand why vanilla coffee-script acts so "forgiving".

arkarkark commented 10 years ago

:-D

flying-sheep commented 8 years ago

OK, so why is this invalid?

a(b,
  c)

→ Syntax error on line 1, column 5: unexpected '\n' (\u000A)

isn’t the grouping given via parens?

it’s especially of interest for me because coffee-react-transform creates such code:

Car = React.createClass
  render: ->
    React.createElement(Vehicle, {"doors": (4), "locked": (isLocked()), "data-colour": "red", "on": true},
      React.createElement(Parts.FrontSeat, null),
      React.createElement(Parts.BackSeat, null),
      React.createElement("p", {"className": "seat"}, "Which seat can I take? ", (@props?.seat or 'none'))
    )
SuperPaintman commented 8 years ago

@flying-sheep

a(b
   ,c)

# or

a \
      b
    , c
flying-sheep commented 8 years ago

thanks but:

  1. i didn’t create the code, coffee-react-transform did, so i have no control over it.
  2. there’s absolutely no ambiguity so there’s no reason why it shouldn’t be accepted
  3. the grammar says nothing about whitespace in argument lists