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

if() not described correctly #285

Open
MartijnR opened this issue Jun 13, 2021 · 7 comments
Open

if() not described correctly #285

MartijnR opened this issue Jun 13, 2021 · 7 comments

Comments

@MartijnR
Copy link
Contributor

The current ODK Collect and the old Enketo XPath evaluator does not convert the 2nd and 3rd parameter of if() to a string, i.e. it evaluates if(false(), false(), false()) or false() to "false".

The spec says both parameters should be converted to strings and thus the above expression would return "true". The new Enketo evaluator actually follows this spec, hence this issue was discovered. It actually evaluates if(false(), "false", "false") or false() which becomes "false" or false() and is true.

I think the spec should be corrected to accept any type as 2nd and 3rd parameter (even node-set so count(if( true(), //a, //b)) would also work.

@MartijnR
Copy link
Contributor Author

Hmm. The W3C XForms spec (that we refer to) actually uses string... https://www.w3.org/TR/2003/REC-xforms-20031014/slice7.html#fn-if

@lognaturel
Copy link
Member

lognaturel commented Jun 13, 2021

We have a general problem with XPath Boolean since its definition of truthiness is unexpected in many ways. @pbowen-oc’s usage makes sense and it’s surprising that it wouldn’t work.

In general, when the spec documents something other than what has been implemented for a long time, I’d tend to prefer we make sure the spec matches the real implementation. We know users make use of the behavior and changing it is disruptive. Users don’t refer to the spec when authoring forms.

Is there any way Enketo can implement the original behavior now or is that door closed? Maybe Enketo can detect attempts to use and or or with if and suggest alternatives or something like that?

@lognaturel
Copy link
Member

Oh good, I for some reason thought you were proposing aligning with the W3C spec. PR looks great. 🙏

@alxndrsn
Copy link
Contributor

Hi @lognaturel!

I think we all agree that it's surprising and quite unhelpful for if() to coerce its return values into strings. Redefining this behaviour for ODK seems like a win, especially if that's already how it works 😄

I'd be interested to dig a bit more into this:

We have a general problem with XPath Boolean since its definition of truthiness is unexpected in many ways.

From the spec:

  • a number is true if and only if it is neither positive or negative zero nor NaN

  • a node-set is true if and only if it is non-empty

  • a string is true if and only if its length is non-zero

  • an object of a type other than the four basic types is converted to a boolean in a way that is dependent on that type

-- https://www.w3.org/TR/1999/REC-xpath-19991116/#function-boolean

This seems quite reasonable to me; is there something specific here that seems odd? Or are there more subtle implications to these definitions which show up in more complicated situations?

@lognaturel
Copy link
Member

See #115

@alxndrsn
Copy link
Contributor

@lognaturel I'm not sure I've understood. Is the suggestion that boolean("false") should evaluate as false?

@lognaturel
Copy link
Member

The confusing behavior illustrated by this issue is that you can have a Boolean expression that evaluates to false that gets serialized to the string “false” which is then treated as truthy in any subsequent expressions unless explicitly parsed. I’m not suggesting that is wrong, but as the thread brings up, it’s probably best to avoid these kinds of type conversions and more broadly avoid the Boolean type as much as possible.

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

No branches or pull requests

3 participants