Skip to content

Fix some issues around merge key/anchor handling #2400

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

stevenwdv
Copy link

@stevenwdv stevenwdv commented Jun 13, 2025

This PR fixes multiple issues related to merge keys (!!merge <<):

  • Allow inline maps instead of just aliases (e.g. {<<: {a: 42}}, {<<: [{a: 42}]}), both for traversing and exploding (closes Merge key with inline map broken #2386)
  • Allow aliases to sequences to be used (e.g. {s: &s [{a: 42}], m: {<<: *s}}), both for traversing and exploding (closes Explode with merge aliases throw an error #2178)
  • Disallow nested sequences (e.g. {<<: [[{a: 42}]]})
  • Disallow other (inline) types (e.g. {<<: 42})
  • However: Ignore invalid merge keys when traversing maps. Previously cases like {a: &a 42, m: {<<: *a}} were reported but cases like {m: {<<: 42}} were not. I chose to ignore all errors because otherwise traversing such a map (e.g. to correct types) becomes impossible.
  • Errors are not ignored when using explode. Previously explode would just leave out invalid merge keys. Another potential solution would be to leave them as-is.
  • Fix precedence of merge anchor sequence for traverse (explode was already correct), now it correctly lets earlier keys overwrite later keys according to the spec.
  • Fix excessive exploding with merge keys, e.g. previously echo '{b: &b {a: &a 42}, r: *a, c: {<<: *b}}' | yq 'explode(.c)' would yield {b: &b {a: 42}, r: *a, c: {a: 42}}, making r: *a a dangling reference. Now it yields {b: &b {a: &a 42}, r: *a, c: {a: &a 42}}.
  • For exploding, detect merge keys using tag (!!merge) rather than value (<<).

I do still have a question: I saw in operator_traverse_path_test.go that goccy would reject at least some invalid merge types at parse time, which might make using custom tags like <<: !include file.yaml (see #2386) impossible. Am I right about that? And does that mean that this usage will not be supported in the future? If so, that would be a bit sad. See also the note above about ignoring invalid merge keys.

- Allow inline maps instead of just aliases
- Disallow nested sequences
- Disallow other types

Closes mikefarah#2386
@stevenwdv stevenwdv marked this pull request as draft June 13, 2025 19:35
@stevenwdv
Copy link
Author

Wait, this actually doesn't fix explode(.) yet. Converting to draft for now, I'll do that later.

- Allow inline maps instead of just aliases
- Allow aliased sequences
- Disallow other types
- Use tag `!!merge` instead of key `<<`
- Fix insertion index for sequence merge

Closes mikefarah#2386
@stevenwdv stevenwdv changed the title Fix merge anchor traversing Fix merge anchor handling Jun 16, 2025
@stevenwdv stevenwdv marked this pull request as ready for review June 16, 2025 12:57
@stevenwdv stevenwdv changed the title Fix merge anchor handling Fix merge key/anchor handling Jun 16, 2025
@stevenwdv stevenwdv changed the title Fix merge key/anchor handling Fix issues around merge key/anchor handling Jun 16, 2025
@stevenwdv stevenwdv changed the title Fix issues around merge key/anchor handling Fix some issues around merge key/anchor handling Jun 16, 2025
@stevenwdv
Copy link
Author

stevenwdv commented Jun 16, 2025

EDIT: nvm fixed it


Ok, the excessive exploding fix I mentioned above may actually cause problems of its own:
echo '{a: {b: &b 42}, <<: {c: *b}}' | yq 'explode(.)' now returns {a: {b: 42}, c: *b}, which also has a dangling alias...

However, I don't really know how to solve this, because it depends on if the mapping node to which the merge anchor refers is also being exploded, e.g. with {r: &r {...}, m: {<<: *r}} we don't want to explode .r, although we might make a copy and then explode it.

And apparently this issue is already present when partially exploding: echo '{a: {b: &b 42}, c: *b}' | yq 'explode(.a)' in the latest release v4.45.4 returns {a: {b: 42}, c: *b}, which also has a dangling alias. Is that a bug or intentional?

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.

Merge key with inline map broken Explode with merge aliases throw an error
1 participant