[Preview Discussion] Argument Coercion Proposal

[TotalTechGeek]

Hey folks 👋🏻

We still need to set community guidelines and set up a process and repository for presenting proposals.

I wanted to bring up one item a bit early that I feel might be controversial and may not end up being adopted. I fully expect to lose, but I hope that I'll be able to convice 😉

Background

So, to get things started, let's establish some background, JSON Logic is basically an AST / serializable S-Expressions in JSON.

To call a method, you use a key in an object, and the value is treated as your arguments for the expression.

{ "max": [1, 2, 3] }

Is conceptually the same as calling max(1, 2, 3)

So far so good.

Per mainline JSON Logic, when you call a method directly a value, this is treated as shorthand for calling with a single argument array.

Therefore, { "max": 1 } is the same as { "max": [1] }, which translates to max(1)

This is also fairly reasonable, though has caused some confusion with { "not": 0 } vs { "not": [0] }. Both of these expressions should translate to not(0) because of the shorthand sugaring; but I ultimately believe this design decision makes sense, and I believe needs to be preserved.

Proposal

Where I diverge slightly is in how I wish to treat chained logic statements.

Imagine I have {"max": {"var": "data"} } and { "data": [1, 2, 3] }

If we treat chained logic the same way as values, we limit our capabilities significantly, or make operator implementation quite a bit more complex.

For example, if {"max": {"var": "data"} } is interpreted as {"max": [{"var": "data"}] }, it would be equivalent to max([1, 2, 3]) instead of max(1, 2, 3), which unless we create a rather robust implementation of max, would likely return awkward results.

Similarly, if I had { "cat": { "merge": [["Hello "], ["World", "!"]] } }, cat would receive [["Hello ", "World", "!"]] as its argument list.

In json-logic-js, this ends up producing "Hello ,World,!" as it automatically calls toString on the array..

What if Instead of the sugaring operation mentally being (x is not array) ? [apply x] : apply x, the sugaring operation could instead be concat([], apply x).

Then we'd be able to chain function calls and essentially treat them as if they spread into the argument list, so if I say:

{ "cat": { "var": "text" } }

With the data

{ "text": ["Hello ", "World", "!"] }

It will result in Hello World!

So instead of { "cat": { "var": "text" } } being automatically sugared into { "cat": [{ "var": "text" }] } as cat([var("text")]), it's instead parsed as cat(...var("text"))

Making it possible to do things like:

{
  "max": {
      "map": [
        {
          "filter": [
            { "var": "people" }, 
            { "===": [{ "var": "department" }, "Engineering"] }
          ]
        }, 
        { "var": "salary" }
    ]
  }
}

With the following data:

{
  "people": [
    { "name": "Jay Ortiz", "salary": 100414, "department": "Engineering"},
    { "name": "Louisa Hall", "salary": 133601, "department": "Sales"},
    { "name": "Kyle Carlson", "salary": 139803, "department": "Sales"},
    { "name": "Grace Ortiz", "salary": 147068, "department": "Engineering"},
    { "name": "Isabelle Harrington", "salary": 112704, "department": "Marketing"},
    { "name": "Harold Moore", "salary": 125221, "department": "Sales"},
    { "name": "Clarence Schultz", "salary": 127985, "department": "Sales"},
    { "name": "Jesse Keller", "salary": 149212, "department": "Engineering"},
    { "name": "Phillip Holland", "salary": 105888, "department": "Marketing"},
    { "name": "Mason Sullivan", "salary": 147161, "department": "Engineering" }
  ]
}

And output 149212

Arguments Against

I believe a solid argument against this proposal is that it diverges significantly from the current mental model for the sugaring operation.

concat([], apply x) is a bit more advanced of an operation to deal with sugaring.

Arguments For

This approach would make JSON Logic significantly more flexible and capable.

It'd be easy to pass the output of a filter & map statement into cat or + and have it produce a reasonable result.

It also provides this capability without embedding a ton of edge cases into each operator to be able to support "If I receive a single argument, and that argument is a list"

It's also still the same operation, it just changes when the "is array" evaluation occurs by one step.

Backwards Compatibility

I don't believe this tweak would break anyone's existing logic; and I believe that this would enable use-cases that otherwise would not be possible.

[TotalTechGeek]

Note: Going forward, I think that proposals should be presented in JSON Blocks,

[
  "Basic Tests",
  {
    "description": "Adds two numbers together",
    "rule": { "+": [1, 2] }
    "result": 3,
    "data": {}
  }
]

Format outlined here: https://github.com/json-logic/.github/blob/main/TEST_FORMAT.md

To make it easier to extract and play with.

Here are some of the proposed behaviors:

[
    "# These are tests from https://github.com/orgs/json-logic/discussions/2",
    {
        "description": "Standard Max",
        "rule": { "max": [1, 2, 3] },
        "data": {},
        "result": 3
    },
    {
        "description": "Standard Max, Single Argument Sugared",
        "rule": {"max": 1},
        "data": {},
        "result": 1
    },
    {
        "description": "Max with Logic Chaining",
        "rule": { "max": {"var": "data"} },
        "data": { "data": [1, 2, 3] },
        "result": 3
    },
    {
        "description": "Cat with Logic Chaining",
        "rule": {
            "cat": {
                "merge": [ ["Hello "], ["World", "!"] ]
            }
        },
        "data": {},
        "result": "Hello World!"
    },
    {
        "description": "Cat with Logic Chaining (Simple)",
        "rule": { "cat": {"var": "text"} },
        "data": { "text": ["Hello ", "World", "!"] },
        "result": "Hello World!"
    },
    {
        "rule": {
            "max": {
                "map": [
                    {
                        "filter": [
                            {"var": "people"},
                            {
                                "===": [ {"var": "department"}, "Engineering" ]
                            }
                        ]
                    },
                    {"var": "salary"}
                ]
            }
        },
        "data": {
            "people": [
                {"name": "Jay Ortiz"          , "salary": 100414, "department": "Engineering"},
                {"name": "Louisa Hall"        , "salary": 133601, "department": "Sales"      },
                {"name": "Kyle Carlson"       , "salary": 139803, "department": "Sales"      },
                {"name": "Grace Ortiz"        , "salary": 147068, "department": "Engineering"},
                {"name": "Isabelle Harrington", "salary": 112704, "department": "Marketing"  },
                {"name": "Harold Moore"       , "salary": 125221, "department": "Sales"      },
                {"name": "Clarence Schultz"   , "salary": 127985, "department": "Sales"      },
                {"name": "Jesse Keller"       , "salary": 149212, "department": "Engineering"},
                {"name": "Phillip Holland"    , "salary": 105888, "department": "Marketing"  },
                {"name": "Mason Sullivan"     , "salary": 147161, "department": "Engineering"}
            ]
        },
        "result": 149212,
        "description": "Max with Logic Chaining (Complex)"
    }
]

[TotalTechGeek]

@json-logic/core Hey folks, this is a "test" proposal to solicit feedback on the best way to organize proposals.

I do think that each proposal should have the relevant JSON attached.

If y'all have suggestions on how to organize proposals going forward, we can definitely incorporate that into the initiative's guidelines

[josephdpurcell]

One more thing popped up, what is the expectation for a "passing" test? I understand it to be that a given engine takes the data and runs the rule to get the result, and then a deep and strict equivalence is done to match the test result to the engines result. But can we conceive of cases that languages will have trouble with this? Or cases where the result is non-deterministic? I can't think of any right now but it's worth validating.

Or maybe this is a topic that is addressed by saying there will be test cases the engine must implement due to language-specific challenges that are not appropriate to be made into general specification. For example, do all languages even have an undefined type?

My conclusion having explored this for 60 seconds is to handle it similar to PHP FIG where the test cases are more like examples. "Here are examples of JSON Logic behavior that should be consistent across engines, anything not specified is up to the engine to provide the rigorous testing/definition."

[TotalTechGeek]

The grouping concept feels iffy?

There is no grouping concept; it's just inherited comments from https://jsonlogic.com/tests.json.

I should probably tweak the markdown to make it clear that it's just a way to embed some additional human-readable data into it.

It's not intended to be used by the test suite.

The rule as an any type is problematic. I would expect the rule type to be rigorously defined in its structure

The rule has to be an any type. I could potentially re-describe it as "any valid JSON value" (JSON RFC 8259), but mainline JSON Logic says that true is a valid rule that is evaluated to be true. I do not want to break backwards compatibility / redefine that fundamental.

It's also a detail that most engines use to evaluate arguments, so that logic.apply can be run recursively.

I think I object to data being an object

This is a documentation mistake. Looking at the file will show that this is not actually a requirement.

You are correct, you might pass in 5 or [1, 2, 3] as data 😄

Additionally I don't think it should default to an empty object. The field should be required and if it's omitted the expectation is the engine should treat it as the equivalent of "undefined" in that language.

null would be more appropriate, given that undefined does not exist in JSON.

I think this might be bikeshed territory, and this isn't somewhere I particularly feel very strongly one way or another,

So I think I might make it mandatory as well. Less room for disagreement 😆

[TotalTechGeek]

I've made some tweaks to the doc.

Or cases where the result is non-deterministic?

Non-deterministic results would be rather hard to test for 😉.

But I get what you're referring to;

For example, if we proposed an extension that allowed someone to use the current time, then the result would naturally be non-deterministic and hard to encode into the spec.

I think this serves as a relatively decent v1 that can handle 99% of our use-cases.

I think we can cover those edge cases and address that if we encounter them; but I don't believe we will be incorporating non-deterministic operators into our specification in the near-term.

I think if we encounter something, we should be able to extend the test specification.

[TotalTechGeek]

then a deep and strict equivalence is done to match the test result to the engines result. But can we conceive of cases that languages will have trouble with this
equivalent of "undefined" in whatever language the engine is implemented in

I think we'll allow implementers to define equivalence classes in their implementation of the test runner, particularly for that null case.

If the implementer wants to return their language's version of null rather than some weird JSONNull object, if it would be ergonomic for their users, I think that should be fine.

My reaction to undefined was partially driven by none of the currently defined operators being allowed to return undefined. { var: 'name' } where the data is {} would return null.

[TotalTechGeek]

Or cases where the result is non-deterministic?

@beeme1mr Brought up Cucumber in the compat table discussion, and I believe if we encounter more significant scenarios that require non-deterministic outputs (like a spec that uses the current time), Cucumber could help provide the scaffolding necessary to validate these use cases.

I think it tends to be overkill for unit testing, but if we need to measure side-effects or non-deterministic results, it can be an ideal framework for that.

[gregsdennis]

I think part of the confusion is that you're trying to solve two problems simultaneously. Separate the chaining from the "sugaring adjustment".

{ "cat": { "var": "text" } }

I propose that there be a two step process.

1. Resolve all vars, and do so iteratively until there are no more.
2. Process the result as is.

This means that { "var": "text" } in the above would be replaced with the data value for text. If that's an array, e.g. ["x", "y"], then process cat("x", "y"). If it's a single value, you first apply the sugaring and then process.

The obvious gotcha is how to retain an array in the data as a single value. The answer is to require that it explicitly contained in an array.

{ "cat": [{ "var": "text" }] }

This kind of thing is done all the time in other syntaxes and languages, where the shorthand can only be used in specific cases, and for other scenarios the full syntax MUST be used. For example, in RFC 9535 (JSON Path), the dot -syntax for a property selector (.foo) can only be used if the property name is alphanumeric (plus underscore) and doesn't start with a number. Otherwise, you must use the bracketed syntax (['$foo']).

It's perfectly acceptable to have cases where the sugaring can't be used as long as we can specify it well, and I think we can in this case.

[gregsdennis]

concat([], apply 1)

This syntax doesn't make sense to me. I would avoid using language (even pseudo-language) syntax.

[TotalTechGeek]

So the process would be resolved bottom-up, that is, from leaf to root.

If at any point a non-array is returned where an array is required, it is "wrapped" in an array and processed accordingly. If the logic author wants an array to be considered as a value, they MUST explicitly include the "parameter" array.

👍🏻

This syntax doesn't make sense to me.

Did my follow up post below make the syntax / expectation a bit more clear?

Breaking down what I meant, concat([], apply x) was my way of saying "x is evaluated, and then concatenated against an empty array".

In JavaScript, when you concat an array to an array, it joins them to create one array. concat([], ['a']) becomes ['a'], and if you concat a value to an array, it becomes an element of the array, concat([], 'a') becomes ['a']

apply 1 just evaluates to 1, while apply { "+": [1,2] } becomes 3

We need to be careful about specifying this in a language/implementation-agnostic way.

Fully agree, I was being a bit loose in my description.

I do think that in order to support map, if, and similar operators, there does need to be some concept of being able to label operators as "lazily evaluated" to give the operator complete control of how it interprets the arguments.

When an operator is lazy or not automatically traversed or functional however we want to represent that, the leaf is not automatically evaluated.

[TotalTechGeek]

My implementation doesn't perform this wrapping and instead merely operates directly on the value. I'm not sure the site [strictly speaking, it's not a spec] explicitly declares that a raw value is shorthand for a parameter array; just that the system also works with raw values. It does say "we support syntactic sugar on unary operators to skip the array around values", but that's hardly instruction to an implementation that raw values should be treated as param arrays. If this is the direction we want to take this, which could make sense, this operation needs to be more explicit in a specification.

Mine actually did not explicitly perform this either until last night.
json-logic/json-logic-engine@8932bcf#diff-9f6a5a68bd112cdd9fc270b72abdf7addf40ce02b6e26a036012ec0b820d2a4bL74

But this makes implementing custom operators much more consistent (so you will always receive an arguments array), and many implementations have implemented this, so I've implemented it.

[gregsdennis]

x is evaluated, and then concatenated against an empty array

No, this doesn't make sense. Maybe "concatenate" is the wrong word to use. You can't concatenate non-arrays (strings are just char arrays). You can append a value to an array.

The site uses the word "cast", which I'm not sure is exactly precise, but it does make more sense than "concatenate".

When an operator is lazy or not automatically traversed or functional however we want to represent that, the leaf is not automatically evaluated.

Even "laziness" can be an implementation decision. We don't want to influence implementations. We state requirements.

[TotalTechGeek]

Maybe "concatenate" is the wrong word to use. You can't concatenate non-arrays

A few languages have called this operation concatenation (like Haskell), perhaps it's the wrong term to use; but I'm willing to defer on terminology; I don't really mind.

Even "laziness" can be an implementation decision. We don't want to influence implementations. We state requirements.

OK.

How do we want to state that operations like '+', '-', 'cat', and should expect to have this mechanism applied,

While operations like map, filter, reduce are not evaluated in this way.

[TotalTechGeek]

And maybe I'll explain it in one more way (though I'll be repeating some things):

Right now, traversed arguments are passed to methods with the following rule:

args is array ? eval args : [eval args]

eval x in this is notation to execute logic.

• eval 1 is 1
• eval [1] is [1]
• eval { "+": [1, 2] } is 3

Imagine { "log": args }

With the current rule, given data is {"a": 1, "b": [1], "c": [1, 2] } when args is:

• 1, the output is [1]
• "hello", the output is ["hello"]
• [1] the output is [1]
• [1,2] the output is [1, 2]
• {"var": "a"} the output is [1]
• {"var": "b"} the output is [[1]]
• [{"var": "b"}] the output is [[1]]
• {"var": "c"} the output is [[1, 2]]
• [{"var": "c"}] the output is [[1, 2]]
• {"map": [[1,2], {"+": [{"var":""}, 1]}]} the output is [[2,3]]

I'd like to adjust this slightly, and I believe it would be a backwards compatible change because this is not done very often.

I'd like to switch the rule to y = eval args, y is array ? y : [y], alternatively written as concat([], eval args)

This tweak also passes every test in the public test suite.

With the proposed rule, given data is {"a": 1, "b": [1], "c": [1, 2] } when args is:

• 1, the output is [1]
• "hello", the output is ["hello"]
• [1] the output is [1]
• [1,2] the output is [1, 2]
• {"var": "a"} the output is [1]
• {"var": "b"} the output is [1]
• [{"var": "b"}] the output is [[1]]
• {"var": "c"} the output is [1, 2]
• [{"var": "c"}] the output is [[1, 2]]
• {"map": [[1,2], {"+": [{"var":""}, 1]}]} the output is [2,3]

This would make it significantly easier to chain logic together, so you could call max or + or cat on the output of another method easily.

[gregsdennis]

• args is array ? eval args : [eval args]
• y = eval args, y is array ? y : [y]

These are the same. In both cases, you're processing the eval args and determining if the output is an array. If not, wrap it in an array.

I'm suggesting that we do it on input. Something more akin to

args is array ? eval args : eval [args]

or 

y = args is array ? args : [args], eval y

Doing this, you can define that specific operators always take arrays of arguments, and that there's a potential "wrap" in order to adapt arguments. Other operators might explicitly take values, and this "wrapping" doesn't occur.

[gregsdennis]

I do think we're arguing for the same thing, though.

[TotalTechGeek]

I think we're arguing for the same thing -- I think we're missing each other on the best way to represent it / how we understand it to work 😂

That first bullet point is supposed to represent: "If the argument on input is an array, eval it, or wrap it in an array and eval it"
The second bullet point represents "If the argument on output is an array, pass it along or wrap it"

I think we're aiming for the same outcome,

I appreciate your input.

I don't think args is array ? eval args : eval [args] will work, because if args returns an array, like { "var": "data" }, it'll be inside of an array, unless we have a difference in how we interpret eval [args], where it gets unwrapped from the array automatically once evaluated. Which might be because I introduced a pseudosyntax without explaining how it works / gets interpreted 😳

[TotalTechGeek]

My current conceptual understanding for JSON Logic is that there are two classifications of operator,

• Operators that are resolved from the leaf, and MUST coerce the leaf to an array upon evaluation. Eagerly traversed. (I liked your explanation here: https://github.com/orgs/json-logic/discussions/2#discussioncomment-11477112)
• Operators that are resolved from the root, where the raw arguments are passed along to the operator.

Those might need rephrased or massaging. I welcome your input.

Most of the operators in JSON Logic will be of the first category. +, -, *, cat, max, min, var.
Even the unary operators, as the unary operators will only look at the first argument of the argument list they receive. So ! and !! and so on.

The higher order operators like map, filter, reduce, if are responsible for traversing and handling the raw arguments, without any casting / coercion.

I believe this convention makes it simple for folks to implement custom operators with semi-consistent types. But conventions are not ideal for specs.

I suppose we could omit the traversal piece, since that could be implementation dependent. datalogic-rs evaluated the arguments within the + operator directly.

But I believe to ensure { "max": { "var: "data" } } works as intended and maximum compatibility between operators, the array coercion must happen once the leaf is evaluated.