rspec-expectations: Keyword args support causes expected values to coerce to Hash

In https://github.com/jruby/jruby/issues/6452 we discovered that a recent update to rspec-expectations (https://github.com/rspec/rspec-expectations/pull/1187) started causing Java HashMap to coerce to Hash, triggering a failure in our tests of HashMap integration.

The coercion behavior appears to match CRuby, and we are debating whether Java HashMap should even support to_hash, but the behavior in rspec may be problematic. Any expected value object passed through the same logic will end up coercing to Hash if it defines an appropriate to_hash, which seems…unexpected.

I do not have a solution to offer, but wanted to bring it up here since it seems peculiar that an RSpec expectation would cause a value to coerce… as in our case I would expect it to pass through without modification or conversion.

Note that the behavior of coercing the last argument to a Hash will go away in Ruby 3.0, which will avoid this behavior… but it affects 2.7 (with a warning) and lower.

Isolated example not using JRuby Java integration (this simulates passing through the code from #1187):

h = Class.new { def to_hash; {a:1}; end}.new
class X
  def blah(*args, **kwargs)
    [args, kwargs]
  end
end
class Y < X
  def blah(*args)
    super
  end
end
args, kwargs = Y.new.blah(h)
p args
p kwargs

CC @eregon @jeremyevans @marcandre since I believe they have been part of debates on kwargs behavior going forward.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 21 (11 by maintainers)

Most upvoted comments

Closing this since the issue has apparently been resolved.

pirj on Dec 29, 2020

@eregon Yes, we definitely plan to work on this. In the new few days we’re focused on pushing rspec-rails 4.1 to support Rails 6.1, and RSpec 4 where we drop support for old rubies.

I keep a bookmark to this comment of yours that explains that ruby2_keywords is the only way to make it compatible and warning-free on both Ruby 2 & 3, and I’ve seen your PoC change to ruby2_keywords.

What concerns me is that ruby2_keywords is hard to apply in certain cases. You have probably seen WithKeywordsWhenNeeded and its applications, e.g. in rspec-expectations:

          klass = class << self
            # See `Macros#define_user_override` above, for an explanation.
            include(@user_method_defs = Module.new)
            self
          end
          RSpec::Support::WithKeywordsWhenNeeded.class_exec(klass, *expected, &declarations)

What is the correct approach for the above case?

Also, I haven’t checked Jeremy’s note above in detail, but it sounds like in some cases we’re might be cornered with clean support of both recent major Ruby versions.

It’s not that I’m trying to push the responsibility to you, but an informed advise would help to move this forward a bit quicker.

pirj on Dec 22, 2020

(*args, **kwargs)-delegation does not work on Ruby < 3, so I’d suggest to replace all those cases using def m(*args, **kwargs) in RSpec with ruby2_keywords def m(*args). Nothing else works reliably for delegation in Ruby 2.7.

BasicObject#equal? doesn’t accept keyword arguments, so it should work fine for this chain, if no ** is between the spec and the call to BasicObject#equal?.

eregon on Dec 12, 2020

If expect/be_equal doesn’t accept keyword arguments, but later passes all received arguments to a method that does, in Ruby 2 it should probably append an empty hash argument, which will be treated as keywords when passed to the method that accepts keywords. I don’t know enough about RSpec to determine if that is the case.

If expect/be_equal accepts keyword arguments, there is nothing you can do in Ruby 2.

jeremyevans on Dec 11, 2020

This fails on rspec-expectations 3.9.3 and later. I think it should pass.

describe "A non-Hash value with to_hash defined" do
  it "does not coerce when passed through an equal? expectation" do
    cls = Class.new { def to_hash; {a:1}; end }
    obj = cls.new
    expect(obj).to be_equal(obj)
  end
end

headius on Dec 11, 2020

This is one of the reasons we separated keyword args in Ruby 3, because if an object responds to to_hash, it could be treated as keywords even if not passed as keywords in Ruby 2.

The approach here (https://github.com/rspec/rspec-expectations/pull/1187/files#diff-4bff1e98c288b9ed0d824e9ac97c878795f2b67718dcf28cb47082620a442a09R184) looks wrong. You should use ruby2_keywords, not separate method definitions. ruby2_keywords is compatible with Ruby <2, since you only need to use *args syntax.

jeremyevans on Dec 11, 2020