Merge pull request #366 from edzhelyov/recognize-hash-as-optional-argument

Recognize hash as optional arg when optional keyword is present

Author: JonRowe

lib/rspec/support/method_signature_verifier.rb

@@ -77,14 +77,19 @@ def invalid_kw_args_from(given_kw_args)
           given_kw_args - @allowed_kw_args
         end
 
+        # If the last argument is Hash, Ruby will treat only symbol keys as keyword arguments
+        # the rest will be grouped in another Hash and passed as positional argument.
         def has_kw_args_in?(args)
-          Hash === args.last && could_contain_kw_args?(args)
+          Hash === args.last &&
+            could_contain_kw_args?(args) &&
+            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)
           return false if args.count <= min_non_kw_args
+
           @allows_any_kw_args || @allowed_kw_args.any?
         end
 
@@ -357,7 +362,14 @@ def unlimited_args?
 
       def split_args(*args)
         kw_args = if @signature.has_kw_args_in?(args)
-                    args.pop.keys
+                    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
                   else
                     []
                   end

spec/rspec/support/method_signature_verifier_spec.rb

@@ -366,6 +366,102 @@ def arity_kw(x, y:1, z:2); end
           end
         end
 
+        if RubyFeatures.kw_args_supported?
+          describe 'a method with optional argument and keyword arguments' do
+            eval <<-RUBY
+              def arity_kw(x, y = {}, z:2); end
+            RUBY
+
+            let(:test_method) { method(:arity_kw) }
+
+            it 'does not require any of the arguments' do
+              expect(valid?(nil)).to eq(true)
+              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)
+            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
+
+            it 'describes invalid arity precisely' do
+              expect(error_for()).to \
+                eq("Wrong number of arguments. Expected 1 to 2, got 0.")
+            end
+
+            it 'does not blow up when given a BasicObject as the last arg' do
+              expect(valid?(BasicObject.new)).to eq(true)
+            end
+
+            it 'does not mutate the provided args array' do
+              args = [nil, nil, { :y => 1 }]
+              described_class.new(signature, args).valid?
+              expect(args).to eq([nil, nil, { :y => 1 }])
+            end
+
+            it 'mentions the arity and optional kw args in the description', :pending => RSpec::Support::Ruby.jruby? && !RSpec::Support::Ruby.jruby_9000? do
+              expect(signature_description).to eq("arity of 1 to 2 and optional keyword args (:z)")
+            end
+
+            it "indicates the optional keyword args" do
+              expect(signature.optional_kw_args).to contain_exactly(:z)
+            end
+
+            it "indicates it has no required keyword args" do
+              expect(signature.required_kw_args).to eq([])
+            end
+
+            describe 'with an expectation object' do
+              it 'matches the exact arity' do
+                expect(validate_expectation 0).to eq(false)
+                expect(validate_expectation 1).to eq(true)
+                expect(validate_expectation 2).to eq(true)
+              end
+
+              it 'matches the exact range' do
+                expect(validate_expectation 0, 1).to eq(false)
+                expect(validate_expectation 1, 1).to eq(true)
+                expect(validate_expectation 1, 2).to eq(true)
+                expect(validate_expectation 1, 3).to eq(false)
+              end
+
+              it 'does not match unlimited arguments' do
+                expect(validate_expectation :unlimited_args).to eq(false)
+              end
+
+              it 'matches optional keywords with the correct arity' do
+                expect(validate_expectation :z).to eq(false)
+                expect(validate_expectation 1, :z).to eq(true) # Are we OK with that?
+                expect(validate_expectation 1, 2, :z).to eq(true)
+                expect(validate_expectation 1, 2, :y).to eq(false)
+              end
+
+              it 'does not match invalid keywords' do
+                expect(validate_expectation :w).to eq(false)
+
+                expect(validate_expectation 2, :w).to eq(false)
+              end
+
+              it 'does not match arbitrary keywords' do
+                expect(validate_expectation :arbitrary_kw_args).to eq(false)
+              end
+            end
+          end
+        end
+
         if RubyFeatures.required_kw_args_supported?
           describe 'a method with required keyword arguments' do
             eval <<-RUBY