Add type declarations where backwards-compatible #8544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonberger
reviewed
Mar 16, 2021
greg0ire
force-pushed
the
bc-type-declarations
branch
from
d44eb39
to
caf8c70
Compare
|
Thanks for the careful review @simonberger , much appreciated 👍 |
simonberger
previously approved these changes
Mar 16, 2021
BackEndTea
reviewed
Mar 19, 2021
greg0ire
force-pushed
the
bc-type-declarations
branch
from
caf8c70
to
bcce2a0
Compare
ostrolucky
requested changes
Apr 8, 2021
lib/Doctrine/ORM/Cache/Persister/Entity/NonStrictReadWriteCachedEntityPersister.php
Outdated
Show resolved
Hide resolved
| { | ||
| $namespace = $this->getClassNamespace($fullClassName); | ||
|
|
||
| return $namespace ? 'namespace ' . $namespace . ';' : ''; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $fullClassName |
greg0ire
force-pushed
the
bc-type-declarations
branch
5 times, most recently
from
5d090db
to
507a115
Compare
ostrolucky
reviewed
Apr 13, 2021
There was a problem hiding this comment.
One comment remaining. Also, you are adding object type declarations, but these are being removed in #8613. That will cause mess of a conflict.
greg0ire
commented
Apr 14, 2021
| use Doctrine\DBAL\LockMode; | ||
| use Doctrine\DBAL\Platforms\AbstractPlatform; | ||
| use Doctrine\DBAL\Statement; |
There was a problem hiding this comment.
Unsure if we should wait for doctrine/dbal#4596 and revert this change in use statements. I think that change might be good because more forward compatible.
There was a problem hiding this comment.
I'll leave you decide. But to me, such change seems a bit unrelated to this PR
|
I will wait until #8613 is resolved |
greg0ire
force-pushed
the
bc-type-declarations
branch
4 times, most recently
from
105670f
to
6afe86a
Compare
ostrolucky
previously approved these changes
Apr 19, 2021
|
The 7.1 PR has been merged, let's rebase to be 100% sure it still works |
greg0ire
force-pushed
the
bc-type-declarations
branch
from
6afe86a
to
e1827c4
Compare
greg0ire
force-pushed
the
bc-type-declarations
branch
from
e1827c4
to
32e3a5a
Compare
This includes: - private methods - return type declarations of final protected methods - return type declarations of public and protected methods of final classes Parameter type declarations are a more delicate matter and should probably be handled separately to make it easier to catch issues during code review. Type declarations can be more trusted than simple phpdoc when it comes to static analysis, having them means we can infer the phpdoc of calling methods with confidence. Note that it seems that some of the phpdoc I initially inferred these declarations from were apparently wrong, in particular some mentioning Doctrine\Dbal\Statement when was is really passed around is Doctrine\Dbal\Driver\Statement.
greg0ire
force-pushed
the
bc-type-declarations
branch
from
32e3a5a
to
55f030f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes:
Parameter type declarations are a more delicate matter and should
probably be handled separately to make it easier to catch issues during
code review.
Type declarations can be more trusted than simple phpdoc when it
comes to static analysis, having them means we can infer the phpdoc of
calling methods with confidence.
Note that it seems that some of the phpdoc I initially inferred these
declarations from were apparently wrong, in particular some mentioning
Doctrine\Dbal\Statementwhen was is really passed around isDoctrine\Dbal\Driver\Statement.Closes #8538