WIP: Relax keyword argument matching

When a method doesn't accept keyword arguments.

In this scenario keyword or Hash-type arguments are assigned as a single
Hash to the last argument without any warnings and strict keyword
matching should not have any effect.

This is an exploratory spike on fixing #593.

* This has highlighted a significant problem with partial mocks in #532.
  The method obtained from the responder is the stub method defined by
  Mocha and not the original. This effectively makes it useless!

* I'm not sure the method_accepts_keyword_arguments? belongs on
  Invocation, but that's the most convenient place for now. It feels as
  if we need to have a bit of a sort out of where various things live
  and perhaps introduce some new classes to make things clearer.

* We might want to think ahead a bit at what we want to do in #149 to
  decide the best way to go about this.

* I'm not sure it's sensible to re-use the Equals matcher; we could
  instead parameterize PositionalOrKeywordHash, although the logic in
  there is already quite complex. Conversely if this is a good approach,
  it might make more sense to do something similar when creating a hash
  matcher for a non-last parameter to further simplify the code.

* I haven't yet introduced any acceptance tests for this and I suspect
  there might be some edge cases yet to come out of the woodwork. In
  particular, I think it's worth exhaustively working through the
  various references mentioned in this comment [1].

[1]: #593 (comment)

lib/mocha/expectation.rb

@@ -694,7 +694,7 @@ def matches_method?(method_name)
 
     # @private
     def match?(invocation, ignoring_order: false)
-      order_independent_match = @method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation.arguments) && @block_matcher.match?(invocation.block)
+      order_independent_match = @method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation) && @block_matcher.match?(invocation.block)
       ignoring_order ? order_independent_match : order_independent_match && in_correct_order?
     end
 

lib/mocha/invocation.rb

@@ -8,11 +8,12 @@ module Mocha
   class Invocation
     attr_reader :method_name, :block
 
-    def initialize(mock, method_name, arguments = [], block = nil)
+    def initialize(mock, method_name, arguments = [], block = nil, responder = nil)
       @mock = mock
       @method_name = method_name
       @arguments = arguments
       @block = block
+      @responder = responder
       @yields = []
       @result = nil
     end
@@ -63,6 +64,12 @@ def full_description
       "\n  - #{call_description} #{result_description}"
     end
 
+    def method_accepts_keyword_arguments?
+      return true unless @responder
+
+      @responder.method(@method_name).parameters.any? { |k, _v| %i[keyreq key keyrest].include?(k) }
+    end
+
     private
 
     def argument_description

lib/mocha/mock.rb

@@ -322,7 +322,7 @@ def method_missing(symbol, *arguments, &block)
     def handle_method_call(symbol, arguments, block)
       check_expiry
       check_responder_responds_to(symbol)
-      invocation = Invocation.new(self, symbol, arguments, block)
+      invocation = Invocation.new(self, symbol, arguments, block, @responder)
 
       matching_expectations = all_expectations.matching_expectations(invocation)
 

lib/mocha/parameter_matchers/instance_methods.rb

@@ -7,10 +7,10 @@ module ParameterMatchers
     # @private
     module InstanceMethods
       # @private
-      def to_matcher(expectation: nil, top_level: false)
+      def to_matcher(expectation: nil, top_level: false, method_accepts_keyword_arguments: true)
         if Base === self
           self
-        elsif Hash === self && top_level
+        elsif Hash === self && (top_level || method_accepts_keyword_arguments)
           Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
         else
           Mocha::ParameterMatchers::Equals.new(self)

lib/mocha/parameters_matcher.rb

@@ -9,18 +9,15 @@ def initialize(expected_parameters = [ParameterMatchers::AnyParameters.new], exp
       @matching_block = matching_block
     end
 
-    def match?(actual_parameters = [])
+    def match?(invocation)
+      actual_parameters = invocation.arguments || []
       if @matching_block
         @matching_block.call(*actual_parameters)
       else
-        parameters_match?(actual_parameters)
+        matchers(invocation).all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
       end
     end
 
-    def parameters_match?(actual_parameters)
-      matchers.all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
-    end
-
     def mocha_inspect
       if @matching_block
         '(arguments_accepted_by_custom_matching_block)'
@@ -31,8 +28,10 @@ def mocha_inspect
       end
     end
 
-    def matchers
-      @expected_parameters.map { |p| p.to_matcher(expectation: @expectation, top_level: true) }
+    def matchers(invocation = nil)
+      @expected_parameters.map do |p|
+        p.to_matcher(expectation: @expectation, top_level: true, method_accepts_keyword_arguments: invocation ? invocation.method_accepts_keyword_arguments? : true)
+      end
     end
   end
 end

test/unit/parameters_matcher_test.rb

@@ -1,90 +1,91 @@
 require File.expand_path('../../test_helper', __FILE__)
 require 'mocha/parameters_matcher'
+require 'mocha/invocation'
 
 class ParametersMatcherTest < Mocha::TestCase
   include Mocha
 
   def test_should_match_any_actual_parameters_if_no_expected_parameters_specified
     parameters_matcher = ParametersMatcher.new
-    assert parameters_matcher.match?([1, 2, 3])
+    assert parameters_matcher.match?(invocation_with_arguments([1, 2, 3]))
   end
 
   def test_should_match_if_actual_parameters_are_same_as_expected_parameters
     parameters_matcher = ParametersMatcher.new([4, 5, 6])
-    assert parameters_matcher.match?([4, 5, 6])
+    assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
   end
 
   def test_should_not_match_if_actual_parameters_are_different_from_expected_parameters
     parameters_matcher = ParametersMatcher.new([4, 5, 6])
-    assert !parameters_matcher.match?([1, 2, 3])
+    assert !parameters_matcher.match?(invocation_with_arguments([1, 2, 3]))
   end
 
   def test_should_not_match_if_there_are_less_actual_parameters_than_expected_parameters
     parameters_matcher = ParametersMatcher.new([4, 5, 6])
-    assert !parameters_matcher.match?([4, 5])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 5]))
   end
 
   def test_should_not_match_if_there_are_more_actual_parameters_than_expected_parameters
     parameters_matcher = ParametersMatcher.new([4, 5])
-    assert !parameters_matcher.match?([4, 5, 6])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
   end
 
   def test_should_not_match_if_not_all_required_parameters_are_supplied
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert !parameters_matcher.match?([4])
+    assert !parameters_matcher.match?(invocation_with_arguments([4]))
   end
 
   def test_should_match_if_all_required_parameters_match_and_no_optional_parameters_are_supplied
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert parameters_matcher.match?([4, 5])
+    assert parameters_matcher.match?(invocation_with_arguments([4, 5]))
   end
 
   def test_should_match_if_all_required_and_optional_parameters_match_and_some_optional_parameters_are_supplied
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert parameters_matcher.match?([4, 5, 6])
+    assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
   end
 
   def test_should_match_if_all_required_and_optional_parameters_match_and_all_optional_parameters_are_supplied
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert parameters_matcher.match?([4, 5, 6, 7])
+    assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 7]))
   end
 
   def test_should_not_match_if_all_required_and_optional_parameters_match_but_too_many_optional_parameters_are_supplied
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert !parameters_matcher.match?([4, 5, 6, 7, 8])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 7, 8]))
   end
 
   def test_should_not_match_if_all_required_parameters_match_but_some_optional_parameters_do_not_match
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert !parameters_matcher.match?([4, 5, 6, 0])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 0]))
   end
 
   def test_should_not_match_if_some_required_parameters_do_not_match_although_all_optional_parameters_do_match
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert !parameters_matcher.match?([4, 0, 6])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 0, 6]))
   end
 
   def test_should_not_match_if_all_required_parameters_match_but_no_optional_parameters_match
     optionals = ParameterMatchers::Optionally.new(6, 7)
     parameters_matcher = ParametersMatcher.new([4, 5, optionals])
-    assert !parameters_matcher.match?([4, 5, 0, 0])
+    assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 0, 0]))
   end
 
   def test_should_match_if_actual_parameters_satisfy_matching_block
     parameters_matcher = ParametersMatcher.new { |x, y| x + y == 3 }
-    assert parameters_matcher.match?([1, 2])
+    assert parameters_matcher.match?(invocation_with_arguments([1, 2]))
   end
 
   def test_should_not_match_if_actual_parameters_do_not_satisfy_matching_block
     parameters_matcher = ParametersMatcher.new { |x, y| x + y == 3 }
-    assert !parameters_matcher.match?([2, 3])
+    assert !parameters_matcher.match?(invocation_with_arguments([2, 3]))
   end
 
   def test_should_remove_outer_array_braces
@@ -108,4 +109,10 @@ def test_should_indicate_that_matcher_logic_is_defined_by_custom_block
     parameters_matcher = ParametersMatcher.new { true }
     assert_equal '(arguments_accepted_by_custom_matching_block)', parameters_matcher.mocha_inspect
   end
+
+  private
+
+  def invocation_with_arguments(arguments)
+    Invocation.new(nil, nil, arguments, nil, nil)
+  end
 end