Skip to content

Commit

Permalink
Fix -1 not in [] expressions (#590)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ckganesan @antonmedv
ckganesan authored and antonmedv committed Apr 13, 2024
1 parent ded019d commit 0e9136e
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 46 deletions.
33 changes: 33 additions & 0 deletions compiler/compiler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,39 @@ func TestCompile_optimizes_jumps(t *testing.T) {
{vm.OpFetch, 0},
},
},
{
`-1 not in [1, 2, 5]`,
[]op{
{vm.OpPush, 0},
{vm.OpPush, 1},
{vm.OpIn, 0},
{vm.OpNot, 0},
},
},
{
`1 + 8 not in [1, 2, 5]`,
[]op{
{vm.OpPush, 0},
{vm.OpPush, 1},
{vm.OpIn, 0},
{vm.OpNot, 0},
},
},
{
`true ? false : 8 not in [1, 2, 5]`,
[]op{
{vm.OpTrue, 0},
{vm.OpJumpIfFalse, 3},
{vm.OpPop, 0},
{vm.OpFalse, 0},
{vm.OpJump, 5},
{vm.OpPop, 0},
{vm.OpPush, 0},
{vm.OpPush, 1},
{vm.OpIn, 0},
{vm.OpNot, 0},
},
},
}

for _, test := range tests {
Expand Down
8 changes: 8 additions & 0 deletions expr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,10 @@ func TestExpr(t *testing.T) {
`Two not in 0..1`,
true,
},
{
`-1 not in [1]`,
true,
},
{
`Int32 in [10, 20]`,
false,
Expand All @@ -797,6 +801,10 @@ func TestExpr(t *testing.T) {
`String matches ("^" + String + "$")`,
true,
},
{
`'foo' + 'bar' not matches 'foobar'`,
false,
},
{
`"foobar" contains "bar"`,
true,
Expand Down
9 changes: 9 additions & 0 deletions parser/operator/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ func IsBoolean(op string) bool {
return op == "and" || op == "or" || op == "&&" || op == "||"
}

func AllowedNegateSuffix(op string) bool {
switch op {
case "contains", "matches", "startsWith", "endsWith", "in":
return true
default:
return false
}
}

var Unary = map[string]Operator{
"not": {50, Left},
"!": {50, Left},
Expand Down
99 changes: 53 additions & 46 deletions parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,73 +126,80 @@ func (p *parser) expect(kind Kind, values ...string) {
// parse functions

func (p *parser) parseExpression(precedence int) Node {
if precedence == 0 {
if p.current.Is(Operator, "let") {
return p.parseVariableDeclaration()
}
if precedence == 0 && p.current.Is(Operator, "let") {
return p.parseVariableDeclaration()
}

nodeLeft := p.parsePrimary()

prevOperator := ""
opToken := p.current
for opToken.Is(Operator) && p.err == nil {
negate := false
negate := opToken.Is(Operator, "not")
var notToken Token

// Handle "not *" operator, like "not in" or "not contains".
if opToken.Is(Operator, "not") {
if negate {
currentPos := p.pos
p.next()
notToken = p.current
negate = true
opToken = p.current
if operator.AllowedNegateSuffix(p.current.Value) {
if op, ok := operator.Binary[p.current.Value]; ok && op.Precedence >= precedence {
notToken = p.current
opToken = p.current
} else {
p.pos = currentPos
p.current = opToken
break
}
} else {
p.error("unexpected token %v", p.current)
break
}
}

if op, ok := operator.Binary[opToken.Value]; ok {
if op.Precedence >= precedence {
p.next()
if op, ok := operator.Binary[opToken.Value]; ok && op.Precedence >= precedence {
p.next()

if opToken.Value == "|" {
identToken := p.current
p.expect(Identifier)
nodeLeft = p.parseCall(identToken, []Node{nodeLeft}, true)
goto next
}
if opToken.Value == "|" {
identToken := p.current
p.expect(Identifier)
nodeLeft = p.parseCall(identToken, []Node{nodeLeft}, true)
goto next
}

if prevOperator == "??" && opToken.Value != "??" && !opToken.Is(Bracket, "(") {
p.errorAt(opToken, "Operator (%v) and coalesce expressions (??) cannot be mixed. Wrap either by parentheses.", opToken.Value)
break
}
if prevOperator == "??" && opToken.Value != "??" && !opToken.Is(Bracket, "(") {
p.errorAt(opToken, "Operator (%v) and coalesce expressions (??) cannot be mixed. Wrap either by parentheses.", opToken.Value)
break
}

if operator.IsComparison(opToken.Value) {
nodeLeft = p.parseComparison(nodeLeft, opToken, op.Precedence)
goto next
}
if operator.IsComparison(opToken.Value) {
nodeLeft = p.parseComparison(nodeLeft, opToken, op.Precedence)
goto next
}

var nodeRight Node
if op.Associativity == operator.Left {
nodeRight = p.parseExpression(op.Precedence + 1)
} else {
nodeRight = p.parseExpression(op.Precedence)
}
var nodeRight Node
if op.Associativity == operator.Left {
nodeRight = p.parseExpression(op.Precedence + 1)
} else {
nodeRight = p.parseExpression(op.Precedence)
}

nodeLeft = &BinaryNode{
Operator: opToken.Value,
Left: nodeLeft,
Right: nodeRight,
}
nodeLeft.SetLocation(opToken.Location)
nodeLeft = &BinaryNode{
Operator: opToken.Value,
Left: nodeLeft,
Right: nodeRight,
}
nodeLeft.SetLocation(opToken.Location)

if negate {
nodeLeft = &UnaryNode{
Operator: "not",
Node: nodeLeft,
}
nodeLeft.SetLocation(notToken.Location)
if negate {
nodeLeft = &UnaryNode{
Operator: "not",
Node: nodeLeft,
}

goto next
nodeLeft.SetLocation(notToken.Location)
}

goto next
}
break

Expand Down
61 changes: 61 additions & 0 deletions parser/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,62 @@ world`},
&UnaryNode{Operator: "not",
Node: &IdentifierNode{Value: "in_var"}},
},
{
"-1 not in [1, 2, 3, 4]",
&UnaryNode{Operator: "not",
Node: &BinaryNode{Operator: "in",
Left: &UnaryNode{Operator: "-", Node: &IntegerNode{Value: 1}},
Right: &ArrayNode{Nodes: []Node{
&IntegerNode{Value: 1},
&IntegerNode{Value: 2},
&IntegerNode{Value: 3},
&IntegerNode{Value: 4},
}}}},
},
{
"1*8 not in [1, 2, 3, 4]",
&UnaryNode{Operator: "not",
Node: &BinaryNode{Operator: "in",
Left: &BinaryNode{Operator: "*",
Left: &IntegerNode{Value: 1},
Right: &IntegerNode{Value: 8},
},
Right: &ArrayNode{Nodes: []Node{
&IntegerNode{Value: 1},
&IntegerNode{Value: 2},
&IntegerNode{Value: 3},
&IntegerNode{Value: 4},
}}}},
},
{
"2==2 ? false : 3 not in [1, 2, 5]",
&ConditionalNode{
Cond: &BinaryNode{
Operator: "==",
Left: &IntegerNode{Value: 2},
Right: &IntegerNode{Value: 2},
},
Exp1: &BoolNode{Value: false},
Exp2: &UnaryNode{
Operator: "not",
Node: &BinaryNode{
Operator: "in",
Left: &IntegerNode{Value: 3},
Right: &ArrayNode{Nodes: []Node{
&IntegerNode{Value: 1},
&IntegerNode{Value: 2},
&IntegerNode{Value: 5},
}}}}},
},
{
"'foo' + 'bar' not matches 'foobar'",
&UnaryNode{Operator: "not",
Node: &BinaryNode{Operator: "matches",
Left: &BinaryNode{Operator: "+",
Left: &StringNode{Value: "foo"},
Right: &StringNode{Value: "bar"}},
Right: &StringNode{Value: "foobar"}}},
},
{
"all(Tickets, #)",
&BuiltinNode{
Expand Down Expand Up @@ -706,6 +762,11 @@ invalid float literal: strconv.ParseFloat: parsing "0o1E+1": invalid syntax (1:6
invalid float literal: strconv.ParseFloat: parsing "1E": invalid syntax (1:2)
| 1E
| .^
1 not == [1, 2, 5]
unexpected token Operator("==") (1:7)
| 1 not == [1, 2, 5]
| ......^
`

func TestParse_error(t *testing.T) {
Expand Down

0 comments on commit 0e9136e

Please sign in to comment.