fix(builtin): handle string pointer in fromJSON #751
Conversation
Support *string input in fromJSON builtin function by dereferencing pointer before JSON unmarshaling. Added tests covering both direct string and pointer string use cases, including toJSON, which already supports this. Signed-off-by: Ville Vesilehto <[email protected]>
builtin/builtin.go
Outdated
| @@ -443,7 +443,11 @@ var Builtins = []*Function{ | |||
| Name: "fromJSON", | |||
| Func: func(args ...any) (any, error) { | |||
| var v any | |||
| err := json.Unmarshal([]byte(args[0].(string)), &v) | |||
| jsonStr := args[0] | |||
| if strPtr, ok := jsonStr.(*string); ok { | |||
There was a problem hiding this comment.
Althoug this is a working solution, we need to understand why OpDeref was not added before calling fromJSON.
Compiler adds OpDeref for arguments before calling a function. But only if needed. Aperently the check is not perfect.
|
We can merge this PR, and it will fix problem with *string and fromJSON. But we still can encounter those errors. |
|
I did a quick glance over the compiler earlier but maybe I missed something. Agree that this would ideally be fixed for other use cases as well than just JSON. I can take a look and keep this as a draft PR until then. |
This commit enables fromJSON to accept both string and *string arguments by performing a runtime check on the provided argument. It also refactors the deref.Type function to preserve interface types, ensuring methods sets are not lost when unwrapping pointers. Changes: - fromJSON now handles `string` and `*string` distinctly, returning an error if the pointer is nil or if an unsupported type is given. - deref.Type preserves the Kind() == reflect.Interface immediately instead of unwrapping further, which prevents losing interface method sets. - Updated unit tests to confirm that interface-wrapped pointers are unwrapped correctly (e.g., reflect.String) and interface types remain interfaces. Signed-off-by: Ville Vesilehto <[email protected]>
thevilledev
changed the title
|
I think I found a way to do this at compile-time, with just a little type hinting at runtime in EDIT: simplified it even further, I don't think we need |
thevilledev
changed the title
|
Well shoot. Looks like my assumption was wrong. Back to the drawing board 😅 |
|
Used this scenario when debugging:
So the compiler can't see through the |
Support
*stringinput in thefromJSONbuiltin function by runtime type handling. Type switch how to handle each case.Added tests covering both direct string and pointer-to-string use cases, including
toJSONfunction.Fixes #739