fix(patcher): ctx into nested custom funcs and Env#883
Uh oh!
There was an error while loading. Please reload this page.
Conversation
thevilledev commented Dec 16, 2025
I had an idea to maybe fix this through a checker-level fix but it turned out to be quite cumbersome due to side-effects to all the other expressions. Lemme know what you think @antonmedv 👍 |
patcher/with_context.go Outdated
| // If callee type is interface{} (unknown), try to look up the function type | ||
| // from the Functions table. This handles cases where the call is nested | ||
| // inside another call with an unknown callee type. | ||
| iffn.Kind() ==reflect.Interface&&w.Functions!=nil{ |
There was a problem hiding this comment.
But why date2() callnode type is any?
There was a problem hiding this comment.
IIUC the checker returns early without visiting date2() because now2() initially has 0 arguments, as the context has not been injected yet. This causes checkArguments to fail. Furthermore it makes the now2() call return unknown type. Then it cascades: memberNode for .After returns unknown, so the outer callNode returns early without visiting its arguments. And it leaves date2() identifier with interface{} type when WithContext runs.
I was playing around with the Checker-level fix like:
ifnt.IsUnknown(&v.config.NtCache){// Still visit arguments to set their types for patchersfor_, arg:=rangenode.Arguments{v.visit(arg) } returnNature{} }But it breaks a bunch of stuff, and I went with this. :)
I'll update the comment a bit.
There was a problem hiding this comment.
Thanks for explanation, let me also see if checker solution can be added.
| typeWithContextstruct{ | ||
| Namestring | ||
| Namestring | ||
| Functions conf.FunctionsTable// Optional: used to look up function types when callee type is unknown. |
There was a problem hiding this comment.
A new function can be also defined as a method of Env. Should it be checked there too?
There was a problem hiding this comment.
Thanks, good point! I'll check this out. And lets add a regression test for it too.
There was a problem hiding this comment.
Functions table and Env.MethodByName() now also included!
WithContext patcher now looks up function types from the Functions table and Env methods when the callee type is interface{}. This fixes context injection for custom functions and Env methods nested as arguments inside method calls with unknown callee types (e.g., Now2().After(Date2())). Also improved the regression test to actually verify context is passed to both functions, and added a test for Env methods. Signed-off-by: Ville Vesilehto <[email protected]>
thevilledevforce-pushed the fix/issue-823-part2 branch from 4857426 to 51c878bComparethevilledev changed the title 3 participants
WithContext patcher now looks up function types from the Functions table and Env methods when the callee type is
interface{}. This fixes context injection for custom functions and Env methods nested as arguments inside method calls with unknown callee types (e.g.,Now2().After(Date2())).Also improved the regression test to actually verify context is passed to both functions, and added a test for Env methods.
Should finally fix#823.