Merge pull request #353 from composerinteralia/raise-merge-errors

Avoid silently overwriting with merge

Author: rafaelfranca

lib/jbuilder.rb

@@ -270,14 +270,14 @@ def _merge_block(key)
   def _merge_values(current_value, updates)
     if _blank?(updates)
       current_value
-    elsif _blank?(current_value) || updates.nil?
+    elsif _blank?(current_value) || updates.nil? || current_value.empty? && ::Array === updates
       updates
-    elsif ::Array === updates
-      ::Array === current_value ? current_value + updates : updates
-    elsif ::Hash === current_value
+    elsif ::Array === current_value && ::Array === updates
+      current_value + updates
+    elsif ::Hash === current_value && ::Hash === updates
       current_value.merge(updates)
     else
-      raise "Can't merge #{updates.inspect} with #{current_value.inspect}"
+      raise MergeError.build(current_value, updates)
     end
   end
 

lib/jbuilder/errors.rb

@@ -14,4 +14,11 @@ def self.build(key)
       new(message)
     end
   end
+
+  class MergeError < ::StandardError
+    def self.build(current_value, updates)
+      message = "Can't merge #{updates.inspect} into #{current_value.inspect}"
+      new(message)
+    end
+  end
 end

test/jbuilder_test.rb

@@ -686,4 +686,31 @@ class JbuilderTest < ActiveSupport::TestCase
       end
     end
   end
+
+  test "throws MergeError when trying to merge array with non-empty hash" do
+    assert_raise Jbuilder::MergeError do
+      jbuild do |json|
+        json.name "Daniel"
+        json.merge! []
+      end
+    end
+  end
+
+  test "throws MergeError when trying to merge hash with array" do
+    assert_raise Jbuilder::MergeError do
+      jbuild do |json|
+        json.array!
+        json.merge!({})
+      end
+    end
+  end
+
+  test "throws MergeError when trying to merge invalid objects" do
+    assert_raise Jbuilder::MergeError do
+      jbuild do |json|
+        json.name "Daniel"
+        json.merge! "Nope"
+      end
+    end
+  end
 end