Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can I patch DateTime? #146

Open
aruku opened this issue Dec 1, 2023 · 10 comments
Open

Can I patch DateTime? #146

aruku opened this issue Dec 1, 2023 · 10 comments
Assignees
Labels

Comments

@aruku
Copy link

aruku commented Dec 1, 2023

I'm trying to mock DateTime to capture some output, but I get this error:

Fatal error: Uncaught ReflectionException: Function \DateTime::__construct() does not exist in /workspace/vendor/antecedent/patchwork/src/CallRerouting.php:430
Stack trace:
#0 /workspace/vendor/antecedent/patchwork/src/CallRerouting.php(430): ReflectionFunction->__construct('\\DateTime::__co...')
#1 /workspace/vendor/antecedent/patchwork/Patchwork.php(138): Patchwork\CallRerouting\createStubsForInternals()
#2 /workspace/vendor/composer/autoload_real.php(41): require('/workspace/vend...')
#3 /workspace/vendor/composer/autoload_real.php(45): {closure}('2f3136d1da93fd8...', '/workspace/vend...')
#4 /workspace/vendor/autoload.php(25): ComposerAutoloaderInitd8810229a0ec6a5cd1df17dc9a81c6a4::getLoader()
#5 /workspace/vendor/phpunit/phpunit/phpunit(105): require('/workspace/vend...')
#6 {main}
  thrown in /workspace/vendor/antecedent/patchwork/src/CallRerouting.php on line 430

I do it like this:
redefine('DateTime::__construct', always(new \DateTime('18:10 30-11-2023')));

And this my patchwork.json:

{
    "redefinable-internals": [
        "\\DateTime::__construct"
    ]
}

Should this work or am I trying something impossible?

Also, I installed the library via Composer but I still had to modify composer.json so it is autoloaded:

    "autoload-dev": {
        "files": [
            "vendor/antecedent/patchwork/Patchwork.php"
        ]
}

I'm using version 2.1.26. Thank you in advance!

@antecedent
Copy link
Owner

Thank you for filing the issue!

Strangely enough, the feature in question has gone undocumented up to this day. I will get to that as soon as I can.

Please put the following in your patchwork.json:

{
    "new-keyword-redefinable": true
}

Once this is in check, redefine as follows:

redefine('new DateTime', always(new \DateTime('18:10 30-11-2023')));

@aruku
Copy link
Author

aruku commented Dec 1, 2023

Thank you!

Any ideas about the autoloading problem?

@aruku
Copy link
Author

aruku commented Dec 1, 2023

I'm getting another error... Did you mean this should work as you indicated and you only need to update the docs, or is there a patch you need to submit?

After the changes you suggested, I get this error:

PHP Fatal error:  During class fetch: Uncaught ParseError: syntax error, unexpected '\' (T_NS_SEPARATOR) in /workspace/vendor/symfony/framework-bundle/Kernel/MicroKernelTrait.php:188
Stack trace:
#0 [internal function]: Symfony\Component\ErrorHandler\DebugClassLoader->loadClass('Symfony\\Bundle\\...')
#1 /workspace/src/Kernel.php(12): spl_autoload_call('Symfony\\Bundle\\...')
#2 /workspace/vendor/symfony/error-handler/DebugClassLoader.php(285): include('/workspace/src/...')
#3 [internal function]: Symfony\Component\ErrorHandler\DebugClassLoader->loadClass('App\\Kernel')
#4 [internal function]: spl_autoload_call('App\\Kernel')
#5 /workspace/vendor/symfony/framework-bundle/Test/KernelTestCase.php(65): class_exists('App\\Kernel')
#6 /workspace/vendor/symfony/framework-bundle/Test/KernelTestCase.php(126): Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::getKernelClass()
#7 /workspace/vendor/symfony/framework-bundle/Test/KernelTestCase.php(81): Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::createKernel(Array)
#8 /workspace in /workspace/src/Kernel.php on line 12

Do I need to include anything other than "new-keyword-redefinable": true on the patchwork.json file?

@antecedent
Copy link
Owner

This was a bug after all. Please update to 2.1.27 and retry.

@antecedent antecedent self-assigned this Dec 3, 2023
@antecedent antecedent added the Bug label Dec 3, 2023
@aruku
Copy link
Author

aruku commented Dec 4, 2023

Thank you for your effort. Unfortunately, now I get two different errors. One of them only seemed to happen two or three times after trying to run the library in my test without dumping the autoloader.
Now I get this one, even when not calling any method of the library:

PDO::setAttribute(): SQLSTATE[HY000]: General error: user-supplied statement class cannot have a public constructor
 /workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:41
 /workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOMySql/Driver.php:25
 /workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:412
 /workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1952
 /workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1290
 /workspace/vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php:747
 /workspace/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php:219
 /workspace/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php:346
 /workspace/vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php:254
 /workspace/tests/WebTestCaseWithUser.php:22
 /workspace/tests/Characterization/SimpleGetRequestTest.php:31

This happens as long as I don't remove the entry in the files array in the Composer file and dump the autoloader, which tells me there is something going on with the autoload mechanism or the initialization of the library.
I'm running Symfony 5.4 on PHP 7.4.
Let me know if there is anything else I can do to help.

@antecedent
Copy link
Owner

Thank you for trying out the patch. It does seem that we are on the right track.

When "new-keyword-redefinable": true is set, Patchwork also overrides the visibility of all (userland) constructors, making them public. It seems that we need to exclude Doctrine's PDOStatement class from this procedure, because, for some reason, PDO disallows statement classes that have public constructors.

Please try adding "blacklist": ["/workspace/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOStatement.php"] to your patchwork.json. The path can also be made relative to patchwork.json's location.

@aruku
Copy link
Author

aruku commented Dec 4, 2023

This is the error now:

Parameter 3 to Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator::__construct() expected to be a reference, value given
 /workspace/vendor/symfony/framework-bundle/Kernel/MicroKernelTrait.php:188
 /workspace/vendor/symfony/dependency-injection/Loader/ClosureLoader.php:39
 /workspace/vendor/symfony/config/Loader/DelegatingLoader.php:40
 /workspace/vendor/symfony/framework-bundle/Kernel/MicroKernelTrait.php:196
 /workspace/vendor/symfony/http-kernel/Kernel.php:649
 /workspace/vendor/symfony/http-kernel/Kernel.php:545
 /workspace/vendor/symfony/http-kernel/Kernel.php:787
 /workspace/vendor/symfony/http-kernel/Kernel.php:128
 /workspace/vendor/symfony/framework-bundle/Test/KernelTestCase.php:82
 /workspace/vendor/symfony/framework-bundle/Test/WebTestCase.php:46
 /workspace/tests/Characterization/SimpleGetRequestTest.php:25

I think this is also the other error I saw earlier but just a few times.

@antecedent
Copy link
Owner

For the time being, could you try blacklisting /workspace/vendor/symfony/framework-bundle/Kernel/MicroKernelTrait.php in an analogous way? To suppress the particular instance of this problem and to see if anything else crops up.

The error that you report reveals an important shortcoming of the constructor redefinition component in all current versions of Patchwork. Thank you for bringing it to light. Unfortunately, this also means that it will take a little bit longer for me to arrive at a satisfactory fix.

@aruku
Copy link
Author

aruku commented Dec 5, 2023

I also added "/workspace/vendor/symfony/dependency-injection/Loader/PhpFileLoader.php" and it finally stop failing; unfortunately, it doesn't seem to be doing what I was expecting (return the same fixed date for new DateTime), so I'm gonna stop trying to incorporate in our project for now.

I'll be happy to provide you with more context if this is something you want to look into.
Thank you for all your work!

@withinboredom
Copy link

@antecedent I'm curious if you ever managed to work this out? I'm working a project (Durable PHP), and it's really easy to forget to NOT use new DateTime (immutable or not) sometimes (due to idempotency issues). I was looking into the library to see about replacing that call to make it easier to read/write idempotent code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants