Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Allow string keys for method kwarg splats #591

Merged
merged 5 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/rspec/support/method_signature_verifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def invalid_kw_args_from(given_kw_args)
def has_kw_args_in?(args)
Hash === args.last &&
could_contain_kw_args?(args) &&
(args.last.empty? || args.last.keys.any? { |x| x.is_a?(Symbol) })
(RubyFeatures.kw_arg_separation? || args.last.empty? || args.last.keys.any? { |x| x.is_a?(Symbol) })
end

# Without considering what the last arg is, could it
Expand Down Expand Up @@ -363,7 +363,7 @@ def unlimited_args?
end

def split_args(*args)
Copy link
Contributor Author

@malcolmohare malcolmohare Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an alternative version of this function, let me know which is preferred.

def split_args(*args)
  return [args.length, []] unless @signature.has_kw_args_in?(args)
  kw_args = if RubyFeatures.kw_arg_separation?
              args.pop.keys
            else
              last = args.pop
              non_kw_args = last.reject { |k, _| k.is_a?(Symbol) }
              if non_kw_args.empty?
                last.keys
              else
                args << non_kw_args
                last.select { |k, _| k.is_a?(Symbol) }.keys
              end
            end
  [args.length, kw_args]
end

kw_args = if @signature.has_kw_args_in?(args)
kw_args = if @signature.has_kw_args_in?(args) && !RubyFeatures.kw_arg_separation?
last = args.pop
non_kw_args = last.reject { |k, _| k.is_a?(Symbol) }
if non_kw_args.empty?
Expand All @@ -372,6 +372,8 @@ def split_args(*args)
args << non_kw_args
last.select { |k, _| k.is_a?(Symbol) }.keys
end
elsif @signature.has_kw_args_in?(args) && RubyFeatures.kw_arg_separation?
args.pop.keys
else
[]
end
Expand Down
11 changes: 11 additions & 0 deletions lib/rspec/support/ruby_features.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ def supports_syntax_suggest?
end
end

if RUBY_VERSION.to_f >= 3.0
# https://rubyreferences.github.io/rubychanges/3.0.html#keyword-arguments-are-now-fully-separated-from-positional-arguments
def kw_arg_separation?
true
end
else
def kw_arg_separation?
false
end
end

if RUBY_VERSION.to_f >= 2.7
def supports_taint?
false
Expand Down
87 changes: 77 additions & 10 deletions spec/rspec/support/method_signature_verifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,17 +384,37 @@ def arity_kw(x, y = {}, z:2); end
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
if RubyFeatures.kw_arg_separation?
context "when ruby has kw arg separation" do
it 'treats symbols as keyword arguments' do
expect(valid?(nil, nil, :z => 1)).to eq(true)
expect(valid?(nil, :z => 1)).to eq(true)
end

it 'fails to match string keys as kw args' do
expect(valid?(nil, nil, 'z' => 1)).to eq(false)
expect(valid?(nil, 'z' => 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: a, b")
end
end
else
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")
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 Expand Up @@ -656,6 +676,50 @@ def arity_required_kw_splat(w, *x, y:, z:, a: 'default'); end
end
end

describe 'a method with only a keyword arg splat' do
eval <<-RUBY
def arity_kw_arg_splat(**rest); end
RUBY

let(:test_method) { method(:arity_kw_arg_splat) }

if RubyFeatures.kw_arg_separation?
it 'allows undeclared keyword args' do
expect(valid?(:x => 1)).to eq(true)
expect(valid?(:x => 1, 'y' => 2)).to eq(true)
expect(valid?('y' => 2)).to eq(true)
end
else
it 'allows undeclared symbol keyword args' do
expect(valid?(:x => 1)).to eq(true)
expect(valid?(:x => 1, 'y' => 2)).to eq(false)
expect(valid?('y' => 2)).to eq(false)
end
end

it 'mentions the required kw args and keyword splat in the description' do
expect(signature_description).to \
eq("any additional keyword args")
end

describe 'with an expectation object' do
it 'allows zero non-kw args' do
expect(validate_expectation 0).to eq(true)
expect(validate_expectation 1).to eq(false)
expect(validate_expectation 0, 0).to eq(true)
expect(validate_expectation 0, 1).to eq(false)
end

it 'does not match unlimited arguments' do
expect(validate_expectation :unlimited_args).to eq(false)
end

it 'matches arbitrary keywords' do
expect(validate_expectation :arbitrary_kw_args).to eq(true)
end
end
end

describe 'a method with required keyword arguments and a keyword arg splat' do
eval <<-RUBY
def arity_kw_arg_splat(x:, **rest); end
Expand All @@ -666,6 +730,7 @@ def arity_kw_arg_splat(x:, **rest); end
it 'allows extra undeclared keyword args' do
expect(valid?(:x => 1)).to eq(true)
expect(valid?(:x => 1, :y => 2)).to eq(true)
expect(valid?(:x => 1, :y => 2, 'z' => 3)).to eq(RubyFeatures.kw_arg_separation?)
end

it 'mentions missing required keyword args in the error' do
Expand Down Expand Up @@ -730,7 +795,9 @@ def arity_kw_arg_splat(x, **rest); end
it 'allows a single arg and any number of keyword args' do
expect(valid?(nil)).to eq(true)
expect(valid?(nil, :x => 1)).to eq(true)
expect(valid?(nil, 'x' => 1)).to eq(RubyFeatures.kw_arg_separation?)
expect(valid?(nil, :x => 1, :y => 2)).to eq(true)
expect(valid?(nil, :x => 1, :y => 2, 'z' => 3)).to eq(RubyFeatures.kw_arg_separation?)
expect(valid?(:x => 1)).to eq(true)
expect(valid?(nil, {})).to eq(true)

Expand Down