jashkenas/coffeescript

Bug: Functions as non-final arguments

Closed this issue · 3 comments

a1q0 commented

CoffeeScript 2.7.0

Expected Behavior:

The compiler should recognize and properly process function literals when they are used as arguments in a function call, especially when these function literals are not the last argument.

Actual Behavior:

The compiler throws a syntax error at the comma following the function.

How to reproduce :

foo(() => 1, 2) 

# bug.coffee:2:13: error: unexpected ,
# foo(() => 1, 2)	
#              ^	

direct equivalent works fine in javascript :

foo(() => 1, 2)

Syntax ambiguity ?

Since object literal braces a: 'a', b: 'b' can be omitted if shorthands properties are used a, b then we can't know if the intent was to pass an object literal with shorthand props a, b or several arguments a, b.

An object literal with braces and shorthand props: foo(() => { 1, 2 }) transpiles to foo(() => { return {1: 1, 2: 2}; }); as expected.

So, wouldn't it be neater if the currently broken foo(() => 1, 2) didn't throw a syntax error, but picked an opiniated default ?

I'd rather write foo(() => {1, 2}) than foo((() => 1), 2) :/

btw foo(() => {1, 2}, 3) doesn't work either. 'unexpected ,'

best workaround for now

this works fine but can't take arguments

foo((-> 1), 2)
foo((=> 1), 2)

alternatively add a line return after the ,

foo(
    () => 1,
    2
)
# transpile to javascript as expected
foo(() => {
  return 1;
}, 2);
a1q0 commented

I might have pinned the issue in rewritter.coffee it appears the normalizeLines layer is responsible, more specifically the detectEnd function that set the position of the OUTDENT.

Rewriter.coffee

BALANCED_PAIRS / EXPRESSION_START EXPRESSION_END

# List of the token pairs that must be balanced.
BALANCED_PAIRS = [
  ['(', ')']
  ['[', ']']
  ['{', '}']
  ['INDENT', 'OUTDENT'],
  ['CALL_START', 'CALL_END']
  ['PARAM_START', 'PARAM_END']
  ['INDEX_START', 'INDEX_END']
  ['STRING_START', 'STRING_END']
  ['INTERPOLATION_START', 'INTERPOLATION_END']
  ['REGEX_START', 'REGEX_END']
]

# The inverse mappings of `BALANCED_PAIRS` we’re trying to fix up, so we can
# look things up from either end.
exports.INVERSES = INVERSES = {}

# The tokens that signal the start/end of a balanced pair.
EXPRESSION_START = []
EXPRESSION_END   = []

for [left, right] in BALANCED_PAIRS
  EXPRESSION_START.push INVERSES[right] = left
  EXPRESSION_END  .push INVERSES[left] = right

normalizeLines rewriter's layer, the detectEnd call at the very end

  # Because our grammar is LALR(1), it can’t handle some single-line
  # expressions that lack ending delimiters. The **Rewriter** adds the implicit
  # blocks, so it doesn’t need to. To keep the grammar clean and tidy, trailing
  # newlines within expressions are removed and the indentation tokens of empty
  # blocks are added.
  normalizeLines: ->
    starter = indent = outdent = null
    leading_switch_when = null
    leading_if_then = null
    # Count `THEN` tags
    ifThens = []

    condition = (token, i) ->
      token[1] isnt ';' and token[0] in SINGLE_CLOSERS and
      not (token[0] is 'TERMINATOR' and @tag(i + 1) in EXPRESSION_CLOSE) and
      not (token[0] is 'ELSE' and
           (starter isnt 'THEN' or (leading_if_then or leading_switch_when))) and
      not (token[0] in ['CATCH', 'FINALLY'] and starter in ['->', '=>']) or
      token[0] in CALL_CLOSERS and
      (@tokens[i - 1].newLine or @tokens[i - 1][0] is 'OUTDENT')

    action = (token, i) ->
      ifThens.pop() if token[0] is 'ELSE' and starter is 'THEN'
      @tokens.splice (if @tag(i - 1) is ',' then i - 1 else i), 0, outdent

    closeElseTag = (tokens, i) =>
      tlen = ifThens.length
      return i unless tlen > 0
      lastThen = ifThens.pop()
      [, outdentElse] = @indentation tokens[lastThen]
      # Insert `OUTDENT` to close inner `IF`.
      outdentElse[1] = tlen*2
      tokens.splice(i, 0, outdentElse)
      # Insert `OUTDENT` to close outer `IF`.
      outdentElse[1] = 2
      tokens.splice(i + 1, 0, outdentElse)
      # Remove outdents from the end.
      @detectEnd i + 2,
        (token, i) -> token[0] in ['OUTDENT', 'TERMINATOR']
        (token, i) ->
            if @tag(i) is 'OUTDENT' and @tag(i + 1) is 'OUTDENT'
              tokens.splice i, 2
      i + 2

    @scanTokens (token, i, tokens) ->
      [tag] = token
      conditionTag = tag in ['->', '=>'] and
        @findTagsBackwards(i, ['IF', 'WHILE', 'FOR', 'UNTIL', 'SWITCH', 'WHEN', 'LEADING_WHEN', '[', 'INDEX_START']) and
        not (@findTagsBackwards i, ['THEN', '..', '...'])
      console.log token[0]
      if tag is 'TERMINATOR'
        if @tag(i + 1) is 'ELSE' and @tag(i - 1) isnt 'OUTDENT'
          tokens.splice i, 1, @indentation()...
          return 1
        if @tag(i + 1) in EXPRESSION_CLOSE
          if token[1] is ';' and @tag(i + 1) is 'OUTDENT'
            tokens[i + 1].prevToken = token
            moveComments token, tokens[i + 1]
          tokens.splice i, 1
          return 0
      if tag is 'CATCH'
        for j in [1..2] when @tag(i + j) in ['OUTDENT', 'TERMINATOR', 'FINALLY']
          tokens.splice i + j, 0, @indentation()...
          return 2 + j
      if tag in ['->', '=>'] and (@tag(i + 1) in [',', ']'] or @tag(i + 1) is '.' and token.newLine)
        [indent, outdent] = @indentation tokens[i]
        tokens.splice i + 1, 0, indent, outdent
        return 1
      if tag in SINGLE_LINERS and @tag(i + 1) isnt 'INDENT' and
         not (tag is 'ELSE' and @tag(i + 1) is 'IF') and
         not conditionTag
        console.log token
        starter = tag
        [indent, outdent] = @indentation tokens[i]
        indent.fromThen   = true if starter is 'THEN'
        if tag is 'THEN'
          leading_switch_when = @findTagsBackwards(i, ['LEADING_WHEN']) and @tag(i + 1) is 'IF'
          leading_if_then = @findTagsBackwards(i, ['IF']) and @tag(i + 1) is 'IF'
        ifThens.push i if tag is 'THEN' and @findTagsBackwards(i, ['IF'])
        # `ELSE` tag is not closed.
        if tag is 'ELSE' and @tag(i - 1) isnt 'OUTDENT'
          i = closeElseTag tokens, i
        
        tokens.splice i + 1, 0, indent

   >>>> @detectEnd i + 2, condition, action

        tokens.splice i, 1 if tag is 'THEN'
        console.log tokens
        console.log (t[0] + '/' + t[1] for t in @tokens).join ' '

        return 1
      return 1

detectEnd

  detectEnd: (i, condition, action, opts = {}) ->
    {tokens} = this
    levels = 0
    while token = tokens[i]
      return action.call this, token, i if levels is 0 and condition.call this, token, i
      if token[0] in EXPRESSION_START
        levels += 1
      else if token[0] in EXPRESSION_END
        levels -= 1
      if levels < 0
        return if opts.returnOnNegativeLevel
        return action.call this, token, i
      i += 1
    i - 1

There are a few other issues about this. This is an implementation issue, for sure, but one that’s require a fairly big lexer refactor, which is not easy.
If you do have the time and motivation, PRs welcome of course.