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

[SoftDeleteableFilter] Fix addFilterConstraint() return type is missing #2782

Closed
wants to merge 1 commit into from

Conversation

cavasinf
Copy link

@cavasinf cavasinf commented Mar 19, 2024

Resolve:

Fatal error: Declaration of Gedmo\SoftDeleteable\Filter\SoftDeleteableFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, $targetTableAlias) must be compatible with Doctrine\ORM\Query\Filter\SQLFilter::addFilterConstraint(Doctrine\ORM\Mapping\ClassMetadata $targetEntity, string $targetTableAlias): string in /var/www/html/vendor/gedmo/doctrine-extensions/src/SoftDeleteable/Filter/SoftDeleteableFilter.php on line 54

https://github.com/doctrine/orm/blob/55c4845d5734bea5579424c2c2a7c5b97dbb4c60/src/Query/Filter/SQLFilter.php#L173

As DoctrineExtensions supports php >= 7, but was denied in #2089 before.

@mbabker
Copy link
Contributor

mbabker commented Mar 19, 2024

It looks like you're trying to use this package with ORM 3.0 given the presence of the return type. This package itself isn't compatible with ORM 3.0 yet, and technically adding this return type (or one of the other dozen or so now mandatory param or return types I've identified working on ORM 3.0 compat) is a B/C break within this package. So this PR on its own can't just be merged and shipped.

@cavasinf
Copy link
Author

OK, I though ORM 3.0 wasn't compatible because of annotation vs attributes left (#2708).
Closing this PR then.

@cavasinf cavasinf closed this Mar 20, 2024
@cavasinf cavasinf deleted the patch-1 branch March 20, 2024 07:58
@mbabker
Copy link
Contributor

mbabker commented Mar 20, 2024

I though ORM 3.0 wasn't compatible because of annotation vs attributes left

That part should be fine as long as you use explicit config. The feature detection bits in this package prefer annotations over attributes for B/C, so when a reader is not explicitly injected and your app environment has doctrine/annotations installed (either as your own explicit app dependency or it's still being pulled in as a transient dependency through one of your other dependencies), the package will create a new Doctrine\Common\Annotations\AnnotationReader instance without any configuration, and that would trigger annotation parsing errors like this one. So to avoid that if your app is using attributes, you can inject the Gedmo\Mapping\Driver\AttributeReader in and avoid the feature detection trying to decide what to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants