Skip to content

Commit 50e2a46

Browse files
committed
refactor(vm): use deref package and add unit tests for fetchFromEmbeddedInterfaces
Replace hand-rolled pointer/interface unwrap loops with deref.Value and deref.Type, consistent with how Fetch already dereferences values. Add white-box unit tests covering each branch (100% coverage of the function): plain struct, embedded interface with/without field, pointer to struct concrete value, nil concrete value, nil embedded pointer, nested embedded struct, nested embedded interface, non-struct concrete value, expr:"-" tag, and empty concrete struct.
1 parent 6fbab1e commit 50e2a46

2 files changed

Lines changed: 185 additions & 39 deletions

File tree

vm/runtime/runtime.go

Lines changed: 12 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -164,48 +164,21 @@ func fetchFromEmbeddedInterfaces(v reflect.Value, fieldName string) (any, bool)
164164
if !f.Anonymous {
165165
continue
166166
}
167-
fv := v.Field(i)
168-
fk := f.Type.Kind()
169-
170-
// Dereference pointers to get to the underlying type.
171-
for fk == reflect.Ptr {
172-
if fv.IsNil() {
173-
break
174-
}
175-
fv = fv.Elem()
176-
fk = fv.Kind()
167+
fv := deref.Value(v.Field(i))
168+
if fv.Kind() != reflect.Struct {
169+
continue
177170
}
178-
179-
switch fk {
180-
case reflect.Interface:
181-
if fv.IsNil() {
182-
continue
183-
}
184-
// Unwrap interface and dereference pointers to reach the
185-
// concrete struct value.
186-
concrete := fv.Elem()
187-
for concrete.Kind() == reflect.Ptr {
188-
if concrete.IsNil() {
189-
break
190-
}
191-
concrete = concrete.Elem()
192-
}
193-
if concrete.Kind() != reflect.Struct {
194-
continue
195-
}
196-
if value, _, ok := findStructField(concrete, fieldName); ok {
171+
// Embedded interfaces need an explicit field lookup on the concrete
172+
// value. Embedded structs are already covered by Go's standard field
173+
// promotion, so we only recurse into them to find further embedded
174+
// interfaces.
175+
if deref.Type(f.Type).Kind() == reflect.Interface {
176+
if value, _, ok := findStructField(fv, fieldName); ok {
197177
return value.Interface(), true
198178
}
199-
// The concrete type itself may have embedded interfaces.
200-
if result, found := fetchFromEmbeddedInterfaces(concrete, fieldName); found {
201-
return result, found
202-
}
203-
204-
case reflect.Struct:
205-
// Recurse into embedded structs to find embedded interfaces.
206-
if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found {
207-
return result, found
208-
}
179+
}
180+
if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found {
181+
return result, true
209182
}
210183
}
211184
return nil, false
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)