Skip to content

Commit

Permalink
Distinct last positional argument hashes from keywords on Ruby 3
Browse files Browse the repository at this point in the history
  • Loading branch information
JonRowe authored and pirj committed Jan 9, 2023
1 parent 60d45b9 commit ca9ee38
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 16 deletions.
52 changes: 49 additions & 3 deletions lib/rspec/support/method_signature_verifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
53 changes: 40 additions & 13 deletions spec/rspec/support/method_signature_verifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ca9ee38

Please sign in to comment.