Skip to content

Commit 4a9e2ad

Browse files
committed
fix: bug in optional chaining typechecker #854
1 parent 593f93f commit 4a9e2ad

File tree

7 files changed

+68
-6
lines changed

7 files changed

+68
-6
lines changed

ast/node.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ type BoolNode struct {
101101
// StringNode represents a string.
102102
type StringNode struct {
103103
base
104-
Value string // Value of the string.
104+
Value string // Value of the string.
105+
Optional bool // If true then the property is optional. Like "foo.bar?.baz".
105106
}
106107

107108
// ConstantNode represents a constant.

checker/checker.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,12 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature {
527527
}
528528

529529
base := v.visit(node.Node)
530+
531+
// If the base is optional and the property is not found, we need to skip the property access.
532+
if base.Skip {
533+
return Nature{Skip: true}
534+
}
535+
530536
prop := v.visit(node.Property)
531537

532538
if base.IsUnknown(&v.config.NtCache) {
@@ -573,6 +579,12 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature {
573579
if field, ok := base.FieldByName(&v.config.NtCache, propertyName); ok {
574580
return v.config.NtCache.FromType(field.Type)
575581
}
582+
// If the property access is optional (via ?.) or the property itself is marked optional,
583+
// allow it to be unknown for optional chaining support
584+
if name.Optional || node.Optional {
585+
base.Skip = true
586+
return Nature{Skip: true}
587+
}
576588
if node.Method {
577589
return v.error(node, "type %v has no method %v", base.String(), propertyName)
578590
}

checker/nature/nature.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ type Nature struct {
6363
// - Array-like types: then Ref is the Elem nature of array type (usually Type is []any, but ArrayOf can be any nature).
6464
Ref *Nature
6565

66+
Skip bool // If the property access is optional (via ?.) and the property is not found, skip the error.
6667
Nil bool // If value is nil.
6768
Strict bool // If map is types.StrictMap.
6869
Method bool // If value retrieved from method. Usually used to determine amount of in arguments.

compiler/compiler.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,11 @@ func (c *compiler) MemberNode(node *ast.MemberNode) {
715715
}
716716

717717
if op == OpFetch {
718+
// If the field is optional, we need to jump over the fetch operation.
719+
if node.Nature().Skip {
720+
c.emit(OpNil)
721+
return
722+
}
718723
c.compile(node.Property)
719724
c.emit(OpFetch)
720725
} else {

expr_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,49 @@ func TestExpr_optional_chaining_array(t *testing.T) {
15221522
assert.Equal(t, nil, got)
15231523
}
15241524

1525+
func TestIssue854_optional_chaining_typechecker(t *testing.T) {
1526+
type User struct {
1527+
Id int
1528+
Name string
1529+
Group string
1530+
// Profile field is missing
1531+
}
1532+
1533+
env := map[string]any{
1534+
"user": User{
1535+
Id: 1,
1536+
Name: "John Doe",
1537+
Group: "admin",
1538+
},
1539+
}
1540+
1541+
tests := []struct {
1542+
code string
1543+
want string
1544+
}{
1545+
{
1546+
code: `user.Profile?.Address ?? "Unknown address"`,
1547+
want: "Unknown address",
1548+
},
1549+
// TODO: This case will be covered in a future update.
1550+
// {
1551+
// code: `user.Profile[0]?.Address ?? "Unknown address"`,
1552+
// want: "Unknown address",
1553+
// },
1554+
}
1555+
1556+
for _, tt := range tests {
1557+
t.Run(tt.code, func(t *testing.T) {
1558+
program, err := expr.Compile(tt.code, expr.Env(env))
1559+
require.NoError(t, err)
1560+
1561+
got, err := expr.Run(program, env)
1562+
require.NoError(t, err)
1563+
assert.Equal(t, tt.want, got)
1564+
})
1565+
}
1566+
}
1567+
15251568
func TestExpr_eval_with_env(t *testing.T) {
15261569
_, err := expr.Eval("true", expr.Env(map[string]any{}))
15271570
assert.Error(t, err)

parser/parser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ func (p *Parser) parsePostfixExpression(node Node) Node {
770770
p.error("expected name")
771771
}
772772

773-
property := p.createNode(&StringNode{Value: propertyToken.Value}, propertyToken.Location)
773+
property := p.createNode(&StringNode{Value: propertyToken.Value, Optional: p.current.Value == "?."}, propertyToken.Location)
774774
if property == nil {
775775
return nil
776776
}

parser/parser_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,7 @@ func TestParse_optional_chaining(t *testing.T) {
10631063
Node: &MemberNode{
10641064
Node: &MemberNode{
10651065
Node: &IdentifierNode{Value: "foo"},
1066-
Property: &StringNode{Value: "bar"},
1066+
Property: &StringNode{Value: "bar", Optional: true},
10671067
},
10681068
Property: &StringNode{Value: "baz"},
10691069
Optional: true,
@@ -1076,7 +1076,7 @@ func TestParse_optional_chaining(t *testing.T) {
10761076
Node: &MemberNode{
10771077
Node: &MemberNode{
10781078
Node: &IdentifierNode{Value: "foo"},
1079-
Property: &StringNode{Value: "bar"},
1079+
Property: &StringNode{Value: "bar", Optional: true},
10801080
Optional: true,
10811081
},
10821082
Property: &StringNode{Value: "baz"},
@@ -1107,7 +1107,7 @@ func TestParse_optional_chaining(t *testing.T) {
11071107
Node: &MemberNode{
11081108
Node: &MemberNode{
11091109
Node: &IdentifierNode{Value: "foo"},
1110-
Property: &StringNode{Value: "bar"},
1110+
Property: &StringNode{Value: "bar", Optional: false}, // TODO: Optional chaining for indexed property access should be determined during semantic analysis, not parsing.
11111111
},
11121112
Property: &ChainNode{
11131113
Node: &MemberNode{
@@ -1128,7 +1128,7 @@ func TestParse_optional_chaining(t *testing.T) {
11281128
Node: &MemberNode{
11291129
Node: &MemberNode{
11301130
Node: &IdentifierNode{Value: "foo"},
1131-
Property: &StringNode{Value: "bar"},
1131+
Property: &StringNode{Value: "bar", Optional: true},
11321132
},
11331133
Property: &IntegerNode{Value: 0},
11341134
Optional: true,

0 commit comments

Comments
 (0)