From 6fbab1ef9d1096986dfccb6d651003033aa3baa2 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 31 Mar 2026 17:19:21 +0400 Subject: [PATCH 1/3] feat: allow field access on concrete types behind interface values --- checker/checker.go | 9 +++ test/issues/951/issue_test.go | 118 ++++++++++++++++++++++++++++++++++ vm/runtime/runtime.go | 102 ++++++++++++++++++++++++----- 3 files changed, 212 insertions(+), 17 deletions(-) create mode 100644 test/issues/951/issue_test.go diff --git a/checker/checker.go b/checker/checker.go index 3620f207..63425af1 100644 --- a/checker/checker.go +++ b/checker/checker.go @@ -578,6 +578,15 @@ func (v *Checker) memberNode(node *ast.MemberNode) Nature { } return base.Elem(&v.config.NtCache) + case reflect.Interface: + // For non-any interface types, we don't know the concrete type at + // compile time. Allow field (non-method) access and defer resolution + // to runtime, where the concrete type can be inspected. + if name, ok := node.Property.(*ast.StringNode); ok && node.Method { + return v.error(node, "type %v has no method %v", base.String(), name.Value) + } + return Nature{} + case reflect.Struct: if name, ok := node.Property.(*ast.StringNode); ok { propertyName := name.Value diff --git a/test/issues/951/issue_test.go b/test/issues/951/issue_test.go new file mode 100644 index 00000000..7c735210 --- /dev/null +++ b/test/issues/951/issue_test.go @@ -0,0 +1,118 @@ +package issue951 + +import ( + "testing" + + "github.com/expr-lang/expr" + "github.com/expr-lang/expr/internal/testify/require" +) + +type Node interface { + ID() string +} + +type Base struct { + Name string +} + +func (b Base) ID() string { return b.Name } + +type Container struct { + Base + Items []*Item +} + +type Item struct { + Kind string + Value string +} + +type Wrapper struct { + Node // embedded interface +} + +type Proxy struct { + *Wrapper +} + +type Nodes []Node + +func (ns Nodes) GetByID(id string) Node { + for _, n := range ns { + if n.ID() == id { + return n + } + } + return nil +} + +func TestFieldAccessThroughEmbeddedInterface(t *testing.T) { + container := &Container{ + Base: Base{Name: "test"}, + Items: []*Item{ + {Kind: "card", Value: "some_value"}, + }, + } + proxy := &Proxy{ + Wrapper: &Wrapper{ + Node: container, + }, + } + + tests := []struct { + name string + expr string + env any + expect any + }{ + { + name: "field through GetByID returning interface", + expr: `data.GetByID("test").Items[0].Value`, + env: map[string]any{"data": Nodes{proxy}}, + expect: "some_value", + }, + { + name: "optional chaining with embedded interface", + expr: `data.GetByID("test")?.Items[0].Value`, + env: map[string]any{"data": Nodes{proxy}}, + expect: "some_value", + }, + { + name: "optional chaining nil result", + expr: `data.GetByID("missing")?.Items`, + env: map[string]any{"data": Nodes{proxy}}, + expect: nil, + }, + { + name: "promoted field through interface", + expr: `data.GetByID("test").Name`, + env: map[string]any{"data": Nodes{proxy}}, + expect: "test", + }, + { + name: "method on interface still works", + expr: `data.GetByID("test").ID()`, + env: map[string]any{"data": Nodes{proxy}}, + expect: "test", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := expr.Eval(tt.expr, tt.env) + require.NoError(t, err) + require.Equal(t, tt.expect, result) + }) + } +} + +func TestFieldAccessEmbeddedInterfaceNil(t *testing.T) { + proxy := &Proxy{ + Wrapper: &Wrapper{ + Node: nil, + }, + } + + _, err := expr.Eval(`Items[0].Value`, proxy) + require.Error(t, err) +} diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index bc6f2b4d..169d66d6 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -79,23 +79,15 @@ func Fetch(from, i any) any { if cv, ok := fieldCache.Load(key); ok { return v.FieldByIndex(cv.([]int)).Interface() } - field, ok := t.FieldByNameFunc(func(name string) bool { - field, _ := t.FieldByName(name) - switch field.Tag.Get("expr") { - case "-": - return false - case fieldName: - return true - default: - return name == fieldName - } - }) - if ok && field.IsExported() { - value := v.FieldByIndex(field.Index) - if value.IsValid() { - fieldCache.Store(key, field.Index) - return value.Interface() - } + if value, field, ok := findStructField(v, fieldName); ok { + fieldCache.Store(key, field.Index) + return value.Interface() + } + // Field isn't found via standard Go promotion. Try to find it + // by traversing embedded interface values whose concrete types + // may contain the requested field. + if result, found := fetchFromEmbeddedInterfaces(v, fieldName); found { + return result } } panic(fmt.Sprintf("cannot fetch %v from %T", i, from)) @@ -143,6 +135,82 @@ func fieldByIndex(v reflect.Value, field *Field) reflect.Value { return v } +func findStructField(v reflect.Value, fieldName string) (reflect.Value, reflect.StructField, bool) { + t := v.Type() + field, ok := t.FieldByNameFunc(func(name string) bool { + sf, _ := t.FieldByName(name) + switch sf.Tag.Get("expr") { + case "-": + return false + case fieldName: + return true + default: + return name == fieldName + } + }) + if ok && field.IsExported() { + value := v.FieldByIndex(field.Index) + if value.IsValid() { + return value, field, true + } + } + return reflect.Value{}, reflect.StructField{}, false +} + +func fetchFromEmbeddedInterfaces(v reflect.Value, fieldName string) (any, bool) { + t := v.Type() + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !f.Anonymous { + continue + } + fv := v.Field(i) + fk := f.Type.Kind() + + // Dereference pointers to get to the underlying type. + for fk == reflect.Ptr { + if fv.IsNil() { + break + } + fv = fv.Elem() + fk = fv.Kind() + } + + switch fk { + case reflect.Interface: + if fv.IsNil() { + continue + } + // Unwrap interface and dereference pointers to reach the + // concrete struct value. + concrete := fv.Elem() + for concrete.Kind() == reflect.Ptr { + if concrete.IsNil() { + break + } + concrete = concrete.Elem() + } + if concrete.Kind() != reflect.Struct { + continue + } + if value, _, ok := findStructField(concrete, fieldName); ok { + return value.Interface(), true + } + // The concrete type itself may have embedded interfaces. + if result, found := fetchFromEmbeddedInterfaces(concrete, fieldName); found { + return result, found + } + + case reflect.Struct: + // Recurse into embedded structs to find embedded interfaces. + if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found { + return result, found + } + } + } + return nil, false +} + type Method struct { Index int Name string From 50e2a460b7802ed4b67a6a26cb510814369abe67 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 19 May 2026 09:19:56 +0400 Subject: [PATCH 2/3] 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. --- vm/runtime/runtime.go | 51 ++------ vm/runtime/runtime_internal_test.go | 173 ++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 39 deletions(-) create mode 100644 vm/runtime/runtime_internal_test.go diff --git a/vm/runtime/runtime.go b/vm/runtime/runtime.go index 169d66d6..716c731b 100644 --- a/vm/runtime/runtime.go +++ b/vm/runtime/runtime.go @@ -164,48 +164,21 @@ func fetchFromEmbeddedInterfaces(v reflect.Value, fieldName string) (any, bool) if !f.Anonymous { continue } - fv := v.Field(i) - fk := f.Type.Kind() - - // Dereference pointers to get to the underlying type. - for fk == reflect.Ptr { - if fv.IsNil() { - break - } - fv = fv.Elem() - fk = fv.Kind() + fv := deref.Value(v.Field(i)) + if fv.Kind() != reflect.Struct { + continue } - - switch fk { - case reflect.Interface: - if fv.IsNil() { - continue - } - // Unwrap interface and dereference pointers to reach the - // concrete struct value. - concrete := fv.Elem() - for concrete.Kind() == reflect.Ptr { - if concrete.IsNil() { - break - } - concrete = concrete.Elem() - } - if concrete.Kind() != reflect.Struct { - continue - } - if value, _, ok := findStructField(concrete, fieldName); ok { + // Embedded interfaces need an explicit field lookup on the concrete + // value. Embedded structs are already covered by Go's standard field + // promotion, so we only recurse into them to find further embedded + // interfaces. + if deref.Type(f.Type).Kind() == reflect.Interface { + if value, _, ok := findStructField(fv, fieldName); ok { return value.Interface(), true } - // The concrete type itself may have embedded interfaces. - if result, found := fetchFromEmbeddedInterfaces(concrete, fieldName); found { - return result, found - } - - case reflect.Struct: - // Recurse into embedded structs to find embedded interfaces. - if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found { - return result, found - } + } + if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found { + return result, true } } return nil, false diff --git a/vm/runtime/runtime_internal_test.go b/vm/runtime/runtime_internal_test.go new file mode 100644 index 00000000..14fbff5d --- /dev/null +++ b/vm/runtime/runtime_internal_test.go @@ -0,0 +1,173 @@ +package runtime + +import ( + "reflect" + "testing" + + "github.com/expr-lang/expr/internal/testify/require" +) + +type Namer interface { + Name() string +} + +type IntHolder interface { + Int() int +} + +type ConcreteWithName struct { + Title string +} + +func (ConcreteWithName) Name() string { return "" } + +type ConcreteWithSkippedField struct { + Title string `expr:"-"` +} + +func (ConcreteWithSkippedField) Name() string { return "" } + +type ConcreteEmptyStruct struct{} + +func (ConcreteEmptyStruct) Name() string { return "" } + +type ConcreteInt int + +func (ConcreteInt) Int() int { return 0 } + +type ConcreteWithEmbeddedInterface struct { + Namer +} + +func (ConcreteWithEmbeddedInterface) Int() int { return 0 } + +type EmbeddedInterfaceOnly struct { + Namer +} + +type EmbeddedNilPointerOnly struct { + *ConcreteWithName +} + +type EmbeddedStructWithInterface struct { + EmbeddedInterfaceOnly +} + +type EmbeddedIntHolder struct { + IntHolder +} + +type PlainStruct struct { + Title string +} + +func TestFetchFromEmbeddedInterfaces(t *testing.T) { + tests := []struct { + name string + input any + fieldName string + want any + ok bool + }{ + { + name: "no anonymous fields", + input: PlainStruct{Title: "ignored"}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded interface with field on concrete struct", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "hello"}, + }, + fieldName: "Title", + want: "hello", + ok: true, + }, + { + name: "embedded interface, concrete missing field", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "hello"}, + }, + fieldName: "Missing", + ok: false, + }, + { + name: "embedded interface holding pointer to struct", + input: EmbeddedInterfaceOnly{ + Namer: &ConcreteWithName{Title: "pointer"}, + }, + fieldName: "Title", + want: "pointer", + ok: true, + }, + { + name: "embedded interface with nil concrete value", + input: EmbeddedInterfaceOnly{Namer: nil}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded nil pointer to struct", + input: EmbeddedNilPointerOnly{ConcreteWithName: nil}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded struct containing embedded interface with field", + input: EmbeddedStructWithInterface{ + EmbeddedInterfaceOnly: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "nested"}, + }, + }, + fieldName: "Title", + want: "nested", + ok: true, + }, + { + name: "embedded interface whose concrete embeds another interface", + input: EmbeddedIntHolder{ + IntHolder: ConcreteWithEmbeddedInterface{ + Namer: ConcreteWithName{Title: "deep"}, + }, + }, + fieldName: "Title", + want: "deep", + ok: true, + }, + { + name: "embedded interface with non-struct concrete value", + input: EmbeddedIntHolder{ + IntHolder: ConcreteInt(5), + }, + fieldName: "Title", + ok: false, + }, + { + name: "field is skipped via expr:\"-\" tag", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithSkippedField{Title: "hidden"}, + }, + fieldName: "Title", + ok: false, + }, + { + name: "embedded interface with empty concrete struct, recurses to nothing", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteEmptyStruct{}, + }, + fieldName: "Title", + ok: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := fetchFromEmbeddedInterfaces(reflect.ValueOf(tt.input), tt.fieldName) + require.Equal(t, tt.ok, ok) + if tt.ok { + require.Equal(t, tt.want, got) + } + }) + } +} From 9522f91989f3bf58e8e6d1a6242465c6f82268d2 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 19 May 2026 09:45:17 +0400 Subject: [PATCH 3/3] feat: allow field access on concrete types behind interface values --- vm/runtime/runtime_test.go | 173 +++++++++++++++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 vm/runtime/runtime_test.go diff --git a/vm/runtime/runtime_test.go b/vm/runtime/runtime_test.go new file mode 100644 index 00000000..14fbff5d --- /dev/null +++ b/vm/runtime/runtime_test.go @@ -0,0 +1,173 @@ +package runtime + +import ( + "reflect" + "testing" + + "github.com/expr-lang/expr/internal/testify/require" +) + +type Namer interface { + Name() string +} + +type IntHolder interface { + Int() int +} + +type ConcreteWithName struct { + Title string +} + +func (ConcreteWithName) Name() string { return "" } + +type ConcreteWithSkippedField struct { + Title string `expr:"-"` +} + +func (ConcreteWithSkippedField) Name() string { return "" } + +type ConcreteEmptyStruct struct{} + +func (ConcreteEmptyStruct) Name() string { return "" } + +type ConcreteInt int + +func (ConcreteInt) Int() int { return 0 } + +type ConcreteWithEmbeddedInterface struct { + Namer +} + +func (ConcreteWithEmbeddedInterface) Int() int { return 0 } + +type EmbeddedInterfaceOnly struct { + Namer +} + +type EmbeddedNilPointerOnly struct { + *ConcreteWithName +} + +type EmbeddedStructWithInterface struct { + EmbeddedInterfaceOnly +} + +type EmbeddedIntHolder struct { + IntHolder +} + +type PlainStruct struct { + Title string +} + +func TestFetchFromEmbeddedInterfaces(t *testing.T) { + tests := []struct { + name string + input any + fieldName string + want any + ok bool + }{ + { + name: "no anonymous fields", + input: PlainStruct{Title: "ignored"}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded interface with field on concrete struct", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "hello"}, + }, + fieldName: "Title", + want: "hello", + ok: true, + }, + { + name: "embedded interface, concrete missing field", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "hello"}, + }, + fieldName: "Missing", + ok: false, + }, + { + name: "embedded interface holding pointer to struct", + input: EmbeddedInterfaceOnly{ + Namer: &ConcreteWithName{Title: "pointer"}, + }, + fieldName: "Title", + want: "pointer", + ok: true, + }, + { + name: "embedded interface with nil concrete value", + input: EmbeddedInterfaceOnly{Namer: nil}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded nil pointer to struct", + input: EmbeddedNilPointerOnly{ConcreteWithName: nil}, + fieldName: "Title", + ok: false, + }, + { + name: "embedded struct containing embedded interface with field", + input: EmbeddedStructWithInterface{ + EmbeddedInterfaceOnly: EmbeddedInterfaceOnly{ + Namer: ConcreteWithName{Title: "nested"}, + }, + }, + fieldName: "Title", + want: "nested", + ok: true, + }, + { + name: "embedded interface whose concrete embeds another interface", + input: EmbeddedIntHolder{ + IntHolder: ConcreteWithEmbeddedInterface{ + Namer: ConcreteWithName{Title: "deep"}, + }, + }, + fieldName: "Title", + want: "deep", + ok: true, + }, + { + name: "embedded interface with non-struct concrete value", + input: EmbeddedIntHolder{ + IntHolder: ConcreteInt(5), + }, + fieldName: "Title", + ok: false, + }, + { + name: "field is skipped via expr:\"-\" tag", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteWithSkippedField{Title: "hidden"}, + }, + fieldName: "Title", + ok: false, + }, + { + name: "embedded interface with empty concrete struct, recurses to nothing", + input: EmbeddedInterfaceOnly{ + Namer: ConcreteEmptyStruct{}, + }, + fieldName: "Title", + ok: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := fetchFromEmbeddedInterfaces(reflect.ValueOf(tt.input), tt.fieldName) + require.Equal(t, tt.ok, ok) + if tt.ok { + require.Equal(t, tt.want, got) + } + }) + } +}