Fix psalm errors: remove override of template type #11385
Closed
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
Instead of ensuring every mapping array has a mappedBy and an inversedBy field, let us do the opposite, and remove them when they are null. Likewise if there is a joinColumns field, it is useless if null or empty.
- Each type is now either final, abstract or an interface. - The mappedBy attribute is no longer nullable and moved down the hierarchy. - The inversedBy attribute is still nullable and also moved down the hierarchy. - Code common to ManyToOneAssociationMapping and OneToOneOwningSideMapping is de-duplicated and moved up the hierarchy - Code inside ToManyInverseSideMapping and ToManyOwningSideMapping comes from a trait to avoid duplication.
Make sure unneeded fields are not present
Rework association mapping hierarchy
Interfaces cannot have properties, and we do not have a concept of sealed classes available to us without installing third party packages. Interfaces can have methods however, which allows us to simplify calling code. I've been avoiding introducing getters for mapping properties because I do not know what the performance implications are, but here, I think it is sensible to make an exception, given the benefits.
Throughout the codebase, there is this pattern where we ensure we have the owning side of an association. It involves accessing it from the associationMappings array. In the end, static analysis cannot know that the association is indeed owning. By introducing this convenience method, we make this clear, and also delegate the complexity to the class metadata factory.
Introduce method to get to the owning side
It allows to remove many assertions.
Add templating for getOwningSide
It has only meaning for ToOneOwningSideMapping
…dnames-down Move joinColumnFieldNames down the class hierarchy
The keys in the join columns array have no meaning, and are just sequential integers.
These methods assert the type of the mapping provided by the collection according to the name of the class they are in: the one to many persister only ever deals with one to many associations, and the many to many persister only ever deals with many to many associations.
…ingclass fix wrong phpdoc
…ping-type Narrow down ClassMetadata::associationMappings type
Introduce convenience methods to narrow types
Maybe we do not know enough about the parameter to determine the type of the returned relationship, but we can at least narrow it down to 3 possibilites.
Not sure how I forgot these.
…e-qs Narrow down parameter types for quote strategies
* 3.0.x: Use enum_exists() for enums
* 2.18.x: Psalm 5.22.2 (doctrine#11326)
* 3.0.x: Psalm 5.22.2 (doctrine#11326)
* 2.19.x:
* Use class from persistence package It is meant to remove duplication between the ORM and the ODM. * Update UPGRADE.md Co-authored-by: Steve Todd <stodd@mashbo.com> --------- Co-authored-by: Alexander M. Turek <me@derrabus.de> Co-authored-by: Steve Todd <stodd@mashbo.com>
* 2.19.x: Refator array_map into simple loop for performance. (doctrine#11332)
* 2.18.x: Fix annotation Bump CI workflows (doctrine#11336) Fix SchemaTool::getSchemaFromMetadata() uniqueConstraint without a predefined name (doctrine#11314)
* 3.0.x: Fix annotation Bump CI workflows (doctrine#11336) Fix SchemaTool::getSchemaFromMetadata() uniqueConstraint without a predefined name (doctrine#11314)
* 2.19.x:
* 2.19.x: Prepare releases 2.19 and 3.1 (doctrine#11335)
Merge 2.19.x up into 3.1.x
It is deprecated.
Avoid array access
Avoid another occurrence of ArrayAccess
Merge 2.19.x up into 3.1.x
|
Psalm and PHPCS are failing. |
|
Updated to address these issues (good that there is a strict CI watching my rookie commits…). The remaining failing check seems unrelated. |
|
Restarted the job, it now passes. |
|
Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble. How to do that?
|
See doctrine/collections#368 for the same issue in doctrine/collections which has been fixed there. The issue happened when using ->contains(). Running psalm emitted > InvalidArgument - Argument 1 of Doctrine\ORM\PersistentCollection::contains > expects > TMaybeContained:fn-doctrine\common\collections\readablecollection::contains > as mixed, but … provided. We should either not define @template TMaybeContained or re-define the psalm docblock from ReadableCollection completely. Repairing the docblock necessitates an update to the psalm baseline: one "known issue" is no longer an issue and could thus be removed.
|
Sorry. I started from the wrong branch. Failed to see #11208 How to choose the right branch. |
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.
See doctrine/collections#368 for the same issue in doctrine/collections which has been fixed there.
The issue happens when using
->contains(). Running psalm emitsWe should either not define
@template TMaybeContainedor re-define the psalm docblock fromReadableCollectioncompletely.