Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
118 changes: 118 additions & 0 deletions test/issues/951/issue_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
75 changes: 58 additions & 17 deletions vm/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -143,6 +135,55 @@ 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) {
Comment thread
antonmedv marked this conversation as resolved.
t := v.Type()
for i := 0; i < t.NumField(); i++ {
f := t.Field(i)
if !f.Anonymous {
continue
}
fv := deref.Value(v.Field(i))
if fv.Kind() != reflect.Struct {
continue
}
// 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
}
}
if result, found := fetchFromEmbeddedInterfaces(fv, fieldName); found {
return result, true
}
}
return nil, false
}

type Method struct {
Index int
Name string
Expand Down
173 changes: 173 additions & 0 deletions vm/runtime/runtime_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package runtime

import (
"reflect"
"testing"

"github.com/expr-lang/expr/internal/testify/require"
)

type Namer interface {

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.22)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.25)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.18)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.20)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.24)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.21)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.19)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.26)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.23)

other declaration of Namer

Check failure on line 10 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / coverage

other declaration of Namer
Name() string
}

type IntHolder interface {

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.22)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.25)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.18)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.20)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.24)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.21)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.19)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.26)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.23)

other declaration of IntHolder

Check failure on line 14 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / coverage

other declaration of IntHolder
Int() int
}

type ConcreteWithName struct {

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.22)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.25)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.18)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.20)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.24)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.21)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.19)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.26)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.23)

other declaration of ConcreteWithName

Check failure on line 18 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / coverage

other declaration of ConcreteWithName
Title string
}

func (ConcreteWithName) Name() string { return "" }

type ConcreteWithSkippedField struct {

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.22)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.25)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.18)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.20)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.24)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.21)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.19)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.26)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.23)

other declaration of ConcreteWithSkippedField

Check failure on line 24 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / coverage

other declaration of ConcreteWithSkippedField
Title string `expr:"-"`
}

func (ConcreteWithSkippedField) Name() string { return "" }

type ConcreteEmptyStruct struct{}

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.22)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.25)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.18)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.20)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.24)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.21)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.19)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.26)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / test (1.23)

other declaration of ConcreteEmptyStruct

Check failure on line 30 in vm/runtime/runtime_internal_test.go

View workflow job for this annotation

GitHub Actions / coverage

other declaration of ConcreteEmptyStruct

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)
}
})
}
}
Loading
Loading