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

Flip conditional extension of legacy AnnotationDriver class #9843

Merged
merged 1 commit into from
Jun 13, 2022

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Jun 13, 2022

This PR flips the if/else block of the conditional CompatibilityAnnotationDriver class declaration. I do this to work around a PHPStan bug (see phpstan/phpstan#7290).

PHPStan 1.7 basically ignores the if block and picks the first of the two declarations. For projects that have upgraded to Persistence 3 already, PHPStan will complain about code like this:

$config->setMetadataDriverImpl(new AnnotationDriver());

My idea is that the implementation that implements the interface directly is the more correct one. Worst-case scenario with my patch is that PHPStan does not know about the inheritance of the two annotation classes when Persistance 2 is still installed. From my PoV, that's the lesser evil.

cc @VincentLanglet

@derrabus derrabus added this to the 2.12.3 milestone Jun 13, 2022
@derrabus derrabus requested a review from greg0ire June 13, 2022 14:08
@derrabus derrabus merged commit eff540a into doctrine:2.12.x Jun 13, 2022
@derrabus derrabus deleted the bugfix/flip-conditional-extend branch June 13, 2022 18:10
derrabus added a commit to derrabus/orm that referenced this pull request Jun 17, 2022
* 2.13.x:
  Deprecate omitting the alias in QueryBuilder (doctrine#9765)
  Run tests on PHP 8.2 (doctrine#9840)
  PHPStan 1.7.13 (doctrine#9844)
  Flip conditional extension of legacy AnnotationDriver class (doctrine#9843)
  PHP CodeSniffer 3.7 (doctrine#9842)
  Make Reflection available to ConvertMappingCommand (doctrine#9619)
  Add missing property declaration
  Use proper API for introspection of tables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants