Skip to content

Commit 2010a11

Browse files
authored
feat: allow field access on concrete types behind interface values (#952)
1 parent 630bbf0 commit 2010a11

4 files changed

Lines changed: 358 additions & 17 deletions

File tree

checker/checker.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,15 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature {
578578
}
579579
return base.Elem(&v.config.NtCache)
580580

581+
case reflect.Interface:
582+
// For non-any interface types, we don't know the concrete type at
583+
// compile time. Allow field (non-method) access and defer resolution
584+
// to runtime, where the concrete type can be inspected.
585+
if name, ok := node.Property.(*ast.StringNode); ok && node.Method {
586+
return v.error(node, "type %v has no method %v", base.String(), name.Value)
587+
}
588+
return Nature{}
589+
581590
case reflect.Struct:
582591
if name, ok := node.Property.(*ast.StringNode); ok {
583592
propertyName := name.Value

test/issues/951/issue_test.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package issue951
2+
3+
import (
4+
"testing"
5+
6+
"github.com/expr-lang/expr"
7+
"github.com/expr-lang/expr/internal/testify/require"
8+
)
9+
10+
type Node interface {
11+
ID() string
12+
}
13+
14+
type Base struct {
15+
Name string
16+
}
17+
18+
func (b Base) ID() string { return b.Name }
19+
20+
type Container struct {
21+
Base
22+
Items []*Item
23+
}
24+
25+
type Item struct {
26+
Kind string
27+
Value string
28+
}
29+
30+
type Wrapper struct {
31+
Node // embedded interface
32+
}
33+
34+
type Proxy struct {
35+
*Wrapper
36+
}
37+
38+
type Nodes []Node
39+
40+
func (ns Nodes) GetByID(id string) Node {
41+
for _, n := range ns {
42+
if n.ID() == id {
43+
return n
44+
}
45+
}
46+
return nil
47+
}
48+
49+
func TestFieldAccessThroughEmbeddedInterface(t *testing.T) {
50+
container := &Container{
51+
Base: Base{Name: "test"},
52+
Items: []*Item{
53+
{Kind: "card", Value: "some_value"},
54+
},
55+
}
56+
proxy := &Proxy{
57+
Wrapper: &Wrapper{
58+
Node: container,
59+
},
60+
}
61+
62+
tests := []struct {
63+
name string
64+
expr string
65+
env any
66+
expect any
67+
}{
68+
{
69+
name: "field through GetByID returning interface",
70+
expr: `data.GetByID("test").Items[0].Value`,
71+
env: map[string]any{"data": Nodes{proxy}},
72+
expect: "some_value",
73+
},
74+
{
75+
name: "optional chaining with embedded interface",
76+
expr: `data.GetByID("test")?.Items[0].Value`,
77+
env: map[string]any{"data": Nodes{proxy}},
78+
expect: "some_value",
79+
},
80+
{
81+
name: "optional chaining nil result",
82+
expr: `data.GetByID("missing")?.Items`,
83+
env: map[string]any{"data": Nodes{proxy}},
84+
expect: nil,
85+
},
86+
{
87+
name: "promoted field through interface",
88+
expr: `data.GetByID("test").Name`,
89+
env: map[string]any{"data": Nodes{proxy}},
90+
expect: "test",
91+
},
92+
{
93+
name: "method on interface still works",
94+
expr: `data.GetByID("test").ID()`,
95+
env: map[string]any{"data": Nodes{proxy}},
96+
expect: "test",
97+
},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
result, err := expr.Eval(tt.expr, tt.env)
103+
require.NoError(t, err)
104+
require.Equal(t, tt.expect, result)
105+
})
106+
}
107+
}
108+
109+
func TestFieldAccessEmbeddedInterfaceNil(t *testing.T) {
110+
proxy := &Proxy{
111+
Wrapper: &Wrapper{
112+
Node: nil,
113+
},
114+
}
115+
116+
_, err := expr.Eval(`Items[0].Value`, proxy)
117+
require.Error(t, err)
118+
}

vm/runtime/runtime.go

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -82,23 +82,15 @@ func Fetch(from, i any) any {
8282
if cv, ok := fieldCache.Load(key); ok {
8383
return v.FieldByIndex(cv.([]int)).Interface()
8484
}
85-
field, ok := t.FieldByNameFunc(func(name string) bool {
86-
field, _ := t.FieldByName(name)
87-
switch field.Tag.Get("expr") {
88-
case "-":
89-
return false
90-
case fieldName:
91-
return true
92-
default:
93-
return name == fieldName
94-
}
95-
})
96-
if ok && field.IsExported() {
97-
value := v.FieldByIndex(field.Index)
98-
if value.IsValid() {
99-
fieldCache.Store(key, field.Index)
100-
return value.Interface()
101-
}
85+
if value, field, ok := findStructField(v, fieldName); ok {
86+
fieldCache.Store(key, field.Index)
87+
return value.Interface()
88+
}
89+
// Field isn't found via standard Go promotion. Try to find it
90+
// by traversing embedded interface values whose concrete types
91+
// may contain the requested field.
92+
if result, found := fetchFromEmbeddedInterfaces(v, fieldName); found {
93+
return result
10294
}
10395
}
10496
panic(fmt.Sprintf("cannot fetch %v from %T", i, from))
@@ -146,6 +138,55 @@ func fieldByIndex(v reflect.Value, field *Field) reflect.Value {
146138
return v
147139
}
148140

141+
func findStructField(v reflect.Value, fieldName string) (reflect.Value, reflect.StructField, bool) {
142+
t := v.Type()
143+
field, ok := t.FieldByNameFunc(func(name string) bool {
144+
sf, _ := t.FieldByName(name)
145+
switch sf.Tag.Get("expr") {
146+
case "-":
147+
return false
148+
case fieldName:
149+
return true
150+
default:
151+
return name == fieldName
152+
}
153+
})
154+
if ok && field.IsExported() {
155+
value := v.FieldByIndex(field.Index)
156+
if value.IsValid() {
157+
return value, field, true
158+
}
159+
}
160+
return reflect.Value{}, reflect.StructField{}, false
161+
}
162+
163+
func fetchFromEmbeddedInterfaces(v reflect.Value, fieldName string) (any, bool) {
164+
t := v.Type()
165+
for i := 0; i < t.NumField(); i++ {
166+
f := t.Field(i)
167+
if !f.Anonymous {
168+
continue
169+
}
170+
fv := deref.Value(v.Field(i))
171+
if fv.Kind() != reflect.Struct {
172+
continue
173+
}
174+
// Embedded interfaces need an explicit field lookup on the concrete
175+
// value. Embedded structs are already covered by Go's standard field
176+
// promotion, so we only recurse into them to find further embedded
177+
// interfaces.
178+
if deref.Type(f.Type).Kind() == reflect.Interface {
179+
if value, _, ok := findStructField(fv, fieldName); ok {
180+
return value.Interface(), true
181+
}
182+
}
183+
if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found {
184+
return result, true
185+
}
186+
}
187+
return nil, false
188+
}
189+
149190
type Method struct {
150191
Index int
151192
Name string

vm/runtime/runtime_test.go

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
package runtime
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/expr-lang/expr/internal/testify/require"
8+
)
9+
10+
type Namer interface {
11+
Name() string
12+
}
13+
14+
type IntHolder interface {
15+
Int() int
16+
}
17+
18+
type ConcreteWithName struct {
19+
Title string
20+
}
21+
22+
func (ConcreteWithName) Name() string { return "" }
23+
24+
type ConcreteWithSkippedField struct {
25+
Title string `expr:"-"`
26+
}
27+
28+
func (ConcreteWithSkippedField) Name() string { return "" }
29+
30+
type ConcreteEmptyStruct struct{}
31+
32+
func (ConcreteEmptyStruct) Name() string { return "" }
33+
34+
type ConcreteInt int
35+
36+
func (ConcreteInt) Int() int { return 0 }
37+
38+
type ConcreteWithEmbeddedInterface struct {
39+
Namer
40+
}
41+
42+
func (ConcreteWithEmbeddedInterface) Int() int { return 0 }
43+
44+
type EmbeddedInterfaceOnly struct {
45+
Namer
46+
}
47+
48+
type EmbeddedNilPointerOnly struct {
49+
*ConcreteWithName
50+
}
51+
52+
type EmbeddedStructWithInterface struct {
53+
EmbeddedInterfaceOnly
54+
}
55+
56+
type EmbeddedIntHolder struct {
57+
IntHolder
58+
}
59+
60+
type PlainStruct struct {
61+
Title string
62+
}
63+
64+
func TestFetchFromEmbeddedInterfaces(t *testing.T) {
65+
tests := []struct {
66+
name string
67+
input any
68+
fieldName string
69+
want any
70+
ok bool
71+
}{
72+
{
73+
name: "no anonymous fields",
74+
input: PlainStruct{Title: "ignored"},
75+
fieldName: "Title",
76+
ok: false,
77+
},
78+
{
79+
name: "embedded interface with field on concrete struct",
80+
input: EmbeddedInterfaceOnly{
81+
Namer: ConcreteWithName{Title: "hello"},
82+
},
83+
fieldName: "Title",
84+
want: "hello",
85+
ok: true,
86+
},
87+
{
88+
name: "embedded interface, concrete missing field",
89+
input: EmbeddedInterfaceOnly{
90+
Namer: ConcreteWithName{Title: "hello"},
91+
},
92+
fieldName: "Missing",
93+
ok: false,
94+
},
95+
{
96+
name: "embedded interface holding pointer to struct",
97+
input: EmbeddedInterfaceOnly{
98+
Namer: &ConcreteWithName{Title: "pointer"},
99+
},
100+
fieldName: "Title",
101+
want: "pointer",
102+
ok: true,
103+
},
104+
{
105+
name: "embedded interface with nil concrete value",
106+
input: EmbeddedInterfaceOnly{Namer: nil},
107+
fieldName: "Title",
108+
ok: false,
109+
},
110+
{
111+
name: "embedded nil pointer to struct",
112+
input: EmbeddedNilPointerOnly{ConcreteWithName: nil},
113+
fieldName: "Title",
114+
ok: false,
115+
},
116+
{
117+
name: "embedded struct containing embedded interface with field",
118+
input: EmbeddedStructWithInterface{
119+
EmbeddedInterfaceOnly: EmbeddedInterfaceOnly{
120+
Namer: ConcreteWithName{Title: "nested"},
121+
},
122+
},
123+
fieldName: "Title",
124+
want: "nested",
125+
ok: true,
126+
},
127+
{
128+
name: "embedded interface whose concrete embeds another interface",
129+
input: EmbeddedIntHolder{
130+
IntHolder: ConcreteWithEmbeddedInterface{
131+
Namer: ConcreteWithName{Title: "deep"},
132+
},
133+
},
134+
fieldName: "Title",
135+
want: "deep",
136+
ok: true,
137+
},
138+
{
139+
name: "embedded interface with non-struct concrete value",
140+
input: EmbeddedIntHolder{
141+
IntHolder: ConcreteInt(5),
142+
},
143+
fieldName: "Title",
144+
ok: false,
145+
},
146+
{
147+
name: "field is skipped via expr:\"-\" tag",
148+
input: EmbeddedInterfaceOnly{
149+
Namer: ConcreteWithSkippedField{Title: "hidden"},
150+
},
151+
fieldName: "Title",
152+
ok: false,
153+
},
154+
{
155+
name: "embedded interface with empty concrete struct, recurses to nothing",
156+
input: EmbeddedInterfaceOnly{
157+
Namer: ConcreteEmptyStruct{},
158+
},
159+
fieldName: "Title",
160+
ok: false,
161+
},
162+
}
163+
164+
for _, tt := range tests {
165+
t.Run(tt.name, func(t *testing.T) {
166+
got, ok := fetchFromEmbeddedInterfaces(reflect.ValueOf(tt.input), tt.fieldName)
167+
require.Equal(t, tt.ok, ok)
168+
if tt.ok {
169+
require.Equal(t, tt.want, got)
170+
}
171+
})
172+
}
173+
}

0 commit comments

Comments
 (0)