From d245226abab23dc5d570bce2e6c2b4cbf6d854fb Mon Sep 17 00:00:00 2001
From: Mario Kostelac <mario.kostelac@gmail.com>
Date: Thu, 5 Aug 2021 08:45:40 +0200
Subject: [PATCH 1/2] Optionally raise on empty hash filter definitions

I've recently spent several hours debugging a problem that was caused by an empty hash definition
```ruby
required do
   hash :hash_name
end
```

Such definition discards any hash content being passed in, which is very
confusing and I reckon rarely useful, but very unintuitive interface
(found other 3 bugs in the codebase caused by the exact same problem).

This change makes such definitions raise, but only if
Mutations.raise_on_empty_hash_filter is set to true (defualt false).
Such optional raising avoids breaking change.

Overall, I recommend changing the interface to raise on bogus definition
as a default, instead of the opt-in.
---
 lib/mutations.rb             |  9 +++++++++
 lib/mutations/hash_filter.rb |  4 ++++
 spec/hash_filter_spec.rb     | 20 ++++++++++++++++++--
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/mutations.rb b/lib/mutations.rb
index be0a856..2dd8ba7 100644
--- a/lib/mutations.rb
+++ b/lib/mutations.rb
@@ -41,7 +41,16 @@ def cache_constants=(val)
     def cache_constants?
       @cache_constants
     end
+
+    def raise_on_empty_hash_filter=(val)
+      @raise_on_empty_hash_filter = val
+    end
+
+    def raise_on_empty_hash_filter
+      @raise_on_empty_hash_filter
+    end
   end
 end
 
 Mutations.cache_constants = true
+Mutations.raise_on_empty_hash_filter = false
diff --git a/lib/mutations/hash_filter.rb b/lib/mutations/hash_filter.rb
index 5f22680..1f0d193 100644
--- a/lib/mutations/hash_filter.rb
+++ b/lib/mutations/hash_filter.rb
@@ -23,6 +23,10 @@ def initialize(opts = {}, &block)
       @required_inputs = {}
       @current_inputs = @required_inputs
 
+      if Mutations.raise_on_empty_hash_filter && !block_given?
+        raise ArgumentError.new("Hash parameter can't be created without passing the block")
+      end
+
       if block_given?
         instance_eval(&block)
       end
diff --git a/spec/hash_filter_spec.rb b/spec/hash_filter_spec.rb
index a1fef96..9d2a4b6 100644
--- a/spec/hash_filter_spec.rb
+++ b/spec/hash_filter_spec.rb
@@ -105,6 +105,22 @@ def input.to_hash
     assert_equal ({"baz" => :integer}), errors.symbolic
   end
 
+  it "allows empty hash definitions" do
+    Mutations::HashFilter.new
+    # the fact that we're here means it didn't raise
+  end
+
+  describe "with raise_on_empty_hash_filter=true" do
+    it "raises for empty hash definition" do
+      old_value = Mutations.raise_on_empty_hash_filter
+      Mutations.raise_on_empty_hash_filter = true
+
+      assert_raises(ArgumentError) { Mutations::HashFilter.new }
+    ensure
+      Mutations.raise_on_empty_hash_filter = old_value
+    end
+  end
+
   describe "optional params and nils" do
     it "bar is optional -- it works if not passed" do
       hf = Mutations::HashFilter.new do
@@ -167,7 +183,7 @@ def input.to_hash
       assert_equal ({"foo" => "bar"}), filtered
       assert_equal nil, errors
     end
-    
+
     it "bar is optional -- discards empty if it needs to be stripped" do
       hf = Mutations::HashFilter.new do
         required do
@@ -182,7 +198,7 @@ def input.to_hash
       assert_equal ({"foo" => "bar"}), filtered
       assert_equal nil, errors
     end
-    
+
     it "bar is optional -- don't discard empty if it's spaces but stripping is off" do
       hf = Mutations::HashFilter.new do
         required do

From 805192bf61a522dac25d96fbcf908b194aa5dd8c Mon Sep 17 00:00:00 2001
From: Mario Kostelac <mario.kostelac@gmail.com>
Date: Thu, 5 Aug 2021 15:13:26 +0200
Subject: [PATCH 2/2] Fix ensure usage

---
 spec/hash_filter_spec.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/spec/hash_filter_spec.rb b/spec/hash_filter_spec.rb
index 9d2a4b6..3d50a62 100644
--- a/spec/hash_filter_spec.rb
+++ b/spec/hash_filter_spec.rb
@@ -112,12 +112,14 @@ def input.to_hash
 
   describe "with raise_on_empty_hash_filter=true" do
     it "raises for empty hash definition" do
-      old_value = Mutations.raise_on_empty_hash_filter
-      Mutations.raise_on_empty_hash_filter = true
+      begin
+        old_value = Mutations.raise_on_empty_hash_filter
+        Mutations.raise_on_empty_hash_filter = true
 
-      assert_raises(ArgumentError) { Mutations::HashFilter.new }
-    ensure
-      Mutations.raise_on_empty_hash_filter = old_value
+        assert_raises(ArgumentError) { Mutations::HashFilter.new }
+      ensure
+        Mutations.raise_on_empty_hash_filter = old_value
+      end
     end
   end