Add zval type coersion #92
Conversation
There was a problem hiding this comment.
I like it, but looks like there's been some lingering bugs here with incorrect login and some overlap in casting and coercion rules.
Its probably best we don't do coersions from null. Just like how php will now insist nulliblity is included in type annotations, we should as require an explicit use of Option. Or an explicit cast. (It also makes me a bit more confident in my assertion that casting should be a free function).
| .or_else(|| zval.bool().map(|b| b.into())) | ||
| .or_else(|| { | ||
| zval.double() | ||
| .map(|d| if d.is_normal() { d.floor() as _ } else { 0 }) |
There was a problem hiding this comment.
As I understand it, PHP only coerces normal floats. Trying to coerce NAN or INF into an int is actually a type error (Argument must be of type int, float given).
When cast, they both go to zero.
function to_integer(int $a): int {
return $a;
}
echo to_integer(NAN); // Type error
echo (int)NAN; // Prints 0| zval.double() | ||
| .map(|d| if d.is_normal() { d.floor() as _ } else { 0 }) | ||
| }) | ||
| .or_else(|| if zval.is_null() { Some(0) } else { None }) |
There was a problem hiding this comment.
Same for null:
echo to_integer(null); // Type error
echo (int)null; // Prints 0Which honestly makes a lot of sense - they want it encoded in the type signature (?int rather than int). Not converting null to zero is probably the better behaviour for rust bindings anyways.
| .or_else(|| { | ||
| if zval.is_null() { | ||
| Some("".to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| }) |
There was a problem hiding this comment.
Also only a cast, not a coercion.
| fn from_zval_coerce(zval: &Zval) -> Option<Self> { | ||
| // https://www.php.net/manual/en/language.types.boolean.php#language.types.boolean.casting | ||
| zval.bool() | ||
| .or_else(|| zval.long().map(|l| l > 0)) |
There was a problem hiding this comment.
Oh, these are wrong. Its the same rules as C: zero is false, everything else is true.
| .or_else(|| zval.long().map(|l| l > 0)) | |
| .or_else(|| zval.long().map(|l| l != 0)) |
| zval.bool() | ||
| .or_else(|| zval.long().map(|l| l > 0)) | ||
| .or_else(|| zval.double().map(|d| d > 0.0)) | ||
| .or_else(|| zval.str().map(|s| !(s.is_empty() || s == "0") || s == "1")) |
There was a problem hiding this comment.
Any other string is considered true.
| .or_else(|| zval.str().map(|s| !(s.is_empty() || s == "0") || s == "1")) | |
| .or_else(|| zval.str().map(|s| !s.is_empty() && s != "0")) |
| .or_else(|| zval.double().map(|d| d > 0.0)) | ||
| .or_else(|| zval.str().map(|s| !(s.is_empty() || s == "0") || s == "1")) | ||
| .or_else(|| zval.array().map(|arr| !arr.is_empty())) | ||
| .or_else(|| if zval.is_null() { Some(false) } else { None }) |
There was a problem hiding this comment.
Null to bool is also a cast, not a coersion.
|
Personally, not a huge fan of this, closing this for now unless someone wants to pick up the implementation and fix outstanding issues. I really don't like PHP's type coercion behavior, and in fact, I believe that the closer this extension is to #298 removes existing type coercion without adding replacement logic. |
Zval::string()will no longer attempt to convert a long into a string. Instead, this is done through the new function on theFromZvaltrait:from_zval_coerce, which broadly follows PHP's type juggling rules.By default, downstream types do not have to implement this function, as its default is to call
from_zvalon the same type.