Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] Throw ValidatorException when validating non-existent property#62238
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:7.4
Are you sure you want to change the base?
Conversation
This fixes issuesymfony#62198 by adding validation checks in validateProperty()and validatePropertyValue() methods to ensure the property exists beforeattempting validation. If a property does not exist, a ValidatorExceptionis thrown with an appropriate error message.The implementation uses hasPropertyMetadata() to check if the propertyexists before calling getPropertyMetadata(), which prevents silentfailures when properties are removed or renamed.
carsonbot commentedOct 30, 2025
Hey! I see that this is your first PR. That is great! Welcome! Symfony has acontribution 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 causedtests 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 shouldreturn 0 violations, while still throwing an exception for propertiesthat don't exist at all.
| 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))) { |
nicolas-grekasOct 31, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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? |
Uh oh!
There was an error while loading.Please reload this page.
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.