jwadhams / json-logic-js

Build complex rules, serialize them as JSON, and execute them in JavaScript
MIT License
1.26k stars 140 forks source link

Better support for others who want to create implementations #93

Open gregsdennis opened 3 years ago

gregsdennis commented 3 years ago

This isn't an issue for this library so much as for JsonLogic in general.

I started on a .Net implementation of JsonLogic, but found there were too many inconsistencies in the online implementation. I've ~complained about~ described some of these inconsistencies in my issue to add this support.

Ideally, I'd like to see:

jwadhams commented 3 years ago

OK, well the good news is that there is a common test suite. The unit tests in this repo are downloading http://jsonlogic.com/tests.json and treating the whole thing as an array, and each row of that array as [rule, data, expected]

That gives me a common 275 item test suite across the two implementations I maintain, PHP and JavaScript.

But you're right, most operators are implemented to just delegate the decision to the language in the fewest lines of code. The entire JavaScript implementation of max is

    "max": function() {
      return Math.max.apply(this, arguments);
    },

The one place I found I just couldn't live with the ambiguity was truthiness, which is why JsonLogic has its own truth table: https://jsonlogic.com/truthy.html It's certainly possible that you could define the same thing for "what is JsonLogic's opinion of -3 > 't'" but it hasn't affected my day job and it doesn't sound like a fun time.

I think if you wanted to make a .NET version that passed the common tests and then went absolutely berserk on type inconsistencies (e.g., if you call {"max":[-1,"t",-3]} it throws an exception like "all arguments must be of the same type" or "all arguments must be numeric") there would absolutely be an audience for that. JsonLogic is loose because the two languages I implemented it in are loose (and in vaguely compatible ways) but you are absolutely not alone in being frustrated by it.

I also think I screwed up the spec when I allowed single-argument operators to skip the [] around args, e.g. {"var":"a"} is treated as identical to {"var":["a"]}. You're not even the first person this month to be thrown by that one. All to save myself two characters in var statements! But I think it's gonna be a colossal breaking change to rules in the wild to remove it now, I haven't even begun to think how I'd migrate my uses in my day job, let alone how I'd advise the community.

gregsdennis commented 3 years ago

Thanks for the explanation and the link to the tests. I might pick it up again.

I suppose the max issue I mentioned is more about casting than truthyness (== vs ===). I'll probably constrain all of the things that aren't explicitly required by the tests. Loose equality isn't something that .Net does natively. It might be interesting to do an in-depth comparison (like this one for JSONPath) of your JS and my .Net implementations (and/or others) to determine which edge cases are supported where.

gregsdennis commented 3 years ago

More on the casting issue, cases like {"/":["1",1]} (from the test suite) are going to be quite difficult for strongly-typed languages.

marvindv commented 3 years ago

I created a port to Rust a while ago (jsonlogic_rs) and faced the same problems. In the end I actually reimplemented the way JavaScript handles types and their coercion and created tests for all examples on jsonlogic.com and all other funny edge cases I could imagine, so stuff like {"max":[-1,"t",-3]} should work just like with the original JsonLogic implementation in JavaScript. Good to know there is http://jsonlogic.com/tests.json, which can be used for other languages.

Even if tedious, it can be done by "just" following the language spec and it was actually a nice insight for me. My personal "highlight" is the specification of abstract equality (==) and its implementation.

gregsdennis commented 3 years ago

Okay. I have all the tests passing except two of the all tests. I can't figure out what they're supposed to do.

1

logic: {"all":[{"var":"integers"},{"<":[{"var":""},1]}]}
data: {"integers":[]}
expected: false
 { "all" :
  [
    { "var" : "integers" },    // (a)
    { "<" :                    // (b)
      [
        { "var" : "" },
        1
      ]
    }
  ]
}

(a) This returns an empty array, so there's nothing to apply the rule to. Do we require that all must have at least one "truthy" element? (b) "" < 1? how an empty string compare to a number? Do the comparison operators follow loose equality (i.e. ==)?

2

logic: {"all":[{"var":"items"},{">=":[{"var":"qty"},1]}]}
data: {"items":[]}
expected: false

Basically the same test, except we have null >= 1. Again, there are no items to run the test on.


Note that if all does require at least one "truthy" element, then an empty array will always fail, and we can put whatever ridiculous rule we want in the logic and it doesn't matter because it'll never run.

gregsdennis commented 3 years ago

Alright... I've published my first version. Passes the entire test suite (with the assumptions above).

gregsdennis commented 3 years ago

How does one go about getting added to the site as an implementation? Would a PR here suffice?