-
Notifications
You must be signed in to change notification settings - Fork 356
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
Allow coercion to boolean from '0' and '1' #345
Allow coercion to boolean from '0' and '1' #345
Conversation
01c73b0
to
80963f8
Compare
I'm not totally sure about this one, mainly because if you want to use "1" and "0" you can just define a type of "integer" and use the string coercion that already exists, right? I want to check a few other libraries and see what their policy is for this kind of thing... will have more information soon. |
That would seem to be contrary to RFC 7159 that defines JSON grammar. There is a caveat that JSON parsers "MAY accept non-JSON forms or extensions", but that could be tricky. The XML Schema Part 2 spec does define It would be great if @awwright could be summoned to give his input on 1/0/true/false for boolean in JSON Schema as opposed to only true/false. |
@moderndeveloperllc I don't think this really has anything to do with the JSON spec, or even the JSON schema spec. We're talking about a piece of functionality that is commonly layered onto validation libraries, specifically. This library already supports some coercion (it has to be invoked explicitly with the I haven't had a chance to research this in depth yet, but one of the most robust and fully featured validation libraries I know of does not coerce string "1" and "0" to boolean (though it does coerce numeric 1 and 0 to boolean): https://github.com/epoberezkin/ajv/blob/master/COERCION.md |
My use case is to validate query parameters ($_GET) with JSON schema, therefore coercion from integers to booleans would not help me as query parameters are always string or array. |
Of course, that exact use case is why we added coercion in the first place. My point is that there is no need for you to coerce your string 1/0 to boolean; you can simply change your expected type to "integer", then check them in your client PHP code for truthiness the exact same way you would check a boolean for truthiness. The end result is functionally the same, and we don't need to push coercion beyond reasonable limits. |
@JanTvrdik I'm not sure what "coercion" means in this case. All a validator does is accept an instance and return "valid" or "invalid". What's the proposed change in behavior, and how does it vary from the current behavior? |
One more comment for @JanTvrdik:
|
Oooh yes. Of course, JSON Schema only specifies validation. But I've implemented "coercion" like this, i.e. take some data (JSON or otherwise) and cast it into JSON valid against a certain schema. I did this for taking a URI and turning it into a JSON-compatible object. This just gets a little complex if you support multiple data types, for example |
I was about to argue that "1" and "0" are different from the cases already handled in that they are not simply a reversal of the stringification performed by operations like
Sooo, I guess I'm not as sure about this, now. I would still like to know why the folks on the other project don't coerce "1" and "0"... |
So we have this elegant counter-example and explanation from the author of ajv: ajv-validator/ajv#395 (comment) Essentially, because coercion of string 1 and 0 to boolean is not reversible, we run into cases where validation on type lists (and possibly other complex nested structures) can unexpectedly fail with coercion turned on, when it would pass with it turned off. So, @JanTvrdik I think your options are still the same:
|
@shmax But his points are irrelevant in context of this library because this library does not suport multiple types with anyOf nor does it support reversing the coercion. You cannot reproduce his example with this library. |
I did try it, and what happened was exactly what he predicted. Did you get different results?
What do you mean? Do you mean that coercion is not fully supported? If you can supply a failing unit test we can investigate it together.
Nobody is saying it should support reversing the coercion. When we say that the coercion of 1 and 0 is not reversible we mean that it is one way; once coerced its original form is lost, so that a further rule that might have otherwise validated its original form will fail (like in the schema sample we've been looking at). It's just too much of a transformation. Like @epoberezkin says, this can happen with the other coercion types as well, but it's much less likely. I guess I don't understand why you're fighting so hard on this? There are many different ways for you to get the functionality you want, and none of them have to involve changing this repo. I'll list out a few for you, complete with samples. Coerce to integer instead of boolean$value = "0"; // json_decode('{}');
$schema = json_decode('{
"type": "integer",
"enum":[0,1]
}');
$validator->coerce($value, $schema);
echo gettype($value); // "integer"
echo $value; // 0 Subclass TypeConstraint and just do whatever you wantclass MyTypeConstraint extends \JsonSchema\Constraints\TypeConstraint {
protected function toBoolean($value) {
if($value == "1") {
return true;
} else if ($value == "0"){
return false;
}
return parent::toBoolean($value);
}
}
$factory = new \JsonSchema\Constraints\Factory();
$factory->setConstraintClass( "type", "\path\to\MyTypeConstraint" );
$validator = new \JsonSchema\Validator($factory);
$value = "0"; // json_decode('{}');
$schema = json_decode('{
"type": "boolean"
}');
$validator->coerce($value, $schema);
echo gettype($value); // "boolean" If you're really determined you could also see about modifying the core coercion code so that coerced values are not passed down the chain when evaluating lists and other complex scenarios, but like the other guy says it might not be worth the added complexity, especially when you already have much lighter alternatives. |
@JanTvrdik @shmax is this PR still in play? |
@bighappyface My opinion is no, especially considering we haven't heard a peep from @JanTvrdik in quite a while. |
Closing until further notice from @JanTvrdik |
For historical reasons we did way too much of assumption to substitute "0" and "1" where a true boolean is not possible, say I'd vote for it if this is where we can start "coercing" things back to standard. For the case of ajv-validator/ajv#395 (comment), I am not sure any coerce should happen for |
Both
'1'
and'0'
are very common string representations of boolean values.