Skip to content
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

Unclear validation error about when type assertions are required #6006

Closed
davidtheclark opened this issue Jan 17, 2018 · 3 comments
Closed
Assignees

Comments

@davidtheclark
Copy link
Contributor

davidtheclark commented Jan 17, 2018

I spent a while today struggling with an expression validation error. I had the following expression:

["length", ["get", "name"]]

The validation error was:

Expected arguments of type (string) | (array), but found (value) instead.

I didn't have any idea what that meant. After tinkering around a while I realized that I could get rid of this error by adding a type expression, changing the overall expression to:

["length", ["string", ["get", "name"]]]

This reminded me at last about the long and fairly difficult section in the documentation about the expression "type system." The validation error didn't help remind me, though — just the solution that I stumbled across on my own. It would be great if the validation error did more to point me in that direction.

In any validation error of the type Expected arguments of type x | y | ..., but found (value) instead, would it make sense to add a suggestion that you may need to use a type assertion or type coercion expression? Or is there any other way we can make this error more helpful?

cc @mapbox/studio

@anandthakker
Copy link
Contributor

Great point -- yeah, we should make this sort of error (and probably several other type errors) much more helpful.

would it make sense to add a suggestion that you may need to use a type assertion or type coercion expression

I think this could make sense, since most of the time, if you have a value where some more specific type is expected, an assertion/coercion is what's needed. (However, note that because of automatic type assertion inference, this specific kind of error is probably pretty rare... so we should definitely also think about ways to improve type error messages more generally.)

@davidtheclark
Copy link
Contributor Author

this specific kind of error is probably pretty rare

Yeah ... my first reaction to the above was despair that we might have to use a type assertion for every get call. Then I realized that the real stumbling block here is length (a much more rare expression) — because it accepts multiple types for its argument.

we should definitely also think about ways to improve type error messages more generally

I and/or other folks in @mapbox/studio will be digging deeper into expression validation soon, so we'll probably be filing more issues like this with more particular ideas.

@jfirebaugh
Copy link
Contributor

jfirebaugh commented Feb 13, 2018

Another approach would be to make ["length", ["get", "name"]] "just work" by relaxing the typing of length so that it accepts a value and checks that it's a string or array at runtime. That's the moral equivalent of what we did for ==/!= in #5840.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants