Respect / Validation

The most awesome validation engine ever created for PHP
https://respect-validation.readthedocs.io
MIT License
5.75k stars 774 forks source link

Check uninitialized properties without throwing exception #1405

Closed gtrbunny closed 5 months ago

gtrbunny commented 1 year ago

PHP 7.4.0+ throws an uninitialized exception when using object validation if the property hasn't been initialized. This additional check should be safe given 7.3 support is dropped.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 87.50% and project coverage change: +1.39% :tada:

Comparison is base (8f545c1) 94.61% compared to head (8ce6fba) 96.01%. Report is 16 commits behind head on 2.3.

:exclamation: Current head 8ce6fba differs from pull request most recent head 88a0df8. Consider uploading reports for the commit 88a0df8 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## 2.3 #1405 +/- ## ============================================ + Coverage 94.61% 96.01% +1.39% + Complexity 973 955 -18 ============================================ Files 195 192 -3 Lines 2285 2231 -54 ============================================ - Hits 2162 2142 -20 + Misses 123 89 -34 ``` | [Files Changed](https://app.codecov.io/gh/Respect/Validation/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Respect) | Coverage Δ | | |---|---|---| | [library/Rules/Attribute.php](https://app.codecov.io/gh/Respect/Validation/pull/1405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Respect#diff-bGlicmFyeS9SdWxlcy9BdHRyaWJ1dGUucGhw) | `93.33% <87.50%> (-6.67%)` | :arrow_down: | ... and [196 files with indirect coverage changes](https://app.codecov.io/gh/Respect/Validation/pull/1405/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Respect)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alganet commented 1 year ago

Hi! Thanks for your contribution. 🐼

I'm switching this to target the 2.3 branch instead of master. You can find more info about this on the CONTRIBUTING.md file.

Some checks are failing. You can fix thos locally using ./vendor/bin/phpunit, ./vendor/bin/phpcs and ./vendor/bin/phpstan analyze and update the PR.

I would also like to ask you to provide a unit/integration test that breaks without your code. For example, something that would throw the exception before the changes you made, and would not throw them after your improvements.

If you need help in any of these, please let me know! We can work on it together.

henriquemoody commented 5 months ago

There's an issue with your fix. If a property exists, the Attribute rule should pass regardless.

I've made a few changes and merged it on 8d7d783698cb1c41dc9a35c919413c586f14b745. This fix will be present in version 2.3.

Thank you for contributing, @gtrbunny! 🐼