[Validator] Throw ValidatorException when validating non-existent property #62238
Conversation
This fixes issue symfony#62198 by adding validation checks in validateProperty() and validatePropertyValue() methods to ensure the property exists before attempting validation. If a property does not exist, a ValidatorException is thrown with an appropriate error message. The implementation uses hasPropertyMetadata() to check if the property exists before calling getPropertyMetadata(), which prevents silent failures when properties are removed or renamed.
carsonbot
changed the title
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Previously, the check only verified if metadata exists, which caused tests to fail for properties that exist but have no constraints defined. Now the check verifies if the property actually exists in the class (using property_exists or getter methods) before throwing an exception. This allows validating properties without constraints, which should return 0 violations, while still throwing an exception for properties that don't exist at all.
carsonbot
changed the title
|
|
||
| if (!$classMetadata->hasPropertyMetadata($propertyName)) { | ||
| $className = \is_object($object) ? $object::class : $object; | ||
| if (!property_exists($className, $propertyName) && !method_exists($className, 'get'.ucfirst($propertyName)) && !method_exists($className, 'is'.ucfirst($propertyName)) && !method_exists($className, 'has'.ucfirst($propertyName))) { |
There was a problem hiding this comment.
this doesn't look appropriate to me: these checks rely on knowledge about what's a property that's none of this class' business
|
From the issue and PR description, it looks like this change will throw an error if the property doesn’t exist. Right now it just does nothing, so changing that might be a breaking change. Maybe we could trigger a warning or deprecation or error message in this version and throw the error in the next major version? |
5 participants
This fixes issue #62198 by adding validation checks in validateProperty() and validatePropertyValue() methods to ensure the property exists before attempting validation. If a property does not exist, a ValidatorException is thrown with an appropriate error message.
The implementation uses hasPropertyMetadata() to check if the property exists before calling getPropertyMetadata(), which prevents silent failures when properties are removed or renamed.