advantagefse / json-logic-swift

A native Swift JsonLogic implementation. This parser accepts JsonLogic rules and executes them.
MIT License
25 stars 22 forks source link

DoubleNegation value parse #21

Closed Rep2 closed 2 years ago

Rep2 commented 3 years ago

Supplying value[0] in DoubleNegation mapping presumed that value is array, which isn't necessarily true. For example for this case the mapping failed and threw .Error(.notSubscriptableType(self.type))

{
  "!!": {
    "missing": [
      "country"
    ]
  }
}

Was value[0] supplied for any specific purpose?

csknns commented 2 years ago

Hello @Rep2 , thanks for your contribution!

The reason for this implementation is because json-logic uses an array for the argument list of an operator. So if after the double negation operator we have an array then that is evaluated as "truthy" is not the array but the contents of the array (the first item specifically). However it seems that the JS implementation accepts also a single argument that is not in an array so you are right this is a bug!!

However with your change the implementation will not work correctly because it is expected that then we have an array we should evaluate the first item for example this { "!!" : [ [] ] } should evaluate to false. If you run the unit tests some will fail. But I think with a small change we can fix it:

struct DoubleNegation: Expression {
    let arg: Expression

    func evalWithData(_ data: JSON?) throws -> JSON {
        let data = try arg.evalWithData(data)
        guard case let JSON.Array(array) = data else {
            return JSON.Bool(data.truthy())
        }
        if let firstItem = array.first {
            return JSON.Bool(firstItem.truthy())
        }
        return JSON.Bool(false)
    }
}

Also if you can pls add in the DoubleNegationTests some unit test for this case:

    func testDoubleNegation_withObject() {
        XCTAssertEqual(true, try applyRule("""
            { "!!" : true }
            """, to: nil))

        XCTAssertEqual(false, try applyRule("""
            { "!!" : false }
            """, to: nil))

        XCTAssertEqual(true, try applyRule("""
            { "!!" : "0" }
            """, to: nil))
    }
csknns commented 2 years ago

I am merging this and I will push the extra code snippets in the develop :)