diff --git a/lib/rspec/support/method_signature_verifier.rb b/lib/rspec/support/method_signature_verifier.rb index c4eb432a..bcd03a22 100644 --- a/lib/rspec/support/method_signature_verifier.rb +++ b/lib/rspec/support/method_signature_verifier.rb @@ -85,12 +85,58 @@ def has_kw_args_in?(args) (args.last.empty? || args.last.keys.any? { |x| x.is_a?(Symbol) }) end - # Without considering what the last arg is, could it - # contain keyword arguments? def could_contain_kw_args?(args) + # If the arg count is less than the required positional args there are no keyword args return false if args.count <= min_non_kw_args - @allows_any_kw_args || @allowed_kw_args.any? + # If there is no keyword splat or allowed keywords there are no keyword args + return false unless @allows_any_kw_args || @allowed_kw_args.any? + + # If the args captured are more than the positional arguments then there are keywords + return true if args.count > max_non_kw_args + + # Otherwise we need to check wether the last argument is a positional non default hash, + # e.g. that this: + # + # def capture(_, last = {}, keyword: 2) + # + # Is valid with both: + # + # [1, {:some => :hash}] + # [1, {:keyword => :value}] + # + # but for different reasons. + + # If the keyword splat is allowed, and there are no specified + # keywords args then we will either have returned true on L100 + # or this is invalid. + return false if @allowed_kw_args.empty? && @allows_any_kw_args + + if Hash === args.last + last_arg_matches_any_keywords = !(args.last.keys & @allowed_kw_args).empty? + has_extra_keys = !(args.last.keys - @allowed_kw_args).empty? + + # If we allow the keyword splat, and the last hash contains + # a specified keyword, then this hash contains keywords + return true if @allows_any_kw_args && last_arg_matches_any_keywords + + # If we don't allow the keyword splat, and the last hash contains extra keys + # then it is a positional hash. + return false if !@allows_any_kw_args && has_extra_keys + + # Otherwise assume we have keywords + return true + else + # If the last argument is a matcher nd we're on Ruby 3 which won't mix keyword args + # and a normal hash... + if RubyFeatures.distincts_kw_args_from_positional_hash? + # we've either returned true above on L100 or this is a positional arg matcher + return false + else + # However on older rubies this could contain keyword arguments + return true + end + end end def arbitrary_kw_args? diff --git a/spec/rspec/support/method_signature_verifier_spec.rb b/spec/rspec/support/method_signature_verifier_spec.rb index d342425a..a3645b4e 100644 --- a/spec/rspec/support/method_signature_verifier_spec.rb +++ b/spec/rspec/support/method_signature_verifier_spec.rb @@ -379,22 +379,49 @@ def arity_kw(x, y = {}, z:2); end expect(valid?(nil, nil)).to eq(true) end - it 'does not allow an invalid keyword arguments' do - expect(valid?(nil, nil, :a => 1)).to eq(false) - expect(valid?(nil, :a => 1)).to eq(false) + it 'treats the final positional argument as a hash' do + expect(valid?(1, { :z => 42 })).to eq(true) + expect(valid?(1, { :foo => 'bar', :baz => 'eggs'})).to eq(true) end - it 'treats symbols as keyword arguments and the rest as optional argument' do - expect(valid?(nil, 'a' => 1)).to eq(true) - expect(valid?(nil, 'a' => 1, :z => 3)).to eq(true) - expect(valid?(nil, 'a' => 1, :b => 3)).to eq(false) - expect(valid?(nil, 'a' => 1, :b => 2, :z => 3)).to eq(false) - end + if RubyFeatures.distincts_kw_args_from_positional_hash? + it 'does not allow an invalid keyword arguments' do + expect(valid?(nil, nil, :a => 1)).to eq(false) + end - it 'mentions the invalid keyword args in the error', :pending => RSpec::Support::Ruby.jruby? && !RSpec::Support::Ruby.jruby_9000? do - expect(error_for(1, 2, :a => 0)).to eq("Invalid keyword arguments provided: a") - expect(error_for(1, :a => 0)).to eq("Invalid keyword arguments provided: a") - expect(error_for(1, 'a' => 0, :b => 0)).to eq("Invalid keyword arguments provided: b") + it 'always treats non matching hashes as positional' do + # Basically on Ruby 3 this will always be a hash. + expect(valid?(nil, 'a' => 1)).to eq(true) + expect(valid?(nil, 'a' => 1, :z => 3)).to eq(true) + expect(valid?(nil, 'a' => 1, :b => 3)).to eq(true) + expect(valid?(nil, 'a' => 1, :b => 2, :z => 3)).to eq(true) + end + + it 'mentions the invalid keyword args in the error', :pending => RSpec::Support::Ruby.jruby? && !RSpec::Support::Ruby.jruby_9000? do + expect(error_for(1, 2, :a => 0)).to eq("Invalid keyword arguments provided: a") + + # Ruby 3 will treat this as a positional hash, so they are valid. + expect(error_for(1, :a => 0)).to eq(nil) + expect(error_for(1, 'a' => 0, :b => 0)).to eq(nil) + end + else + it 'does not allow an invalid keyword arguments' do + expect(valid?(nil, nil, :a => 1)).to eq(false) + expect(valid?(nil, :a => 1)).to eq(false) + end + + it 'treats symbols as keyword arguments and the rest as optional argument' do + expect(valid?(nil, 'a' => 1)).to eq(true) + expect(valid?(nil, 'a' => 1, :z => 3)).to eq(true) + expect(valid?(nil, 'a' => 1, :b => 3)).to eq(false) + expect(valid?(nil, 'a' => 1, :b => 2, :z => 3)).to eq(false) + end + + it 'mentions the invalid keyword args in the error', :pending => RSpec::Support::Ruby.jruby? && !RSpec::Support::Ruby.jruby_9000? do + expect(error_for(1, 2, :a => 0)).to eq("Invalid keyword arguments provided: a") + expect(error_for(1, :a => 0)).to eq("Invalid keyword arguments provided: a") + expect(error_for(1, 'a' => 0, :b => 0)).to eq("Invalid keyword arguments provided: b") + end end it 'describes invalid arity precisely' do