crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.44k stars 1.62k forks source link

Incorrect function args when yield used in argument list #14989

Open RX14 opened 1 month ago

RX14 commented 1 month ago
def argsof(**args)
  pp args
end

def yield_args(&)
  argsof(
    "foo": yield "foo",
    "bar": yield "bar",
  )
end

def yield_args2(&)
  argsof(
    "foo": yield("foo"),
    "bar": yield("bar"),
  )
end

yield_args { |x| x }
yield_args2 { |x| x }

gives output

{foo: "foo"}
{foo: "foo", bar: "bar"}

This should generate the same result for both calls, the second function argument cannot be lost.

RX14 commented 1 month ago

Oh dear, I think this parses as argsof(foo: yield("foo", bar: yield("bar"))). I think perhaps this could be made a syntax error?

devnote-dev commented 1 month ago

Wouldn't that potentially break existing code? It seems yield without parentheses is a lot more common...

straight-shoota commented 1 month ago

The behaviour is identical when calling a method instead of yield:

def argsof(**args)
  pp args
end

def yield_args(&)
  argsof(
    "foo": vield "foo",
    "bar": vield "bar",
  )
end

def yield_args2(&)
  argsof(
    "foo": vield("foo"),
    "bar": vield("bar"),
  )
end

def vield(arg, **args)
  arg
end

yield_args { |x| x }  # => {foo: "foo"}
yield_args2 { |x| x } # => {foo: "foo", bar: "bar"}

So while this looks certainly deceitful, it's syntactically sound and I don't think we can really change much about it without causing a major disruption.

Perhaps the formatter could help clarify the situation (for example by adding parentheses in ambiguous cases like this one). However, the formatter actually fails on this code: expecting ), notDELIMITER_START, , at :8:5.