From 0bb92a4ef5817e4dee496d2dc9c06ea5488be365 Mon Sep 17 00:00:00 2001
From: nick evans <nick@rubinick.dev>
Date: Thu, 26 Sep 2024 10:04:59 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=A5=85=20Improve=20SequenceSet=20frozen?=
 =?UTF-8?q?=20errors?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, SequenceSet modification methods would raise a frozen error
when it updated its internal data structure (currently implemented as an
array of arrays), which was "can't modify frozen Array: [int, int]".
This was unexpected, confusing, and misleading.

This changes the SequenceSet modification methods to raise the expected
FrozenError _prior_ to attempting to modify the internal data structure.
---
 lib/net/imap/sequence_set.rb       |  9 +++++++++
 test/net/imap/test_sequence_set.rb | 26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb
index 2fb8a942..011df314 100644
--- a/lib/net/imap/sequence_set.rb
+++ b/lib/net/imap/sequence_set.rb
@@ -682,6 +682,7 @@ def add(object)
       # Unlike #add, #merge, or #union, the new value is appended to #string.
       # This may result in a #string which has duplicates or is out-of-order.
       def append(object)
+        modifying!
         tuple = input_to_tuple object
         entry = tuple_to_str tuple
         tuple_add tuple
@@ -1317,6 +1318,12 @@ def intersect_tuple?((min, max))
           range.include?(min) || range.include?(max) || (min..max).cover?(range)
       end
 
+      def modifying!
+        if frozen?
+          raise FrozenError, "can't modify frozen #{self.class}: %p" % [self]
+        end
+      end
+
       def tuples_add(tuples)      tuples.each do tuple_add _1      end; self end
       def tuples_subtract(tuples) tuples.each do tuple_subtract _1 end; self end
 
@@ -1331,6 +1338,7 @@ def tuples_subtract(tuples) tuples.each do tuple_subtract _1 end; self end
       #   ---------??===lower==|--|==|----|===upper===|-- join until upper
       #   ---------??===lower==|--|==|--|=====upper===|-- join to upper
       def tuple_add(tuple)
+        modifying!
         min, max = tuple
         lower, lower_idx = tuple_gte_with_index(min - 1)
         if    lower.nil?              then tuples << tuple
@@ -1367,6 +1375,7 @@ def tuple_coalesce(lower, lower_idx, min, max)
       # -------??=====lower====|--|====|---|====upper====|-- 7. delete until
       # -------??=====lower====|--|====|--|=====upper====|-- 8. delete and trim
       def tuple_subtract(tuple)
+        modifying!
         min, max = tuple
         lower, idx = tuple_gte_with_index(min)
         if    lower.nil?        then nil # case 1.
diff --git a/test/net/imap/test_sequence_set.rb b/test/net/imap/test_sequence_set.rb
index 8b215311..929bb325 100644
--- a/test/net/imap/test_sequence_set.rb
+++ b/test/net/imap/test_sequence_set.rb
@@ -57,6 +57,32 @@ def compare_to_reference_set(nums, set, seqset)
     assert_equal set, set.freeze
   end
 
+  data "#clear",       :clear
+  data "#replace seq", ->{ _1.replace SequenceSet[1] }
+  data "#replace num", ->{ _1.replace   1 }
+  data "#replace str", ->{ _1.replace  ?1 }
+  data "#string=",     ->{ _1.string = ?1 }
+  data "#add",         ->{ _1.add       1 }
+  data "#add?",        ->{ _1.add?      1 }
+  data "#<<",          ->{ _1 <<        1 }
+  data "#append",      ->{ _1.append    1 }
+  data "#delete",      ->{ _1.delete    3 }
+  data "#delete?",     ->{ _1.delete?   3 }
+  data "#delete_at",   ->{ _1.delete_at 3 }
+  data "#slice!",      ->{ _1.slice!    1 }
+  data "#merge",       ->{ _1.merge     1 }
+  data "#subtract",    ->{ _1.subtract  1 }
+  data "#limit!",      ->{ _1.limit! max: 10 }
+  data "#complement!", :complement!
+  data "#normalize!",  :normalize!
+  test "frozen error message" do |modification|
+    set = SequenceSet["2:4,7:11,99,999"]
+    msg = "can't modify frozen Net::IMAP::SequenceSet: %p" % [set]
+    assert_raise_with_message FrozenError, msg do
+      modification.to_proc.(set)
+    end
+  end
+
   %i[clone dup].each do |method|
     test "##{method}" do
       orig = SequenceSet.new "2:4,7:11,99,999"