From d79bb05a92b355d3319c74c490fcce6a779959a4 Mon Sep 17 00:00:00 2001 From: ckganesan Date: Fri, 1 Mar 2024 23:07:31 +0530 Subject: [PATCH 1/2] Resolved 'Not' Operator Issue in Binary Node Expression --- compiler/compiler_test.go | 42 ++++++++++++++++++++++ expr_test.go | 8 +++++ parser/operator/operator.go | 9 +++++ parser/parser.go | 71 +++++++++++++++++-------------------- parser/parser_test.go | 61 +++++++++++++++++++++++++++++++ 5 files changed, 153 insertions(+), 38 deletions(-) diff --git a/compiler/compiler_test.go b/compiler/compiler_test.go index 741142a7..01233848 100644 --- a/compiler/compiler_test.go +++ b/compiler/compiler_test.go @@ -541,6 +541,48 @@ 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 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 { diff --git a/expr_test.go b/expr_test.go index 9d9d88af..92343e22 100644 --- a/expr_test.go +++ b/expr_test.go @@ -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, @@ -797,6 +801,10 @@ func TestExpr(t *testing.T) { `String matches ("^" + String + "$")`, true, }, + { + `'foo' + 'bar' not matches 'foobar'`, + false, + }, { `"foobar" contains "bar"`, true, diff --git a/parser/operator/operator.go b/parser/operator/operator.go index 411a0e2b..8535d4aa 100644 --- a/parser/operator/operator.go +++ b/parser/operator/operator.go @@ -20,6 +20,15 @@ func IsBoolean(op string) bool { return op == "and" || op == "or" || op == "&&" || op == "||" } +func IsFunc(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}, diff --git a/parser/parser.go b/parser/parser.go index 9114bc0c..cfc9297f 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -126,48 +126,48 @@ 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 + for opToken := p.current; opToken.Is(Operator) && p.err == nil; opToken = p.current { var notToken Token + negate := opToken.Is(Operator, "not") // 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 op, ok := operator.Binary[opToken.Value]; ok { - if op.Precedence >= precedence { - p.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) + if operator.IsFunc(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 operator.IsComparison(opToken.Value) { - nodeLeft = p.parseComparison(nodeLeft, opToken, op.Precedence) - goto 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) + } else 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 + } else if operator.IsComparison(opToken.Value) { + nodeLeft = p.parseComparison(nodeLeft, opToken, op.Precedence) + } else { var nodeRight Node if op.Associativity == operator.Left { @@ -190,21 +190,16 @@ func (p *parser) parseExpression(precedence int) Node { } nodeLeft.SetLocation(notToken.Location) } - - goto next } + prevOperator = opToken.Value + } else { + break } - break - - next: - prevOperator = opToken.Value - opToken = p.current } if precedence == 0 { nodeLeft = p.parseConditional(nodeLeft) } - return nodeLeft } diff --git a/parser/parser_test.go b/parser/parser_test.go index 9225e102..2a30787a 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -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{ @@ -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) { From 7c44697d6d66d3dbc599f720f0a7d4e365791385 Mon Sep 17 00:00:00 2001 From: ckganesan Date: Mon, 4 Mar 2024 08:03:13 +0530 Subject: [PATCH 2/2] refactoring code as per review comment --- compiler/compiler_test.go | 9 ----- parser/operator/operator.go | 2 +- parser/parser.go | 66 ++++++++++++++++++++++--------------- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/compiler/compiler_test.go b/compiler/compiler_test.go index 01233848..fbd83ec8 100644 --- a/compiler/compiler_test.go +++ b/compiler/compiler_test.go @@ -550,15 +550,6 @@ func TestCompile_optimizes_jumps(t *testing.T) { {vm.OpNot, 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{ diff --git a/parser/operator/operator.go b/parser/operator/operator.go index 8535d4aa..4eeaf80e 100644 --- a/parser/operator/operator.go +++ b/parser/operator/operator.go @@ -20,7 +20,7 @@ func IsBoolean(op string) bool { return op == "and" || op == "or" || op == "&&" || op == "||" } -func IsFunc(op string) bool { +func AllowedNegateSuffix(op string) bool { switch op { case "contains", "matches", "startsWith", "endsWith", "in": return true diff --git a/parser/parser.go b/parser/parser.go index cfc9297f..9cb79cbb 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -133,15 +133,16 @@ func (p *parser) parseExpression(precedence int) Node { nodeLeft := p.parsePrimary() prevOperator := "" - for opToken := p.current; opToken.Is(Operator) && p.err == nil; opToken = p.current { + opToken := p.current + for opToken.Is(Operator) && p.err == nil { + negate := opToken.Is(Operator, "not") var notToken Token - negate := opToken.Is(Operator, "not") // Handle "not *" operator, like "not in" or "not contains". if negate { currentPos := p.pos p.next() - if operator.IsFunc(p.current.Value) { + if operator.AllowedNegateSuffix(p.current.Value) { if op, ok := operator.Binary[p.current.Value]; ok && op.Precedence >= precedence { notToken = p.current opToken = p.current @@ -158,48 +159,59 @@ func (p *parser) parseExpression(precedence int) Node { 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) - } else if prevOperator == "??" && opToken.Value != "??" && !opToken.Is(Bracket, "(") { + 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 - } else if operator.IsComparison(opToken.Value) { + } + + if operator.IsComparison(opToken.Value) { nodeLeft = p.parseComparison(nodeLeft, opToken, op.Precedence) - } else { + 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, } + nodeLeft.SetLocation(notToken.Location) } - prevOperator = opToken.Value - } else { - break + + goto next } + break + + next: + prevOperator = opToken.Value + opToken = p.current } if precedence == 0 { nodeLeft = p.parseConditional(nodeLeft) } + return nodeLeft }