From a834715a2f0596501390443774beca8a9fb9a98d Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 28 Sep 2026 10:14:56 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20`SequenceSet#slice(cardina?= =?UTF-8?q?lity..any)`=20->=20empty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes a `SequenceSet#slice` to return an empty set (rather than `nil`) in the same circumstance. This mimics `Array#slice`, which returns an empty array when starting at the array's size (immediately after the last index). Note that `SequenceSet#slice` indexes the monotonic set numbers, so `#cardinality` is used instead of `#size` (which measures the ordered list data). This could be seen as a minor breaking change. But I'm treating it as a bugfix because: * `SequenceSet#slice` was always intended to mimic `Array#slice`. * The documented API doesn't change, it still returns `nil` or a set. * `SequenceSet#slice` already returns an empty set sometimes. So, I think it's unlikely there is much code that depends on this bug. And any code that _does_ depend on it is (arguably) buggy already, and should be easy to fix. Please note that this is done as a minimal change to avoid refactoring the underlying slice implementation. Since `#cardinality` isn't cached, this currently requires scanning every set element a second time, which is _very_ inefficient for large sets. Fixing `SequenceSet#slice` performance is what led to finding and fixing this bug in the first place. But, for now I'm prioritizing fixing and testing `#slice`'s behavior, even if that leads to some reduced performance in cases like this. --- lib/net/imap/sequence_set.rb | 2 ++ test/net/imap/test_sequence_set.rb | 15 ++++----------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 62db9905c..453e51e91 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -2438,6 +2438,8 @@ def slice_range(range) if min <= max then intersection export_minmax_entry [min, max] else remain_frozen_empty end + elsif first.positive? + remain_frozen_empty if valid_slice_start?(first) end end diff --git a/test/net/imap/test_sequence_set.rb b/test/net/imap/test_sequence_set.rb index 1b95ef8f7..b7955c00c 100644 --- a/test/net/imap/test_sequence_set.rb +++ b/test/net/imap/test_sequence_set.rb @@ -489,13 +489,6 @@ def obj.to_sequence_set; 192_168.001_255 end def pend_slice_bug(what, &) = pend("#slice bug: #{what}", &) - def pend_slice_from_cardinality(actual) - pend_slice_bug "return empty for start == cardinality" do - assert_same SequenceSet.equal, actual - end - assert_nil actual - end - def pend_slice_neg_len(&) pend_slice_bug("allow negative length", &) assert_raise_with_message(ArgumentError, @@ -608,7 +601,7 @@ def pend_slice_zero_len(&) assert_same SequenceSet.empty, set[100.. -101] # i.e: 100.. 99 assert_same SequenceSet.empty, set[199... -1] # i.e: 199...199 assert_same SequenceSet.empty, set[199.. -2] # i.e: 199...198 - pend_slice_from_cardinality set[200.. -1] # i.e: 200.. 199 + assert_same SequenceSet.empty, set[200.. -1] # i.e: 200.. 199 end test "#slice(range) -> empty, for start == cardinality" do @@ -621,9 +614,9 @@ def pend_slice_zero_len(&) assert_equal SequenceSet[ 100], set[-1, 4] # when positive start == cardinality pend_slice_zero_len do assert_equal SequenceSet.empty, set[10, 0] end - pend_slice_from_cardinality set[10, 4] + assert_equal SequenceSet.empty, set[10, 4] assert_equal SequenceSet.empty, set[10...10] - pend_slice_from_cardinality set[10.. 10] + assert_equal SequenceSet.empty, set[10.. 10] assert_equal SequenceSet.empty, set[10.. 9] end @@ -1237,7 +1230,7 @@ def pend_slice_zero_len(&) assert_equal SequenceSet[9..10, 20], set.slice!(3..) assert_equal SequenceSet[5, 7..8], set assert_nil set.slice!(3) - pend_slice_from_cardinality set.slice!(3..) + assert_equal SequenceSet.empty, set.slice!(3..) end test "#delete_at" do