Skip to content

feat: allow field access on concrete types behind interface values#952

Open
snaffi wants to merge 4 commits into
expr-lang:masterfrom
snaffi:feature/get-embedded-interface-properties
Open

feat: allow field access on concrete types behind interface values#952
snaffi wants to merge 4 commits into
expr-lang:masterfrom
snaffi:feature/get-embedded-interface-properties

Conversation

@snaffi
Copy link
Copy Markdown

@snaffi snaffi commented Mar 31, 2026

Issue
#951

Comment thread vm/runtime/runtime.go Outdated
fk := f.Type.Kind()

// Dereference pointers to get to the underlying type.
for fk == reflect.Ptr {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets use our deref package.

Comment thread vm/runtime/runtime.go
return reflect.Value{}, reflect.StructField{}, false
}

func fetchFromEmbeddedInterfaces(v reflect.Value, fieldName string) (any, bool) {
Copy link
Copy Markdown
Member

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.

snaffi added 2 commits May 19, 2026 09:19
…dedInterfaces

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.
@snaffi
Copy link
Copy Markdown
Author

snaffi commented May 19, 2026

@antonmedv hello, I appologize for late response. Yours comments have been addressed.

Title string
}

func TestFetchFromEmbeddedInterfaces(t *testing.T) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonmedv
Copy link
Copy Markdown
Member

Looks like something with tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants