Fix filemtime() warning because of eval'd code #494
Conversation
I appreciate your contribution and I believe that your bugfix is legit. But if you allow me, a little advice for the future: If you want people you don't have any SLAs with to help you in their freetime, please avoid the term "ASAP" in your request. |
| @@ -229,7 +230,7 @@ private function getLastModification(ReflectionClass $class): int | |||
| $parent = $class->getParentClass(); | |||
|
|
|||
| $lastModification = max(array_merge( | |||
| [$filename ? filemtime($filename) : 0], | |||
| [is_file($filename) ? filemtime($filename) : 0], | |||
There was a problem hiding this comment.
$filename is whatever ReflectionClass::getFileName() returns and that could technically be false. I don't want to pass false to is_file().
| [is_file($filename) ? filemtime($filename) : 0], | |
| [$filename !== false && is_file($filename) ? filemtime($filename) : 0], |
That being said, what actually does $filename contain in case of a class that comes from eval'ed code? 🤔
There was a problem hiding this comment.
- Thank you for fast answer and support!
- You are absolutely right, I added check
$filename !== false. -
That being said, what actually does $filename contain in case of a class that comes from eval'ed code?
The filename contains something like (see Issue #432): /home/.../my-project/vendor/lexik/jwt-authentication-bundle/Security/Authenticator/ForwardCompatAuthenticatorTrait.php(14) : eval()'d code.
In PHPUnit test case filename contains /.../tests/Doctrine/Tests/Common/Annotations/PsrCachedReaderTest.php(236) : eval()'d code.
| @@ -220,6 +220,28 @@ public function testReaderIsNotHitIfCacheIsFresh(): void | |||
| ); | |||
| } | |||
|
|
|||
| public function testReaderDoesNotCacheIfFileDoesNotExistSoLastModificationCannotBeDetermined(): void | |||
There was a problem hiding this comment.
Nice test. Can you please repeat this test for the CachedReader as well?
|
I think, you need to pick a different class names and/or namespaces in the two eval'ed snippets. |
I set a different class names in the two eval'ed snippets. |
|
Your new test is failing on PHP 7.1 now. |
|
Let me take this over. Nobody should have to debug stuff on PHP 7.1 anymore these days. 🙈 |
|
Thank you very much! |
* 1.14.x: Fix filemtime() warning because of eval'd code (doctrine#494) Test with Symfony Cache 7 (doctrine#499)
Description
This small PR fix the old problem "
filemtime()warning on eval'd code" described in issue #492 and in PR #436.I faced this bug in my current big project when upgrading PHP from 8.1 to 8.2. But I can't migrate away from doctrine/annotations and use attributes just now. So, please, check and accept this PR ASAP. Thank you!