ORM3 compatibility

[connorhu]

No description provided.

[derrabus]

Thank you for starting the work on this topic. I get the impression that you're trying to cover multiple topic at once. Try to keep your PR focused. For instance, dropping PHP 7 support or upgrading PHP CS Fixer should be separate PRs.

[beberlei]

@connorhu ah, i merged a PR that does some improvements to Lexer compatibility already causing a few conflicts here. Can you integrate it or is that problematic?

[connorhu]

@connorhu ah, i merged a PR that does some improvements to Lexer compatibility already causing a few conflicts here. Can you integrate it or is that problematic?

@beberlei Sure, I can integrate it, because I've done those conversions too. There are still some open questions around code simplification, whether it makes sense or not, whether I should do it or not. I can make a table showing how much repetitive logic is in the code.

[derrabus]

Please revert the changes you've made to the CI. We don't need them.

[derrabus]

Whenever you have time:

• Please make PHPCS happy.
• Please revert everything in the .github directory.
• Please answer the remaining open question.

[connorhu]

To answer your question I have to figure out whats going on here with the carbon.

The types for Carbon were extend from DateType, and some versions did not run tests when combined. This could also mean that 1) some versions didn't work at all, just weren't tested, 2) it would be better to extend the Type class instead 3) the testing is too permissive/incorrect.

[connorhu]

Current implementation is not compatible with DBAL4.
When convertToDatabaseValue method of TimeType, DateType etc classes with DBAL<4 converts the value, it uses DateTimeInterface to check the type.
With DBAL4 it checks for DateTime.
DBA3:
https://github.com/doctrine/dbal/blob/de57e23f00c513263df93b9b10bbbb421bc9239f/src/Types/DateType.php#L49
DBAL4:
https://github.com/doctrine/dbal/blob/1a307d92f8531b0897176e95040051c7414d80a5/src/Types/DateType.php#L38

The problem here is that CarbonImmutable* types do not extend immutable DBAL types. If I fix this, then 4 tests still won't run because they try to save an Immutable type in an non Immutable field:

DoctrineExtensions/tests/Types/CarbonDateTest.php

Line 169 in ffccbed

$entity->date = CarbonImmutable::createFromDate(2015, 1, 1);

If we want these 4 tests to run, we need our own convertToDatabaseValue implementation.

[connorhu]

PR rebased, squasd into one commit. Now we have to wait for release ORM 2.19.0.

[derrabus]

Yes. Thank you very much!

[connorhu]

Thank you for helping the process.

[abozhinov]

When to expect the new release?

[derrabus]

When it's ready.

[connorhu]

@derrabus I was thinking. It is a legitimate wish to use the code with orm3. Which is ready anyway. What if you make an orm3 branch, apply the contents of this PR as a patch, rewrite the requirement for orm3 only and put a note in the install description that until 2.19 is released, the ORM3 compatible code is there? If 2.19 is released, we will merge this PR, drop the orm3 branch and leave the master only.

[derrabus]

I mean, we can merge the branch as is. It's ready anyway and Composer will take care that this package is not installed with ORM 2.18.

[derrabus]

Ah well, we should've done a rebase before, to have PHPStan analyze your changes. I've pushed b6349f3 in order to make the CI green.

[Chris53897]

Thanks. But we should not forget to remove the @dev after the stable release of doctrine/orm 2.19.
https://github.com/beberlei/DoctrineExtensions/blob/master/composer.json#L13

[derrabus]

Sure. Not that it has any impact downstream, but we will remove it.

[derrabus]

https://github.com/beberlei/DoctrineExtensions/releases/tag/v1.5.0