ext/imap: Refactor + Update to modern property write API

[Girgias]

Also use common functions for creating and setting up similar objects.

Split out from #11387 without using the newly introduced APIs as this is a major refactoring.

[nielsdos]

I might've missed some things, I can do another pass when I'm more rested and the comments have been addressed.

I really like the deduplication, and renaming of things. Not only does it help readability and maintainability, it's also less error-prone.
One thing that bothers me a little bit is how verbose the code has become. There's two repetitions: Z_OBJCE_P() and Z_OBJ_P on the same object; and the "fieldname" and strlen("fieldname") is also repetitive. It makes the code longer than it needs to be, and that makes it harder to follow because you have to pattern-match on more lines.
I see you want to get rid of the old add_property_* API, but I'm not sure if that's worth it tbh...
A solution to the verboseness is defining a helper macro to php_imap.c only to reduce the repetition, but then you're just doing the work of add_property_* all over again...

[nielsdos]

I wonder what the historic reason behind these duplicates are...
Anyway, I think you could technically avoid duplicate allocations by creating a zend_string yourself here for en->date and share those instances between the two properties. But I'm not sure if that's really worth it complexity-performance-wise.

Same for "subject" & "Subject" below.

[Girgias]

I don't really know the reason either, probably because the casing was already inconsistent. But this just makes it weirder as sometimes it has the first character uppercased, otherwise no.

Ideally we would have named classes with fixed properties. But I can't really work on ext/imap any more as Fedora has dropped c-client from the libs due to how old and unmaintained it is.

Maybe we should just drop ext/imap....

[Girgias]

One thing that bothers me a little bit is how verbose the code has become. There's two repetitions: Z_OBJCE_P() and Z_OBJ_P on the same object; and the "fieldname" and strlen("fieldname") is also repetitive. It makes the code longer than it needs to be, and that makes it harder to follow because you have to pattern-match on more lines. I see you want to get rid of the old add_property_* API, but I'm not sure if that's worth it tbh... A solution to the verboseness is defining a helper macro to php_imap.c only to reduce the repetition, but then you're just doing the work of add_property_* all over again...

I wasn't planning on removing the "old" add_property_* but the issue around the _str() variant has me worried, also the naming is suboptimal. But I do get the verbosity concern, it just seems like people are not really aware of the "old" API as it looks more like using an stdObject like an array (which is also where the API was located with the array APIs).

But yeah, it seems to be split between using the "old" API, the zend_update_properties and the one where someone just modifies the property directly by knowing the offset in the table. This aspect is a bit of a mess...

[nielsdos]

I wasn't planning on removing the "old" add_property_* but the issue around the _str() variant has me worried, also the naming is suboptimal. But I do get the verbosity concern, it just seems like people are not really aware of the "old" API as it looks more like using an stdObject like an array (which is also where the API was located with the array APIs).

I see. Not sure what to do about the verbosity in this patch, ideally we could avoid it somehow. In any case, stylistic verbosity issue aside, the changes look correct to me.