Deprecate implicit AD::Response splatting and Array conversion

3 files changed, 43 insertions, 5 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index e250450a76..de9722c392 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,16 @@
+*   Deprecate implicit Array conversion for Response objects. It was added
+    (using `#to_ary`) so we could conveniently use implicit splatting:
+
+        status, headers, body = response
+
+    But it also means `response + response` works and `[response].flatten`
+    cascades down to the Rack body. Nonsense behavior. Instead, rely on
+    explicit conversion and splatting with `#to_a`:
+
+        status, header, body = *response
+
+    *Jeremy Kemper*
+
 *   Don't rescue `IPAddr::InvalidAddressError`.
 
     `IPAddr::InvalidAddressError` does not exist in Ruby 1.9.3
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 2fab6be1a5..a58e904c11 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -1,4 +1,5 @@
 require 'active_support/core_ext/module/attribute_accessors'
+require 'active_support/deprecation'
 require 'action_dispatch/http/filter_redirect'
 require 'monitor'
 
@@ -274,12 +275,22 @@ module ActionDispatch # :nodoc:
     end
 
     # Turns the Response into a Rack-compatible array of the status, headers,
-    # and body.
+    # and body. Allows explict splatting:
+    #
+    #   status, headers, body = *response
     def to_a
       rack_response @status, @header.to_hash
     end
     alias prepare! to_a
-    alias to_ary   to_a
+
+    # Be super clear that a response object is not an Array. Defining this
+    # would make implicit splatting work, but it also makes adding responses
+    # as arrays work, and "flattening" responses, cascading to the rack body!
+    # Not sensible behavior.
+    def to_ary
+      ActiveSupport::Deprecation.warn 'ActionDispatch::Response#to_ary no longer performs implicit conversion to an Array. Please use response.to_a instead, or a splat like `status, headers, body = *response`'
+      to_a
+    end
 
     # Returns the response cookies, converted to a Hash of (name => value) pairs
     #
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index 187b9a2420..af188191e3 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -220,9 +220,9 @@ class ResponseTest < ActiveSupport::TestCase
     assert @response.respond_to?(:method_missing, true)
   end
 
-  test "can be destructured into status, headers and an enumerable body" do
+  test "can be explicitly destructured into status, headers and an enumerable body" do
     response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found'])
-    status, headers, body = response
+    status, headers, body = *response
 
     assert_equal 404, status
     assert_equal({ 'Content-Type' => 'text/plain' }, headers)
@@ -231,12 +231,26 @@ class ResponseTest < ActiveSupport::TestCase
 
   test "[response].flatten does not recurse infinitely" do
     Timeout.timeout(1) do # use a timeout to prevent it stalling indefinitely
-      status, headers, body = [@response].flatten
+      status, headers, body = assert_deprecated { [@response].flatten }
       assert_equal @response.status, status
       assert_equal @response.headers, headers
       assert_equal @response.body, body.each.to_a.join
     end
   end
+
+  test "implicit destructuring and Array conversion is deprecated" do
+    response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found'])
+
+    assert_deprecated do
+      status, headers, body = response
+
+      assert_equal 404, status
+      assert_equal({ 'Content-Type' => 'text/plain' }, headers)
+      assert_equal ['Not Found'], body.each.to_a
+    end
+
+    assert_deprecated { response.to_ary }
+  end
 end
 
 class ResponseIntegrationTest < ActionDispatch::IntegrationTest