-
Notifications
You must be signed in to change notification settings - Fork 8
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
Unexpected type coercion in value
#146
Comments
Hmm. I think you're sending us down a slippery slope here... I'm not sure what the right answer is, but some observations:
Some options to consider:
Are there other alternatives? I think I'd be happiest with option 1 |
Yes, that's where a lot of issues I had came from. It is pretty reasonable to assume, that allowing more types for an argument won't disable implicit type coercion. But there is no explicit ruling on how to handle it... I've tried coming with some scheme as "try type coercion in the order of types in the sum type definition and use the first one compatible", which, usually, came to the same result as the reference implementation, but it is rather insane tbf.
I tried to avoid that, as this often opens an unpleasant can of worms, but in this case I assumed this was an explicit description for this:
Yes, I have all those cases written and wanted to bring them up. In some cases it can be converted to multiple, which causes issues. I'll do a separate issue with all of them, so that we could look at them as a whole.
I agree, that this looks absurd, since it is :) . But you will have those cases occur "naturally", when arguments are not literals, but complex expressions or lousy-typed data. Which in turn could cause those annoying bug reports, where you know the expression doesn't make sense, but it works in some library, so you might as well behave the same (queue unpleasant memories of working on .zip support :) ). I would even argue a case like
Seems like a fine solution. Though, imo, it is a bit unexpected to have implicit type coercion be turned off like that. Also this might cause backwards-compatibility concerns, if we were at some point "upgrade" a single type argument to a sum-type, which will cause corner cases to pop back up, and they might be rather odd to write them up in spec.
I don't think it will solve all of them, but it would remove a lot of somewhat pointless cases I've encountered. It is very easy to "cast" to an array and there is always Also, off the top of my head, I cannot think of an existing language, that does such conversions implicitly, except for the "string is an array of code points" case. Overall, this would be a 👍 from me.
I was trying to figure out such a rule and have exceptions written in the function descriptions themselves, but the more I though about it, the more convoluted it became to handle everything... In the end, I think the easiest way would be to not think about it as type coercion anymore, but interpret all those cases as functions accepting My rationale is that the majority of cases, where this pops-up, are functions, which would often be separate, but are hiding under one name for convenience. You could say that we are, practically speaking, calling a type-erasing proxy function, which forwards the call to internal functions based on the types provided, which is documented in the function description. Ex. Also, if we were to remove the array coercion rule, some of this will become easier as well (like the What do you think? Btw, sorry for spamming issues this much. Writing in C++ again brought the "language lawyer" thinking back with it. |
Thinking on this some more, I'm worried that we haven't thought through all the cases where it's useful. e.g.:
And, while stopping array coercion fixes many problems, it doesn't fix them all. So by itself it's not a solution, it would have to be combined with some other rule change.
Perhaps. But I'd argue that an unpredictable coercion is worse
But this is true in general -- we can't change any parameter types in a backward-compatible way. If we really wanted to change parameter types we'd have to introduce new functions instead.
Well, I wouldn't call this the easiest solution. It would be a large change to the JS implementation and spec. IMO Having type coercion centralized is more consistent than having function-specific coercion rules. But I think you are correct in that we can't create rules that allow us to do all coercions before the function call.
Well, actually... the way it is now there are many more than 4 variations -- given the various coercions of different types to arrays, strings and integers. I guess there is yet another option: stop allowing multiple parameter types. In each case, create multiple functions. e.g. Instead of I think my current leaning is:
|
I wanted to bring this up, but for safety you would still want to write
I also was thinking of this... But my thinking was that adding two brackets doesn't make it much more messy, and also that in Python, where something like But I get your point, yes.
True
I was thinking of cases, when it would have been a TypeError before. For example, if
I assumed in the code it would mostly boil down to changing types to ANY in the signature internally and making sure cast are present in the body. But I haven't looked through the code enough to know, so I will trust you on that. :)
Yeah, this would look pretty out-of-place and unnecessarily verbose for such a language.
To be fair, we always have I think we would also need to add |
Summary:
Recommendation
|
agreed |
@JohnBrinkman Is there a mention of "Stop coercing when there are multiple parameter types. The provided parameter must be an exact match to one of the allowed types" in the spec now? I cannot find it... |
You're right. That part is missing. |
From reading the spec, for
value
I assumed the following.object
, then coerce second argument tostring
.array
, then coerce second argument tointeger
.object
andarray
as-is.null
coercion doesn't matter, as result will benull
anyway.array
, as coercion toobject
is invalid.This is different from what I see in the implementation.
value(`false`, 0)
value(42, 0)
value("abc", 0)
These cases throw a TypeError, even though first argument can be converted to array.
Same behavior, if both arguments need conversion, as it already fails on the first one.
value([1, 2, 3], `false`)
value([1, 2, 3], `true`)
value([1, 2], "sdfgesrg")
In these cases
null
is returned instead of an element. There are "Failed to convert X to number" messages in the debug log, even though second argument has a valid integer conversion.The text was updated successfully, but these errors were encountered: