Skip to content

feat: support receiving ast.Node in custom unmarshalers #642

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

Merged
merged 2 commits into from
Mar 16, 2025

Conversation

x1unix
Copy link
Contributor

@x1unix x1unix commented Feb 7, 2025

The go-yaml library in addition to custom unmarshalers, also allows partial unmarshaling into ast.Node struct members.
This can be useful when working with dynamic documents.

Unfortunately this is not enough when there is a need to have a custom type which needs to be constructed out of ast.Node but you also would like to avoid inner unmarshaling inside TextUnmarshaler.

In my case, this is also useful in order to provide a detailed information about position in returned syntax error.

Use-case demo

Let's assume you've an application which can accept some dynamic value which needs to be validated later.

Application accepts an input something like this:

tasks:
  build:
    - action: foo
      timeout: 10s
      with:
        # Here is a dynamic content
        key: value
        items:
          - 1
          - ${foo.bar.baz} # App supports might want to pre-process some fields
          - true

Most of the content is static except tasks.[*].with which can have different body depending on context.
Currently in order to process that, you would need to unmarshal whole document into a struct below and process ast.Node fields separately.

type Doc struct {
	Tasks map[string]struct{
		Action string
		With ast.Node
	}
}

This PR introduces NodeUnmarshaler and NodeUnmarshalerContext interfaces that allow moving AST traversal into an unmarshaler.

Testing

I've added tests for both new interfaces into yaml_test.go

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.36%. Comparing base (8d4b192) to head (423164a).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
+ Coverage   77.34%   77.36%   +0.01%     
==========================================
  Files          22       22              
  Lines        7905     7903       -2     
==========================================
  Hits         6114     6114              
+ Misses       1372     1370       -2     
  Partials      419      419              

@goccy
Copy link
Owner

goccy commented Feb 12, 2025

@x1unix It is already possible to use ast.Node in struct fields. There is no need to add new functionality.

@goccy goccy added the reviewed label Feb 12, 2025
@x1unix
Copy link
Contributor Author

x1unix commented Feb 13, 2025

@goccy yes, you can unmarshal to ast.Node but then struct needs additional processing.

@asmfreak
Copy link
Contributor

Another good thing about this proposal is that it makes possible to output beautiful errors just as the rest of the library does. As with ast.Node, we could also return errors using node.Token(). For some reason, if you get ast.Node from inside InterfaceUnmarshaller, its token is nil.

@goccy goccy merged commit 53701cc into goccy:master Mar 16, 2025
20 checks passed
@x1unix x1unix deleted the feat/node-unmarshaler branch March 28, 2025 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants