Change error reporting for invalid types with multiple valid types #294
Conversation
| // with a new type constraint | ||
| if (is_object($tp)) { | ||
| if (!$isValid) { | ||
| $validator = new static($this->checkMode); |
There was a problem hiding this comment.
I thought we could get rid of this (at least for me) ugly looking workaround, but it seems it's the best solution we currently have :/
|
I'm not sure, but the failed test https://travis-ci.org/justinrainbow/json-schema/jobs/147291856 looks unrelated to this change? |
|
@bighappyface Have you found some time to take a look at this commit? :-) |
|
@FlorianSW did we lose the test? |
|
What test? I'm pretty sure we don't need extra test for this change as it's already covered by existing tests? :-) |
|
I feel like a test was added that would have encouraged me to remove the "needs tests" label. I don't recall it exactly but the comment history suggests a test was created. Regardless, I feel because this PR changes behavior that a new test exhibiting the new behavior is desirable to ensure the outcome described in #293 has been achieved. |
|
Ah, my bad, in my second commit I had in focus to unbreak existing tests (because of my refactoring), so I haven't noticed anymore, that I added some functionality. :D Let me add some tests! :) |
Currently, if a property has multiple valid types, an error message when a value doesn't conform to the schema, would mention only the last valid type, without mentioning the other valid types. This commit changes, how the TypeConstraint works internally. It does a little bit of refactoring (removing also the "Refacter this" ToDo) and now mentions any valid types on a failure. Fixes jsonrainbow#293
|
@bighappyface Maybe you haven't seen it, but I've added some tests to an already existing test case :-) |
|
+1 |
Currently, if a property has multiple valid types, an error message
when a value doesn't conform to the schema, would mention only the
last valid type, without mentioning the other valid types.
This commit changes, how the TypeConstraint works internally. It
does a little bit of refactoring (removing also the "Refacter this"
ToDo) and now mentions any valid types on a failure.
Fixes #293