Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support optional array, string access and deep nil coalescing #583

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: support optional array, string access and deep nil coalescing #583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,12 @@ func (v *checker) MemberNode(node *ast.MemberNode) (reflect.Type, info) {
}
return base.Elem(), info{}

case reflect.String:
if !isInteger(prop) && !isAny(prop) {
return v.error(node.Property, "string elements can only be selected using an integer (got %v)", prop)
}
return stringType, info{}

case reflect.Struct:
if name, ok := node.Property.(*ast.StringNode); ok {
propertyName := name.Value
Expand Down
6 changes: 5 additions & 1 deletion compiler/compiler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,11 @@ func (c *compiler) MemberNode(node *ast.MemberNode) {

if op == OpFetch {
c.compile(node.Property)
c.emit(OpFetch)
if node.Optional {
c.emit(OpOptionalFetch)
} else {
c.emit(op)
}
} else {
c.emitLocation(node.Location(), op, c.addConstant(
&runtime.Field{Index: index, Path: path},
Expand Down
82 changes: 79 additions & 3 deletions compiler/compiler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,71 @@ func TestCompile_optimizes_jumps(t *testing.T) {
{vm.OpNil, 0},
},
},
{
`let m = {"a": {"b": {"c": 1}}}; m.a.b.c ?? 'nil coalescing'`,
[]op{
{vm.OpPush, 0},
{vm.OpPush, 1},
{vm.OpPush, 2},
{vm.OpPush, 3},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpStore, 0},
{vm.OpLoadVar, 0},
{vm.OpJumpIfNil, 8},
{vm.OpPush, 0},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 5},
{vm.OpPush, 1},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 2},
{vm.OpPush, 2},
{vm.OpOptionalFetch, 0},
{vm.OpDeref, 0},
{vm.OpJumpIfNotNil, 2},
{vm.OpPop, 0},
{vm.OpPush, 4},
},
},
{
`let m = [{"a": {"b": {"c": 1}}}]; m[5].a.b.c ?? 'nil coalescing'`,
[]op{
{vm.OpPush, 0},
{vm.OpPush, 1},
{vm.OpPush, 2},
{vm.OpPush, 3},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpPush, 3},
{vm.OpMap, 0},
{vm.OpPush, 3},
{vm.OpArray, 0},
{vm.OpStore, 0},
{vm.OpLoadVar, 0},
{vm.OpJumpIfNil, 11},
{vm.OpPush, 4},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 8},
{vm.OpPush, 0},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 5},
{vm.OpPush, 1},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 2},
{vm.OpPush, 2},
{vm.OpOptionalFetch, 0},
{vm.OpDeref, 0},
{vm.OpJumpIfNotNil, 2},
{vm.OpPop, 0},
{vm.OpPush, 5},
},
},
{
`let m = {"a": {"b": {"c": 1}}}; m?.a?.b?.c`,
[]op{
Expand All @@ -532,13 +597,24 @@ func TestCompile_optimizes_jumps(t *testing.T) {
{vm.OpLoadVar, 0},
{vm.OpJumpIfNil, 8},
{vm.OpPush, 0},
{vm.OpFetch, 0},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 5},
{vm.OpPush, 1},
{vm.OpFetch, 0},
{vm.OpOptionalFetch, 0},
{vm.OpJumpIfNil, 2},
{vm.OpPush, 2},
{vm.OpFetch, 0},
{vm.OpOptionalFetch, 0},
},
},
{
`let m = [1, 2, 3]; m?.[5]`,
[]op{
{vm.OpPush, 0},
{vm.OpStore, 0},
{vm.OpLoadVar, 0},
{vm.OpJumpIfNil, 2},
{vm.OpPush, 1},
{vm.OpOptionalFetch, 0},
},
},
}
Expand Down
30 changes: 30 additions & 0 deletions expr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,18 @@ func TestExpr(t *testing.T) {
`1 < 2 < 3 == true`,
true,
},
{
`[1, 2, 3]?.[5]`,
nil,
},
{
`'string'?.[5]`,
"g",
},
{
`'string'?.[7]`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not exactly correct behavior I'm looking for.

In case array?.[7] operator ?. checks what array itself defined and not nil.

So the code

array?.[7]

will panic in there is no 7 element index.

What we can do is to alter [7] behavior with operator ?. after it.

array[7]?.field

In this case we can alter OpFetch and do not panic if 7 index does not exist.

This mean we, unfortunately, does not have a syntax to alter [7] behavior if it is the last postfix op.

But we can alter the whole "chain" in presence of ?? operator!

array[7].field ?? 'default'

nil,
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -1967,6 +1979,24 @@ func TestRun_NilCoalescingOperator(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "default", out)
})

t.Run("default without chain", func(t *testing.T) {
p, err := expr.Compile(`foo.foo.bar ?? "default"`, expr.Env(env))
assert.NoError(t, err)

out, err := expr.Run(p, map[string]any{})
assert.NoError(t, err)
assert.Equal(t, "default", out)
})

t.Run("array default without chain", func(t *testing.T) {
p, err := expr.Compile(`foo.foo[10].bar ?? "default"`, expr.Env(env))
assert.NoError(t, err)

out, err := expr.Run(p, map[string]any{})
assert.NoError(t, err)
assert.Equal(t, "default", out)
})
}

func TestEval_nil_in_maps(t *testing.T) {
Expand Down
126 changes: 67 additions & 59 deletions parser/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ func (p *parser) parseExpression(precedence int) Node {
nodeRight = p.parseExpression(op.Precedence)
}

if opToken.Value == "??" {
nodeLeft = p.flattenChainedOptionals(nodeLeft)
}

nodeLeft = &BinaryNode{
Operator: opToken.Value,
Left: nodeLeft,
Expand Down Expand Up @@ -579,65 +583,66 @@ end:
}

func (p *parser) parsePostfixExpression(node Node) Node {
postfixToken := p.current
for (postfixToken.Is(Operator) || postfixToken.Is(Bracket)) && p.err == nil {
optional := postfixToken.Value == "?."
parseToken:
if postfixToken.Value == "." || postfixToken.Value == "?." {
p.next()

propertyToken := p.current
if optional && propertyToken.Is(Bracket, "[") {
postfixToken = propertyToken
goto parseToken
}
for postfixToken := p.current; (postfixToken.Is(Operator) || postfixToken.Is(Bracket)) && p.err == nil; postfixToken = p.current {
switch postfixToken.Value {
case ".", "?.":
p.next()

if propertyToken.Kind != Identifier &&
// Operators like "not" and "matches" are valid methods or property names.
(propertyToken.Kind != Operator || !utils.IsValidIdentifier(propertyToken.Value)) {
p.error("expected name")
}

property := &StringNode{Value: propertyToken.Value}
property.SetLocation(propertyToken.Location)

chainNode, isChain := node.(*ChainNode)
optional := postfixToken.Value == "?."

if isChain {
node = chainNode.Node
}
propertyToken := p.current
isOptional := postfixToken.Value == "?."

memberNode := &MemberNode{
Node: node,
Property: property,
Optional: optional,
}
memberNode.SetLocation(propertyToken.Location)

if p.current.Is(Bracket, "(") {
memberNode.Method = true
node = &CallNode{
Callee: memberNode,
Arguments: p.parseArguments([]Node{}),
if isOptional && p.current.Is(Bracket, "[") {
p.next()
node = &MemberNode{
Node: node,
Property: p.parseExpression(0),
Optional: true,
}
node.SetLocation(propertyToken.Location)
p.expect(Bracket, "]")
} else {
node = memberNode
}
p.next()

if propertyToken.Kind != Identifier &&
// Operators like "not" and "matches" are valid methods or property names.
(propertyToken.Kind != Operator || !utils.IsValidIdentifier(propertyToken.Value)) {
p.error("expected name")
}

if isChain || optional {
property := &StringNode{Value: propertyToken.Value}
property.SetLocation(propertyToken.Location)

memberNode := &MemberNode{
Node: node,
Property: property,
Optional: isOptional,
}
memberNode.SetLocation(propertyToken.Location)

if p.current.Is(Bracket, "(") {
memberNode.Method = true
node = &CallNode{
Callee: memberNode,
Arguments: p.parseArguments([]Node{}),
}
node.SetLocation(propertyToken.Location)
} else {
node = memberNode
}
}
if isChain || isOptional {
node = &ChainNode{Node: node}
}

} else if postfixToken.Value == "[" {
case "[":
p.next()
var from, to Node

if p.current.Is(Operator, ":") { // slice without from [:1]
p.next()

var to Node
if !p.current.Is(Bracket, "]") { // slice without from and to [:]
to = p.parseExpression(0)
}
Expand All @@ -646,16 +651,13 @@ func (p *parser) parsePostfixExpression(node Node) Node {
Node: node,
To: to,
}
node.SetLocation(postfixToken.Location)
p.expect(Bracket, "]")

} else {

from = p.parseExpression(0)
from := p.parseExpression(0)

if p.current.Is(Operator, ":") {
p.next()

var to Node
if !p.current.Is(Bracket, "]") { // slice without to [1:]
to = p.parseExpression(0)
}
Expand All @@ -665,28 +667,20 @@ func (p *parser) parsePostfixExpression(node Node) Node {
From: from,
To: to,
}
node.SetLocation(postfixToken.Location)
p.expect(Bracket, "]")

} else {
// Slice operator [:] was not found,
// it should be just an index node.
node = &MemberNode{
Node: node,
Property: from,
Optional: optional,
}
node.SetLocation(postfixToken.Location)
if optional {
node = &ChainNode{Node: node}
}
p.expect(Bracket, "]")
}
}
} else {
break
node.SetLocation(postfixToken.Location)
p.expect(Bracket, "]")
default:
return node
}
postfixToken = p.current
}
return node
}
Expand Down Expand Up @@ -721,3 +715,17 @@ func (p *parser) parseComparison(left Node, token Token, precedence int) Node {
}
return rootNode
}

func (p *parser) flattenChainedOptionals(nodeLeft Node) Node {
switch node := nodeLeft.(type) {
case *MemberNode:
node.Optional = true
node.Node = p.flattenChainedOptionals(node.Node)
return &ChainNode{Node: node}
case *ChainNode:
node.Node = p.flattenChainedOptionals(node.Node)
return node
default:
return nodeLeft
}
}
19 changes: 19 additions & 0 deletions parser/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,25 @@ world`},
Right: &IdentifierNode{Value: "bar"}},
Right: &IdentifierNode{Value: "baz"}},
},
{
"foo.bar.baz ?? 'nil'",
&BinaryNode{Operator: "??",
Left: &ChainNode{
Node: &MemberNode{
Node: &ChainNode{
Node: &MemberNode{
Node: &IdentifierNode{Value: "foo"},
Property: &StringNode{Value: "bar"},
Optional: true,
},
},
Property: &StringNode{Value: "baz"},
Optional: true,
},
},
Right: &StringNode{Value: "nil"},
},
},
{
"foo ?? (bar || baz)",
&BinaryNode{Operator: "??",
Expand Down
1 change: 1 addition & 0 deletions vm/opcodes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (
OpLoadEnv
OpFetch
OpFetchField
OpOptionalFetch
OpMethod
OpTrue
OpFalse
Expand Down
Loading
Loading