-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[PropertyInfo] Bug/BC break? - Getter return type seems to have higher priority than property type #38416
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
Comments
Because reflection was moved down into the list of guessers all others take precedence not only the getter |
Anything that can be done here? Was the breaking change intentional? Or could we revert the behaviour? |
Any news on this one? I'm also having the same problem. Was this change intentional? |
I guess this is related to different expectations. The constructor argument is nullable, but the getter is not. Having one type info used for both is what causes the issue. |
@stof But would you agree, that the behaviour should be somehow stable/predictable and not change between minor releases? Looks like there are no tests covering this, otherwise they would have failed when changing it in #38041 Regarding expectations - as this component is called |
I stumbled upon something similar that is also related to this issue: The PhpDocExtractor has a higher priority than the ReflectionExtractor (at least in the default symfony distribution) and when using the example code from the documentation. To clarify this, consider this example (based on the example from @rogamoore):
This will now return the nullable string type. If you remove the docblock, it's a non-nullable string again. |
Another situation where this seems to be a problem. Let's say you have a DTO like the following, that to me seems to be a perfect normal use case. final class Product
{
private string $currency;
public function __construct(int $amount, string $currency)
{
$this->currency = $currency;
}
public function getCurrency(): Currency
{
return Currency::fromString($this->currency);
}
}
final class Currency
{
private string $currency;
private function __construct(string $currency)
{
$this->currency = $currency;
}
public static function fromString(string $currency): self
{
return new self($currency);
}
} With this last change to the property accessor, if you try to deserialize something into this class there will be an error because it tries to create a |
I think I may have found a workaround for this I discovered this PR #30335 (available for Symfony 5.2) and although there's no documentation yet for it this could be used to give priority to extract the types from constructor, internally this new
|
Hey, thanks for your report! |
Friendly reminder that this issue exists. If I don't hear anything I'll close this. |
We have worked around this problem, but I think the issue in general is still valid. |
Hey, thanks for your report! |
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
Hey, I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen! |
In Symfony 6.3 this weird behaviour seems to keep happening. Is this something that is intended to be fixed or should we rather use a workaround? |
Symfony version(s) affected: 5.1.6, 5.1.7
Description
Hello!
In one of our projects we have a class similar to this:
It's used in the context of api platform, where incoming data is being deserialized to this object.
Before with Symfony 5.1.5 the
example
property was detected as type?string
, after upgrading to 5.1.6/5.1.7 it's only detected as typestring
. Now when the incoming data contains a null value, it will fail withThe type of the "example" attribute must be "string", "NULL" given.
I did some tests and was able to track this new behaviour down to the property-info component. I suspect this change to be the reason:
#38041
It looks like now the return type of
getExample()
is used instead of the property type hint (?string
).So I would like to ask if this is the expected behaviour and if yes, how can I make the old way work again (somehow influence the priorities?). Thanks in advance!
How to reproduce
I created a reproducer here:
https://github.com/rogamoore/property-info-bug
Steps:
composer install
symfony server:start
http://127.0.0.1:8000/
composer install
symfony server:start
http://127.0.0.1:8000/
Additional context
PHP 7.4.10
The text was updated successfully, but these errors were encountered: