Uh oh!
There was an error while loading. Please reload this page.
IMAP: Resource object constructor and stub - #6534
Conversation
Girgias
left a comment
There was a problem hiding this comment.
Thank you for this :D
Could you add a separate test checking that one cannot extend the class?
There was a problem hiding this comment.
Just a nitpick but this test doesn't need to check for the presence of a mailbox, so could you change this to
| <?php | |
| require_once__DIR__ . '/setup/skipif.inc'; | |
| <?php | |
| extension_loaded('imap') ordie('skip imap extension not available in this build'); |
There was a problem hiding this comment.
Thank you, I just force-pushed an updated commit with this test simplified like you suggested.
Disallows constructing an `IMAPConnection` class directly with `new IMAPConnection` construct, by throwing an `Error` exception if attempted. `imap_open` is still the only way to create `IMAPConnection` objects.
Updates the `IMAPConnection` class stub to make sure it has the `final` flag, and adds a test to verify it.
Ayesh
commented
Dec 23, 2020
Thanks a lot for the review @Girgias. I update both commits and force-pushed with updated test for constructor check, and a new test for a Thank you. |
Girgias
left a comment
There was a problem hiding this comment.
LGTM, will wait for CI to clear before merging. :)
Girgias
commented
Dec 23, 2020
Ayesh
commented
Dec 24, 2020
Thanks a lot @Girgias 🙏🏼 |
This is similar to #6533 (
FTPConnection), but forIMAPConnectionresource class.As of now, direct construction of
IMAPConnectionclass is allowed, and I think it was meant to be disallowed just like other new resource objects. In this PR:new IMAPConnectionconstructor with\ErrorexceptionCannot directly construct IMAPConnection, use imap_open() instead.IMAPConnectionwithfinalflag.Thank you.