-
-
Notifications
You must be signed in to change notification settings - Fork 497
feat: allow field access on concrete types behind interface values #952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
6fbab1e
50e2a46
9522f91
7ee7536
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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) | ||
| } |
| 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
|
||
| Name() string | ||
| } | ||
|
|
||
| type IntHolder interface { | ||
|
Check failure on line 14 in vm/runtime/runtime_internal_test.go
|
||
| Int() int | ||
| } | ||
|
|
||
| type ConcreteWithName struct { | ||
|
Check failure on line 18 in vm/runtime/runtime_internal_test.go
|
||
| Title string | ||
| } | ||
|
|
||
| func (ConcreteWithName) Name() string { return "" } | ||
|
|
||
| type ConcreteWithSkippedField struct { | ||
|
Check failure on line 24 in vm/runtime/runtime_internal_test.go
|
||
| Title string `expr:"-"` | ||
| } | ||
|
|
||
| func (ConcreteWithSkippedField) Name() string { return "" } | ||
|
|
||
| type ConcreteEmptyStruct struct{} | ||
|
Check failure on line 30 in vm/runtime/runtime_internal_test.go
|
||
|
|
||
| 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) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be nice to test this function in isolation. A better unit tests for this function, ideally with 100% coverage for this function.