Grape: Broken detection of Proc

[ioquatix]

https://github.com/ruby-grape/grape/blob/d58dc0ab7a0b51625217deedd8110d1030be7cf7/lib/grape/middleware/stack.rb#L80-L84

This code is buggy. The correct solution is have a separate field for &block.

https://bugs.ruby-lang.org/issues/16500

cc @dblock

[ioquatix]

Caused initially by something here:

https://github.com/ruby-grape/grape/blob/d58dc0ab7a0b51625217deedd8110d1030be7cf7/lib/grape/dsl/middleware.rb#L12-L53

[anatol]

Note that this bug breaks gitlab https://gitlab.com/groups/gitlab-org/-/epics/2380

[dblock]

Can someone please help with a failing spec for this? I'd be happy to try and fix.

[dblock]

Note that I tried. There's a bunch of specs around https://github.com/ruby-grape/grape/blob/master/spec/grape/api_spec.rb#L1308 and I couldn't get anything to fail.

[dm1try]

Can someone please help with a failing spec for this? I'd be happy to try and fix.
Note that I tried. There's a bunch of specs around https://github.com/ruby-grape/grape/blob/master/spec/grape/api_spec.rb#L1308 and I couldn't get anything to fail.

according to the description Stack#merge_with should be tested
I'm going to install 2.7.0 and check this)

[dm1try]

@dblock I've added the spec based on the context @ioquatix provided above, see the ref

[dm1try]

guys, take a look https://github.com/ruby-grape/grape/pull/1968/commits/dec3e1ff5dbf3215a714565e62b12bd2ef6b0ddb
is it an acceptable fix?)

[mame]

All versions of Ruby seem to have a bug in combination with rest and block arguments.

def foo(*ary)
  p ary
end
ary = [1, 2, 3, proc {}]
foo(*ary, &ary.pop) #=> [1, 2, 3], expected: [1, 2, 3, #<Proc:...>]

I believe it should be fixed on the side of the interpreter. https://github.com/ruby-grape/grape/commit/dec3e1ff5dbf3215a714565e62b12bd2ef6b0ddb looks good to me. It will work even after the interpreter is fixed.

https://bugs.ruby-lang.org/issues/16500 might be a different issue from this.

[ioquatix]

@mame after your investigation I conclude there is one case where this is a bug in the interpreter. But the proposed fix by @dm1try here is still required because it's ambiguous/undefined behaviour at best without it.

Therefore, you (Grape team) should continue with the fix here, and I suggest we re-open https://bugs.ruby-lang.org/issues/16500 for the cases we (Ruby team) are currently discussing on Slack.

Thanks everyone for your effort!

[mame]

Oh, no! Actually 2.7 fixes a bug "partially".

# in 2.6 or before
args = [1, 2, -> {}]; foo(*args, &args.pop) #=> passes [1, 2] (bug; [1, 2, ->{}] is expected)
args = [1, 2, -> {}]; foo(0, *args, &args.pop) #=> passes [0, 1, 2] (bug; [0, 1, 2, ->{}] is expected)

# in 2.7
args = [1, 2, -> {}]; foo(*args, &args.pop) #=> passes [1, 2] (bug; [1, 2, ->{}] is expected)
args = [1, 2, -> {}]; foo(0, *args, &args.pop) #=> passes [0, 1, 2, ->{}] (good)

So https://github.com/ruby-grape/grape/commit/dec3e1ff5dbf3215a714565e62b12bd2ef6b0ddb is a good fix. Also, args = [1, 2, -> {}]; foo(*args, &args.pop) should be fixed on the side of the interpreter.

[dblock]

Closed via #1968