Integration test suite failure seems unrelated

Good catch. We have the same problem with the$messageTemplate property in that class if I'm not mistaken. In both cases, I'd suggest to perform the string cast inside the constructor already. Either way, it looks like a mistake to me that we allownull to be passed to the constructor.

Either way, it looks like a mistake to me that we allownull to be passed to the constructor.

That ship has sailed though, and there's active design around the property path being missing.

That ship has sailed though, and there's active design around the property path being missing.

Yeah, for sure, we cannot just change the type now. But if it's a mistake, we could try to correct it for Symfony 6. The parameter types for$propertyPath and$messageTemplate have been added with#24722 despite the PHPDoc block documenting those parameters as non-nullable.

Our own code (ExecutionContext::addViolation() andConstraintViolationBuilder::addViolation()) should make sure that we don't passnull to either of them. May I ask how you got yourself aConstraintViolation instance with anull property path? Maybe I missed something.

Our own code (ExecutionContext::addViolation() andConstraintViolationBuilder::addViolation()) should make sure that we don't passnull to either of them. May I ask how you got yourself aConstraintViolation instance with anull property path? Maybe I missed something.

I was copying violations reported from one validator to another, pretty much likeConstraintViolationBuilderInterface#atPath(ConstraintViolationInterface#getPropertyPath())

Thank you. Could you do the same fix for the$messageTemplate please? The same bug is forgetMessageTemplate() too.

Will send a separate patch

I still think we should perform the string cast inside the constructor though. 🙃

btw: is the 4.4 branch affected as well?

Hm, the small benefit to cast it to a string in the constructor is that we can remove the cast in the__toString() method.

I still think we should perform the string cast inside the constructor though. upside_down_face

You do that, you break subclassing types for little benefit.

I can imagine that some subclassing will likely rely on the fact that$this->foo !== null to take internal decisions.

EDIT: keep in mind that the scope of this is abugfix, not a refactoring. That would probably target5.x instead.

btw: is the 4.4 branch affected as well?

Possibly, didn't check, since I'd rather leave 4.4 alone. This is the sort of bugfix that I would never ever want to see in an LTS, since there's a subtle change in behavior.

I still think we should perform the string cast inside the constructor though. upside_down_face

You do that, you break subclassing types for little benefit.

$propertyPath is declared private, so a subclass would need access it via the very same getter that you are patching right now.

Ah, true that 👍

Thank you@Ocramius.