Logic error when evaluating with semvers

[hairyhenderson]

Given an input of {"x": "abc", "y": "11.3.0"}, I expect these two rules to behave the same:

• y gt 11.1.0-0 and not(x IN ["cde","fgh"])
• not(x IN ["cde","fgh"]) and y gt 11.1.0-0

But they do not - the first evaluates to true (incorrect) while the second evaluates to false (correct). Order seems to matter where it shouldn't.

I believe this to be a bug specifically with version handling, because using numbers instead (5 in the input and gt 4 in the rules) results in the same result.

Here's a couple unit tests that illustrate the problem (adapted from the tests in this repo):

package testrules

import (
	"fmt"
	"testing"

	"github.com/nikunjy/rules/parser"
	"github.com/stretchr/testify/assert"
)

func TestAndWithVersionAndStrings(t *testing.T) {
	input := map[string]any{
		"x": "abc",
		"y": "11.3.0",
	}

	tests := []struct {
		expected bool
		rule     string
	}{
		// true and not(true) is false
		{false, `y gt 11.1.0-0 and not(x IN ["abc","cde","fgh"])`},
		// not(true) and true is false
		{false, `not(x IN ["abc","cde","fgh"]) and y gt 11.1.0-0`},
		// true and not(false) is true
		{true, `y gt 11.1.0-0 and not(x IN ["cde","fgh"])`},
		// not(false) and true is true
		{true, `not(x IN ["cde","fgh"]) and y gt 11.1.0-0`},
	}

	for i, tt := range tests {
		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
			assert.Equal(t, tt.expected, parser.Evaluate(tt.rule, input))
		})
	}
}

func TestAndWithNumberAndStrings(t *testing.T) {
	input := map[string]any{
		"x": "abc",
		"y": 5,
	}

	tests := []struct {
		expected bool
		rule     string
	}{
		// true and not(true) is false
		{false, `y gt 4 and not(x IN ["abc","cde","fgh"])`},
		// not(true) and true is false
		{false, `not(x IN ["abc","cde","fgh"]) and y gt 4`},
		// true and not(false) is true
		{true, `y gt 4 and not(x IN ["cde","fgh"])`},
		// not(false) and true is true
		{true, `not(x IN ["cde","fgh"]) and y gt 4`},
	}

	for i, tt := range tests {
		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
			assert.Equal(t, tt.expected, parser.Evaluate(tt.rule, input))
		})
	}
}

The result of running this is:

$ go test .

--- FAIL: TestAndWithVersionAndStrings (0.00s)
    --- FAIL: TestAndWithVersionAndStrings/0 (0.00s)
        testrules_test.go:33: 
            	Error Trace:	testrules_test.go:33
            	Error:      	Not equal: 
            	            	expected: false
            	            	actual  : true
            	Test:       	TestAndWithVersionAndStrings/0
FAIL
coverage: [no statements]
FAIL	example.com/testrules	0.168s
FAIL

[hairyhenderson]

🤔 I think the bug is slightly different than what I thought - if I change the versions to not contain prerelease modifiers (i.e. 11.1.0 rather than 11.1.0-0), the tests pass...

Perhaps there's a parsing bug here?

[hairyhenderson]

Another strange thing I'm noticing:

• x gt 2.1.0-0 and not(y in ["a", "b", "c"]) fails
• (x gt 2.1.0-0) and not(y in ["a", "b", "c"]) fails
• ((x gt 2.1.0-0) and not(y in ["a", "b", "c"])) works as expected
• (x gt 2.1.0-0 and not(y in ["a", "b", "c"])) fails

Something to do with how clauses are surrounded by parentheses is changing how the rule is parsed...

[nikunjy]

I do not have time to fix this unfortunately. I can review and accept PR if you have one @hairyhenderson