ruby / net-imap

Ruby client api for Internet Message Access Protocol
https://ruby.github.io/net-imap
Other
62 stars 29 forks source link

✨ Add Data polyfill for ruby 3.1 #352

Open nevans opened 6 days ago

nevans commented 6 days ago

For new data structs, I'd prefer frozen by default and I don't want to support the entire Struct API. Data is almost exactly what I want, but it's not available until ruby 3.2.

So this adds a DataLite class that closely matches ruby 3.2's Data class, and it can be a drop-in replacement for Data. Net::IMAP::Data is an alias for Net::IMAP::DataLite, so when we remove our implementation, the constant will resolve to ruby's ::Data.

Ideally, we wouldn't define this on newer ruby versions at all, but that breaks the YAML serialization for our test fixtures.

I originally implemented this by hand, with my own tests and partially by looking at ruby's struct.c. But I later copied all of the tests (and some of the implementation) from the polyfill-data gem. I've updated the tests so that they use Data as it is resolved inside the Net::IMAP namespace. Copyright notices have been added to the appropriate files to satisfy the MIT license terms.


This is a prerequisite for the following PRs (as currently implemented):

nevans commented 6 days ago

@shugo what do you think about adding this, temporarily. We'd remove it with v0.6.0, when we raise the minimum required ruby to 3.2.

I actually started with PORO classes, but then I needed to implement the same #==, #eql?, #hash, #deconstruct, #deconstruct_keys multiple times.. at which point I wound up with this!

Alternatively, I could just use Struct as we've been doing. But I'd rather not need to support Struct's larger API surface.

nevans commented 6 days ago

@saturnflyer I'm tagging you too, just so you know that I've copied your tests with very little modification. I'd already started down the path of my own implementation and I kept some of that, but I copied a bunch of your implementation too. Honestly, I should probably amend the commit to add you as Co-authored-by. 🙂