Proposal - Improve test cases for var

[luismoyano]

Hello JsonLogic! I found a case that has never been covered by any standard and could prevent naive implementations to catch bugs easier (I'm not sure yet about how many implementations might include this bug, thus including the following test cases might helps us finding out:

The var operator includes the option of providing a default value, naive implementations could be developed in a similar manner to

value = data[var_key] || default_value

If the value is falsy AND a default was not provided, this implementation would lead to unexpected results;

# Calling with initial rules & values
JsonLogic.apply({var: "my_var"}, {my_var: false})

# Internally we access the context + try to find the default value
default = rules_subject[1] # => nil
value = data[var_key] || default_value # => false || nil => nil

# expected => false
#=> nil

Even though var is bound to be deprecated, I'd still like to include some tests to help maintainers to catch this issue as it has never been done before

Why does this matter?
Imagine the following operation to grant something to a specific user in a features flag engine or others while using JsonLogic with a naive implementation:

// Rules
{ // ...
  "===": [{ "var": "is_power_user" }, false]
}

// data
{ "is_power_user": false }

The expected result is true and the obtained result is false. There's no real coverage for this case yet.

Here's a set of test I'd like to introduce

[
    {
        "description": "var returns false when key exists with false value",
        "rule": { "var": "active" },
        "data": { "active": false },
        "result": false
    },
    {
        "description": "var returns 0 when key exists with 0 value",
        "rule": { "var": "count" },
        "data": { "count": 0 },
        "result": 0
    },
    {
        "description": "var returns empty string when key exists with empty string",
        "rule": { "var": "name" },
        "data": { "name": "" },
        "result": ""
    },
    {
        "description": "var with default does NOT use default when value is false",
        "rule": { "var": ["active", true] },
        "data": { "active": false },
        "result": false
    },
    {
        "description": "var with default does NOT use default when value is 0",
        "rule": { "var": ["count", 99] },
        "data": { "count": 0 },
        "result": 0
    },
    {
        "description": "var with default does NOT use default when value is empty string",
        "rule": { "var": ["name", "anonymous"] },
        "data": { "name": "" },
        "result": ""
    },
    {
        "description": "var with default DOES use default when key is missing",
        "rule": { "var": ["missing", "default"] },
        "data": { "other": "value" },
        "result": "default"
    },
    {
        "description": "nested var returns false from nested path",
        "rule": { "var": "user.active" },
        "data": { "user": { "active": false } },
        "result": false
    },
    {
        "description": "nested var returns 0 from nested path",
        "rule": { "var": "user.score" },
        "data": { "user": { "score": 0 } },
        "result": 0
    },
    {
        "description": "equality check with false value from var",
        "rule": { "==": [{ "var": "is_beta" }, false] },
        "data": { "is_beta": false },
        "result": true
    },
    {
        "description": "negation of false value from var",
        "rule": { "!": [{ "var": "disabled" }] },
        "data": { "disabled": false },
        "result": true
    },
    {
        "description": "and with false value from var",
        "rule": { "and": [{ "var": "enabled" }, true] },
        "data": { "enabled": false },
        "result": false
    }
]

I'd suggest either adding these cases to compatible.json in the var section (since this is core var behavior) or creating a new var-falsy.json suite file

Happy to submit a PR with whichever approach you prefer.

Thanks a lot!!

[codetiger]

Good suggestion @luismoyano , I would suggest adding a new test suite file. Compatible list is directly the original cases created by @jwadhams so let us keep that as is and we have added more cases like these.

[luismoyano]

Thanks! I'll open the PR's

[luismoyano]

@gregsdennis @toddbaert @TotalTechGeek Can we have some words here? Would be nice to have a vote in order to decide whether this is introduced in the corpus of the official test suite or not