From 4a9e2addf6ad1dbd52c0bd3dfc72db63cc970c62 Mon Sep 17 00:00:00 2001 From: Ehab Terra Date: Tue, 9 Dec 2025 01:36:08 +0200 Subject: [PATCH] fix: bug in optional chaining typechecker #854 --- ast/node.go | 3 ++- checker/checker.go | 12 +++++++++++ checker/nature/nature.go | 1 + compiler/compiler.go | 5 +++++ expr_test.go | 43 ++++++++++++++++++++++++++++++++++++++++ parser/parser.go | 2 +- parser/parser_test.go | 8 ++++---- 7 files changed, 68 insertions(+), 6 deletions(-) diff --git a/ast/node.go b/ast/node.go index f156f69be..5c3d279b7 100644 --- a/ast/node.go +++ b/ast/node.go @@ -101,7 +101,8 @@ type BoolNode struct { // StringNode represents a string. type StringNode struct { base - Value string // Value of the string. + Value string // Value of the string. + Optional bool // If true then the property is optional. Like "foo.bar?.baz". } // ConstantNode represents a constant. diff --git a/checker/checker.go b/checker/checker.go index 0210b4166..1224b9c75 100644 --- a/checker/checker.go +++ b/checker/checker.go @@ -527,6 +527,12 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature { } base := v.visit(node.Node) + + // If the base is optional and the property is not found, we need to skip the property access. + if base.Skip { + return Nature{Skip: true} + } + prop := v.visit(node.Property) if base.IsUnknown(&v.config.NtCache) { @@ -573,6 +579,12 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature { if field, ok := base.FieldByName(&v.config.NtCache, propertyName); ok { return v.config.NtCache.FromType(field.Type) } + // If the property access is optional (via ?.) or the property itself is marked optional, + // allow it to be unknown for optional chaining support + if name.Optional || node.Optional { + base.Skip = true + return Nature{Skip: true} + } if node.Method { return v.error(node, "type %v has no method %v", base.String(), propertyName) } diff --git a/checker/nature/nature.go b/checker/nature/nature.go index ae406acbf..2d47a1b7d 100644 --- a/checker/nature/nature.go +++ b/checker/nature/nature.go @@ -63,6 +63,7 @@ type Nature struct { // - Array-like types: then Ref is the Elem nature of array type (usually Type is []any, but ArrayOf can be any nature). Ref *Nature + Skip bool // If the property access is optional (via ?.) and the property is not found, skip the error. Nil bool // If value is nil. Strict bool // If map is types.StrictMap. Method bool // If value retrieved from method. Usually used to determine amount of in arguments. diff --git a/compiler/compiler.go b/compiler/compiler.go index d8a532440..40da7e38f 100644 --- a/compiler/compiler.go +++ b/compiler/compiler.go @@ -715,6 +715,11 @@ func (c *compiler) MemberNode(node *ast.MemberNode) { } if op == OpFetch { + // If the field is optional, we need to jump over the fetch operation. + if node.Nature().Skip { + c.emit(OpNil) + return + } c.compile(node.Property) c.emit(OpFetch) } else { diff --git a/expr_test.go b/expr_test.go index 01ccdeeb9..70a9882fd 100644 --- a/expr_test.go +++ b/expr_test.go @@ -1522,6 +1522,49 @@ func TestExpr_optional_chaining_array(t *testing.T) { assert.Equal(t, nil, got) } +func TestIssue854_optional_chaining_typechecker(t *testing.T) { + type User struct { + Id int + Name string + Group string + // Profile field is missing + } + + env := map[string]any{ + "user": User{ + Id: 1, + Name: "John Doe", + Group: "admin", + }, + } + + tests := []struct { + code string + want string + }{ + { + code: `user.Profile?.Address ?? "Unknown address"`, + want: "Unknown address", + }, + // TODO: This case will be covered in a future update. + // { + // code: `user.Profile[0]?.Address ?? "Unknown address"`, + // want: "Unknown address", + // }, + } + + for _, tt := range tests { + t.Run(tt.code, func(t *testing.T) { + program, err := expr.Compile(tt.code, expr.Env(env)) + require.NoError(t, err) + + got, err := expr.Run(program, env) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + func TestExpr_eval_with_env(t *testing.T) { _, err := expr.Eval("true", expr.Env(map[string]any{})) assert.Error(t, err) diff --git a/parser/parser.go b/parser/parser.go index 6034403bf..5f5bb28e1 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -770,7 +770,7 @@ func (p *Parser) parsePostfixExpression(node Node) Node { p.error("expected name") } - property := p.createNode(&StringNode{Value: propertyToken.Value}, propertyToken.Location) + property := p.createNode(&StringNode{Value: propertyToken.Value, Optional: p.current.Value == "?."}, propertyToken.Location) if property == nil { return nil } diff --git a/parser/parser_test.go b/parser/parser_test.go index 16dac6b79..91836483d 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -1063,7 +1063,7 @@ func TestParse_optional_chaining(t *testing.T) { Node: &MemberNode{ Node: &MemberNode{ Node: &IdentifierNode{Value: "foo"}, - Property: &StringNode{Value: "bar"}, + Property: &StringNode{Value: "bar", Optional: true}, }, Property: &StringNode{Value: "baz"}, Optional: true, @@ -1076,7 +1076,7 @@ func TestParse_optional_chaining(t *testing.T) { Node: &MemberNode{ Node: &MemberNode{ Node: &IdentifierNode{Value: "foo"}, - Property: &StringNode{Value: "bar"}, + Property: &StringNode{Value: "bar", Optional: true}, Optional: true, }, Property: &StringNode{Value: "baz"}, @@ -1107,7 +1107,7 @@ func TestParse_optional_chaining(t *testing.T) { Node: &MemberNode{ Node: &MemberNode{ Node: &IdentifierNode{Value: "foo"}, - Property: &StringNode{Value: "bar"}, + Property: &StringNode{Value: "bar", Optional: false}, // TODO: Optional chaining for indexed property access should be determined during semantic analysis, not parsing. }, Property: &ChainNode{ Node: &MemberNode{ @@ -1128,7 +1128,7 @@ func TestParse_optional_chaining(t *testing.T) { Node: &MemberNode{ Node: &MemberNode{ Node: &IdentifierNode{Value: "foo"}, - Property: &StringNode{Value: "bar"}, + Property: &StringNode{Value: "bar", Optional: true}, }, Property: &IntegerNode{Value: 0}, Optional: true,