diff --git a/ast/node.go b/ast/node.go index fbb9ae825..3ed49560b 100644 --- a/ast/node.go +++ b/ast/node.go @@ -168,9 +168,10 @@ type MemberNode struct { // array[1:4] type SliceNode struct { base - Node Node // Node of the slice. Like "array" in "array[1:4]". - From Node // From an index of the array. Like "1" in "array[1:4]". - To Node // To an index of the array. Like "4" in "array[1:4]". + Node Node // Node of the slice. Like "array" in "array[1:4]". + From Node // From an index of the array. Like "1" in "array[1:4]". + To Node // To an index of the array. Like "4" in "array[1:4]". + Optional bool // If true then the slice access is optional. Like "foo?.[1:4]". } // CallNode represents a function or a method call. diff --git a/ast/print.go b/ast/print.go index 1c197445e..0c32fa69d 100644 --- a/ast/print.go +++ b/ast/print.go @@ -162,16 +162,20 @@ func (n *MemberNode) String() string { } func (n *SliceNode) String() string { + op := "" + if n.Optional { + op = "?." + } if n.From == nil && n.To == nil { - return fmt.Sprintf("%s[:]", n.Node.String()) + return fmt.Sprintf("%s%s[:]", n.Node.String(), op) } if n.From == nil { - return fmt.Sprintf("%s[:%s]", n.Node.String(), n.To.String()) + return fmt.Sprintf("%s%s[:%s]", n.Node.String(), op, n.To.String()) } if n.To == nil { - return fmt.Sprintf("%s[%s:]", n.Node.String(), n.From.String()) + return fmt.Sprintf("%s%s[%s:]", n.Node.String(), op, n.From.String()) } - return fmt.Sprintf("%s[%s:%s]", n.Node.String(), n.From.String(), n.To.String()) + return fmt.Sprintf("%s%s[%s:%s]", n.Node.String(), op, n.From.String(), n.To.String()) } func (n *CallNode) String() string { diff --git a/ast/print_test.go b/ast/print_test.go index bcdad782c..6cc58695f 100644 --- a/ast/print_test.go +++ b/ast/print_test.go @@ -74,6 +74,10 @@ func TestPrint(t *testing.T) { {`a[1:]`, `a[1:]`}, {`a[1:]`, `a[1:]`}, {`a[:]`, `a[:]`}, + {`a?.[1:-1]`, `a?.[1:-1]`}, + {`a?.[1:]`, `a?.[1:]`}, + {`a?.[:1]`, `a?.[:1]`}, + {`a?.[:]`, `a?.[:]`}, {`(nil ?? 1) > 0`, `(nil ?? 1) > 0`}, {`{("a" + "b"): 42}`, `{("a" + "b"): 42}`}, {`(One == 1 ? true : false) && Two == 2`, `(One == 1 ? true : false) && Two == 2`}, diff --git a/checker/checker.go b/checker/checker.go index 63425af1f..6646ece26 100644 --- a/checker/checker.go +++ b/checker/checker.go @@ -622,7 +622,12 @@ func (v *Checker) sliceNode(node *ast.SliceNode) Nature { case reflect.String, reflect.Array, reflect.Slice: // ok default: - return v.error(node, "cannot slice %s", nt.String()) + // Optional chaining only guards against a nil value, so it must not + // hide the error for a type that can never be sliced. + if !node.Optional || !nt.Nil { + return v.error(node, "cannot slice %s", nt.String()) + } + nt = Nature{} } if node.From != nil { diff --git a/compiler/compiler.go b/compiler/compiler.go index 685175350..708dd63ac 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -760,6 +760,12 @@ func (c *compiler) MemberNode(node *ast.MemberNode) { func (c *compiler) SliceNode(node *ast.SliceNode) { c.compile(node.Node) + // If the slice is optional, we need to jump over the slice operation. + // If no ChainNode (none c.chains) is used, do not compile the optional slice. + if node.Optional && len(c.chains) > 0 { + ph := c.emit(OpJumpIfNil, placeholder) + c.chains[len(c.chains)-1] = append(c.chains[len(c.chains)-1], ph) + } if node.To != nil { c.compile(node.To) c.derefInNeeded(node.To) diff --git a/parser/parser.go b/parser/parser.go index 9e24a71e4..60f781602 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -848,12 +848,19 @@ func (p *Parser) parsePostfixExpression(node Node) Node { } node = p.createNode(&SliceNode{ - Node: node, - To: to, + Node: node, + To: to, + Optional: optional, }, postfixToken.Location) if node == nil { return nil } + if optional { + node = p.createNode(&ChainNode{Node: node}, postfixToken.Location) + if node == nil { + return nil + } + } p.expect(Bracket, "]") } else { @@ -868,13 +875,20 @@ func (p *Parser) parsePostfixExpression(node Node) Node { } node = p.createNode(&SliceNode{ - Node: node, - From: from, - To: to, + Node: node, + From: from, + To: to, + Optional: optional, }, postfixToken.Location) if node == nil { return nil } + if optional { + node = p.createNode(&ChainNode{Node: node}, postfixToken.Location) + if node == nil { + return nil + } + } p.expect(Bracket, "]") } else { diff --git a/parser/parser_test.go b/parser/parser_test.go index 963a23050..fe74dd2e5 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -1136,6 +1136,27 @@ func TestParse_optional_chaining(t *testing.T) { }, }, }, + { + "array?.[1:2]", + &ChainNode{ + Node: &SliceNode{ + Node: &IdentifierNode{Value: "array"}, + From: &IntegerNode{Value: 1}, + To: &IntegerNode{Value: 2}, + Optional: true, + }, + }, + }, + { + "array?.[:2]", + &ChainNode{ + Node: &SliceNode{ + Node: &IdentifierNode{Value: "array"}, + To: &IntegerNode{Value: 2}, + Optional: true, + }, + }, + }, { "!foo?.bar.baz", &UnaryNode{ diff --git a/test/issues/822/issue_test.go b/test/issues/822/issue_test.go new file mode 100644 index 000000000..8e3f5bcf3 --- /dev/null +++ b/test/issues/822/issue_test.go @@ -0,0 +1,81 @@ +package issue_test + +import ( + "testing" + + "github.com/expr-lang/expr" + "github.com/expr-lang/expr/internal/testify/require" +) + +func TestIssue822(t *testing.T) { + var tests = []struct { + input string + want any + err string + }{ + { + input: `let x = nil; x?.[0:1]`, + }, + { + input: `let x = nil; x?.[:1]`, + }, + { + input: `let x = nil; x?.[1:]`, + }, + { + input: `let x = [1, 2, 3]; x?.[0:2]`, + want: []any{1, 2}, + }, + { + input: `let x = "test"; x?.[5:10]`, + want: "", + }, + { + input: `let x = 1; x?.[0:1]`, + err: "cannot slice int", + }, + { + input: `let x = 1.5; x?.[0:1]`, + err: "cannot slice float64", + }, + { + input: `let x = true; x?.[0:1]`, + err: "cannot slice bool", + }, + { + input: `let x = {a: 1}; x?.[0:1]`, + err: "cannot slice map[string]interface {}", + }, + { + input: `let x = nil; x?.[true:false]`, + err: "non-integer slice index bool", + }, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + program, err := expr.Compile(tt.input) + + if tt.err != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.err) + } else { + require.NoError(t, err) + out, err := expr.Run(program, nil) + require.NoError(t, err) + require.Equal(t, tt.want, out) + } + }) + } +} + +func TestIssue822_nil_from_env(t *testing.T) { + env := map[string]any{"a": nil} + + program, err := expr.Compile(`a?.[0:1]`, expr.Env(env)) + require.NoError(t, err) + + out, err := expr.Run(program, env) + require.NoError(t, err) + require.Nil(t, out) +}