Provide context to Unmarshaler

[mugabe]

For my project, I need a way to inject dependencies into an unmarshaled type or modify its behavior based on the unmarshaling context.

I'm thinking of something like this:

type Foo struct {
  intValue int
}

func (f *Foo) UnmarshalYAML(ctx context.Context, value *Node) error {
  var strValue string
  value.Decode(&strValue)

  transformer := ctx.Value("transformer").(Transformer)
  f.intValue = transformer.Transform(strValue)
}

func main() {
  ctx := context.WithValue(context.Background(), "transformer", NewTransformer())
 
  var foo Foo
  yaml.UnmarshalWithContext(ctx, []byte{"foo"}, &foo)
}

I understand that my specific needs might go against some principles, and perhaps a different approach, such as transforming the decoded configuration into another type after unmarshaling, would be more suitable.

However, I believe that providing a context.Context to an Unmarshaler could be beneficial for other reasons as well.

My idea is to introduce a new UnmarshalerWithContext interface and an UnmarshalWithContext method. This method would forward the context.Context to the UnmarshalerWithContext interface if the target type implements it.

type UnmarshalerWithContext interface {
  UnmarshalYAML(ctx context.Context, value *Node) error
}

func UnmarshalWithContext(ctx context.Context, in []byte, out any) (err error) {
  ...
}

I'm aware that there's an ongoing discussion about a new Unmarshaler interface in issue #56, so it might be worth considering adding context there.

Additionally, I like the idea of adding options to Unmarshal as proposed in issue #155. In that scenario, the context could be provided via an option rather than by introducing new methods.

I've already implemented this functionality for my own needs in my fork (Draft PR #186). While I'm quite happy with it as is, it would be perfect if something similar could get a chance to be introduced in a future v4 release.

[ingydotnet]

Additionally, I like the idea of adding options to Unmarshal as proposed in issue #155. In that scenario, the context could be provided via an option rather than by introducing new methods.

To consider this I think it would need to be introduced as an option, not a major API addition.

[ingydotnet]

Well, the only API change I'm considering is Load() and Dump() which are present in most YAML implementations. They sit above Decode() and Encode() and use a Node's tag to identify a transform function to be applied to the node.
All nodes are tagged by the Unmarshal process.
We also want to add options to make the node tag resolution process be highly configurable.

This might accomplish what you are looking for here.

[ccoVeille]

Additionally, I like the idea of adding options to Unmarshal as proposed in issue #155. In that scenario, the context could be provided via an option rather than by introducing new methods.

To consider this I think it would need to be introduced as an option, not a major API addition.

Here, I have a different point of view based on my Go experience.

• context should always be the first argument
• context should never be stored in a struct

Unless I missed something in the discussion, using an option to pass a context means we will have a context stored in an struct, which is an anti pattern.

So, I agree with the needs described in the issue here:

• Allowing the Decode and others to react to the presence of an UnmarshalYAML method on a struct with a context being available sounds a good thing.

I will have to check, but I feel like goccy/go-yaml allows it. It could a good thing to inspire from what was done here.

Also, I will have to check what was done with std lib json/v2, I feel like if they didn't add the support of context, there could be a reason. So maybe it's not a good idea to open the Pandora's box and support them

The implementation details may differs from what was trued with #186, but it sounds OK to consider what you are suggesting.