From a84ee71474de98e96a9d7bc0dc827554a41e7fdb Mon Sep 17 00:00:00 2001
From: nick evans <nick@rubinick.dev>
Date: Sun, 6 Oct 2024 17:14:33 -0400
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Responses=20with=20type=20can=20ret?=
 =?UTF-8?q?urn=20frozen=20dup=20array?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Because responses(type) is relatively new and has always raised an
exception, we can update it to return a frozen dup array without
breaking backward compatibility.

Additionally, `config.responses_without_args` was added as an alias for
`config.responses_without_block`.  The original name is a little
misleading now, but it's kept for backward compatibility.
---
 lib/net/imap.rb                      | 13 ++++++++++---
 lib/net/imap/config.rb               |  8 +++++---
 test/net/imap/test_imap_responses.rb | 29 +++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/lib/net/imap.rb b/lib/net/imap.rb
index 96152a08..8bf82356 100644
--- a/lib/net/imap.rb
+++ b/lib/net/imap.rb
@@ -2497,6 +2497,7 @@ def idle_done
     private_constant :RESPONSES_DEPRECATION_MSG
 
     # :call-seq:
+    #   responses(type) -> frozen array
     #   responses       {|hash|  ...} -> block result
     #   responses(type) {|array| ...} -> block result
     #
@@ -2522,8 +2523,14 @@ def idle_done
     #   until the block completes.  Accessing either the response hash or its
     #   response type arrays outside of the block is unsafe.
     #
-    #   Calling without a block is unsafe and deprecated.  Future releases will
-    #   raise ArgumentError unless a block is given.
+    #   For safety, calling #responses with +type+ but without a block will
+    #   return a frozen duplicate of the array.  Note that the array is not
+    #   deeply frozen, and it is still unsafe to modify elements in the array
+    #   from multiple threads.
+    #
+    #   Calling without +type+ or a block is unsafe and deprecated.  Future
+    #   releases may raise ArgumentError unless a block is given.
+    #
     #   See Config#responses_without_block.
     #
     # Previously unhandled responses are automatically cleared before entering a
@@ -2547,7 +2554,7 @@ def responses(type = nil)
       if block_given?
         synchronize { yield(type ? @responses[type.to_s.upcase] : @responses) }
       elsif type
-        raise ArgumentError, "Pass a block or use #clear_responses"
+        synchronize { @responses[type.to_s.upcase].dup.freeze }
       else
         case config.responses_without_block
         when :raise
diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb
index bcf5b72d..e3a897ed 100644
--- a/lib/net/imap/config.rb
+++ b/lib/net/imap/config.rb
@@ -241,9 +241,9 @@ def self.[](config)
 
       # :markup: markdown
       #
-      # Controls the behavior of Net::IMAP#responses when called without a
-      # block.  Valid options are `:warn`, `:raise`, or
-      # `:silence_deprecation_warning`.
+      # Controls the behavior of Net::IMAP#responses when called without any
+      # arguments (a type or block).  Valid options are `:warn`,
+      # `:silence_deprecation_warning`, or `:raise`.
       #
       # | Starting with version   | The default value is           |
       # |-------------------------|--------------------------------|
@@ -253,6 +253,8 @@ def self.[](config)
       attr_accessor :responses_without_block, type: [
         :silence_deprecation_warning, :warn, :raise,
       ]
+      alias responses_without_args  responses_without_block
+      alias responses_without_args= responses_without_block=
 
       # Creates a new config object and initialize its attribute with +attrs+.
       #
diff --git a/test/net/imap/test_imap_responses.rb b/test/net/imap/test_imap_responses.rb
index ee710f26..233235bf 100644
--- a/test/net/imap/test_imap_responses.rb
+++ b/test/net/imap/test_imap_responses.rb
@@ -89,12 +89,35 @@ def for_each_config_option(imap)
     end
   end
 
-  # with with a type and no block: always raise an exception
+  # with with a type and no block: always returns a frozen duplicate
   test "#responses(type, &nil)" do
     with_fake_server do |server, imap|
-      for_each_config_option(imap) do
-        assert_raise(ArgumentError) do imap.responses("CAPABILITY") end
+      stderr = EnvUtil.verbose_warning do
+        # Config options make no difference to responses(type)
+        for_each_config_option(imap) do
+          # responses available before SELECT/EXAMINE
+          assert imap.responses("CAPABILITY").frozen?
+          assert_equal(%w[IMAP4REV1 NAMESPACE MOVE IDLE UTF8=ACCEPT],
+                       imap.responses("CAPABILITY").last)
+        end
+        # responses are cleared after SELECT/EXAMINE
+        imap.select "INBOX"
+        for_each_config_option(imap) do
+          assert imap.responses("CAPABILITY").frozen?
+          assert imap.responses("EXISTS").frozen?
+          assert imap.responses("UIDVALIDITIY").frozen?
+          assert_equal [],           imap.responses("CAPABILITY")
+          assert_equal [172],        imap.responses("EXISTS")
+          assert_equal [3857529045], imap.responses("UIDVALIDITY")
+          assert_equal 1, imap.responses("RECENT").last
+          assert imap.responses("UIDNEXT").frozen?
+          assert_equal [4392], imap.responses("UIDNEXT")
+          assert imap.responses("FLAGS").frozen?
+          assert_equal(%i[Answered Flagged Deleted Seen Draft],
+                       imap.responses("FLAGS").last)
+        end
       end
+      assert_empty stderr # never warn when type is given
     end
   end