-
Notifications
You must be signed in to change notification settings - Fork 97
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
Support Respect/Validation v2 #112
Support Respect/Validation v2 #112
Conversation
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.
LGTM 👍
Thanks @t0mmy742 ! |
} catch (ExceptionInterface $e) { | ||
if (class_exists(NumericVal::class)) { | ||
Validator::numericVal()->assert($data); | ||
Validator::numericVal()->positive()->assert($minimum); |
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 change added a new validation which doesn't allow minimum values to be equal, or lower than zero (see https://respect-validation.readthedocs.io/en/latest/rules/Positive/).
openapi specification seems to allow zero or negative values for both minimum
and maximum
, although not explicitly.
Was this intentional?
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.
I think it's a copypaste from MultipleOf
. Would you mind to submit a PR? I think there are some other problems of the same kind
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.
Also maybe we need more tests around this, since this had pass unseen
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.
Sure! I spotted only minimum and maximum with this extra rule, I'll add any others if I find them.
Fixes #102
Closes #103 (and based on these discussions)
Also completed
ignoreErrors
to phpstan.neon file since static analysis check is done with PHP 7.2 on GitHub CI.This is the best thing we can do for the moment.
Adding a
DataValidator
(see discussion) does not help since we loose a lot of things (auto-completion, static checks,...).Possibly not the cleanest, but it supports v1 AND v2.
Plus we need to check with
class_exists
and notmethod_exists
. These methods (numericVal,...) do not really exist, but use magic functions (__call, __callStatic).One day, we could possibly think about dropping Respect/Validation v1 support (last release more than one year ago, 2019-05-28). Respect/Validation v2.0 supports PHP^7.2, while v2.1 supports PHP^7.3.