jwadhams/json-logic

Inconsistent behavior of truthiness in "!!" operator

Opened this issue · 2 comments

While trying out rules on jsonlogic.com, I spotted unexpected behavior when using "!!" operator. Unfortunately, I cannot confirm if that is the problem with JS implementation, used on website, since I am working with an implementation in Go that doesn't support this operator.

The following two rules produce different result:

{
    "if": [
        [0, 1, 2],
        "True",
        "False"
    ]
}

returns "True" as expected

{
    "if": [
        {
            "!!": [0, 1, 2]
        },
        "True",
        "False"
    ]
}

returns "False", which is not what I expected, since any non-empty array should be true per documentation. And !! should correspond to JSONLogic's truthy specs.

I believe that the problem lies in !! as it returns false for any array, which first element is falsy value.
{ "!!": [0, 2, 3] } -> false
{ "!!": ["", "b", "c"] } -> false
{ "!!": [false, true] } -> false
But
{ "!!": [1, 2, 3] } -> true
{ "!!": ["a", "b", "c"] } -> true
{ "!!": [true, false] } -> true

I'm starting to think the "unary operators can be passed one argument without a surrounding array" was a major design misstep, if there's ever a 2.0 that'll be one of the first things I change.

The problem is, the parser always assumes that the "value" of an operator is an array of arguments. If it's not an array, it assumes it's a single argument, and wraps an array around it. This is a real blunt instrument, and I regret it more each year.

Since your if always has an array of args, it's correctly treating it like

if ([0,1,2]) {

Then using our compromise truthiness table to decide a non-empty array is true.

So intuitively we both thought it was treating your second example like

if (bangbang([0,1,2])) {

but it's actually treating it like

if (bangbang(0, 1, 2)) {

where the bangbang operator only uses the first argument, but doesn't complain about receiving three arguments.

There's a handful of ways out of this.

As an author of rules, I've gotten strict about always using an array of arguments even for unary operators, like

{"!!":[[0,1,2]]}

That keeps me safe, but doesn't lift all boats.

Or I guess we could patch !! to throw an exception if it receives more than one argument? I'm not sure what unintended consequences this could have but it seems like a net positive.

Or we could add a flag to the parser that lets the caller decide if the unary sugar should be enabled? It could be on by default for existing users, but able to be turned off by people hitting these more sophisticated bugs? (And probably recommend turning it off for new users so they develop good habits?)

Thank you for explanation. Yes, it makes sense now.
As of how to proceed with this issue, I don't think that my opinion is valuable here, since I don't use JS implementation, and just wanted to point out this behavior.
Feel free to close this issue if you think that unary operators' behavior should be discussed in dedicated thread