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

Inconsistent behavior of truthiness in "!!" operator #44

Open
BohdanCh-w opened this issue Apr 29, 2024 · 2 comments
Open

Inconsistent behavior of truthiness in "!!" operator #44

BohdanCh-w opened this issue Apr 29, 2024 · 2 comments

Comments

@BohdanCh-w
Copy link

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

@jwadhams
Copy link
Owner

jwadhams commented Apr 29, 2024

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?)

@BohdanCh-w
Copy link
Author

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

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

2 participants