ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

Negation operator is not working #158

Closed jaroslawherod closed 8 months ago

jaroslawherod commented 9 months ago
func TestScript_Match(t *testing.T) {
    data := map[string]any{
        "text": "my Expected text NotExpected",
    }
    // Switch to only negation  !(@.text ~= /(?i)notexpected/) cause parsing error 
    expr := jp.MustNewScript("( @.text ~= /(?i)expected/) && !(@.text ~= /(?i)notexpected/)") 
    assert.False(t, expr.Match(data))
}

The script should match the text field if contains expected text but not notexpected. Expected:

Actual:

Is the wrong usage of negation operator or that's parsing issue ?

ohler55 commented 9 months ago

I'll look into it.

ohler55 commented 9 months ago

Please try the develop branch. It has the fix. Released v1.21.1 with the fix.

jaroslawherod commented 9 months ago

I've tested it with version 1.21.1 but it doesn't work . The test that I've added initially is not passing. Additionally such expression is failing to parse Test:

func TestScript_Match(t *testing.T) {
    data := map[string]any{
        "text": "my Expected text NotExpected",
    }
    expr := jp.MustNewScript("  @.text ~= /(?i)expected/ && !(@.text ~= /(?i)notexpected/)")
    assert.False(t, expr.Match(data))
}

with

panic: expected a value at 31 in   @.text ~= /(?i)expected/ && !(@.text ~= /(?i)notexpected/) [recovered]
    panic: expected a value at 31 in   @.text ~= /(?i)expected/ && !(@.text ~= /(?i)notexpected/)

goroutine 8 [running]:
testing.tRunner.func1.2({0x5f69e0, 0xc00002e490})
    /usr/lib/go-1.21/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
    /usr/lib/go-1.21/src/testing/testing.go:1548 +0x397
panic({0x5f69e0?, 0xc00002e490?})
    /usr/lib/go-1.21/src/runtime/panic.go:914 +0x21f
github.com/ohler55/ojg/jp.(*parser).raise(0xc000059ef0, {0x635773?, 0x28?}, {0x0?, 0x0?, 0x7f979efb6e78?})
    /home/jaro/go/pkg/mod/github.com/ohler55/ojg@v1.21.1/jp/parse.go:779 +0xd4
github.com/ohler55/ojg/jp.(*parser).readEqValue(0xc000059ef0?)
    /home/jaro/go/pkg/mod/github.com/ohler55/ojg@v1.21.1/jp/parse.go:662 +0x4d3
github.com/ohler55/ojg/jp.(*parser).readEquation(0xc00012bef0)
    /home/jaro/go/pkg/mod/github.com/ohler55/ojg@v1.21.1/jp/parse.go:603 +0x336
github.com/ohler55/ojg/jp.MustNewScript({0x63f857?, 0xc000100f90?})
    /home/jaro/go/pkg/mod/github.com/ohler55/ojg@v1.21.1/jp/script.go:113 +0xb9
ohler55 commented 9 months ago

I guess I need to expand my tests. Thanks for your patients.

jaroslawherod commented 9 months ago

No worries :) I'm glad that you are so committed to provide fixes

ohler55 commented 8 months ago

I ended up reworking the parsing. I think overall it is better and added your equation into the unit tests. Please checkout the develop branch. I've got another issue to resolved before releasing but please feel free to test.

ohler55 commented 8 months ago

Released v1.21.2 with the fix.

jaroslawherod commented 8 months ago

Test case still fails :

func TestScript_Match(t *testing.T) {
    data := map[string]any{
        "text": "my Expected text NotExpected",
    }
    expr := jp.MustNewScript("!(@.text ~= /(?i)NotExpected/)")
    assert.False(t, expr.Match(data))
}

I've simplified it to something like that but it still returns "true" instead of "false". Maybe I'm doing something wrong here ?

ohler55 commented 8 months ago

I was focused on getting the parsing and should have spent more time on the evaluation. I'll get that fixed. Sorry.

ohler55 commented 8 months ago

Please try the "group-eval-fix" branch. I have to clean it up and there is an issue with printing equations but I think that fixes the evaluation of the expression you have above.

Updated. I believe it is now ready for a release.

ohler55 commented 8 months ago

Released v1.21.3 with the fix.