Improve keyword argument deprecation warning

By including the source location of the stub definition.

This change is a necessarily a bit pervasive and unfortunately makes the
code and particularly the tests even more complicated. However, I think
it will really help developers fix the deprecation warnings and
(hopefully) it will only be necessary until the next significant
release at which point we should be able to remove it!

lib/mocha/expectation.rb

@@ -12,6 +12,7 @@
 require 'mocha/cardinality'
 require 'mocha/configuration'
 require 'mocha/block_matcher'
+require 'mocha/backtrace_filter'
 
 module Mocha
   # Methods on expectations returned from {Mock#expects}, {Mock#stubs}, {ObjectMethods#expects} and {ObjectMethods#stubs}.
@@ -266,7 +267,7 @@ def at_most_once
     #   object.expected_method(17)
     #   # => verify fails
     def with(*expected_parameters_or_matchers, &matching_block)
-      @parameters_matcher = ParametersMatcher.new(expected_parameters_or_matchers, &matching_block)
+      @parameters_matcher = ParametersMatcher.new(expected_parameters_or_matchers, self, &matching_block)
       self
     end
     ruby2_keywords(:with)
@@ -692,5 +693,11 @@ def method_signature
       signature << " #{@block_matcher.mocha_inspect}" if @block_matcher.mocha_inspect
       signature
     end
+
+    # @private
+    def definition_location
+      filter = BacktraceFilter.new
+      filter.filtered(backtrace)[0]
+    end
   end
 end

lib/mocha/parameter_matchers/base.rb

@@ -3,7 +3,7 @@ module ParameterMatchers
     # @abstract Subclass and implement +#matches?+ and +#mocha_inspect+ to define a custom matcher. Also add a suitably named instance method to {ParameterMatchers} to build an instance of the new matcher c.f. {#equals}.
     class Base
       # @private
-      def to_matcher
+      def to_matcher(_expectation = nil)
         self
       end
 

lib/mocha/parameter_matchers/instance_methods.rb

@@ -6,7 +6,7 @@ module ParameterMatchers
     # @private
     module InstanceMethods
       # @private
-      def to_matcher
+      def to_matcher(_expectation = nil)
         Mocha::ParameterMatchers::Equals.new(self)
       end
     end
@@ -21,7 +21,7 @@ class Object
 # @private
 class Hash
   # @private
-  def to_matcher
-    Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self)
+  def to_matcher(expectation = nil)
+    Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
   end
 end

lib/mocha/parameter_matchers/positional_or_keyword_hash.rb

@@ -6,8 +6,9 @@ module Mocha
   module ParameterMatchers
     # @private
     class PositionalOrKeywordHash < Base
-      def initialize(value)
+      def initialize(value, expectation)
         @value = value
+        @expectation = expectation
       end
 
       def matches?(available_parameters)
@@ -38,10 +39,11 @@ def same_type_of_hash?(actual, expected)
       end
 
       def deprecation_warning(actual, expected)
-        details = "Expected #{hash_type(expected)} (#{expected.mocha_inspect}), but received #{hash_type(actual)} (#{actual.mocha_inspect})."
+        details1 = "Expectation #{expectation_definition} expected #{hash_type(expected)} (#{expected.mocha_inspect}),".squeeze(' ')
+        details2 = "but received #{hash_type(actual)} (#{actual.mocha_inspect})."
         sentence1 = 'These will stop matching when strict keyword argument matching is enabled.'
         sentence2 = 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
-        Deprecation.warning([details, sentence1, sentence2].join(' '))
+        Deprecation.warning([details1, details2, sentence1, sentence2].join(' '))
       end
 
       def hash_type(hash)
@@ -51,6 +53,12 @@ def hash_type(hash)
       def ruby2_keywords_hash?(hash)
         hash.is_a?(Hash) && ::Hash.ruby2_keywords_hash?(hash)
       end
+
+      def expectation_definition
+        return nil unless @expectation
+
+        "defined at #{@expectation.definition_location}"
+      end
     end
   end
 end

lib/mocha/parameters_matcher.rb

@@ -3,8 +3,9 @@
 
 module Mocha
   class ParametersMatcher
-    def initialize(expected_parameters = [ParameterMatchers::AnyParameters.new], &matching_block)
+    def initialize(expected_parameters = [ParameterMatchers::AnyParameters.new], expectation = nil, &matching_block)
       @expected_parameters = expected_parameters
+      @expectation = expectation
       @matching_block = matching_block
     end
 
@@ -27,7 +28,7 @@ def mocha_inspect
     end
 
     def matchers
-      @expected_parameters.map(&:to_matcher)
+      @expected_parameters.map { |p| p.to_matcher(@expectation) }
     end
   end
 end

test/acceptance/keyword_argument_matching_test.rb

@@ -1,6 +1,7 @@
 require File.expand_path('../acceptance_test_helper', __FILE__)
 
 require 'deprecation_disabler'
+require 'execution_point'
 require 'mocha/deprecation'
 
 class KeywordArgumentMatchingTest < Mocha::TestCase
@@ -15,14 +16,17 @@ def teardown
   end
 
   def test_should_match_hash_parameter_with_keyword_args
+    test_name = __method__
     test_result = run_as_test do
       mock = mock()
-      mock.expects(:method).with(:key => 42)
+      mock.expects(:method).with(:key => 42); execution_point = ExecutionPoint.current
       DeprecationDisabler.disable_deprecations do
         mock.method({ :key => 42 }) # rubocop:disable Style/BracesAroundHashParameters
       end
       if Mocha::RUBY_V27_PLUS
-        assert_includes Mocha::Deprecation.messages.last, 'Expected keyword arguments (:key => 42), but received positional hash ({:key => 42}).'
+        location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
+        assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
+        assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
       end
     end
     assert_passed(test_result)
@@ -42,14 +46,17 @@ def test_should_not_match_hash_parameter_with_keyword_args_when_strict_keyword_m
   end
 
   def test_should_match_hash_parameter_with_splatted_keyword_args
+    test_name = __method__
     test_result = run_as_test do
       mock = mock()
-      mock.expects(:method).with(**{ :key => 42 })
+      mock.expects(:method).with(**{ :key => 42 }); execution_point = ExecutionPoint.current
       DeprecationDisabler.disable_deprecations do
         mock.method({ :key => 42 }) # rubocop:disable Style/BracesAroundHashParameters
       end
       if Mocha::RUBY_V27_PLUS
-        assert_includes Mocha::Deprecation.messages.last, 'Expected keyword arguments (:key => 42), but received positional hash ({:key => 42}).'
+        location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
+        assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
+        assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
       end
     end
     assert_passed(test_result)
@@ -87,14 +94,17 @@ def test_should_match_splatted_hash_parameter_with_splatted_hash
   end
 
   def test_should_match_positional_and_keyword_args_with_last_positional_hash
+    test_name = __method__
     test_result = run_as_test do
       mock = mock()
-      mock.expects(:method).with(1, { :key => 42 }) # rubocop:disable Style/BracesAroundHashParameters
+      mock.expects(:method).with(1, { :key => 42 }); execution_point = ExecutionPoint.current # rubocop:disable Style/BracesAroundHashParameters
       DeprecationDisabler.disable_deprecations do
         mock.method(1, :key => 42)
       end
       if Mocha::RUBY_V27_PLUS
-        assert_includes Mocha::Deprecation.messages.last, 'Expected positional hash ({:key => 42}), but received keyword arguments (:key => 42).'
+        location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
+        assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected positional hash ({:key => 42})"
+        assert_includes Mocha::Deprecation.messages.last, 'but received keyword arguments (:key => 42)'
       end
     end
     assert_passed(test_result)
@@ -114,14 +124,17 @@ def test_should_not_match_positional_and_keyword_args_with_last_positional_hash_
   end
 
   def test_should_match_last_positional_hash_with_keyword_args
+    test_name = __method__
     test_result = run_as_test do
       mock = mock()
-      mock.expects(:method).with(1, :key => 42)
+      mock.expects(:method).with(1, :key => 42); execution_point = ExecutionPoint.current
       DeprecationDisabler.disable_deprecations do
         mock.method(1, { :key => 42 }) # rubocop:disable Style/BracesAroundHashParameters
       end
       if Mocha::RUBY_V27_PLUS
-        assert_includes Mocha::Deprecation.messages.last, 'Expected keyword arguments (:key => 42), but received positional hash ({:key => 42}).'
+        location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
+        assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
+        assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
       end
     end
     assert_passed(test_result)

test/unit/parameter_matchers/positional_or_keyword_hash_test.rb

@@ -1,9 +1,11 @@
 require File.expand_path('../../../test_helper', __FILE__)
 
 require 'deprecation_disabler'
+require 'execution_point'
 require 'mocha/parameter_matchers/positional_or_keyword_hash'
 require 'mocha/parameter_matchers/instance_methods'
 require 'mocha/inspect'
+require 'mocha/expectation'
 
 class PositionalOrKeywordHashTest < Mocha::TestCase
   include Mocha::ParameterMatchers
@@ -34,27 +36,33 @@ def test_should_match_keyword_args_with_keyword_args
   end
 
   def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning_if_appropriate
-    matcher = Hash.ruby2_keywords_hash({ :key_1 => 1, :key_2 => 2 }).to_matcher # rubocop:disable Style/BracesAroundHashParameters
+    expectation = Mocha::Expectation.new(self, :foo); execution_point = ExecutionPoint.current
+    matcher = Hash.ruby2_keywords_hash({ :key_1 => 1, :key_2 => 2 }).to_matcher(expectation) # rubocop:disable Style/BracesAroundHashParameters
     DeprecationDisabler.disable_deprecations do
       assert matcher.matches?([{ :key_1 => 1, :key_2 => 2 }])
     end
     return unless Mocha::RUBY_V27_PLUS
 
     message = Mocha::Deprecation.messages.last
-    assert_includes message, 'Expected keyword arguments (:key_1 => 1, :key_2 => 2), but received positional hash ({:key_1 => 1, :key_2 => 2}).'
+    location = "#{execution_point.file_name}:#{execution_point.line_number}:in `new'"
+    assert_includes message, "Expectation defined at #{location} expected keyword arguments (:key_1 => 1, :key_2 => 2)"
+    assert_includes message, 'but received positional hash ({:key_1 => 1, :key_2 => 2})'
     assert_includes message, 'These will stop matching when strict keyword argument matching is enabled.'
     assert_includes message, 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
   end
 
   def test_should_match_keyword_args_with_hash_arg_but_display_deprecation_warning_if_appropriate
-    matcher = { :key_1 => 1, :key_2 => 2 }.to_matcher
+    expectation = Mocha::Expectation.new(self, :foo); execution_point = ExecutionPoint.current
+    matcher = { :key_1 => 1, :key_2 => 2 }.to_matcher(expectation)
     DeprecationDisabler.disable_deprecations do
       assert matcher.matches?([Hash.ruby2_keywords_hash({ :key_1 => 1, :key_2 => 2 })]) # rubocop:disable Style/BracesAroundHashParameters
     end
     return unless Mocha::RUBY_V27_PLUS
 
     message = Mocha::Deprecation.messages.last
-    assert_includes message, 'Expected positional hash ({:key_1 => 1, :key_2 => 2}), but received keyword arguments (:key_1 => 1, :key_2 => 2).'
+    location = "#{execution_point.file_name}:#{execution_point.line_number}:in `new'"
+    assert_includes message, "Expectation defined at #{location} expected positional hash ({:key_1 => 1, :key_2 => 2})"
+    assert_includes message, 'but received keyword arguments (:key_1 => 1, :key_2 => 2)'
     assert_includes message, 'These will stop matching when strict keyword argument matching is enabled.'
     assert_includes message, 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
   end
@@ -101,5 +109,17 @@ def test_should_not_match_keyword_args_with_hash_arg_when_strict_keyword_args_is
         assert !matcher.matches?([Hash.ruby2_keywords_hash({ :key_1 => 1, :key_2 => 2 })]) # rubocop:disable Style/BracesAroundHashParameters
       end
     end
+
+    def test_should_display_deprecation_warning_even_if_parent_expectation_is_nil
+      expectation = nil
+      matcher = { :key_1 => 1, :key_2 => 2 }.to_matcher(expectation)
+      DeprecationDisabler.disable_deprecations do
+        matcher.matches?([Hash.ruby2_keywords_hash({ :key_1 => 1, :key_2 => 2 })]) # rubocop:disable Style/BracesAroundHashParameters
+      end
+
+      message = Mocha::Deprecation.messages.last
+      assert_includes message, 'Expectation expected positional hash ({:key_1 => 1, :key_2 => 2})'
+      assert_includes message, 'but received keyword arguments (:key_1 => 1, :key_2 => 2)'
+    end
   end
 end