Fixing merge anchor key order

This commit is contained in:
Mike Farah 2025-07-19 15:27:44 +10:00
parent 4019d42d60
commit 23a7b173bf
6 changed files with 345 additions and 143 deletions

View File

@ -169,6 +169,18 @@ func (n *CandidateNode) getParsedKey() interface{} {
}
func (n *CandidateNode) FilterMapContentByKey(keyPredicate func(*CandidateNode) bool) []*CandidateNode {
var result []*CandidateNode
for index := 0; index < len(n.Content); index = index + 2 {
keyNode := n.Content[index]
valueNode := n.Content[index+1]
if keyPredicate(keyNode) {
result = append(result, keyNode, valueNode)
}
}
return result
}
func (n *CandidateNode) GetPath() []interface{} {
key := n.getParsedKey()
if n.Parent != nil && key != nil {

View File

@ -34,68 +34,6 @@ y: 2
r: 10
```
## Merge multiple maps
see https://yaml.org/type/merge.html
Given a sample.yml file of:
```yaml
- &CENTER
x: 1
y: 2
- &LEFT
x: 0
y: 2
- &BIG
r: 10
- &SMALL
r: 1
- !!merge <<:
- *CENTER
- *BIG
```
then
```bash
yq '.[4] | explode(.)' sample.yml
```
will output
```yaml
r: 10
x: 1
y: 2
```
## Override
see https://yaml.org/type/merge.html
Given a sample.yml file of:
```yaml
- &CENTER
x: 1
y: 2
- &LEFT
x: 0
y: 2
- &BIG
r: 10
- &SMALL
r: 1
- !!merge <<:
- *BIG
- *LEFT
- *SMALL
x: 1
```
then
```bash
yq '.[4] | explode(.)' sample.yml
```
will output
```yaml
r: 10
x: 1
y: 2
```
## Get anchor
Given a sample.yml file of:
```yaml
@ -254,7 +192,101 @@ f:
cat: b
```
## Explode with merge anchors
## Dereference and update a field
Use explode with multiply to dereference an object
Given a sample.yml file of:
```yaml
item_value: &item_value
value: true
thingOne:
name: item_1
!!merge <<: *item_value
thingTwo:
name: item_2
!!merge <<: *item_value
```
then
```bash
yq '.thingOne |= explode(.) * {"value": false}' sample.yml
```
will output
```yaml
item_value: &item_value
value: true
thingOne:
name: item_1
value: false
thingTwo:
name: item_2
!!merge <<: *item_value
```
## Merge multiple maps
see https://yaml.org/type/merge.html
Given a sample.yml file of:
```yaml
- &CENTER
x: 1
y: 2
- &LEFT
x: 0
y: 2
- &BIG
r: 10
- &SMALL
r: 1
- !!merge <<:
- *CENTER
- *BIG
```
then
```bash
yq '.[4] | explode(.)' sample.yml
```
will output
```yaml
r: 10
x: 1
y: 2
```
## Override
see https://yaml.org/type/merge.html
Given a sample.yml file of:
```yaml
- &CENTER
x: 1
y: 2
- &LEFT
x: 0
y: 2
- &BIG
r: 10
- &SMALL
r: 1
- !!merge <<:
- *BIG
- *LEFT
- *SMALL
x: 1
```
then
```bash
yq '.[4] | explode(.)' sample.yml
```
will output
```yaml
r: 10
x: 1
y: 2
```
## LEGACY: Explode with merge anchors
Caution: this is for when --yaml-fix-merge-anchor-to-spec=false; it's not to YAML spec because the merge anchors incorrectly override the object values. Flag will default to true in late 2025
Given a sample.yml file of:
```yaml
foo: &foo
@ -301,33 +333,52 @@ foobar:
thing: foobar_thing
```
## Dereference and update a field
Use explode with multiply to dereference an object
## FIXED: Explode with merge anchors
Set `--yaml-fix-merge-anchor-to-spec=true` to get this correct merge behaviour. Flag will default to true in late 2025
Given a sample.yml file of:
```yaml
item_value: &item_value
value: true
thingOne:
name: item_1
!!merge <<: *item_value
thingTwo:
name: item_2
!!merge <<: *item_value
foo: &foo
a: foo_a
thing: foo_thing
c: foo_c
bar: &bar
b: bar_b
thing: bar_thing
c: bar_c
foobarList:
b: foobarList_b
!!merge <<:
- *foo
- *bar
c: foobarList_c
foobar:
c: foobar_c
!!merge <<: *foo
thing: foobar_thing
```
then
```bash
yq '.thingOne |= explode(.) * {"value": false}' sample.yml
yq 'explode(.)' sample.yml
```
will output
```yaml
item_value: &item_value
value: true
thingOne:
name: item_1
value: false
thingTwo:
name: item_2
!!merge <<: *item_value
foo:
a: foo_a
thing: foo_thing
c: foo_c
bar:
b: bar_b
thing: bar_thing
c: bar_c
foobarList:
b: foobarList_b
a: foo_a
thing: foo_thing
c: foobarList_c
foobar:
c: foobar_c
a: foo_a
thing: foobar_thing
```

View File

@ -28,6 +28,17 @@ func GetLogger() *logging.Logger {
return log
}
func getContentValueByKey(content []*CandidateNode, key string) *CandidateNode {
for index := 0; index < len(content); index = index + 2 {
keyNode := content[index]
valueNode := content[index+1]
if keyNode.Value == key {
return valueNode
}
}
return nil
}
func recurseNodeArrayEqual(lhs *CandidateNode, rhs *CandidateNode) bool {
if len(lhs.Content) != len(rhs.Content) {
return false

View File

@ -138,6 +138,41 @@ func explodeOperator(d *dataTreeNavigator, context Context, expressionNode *Expr
return context, nil
}
func fixedReconstructAliasedMap(node *CandidateNode) error {
var newContent = []*CandidateNode{}
for index := 0; index < len(node.Content); index = index + 2 {
keyNode := node.Content[index]
valueNode := node.Content[index+1]
if keyNode.Value != "<<" {
// always add in plain nodes
newContent = append(newContent, keyNode, valueNode)
} else {
sequence := valueNode
if sequence.Kind != SequenceNode {
sequence = &CandidateNode{Content: []*CandidateNode{sequence}}
}
for index := 0; index < len(sequence.Content); index = index + 1 {
// for merge anchors, we only set them if the key is not already in node or the newContent
mergeNodeSeq := sequence.Content[index]
if mergeNodeSeq.Kind == AliasNode {
mergeNodeSeq = mergeNodeSeq.Alias
}
if mergeNodeSeq.Kind != MappingNode {
return fmt.Errorf("merge anchor only supports maps, got !!seq instead")
}
itemsToAdd := mergeNodeSeq.FilterMapContentByKey(func(keyNode *CandidateNode) bool {
return getContentValueByKey(node.Content, keyNode.Value) == nil &&
getContentValueByKey(newContent, keyNode.Value) == nil
})
newContent = append(newContent, itemsToAdd...)
}
}
}
node.Content = newContent
return nil
}
func reconstructAliasedMap(node *CandidateNode, context Context) error {
var newContent = list.New()
// can I short cut here by prechecking if there's an anchor in the map?
@ -215,6 +250,10 @@ func explodeNode(node *CandidateNode, context Context) error {
}
if hasAlias {
if ConfiguredYamlPreferences.FixMergeAnchorToSpec {
return fixedReconstructAliasedMap(node)
}
log.Warning("--yaml-fix-merge-anchor-to-spec is false; causing merge anchors to override the existing values which isn't to the yaml spec. This flag will default to true in late 2025.")
// this is a slow op, which is why we want to check before running it.
return reconstructAliasedMap(node, context)
}
@ -272,10 +311,7 @@ func overrideEntry(node *CandidateNode, key *CandidateNode, value *CandidateNode
log.Debugf("checking new content %v:%v", keyNode.Value, valueEl.Value.(*CandidateNode).Value)
if keyNode.Value == key.Value && keyNode.Alias == nil && key.Alias == nil {
log.Debugf("overridign new content")
if !ConfiguredYamlPreferences.FixMergeAnchorToSpec {
log.Warning("--yaml-fix-merge-anchor-to-spec is false; causing the merge anchor to override the existing value at %v which isn't to the yaml spec. This flag will default to true in late 2025.", keyNode.GetNicePath())
valueEl.Value = value
}
valueEl.Value = value
return nil
}
newEl = valueEl // move forward twice

View File

@ -34,6 +34,25 @@ thingTwo:
!!merge <<: *item_value
`
var explodeMergeAnchorsFixedExpected = `D0, P[], (!!map)::foo:
a: foo_a
thing: foo_thing
c: foo_c
bar:
b: bar_b
thing: bar_thing
c: bar_c
foobarList:
b: foobarList_b
a: foo_a
thing: foo_thing
c: foobarList_c
foobar:
c: foobar_c
a: foo_a
thing: foobar_thing
`
var explodeMergeAnchorsExpected = `D0, P[], (!!map)::foo:
a: foo_a
thing: foo_thing
@ -83,23 +102,115 @@ var explodeWhenKeysExistExpected = `D0, P[], (!!map)::objects:
var fixedAnchorOperatorScenarios = []expressionScenario{
{
skipDoc: true,
description: "merge anchor after existing keys",
subdescription: "legacy: overrides existing keys",
document: explodeWhenKeysExistDocument,
expression: "explode(.)",
expected: []string{explodeWhenKeysExistExpected},
skipDoc: true,
description: "merge anchor after existing keys",
document: explodeWhenKeysExistDocument,
expression: "explode(.)",
expected: []string{explodeWhenKeysExistExpected},
},
{
skipDoc: true, // skip doc for now, only difference is order of keys
description: "Merge multiple maps",
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *CENTER, *BIG ]\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::x: 1\ny: 2\nr: 10\n"},
},
{
skipDoc: true, // skip doc for now, only difference is order of keys
description: "Override",
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *BIG, *LEFT, *SMALL ]\n x: 1\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::r: 10\ny: 2\nx: 1\n"},
},
{
description: "FIXED: Explode with merge anchors",
subdescription: "Set `--yaml-fix-merge-anchor-to-spec=true` to get this correct merge behaviour. Flag will default to true in late 2025 ",
document: mergeDocSample,
expression: `explode(.)`,
expected: []string{explodeMergeAnchorsFixedExpected},
},
{
skipDoc: true,
document: mergeDocSample,
expression: `.foo* | explode(.) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: foobarList_b, a: foo_a, thing: foo_thing, c: foobarList_c}\n",
"D0, P[foobar], (!!map)::{c: foobar_c, a: foo_a, thing: foobar_thing}\n",
},
},
{
skipDoc: true,
document: mergeDocSample,
expression: `.foo* | explode(explode(.)) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: foobarList_b, a: foo_a, thing: foo_thing, c: foobarList_c}\n",
"D0, P[foobar], (!!map)::{c: foobar_c, a: foo_a, thing: foobar_thing}\n",
},
},
}
var badAnchorOperatorScenarios = []expressionScenario{
{
skipDoc: true, // incorrect overrides
description: "LEGACY: merge anchor after existing keys",
document: explodeWhenKeysExistDocument,
expression: "explode(.)",
expected: []string{explodeWhenKeysExistLegacy},
},
{
description: "Merge multiple maps", // functionally correct, but key order gets mangled
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *CENTER, *BIG ]\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::r: 10\nx: 1\ny: 2\n"},
},
{
description: "Override", // functionally correct, but key order gets mangled
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *BIG, *LEFT, *SMALL ]\n x: 1\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::r: 10\nx: 1\ny: 2\n"},
},
{
description: "LEGACY: Explode with merge anchors", // incorrect overrides
subdescription: "Caution: this is for when --yaml-fix-merge-anchor-to-spec=false; it's not to YAML spec because the merge anchors incorrectly override the object values. Flag will default to true in late 2025",
document: mergeDocSample,
expression: `explode(.)`,
expected: []string{explodeMergeAnchorsExpected},
},
{
skipDoc: true,
document: mergeDocSample, // incorrect overrides
expression: `.foo* | explode(.) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: bar_b, thing: foo_thing, c: foobarList_c, a: foo_a}\n",
"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
},
},
{
skipDoc: true,
document: mergeDocSample,
expression: `.foo* | explode(explode(.)) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: bar_b, thing: foo_thing, c: foobarList_c, a: foo_a}\n",
"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
},
},
}
var anchorOperatorScenarios = []expressionScenario{
{
skipDoc: true,
description: "merge anchor after existing keys",
subdescription: "legacy: overrides existing keys",
document: explodeWhenKeysExistDocument,
expression: "explode(.)",
expected: []string{explodeWhenKeysExistLegacy},
skipDoc: true,
description: "merge anchor to alias alias",
document: "b: &b 10\na: &a { k: *b }\nc:\n <<: [*a]",
expression: "explode(.)",
expected: []string{"D0, P[], (!!map)::b: 10\na: {k: 10}\nc:\n k: 10\n"},
},
{
skipDoc: true,
@ -115,20 +226,6 @@ var anchorOperatorScenarios = []expressionScenario{
expression: ".[4] | explode(.)",
expected: []string{expectedSpecResult},
},
{
description: "Merge multiple maps",
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *CENTER, *BIG ]\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::r: 10\nx: 1\ny: 2\n"},
},
{
description: "Override",
subdescription: "see https://yaml.org/type/merge.html",
document: specDocument + "- << : [ *BIG, *LEFT, *SMALL ]\n x: 1\n",
expression: ".[4] | explode(.)",
expected: []string{"D0, P[4], (!!map)::r: 10\nx: 1\ny: 2\n"},
},
{
description: "Get anchor",
document: `a: &billyBob cat`,
@ -289,32 +386,6 @@ var anchorOperatorScenarios = []expressionScenario{
"D0, P[], (!!map)::{f: {a: cat, cat: b}}\n",
},
},
{
description: "Explode with merge anchors",
document: mergeDocSample,
expression: `explode(.)`,
expected: []string{explodeMergeAnchorsExpected},
},
{
skipDoc: true,
document: mergeDocSample,
expression: `.foo* | explode(.) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: bar_b, thing: foo_thing, c: foobarList_c, a: foo_a}\n",
"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
},
},
{
skipDoc: true,
document: mergeDocSample,
expression: `.foo* | explode(explode(.)) | (. style="flow")`,
expected: []string{
"D0, P[foo], (!!map)::{a: foo_a, thing: foo_thing, c: foo_c}\n",
"D0, P[foobarList], (!!map)::{b: bar_b, thing: foo_thing, c: foobarList_c, a: foo_a}\n",
"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
},
},
{
skipDoc: true,
document: `{f : {a: &a cat, b: &b {foo: *a}, *a: *b}}`,
@ -333,16 +404,18 @@ var anchorOperatorScenarios = []expressionScenario{
}
func TestAnchorAliasOperatorScenarios(t *testing.T) {
for _, tt := range anchorOperatorScenarios {
for _, tt := range append(anchorOperatorScenarios, badAnchorOperatorScenarios...) {
testScenario(t, &tt)
}
documentOperatorScenarios(t, "anchor-and-alias-operators", anchorOperatorScenarios)
documentOperatorScenarios(t, "anchor-and-alias-operators", append(anchorOperatorScenarios, badAnchorOperatorScenarios...))
}
func TestAnchorAliasOperatorAlignedToSpecScenarios(t *testing.T) {
ConfiguredYamlPreferences.FixMergeAnchorToSpec = true
for _, tt := range fixedAnchorOperatorScenarios {
for _, tt := range append(fixedAnchorOperatorScenarios, anchorOperatorScenarios...) {
testScenario(t, &tt)
}
appendOperatorDocumentScenario(t, "anchor-and-alias-operators", fixedAnchorOperatorScenarios)
ConfiguredYamlPreferences.FixMergeAnchorToSpec = false
}

View File

@ -6,6 +6,7 @@ import (
"container/list"
"fmt"
"io"
"io/fs"
"os"
"sort"
"strings"
@ -37,7 +38,7 @@ var goccyTesting = false
var testingDecoder = NewYamlDecoder(ConfiguredYamlPreferences)
func TestMain(m *testing.M) {
logging.SetLevel(logging.ERROR, "")
logging.SetLevel(logging.WARNING, "")
if os.Getenv("DEBUG") == "true" {
logging.SetLevel(logging.DEBUG, "")
}
@ -75,6 +76,7 @@ func testScenario(t *testing.T, s *expressionScenario) {
if s.skipForGoccy {
return
}
log.Debugf("\n\ntesting scenario %v", s.description)
var err error
node, err := getExpressionParser().ParseExpression(s.expression)
if err != nil {
@ -220,7 +222,8 @@ func formatYaml(yaml string, filename string) string {
type documentScenarioFunc func(t *testing.T, writer *bufio.Writer, scenario interface{})
func documentScenarios(t *testing.T, folder string, title string, scenarios []interface{}, documentScenario documentScenarioFunc) {
f, err := os.Create(fmt.Sprintf("doc/%v/%v.md", folder, title))
filename := fmt.Sprintf("doc/%v/%v.md", folder, title)
f, err := os.Create(filename)
if err != nil {
t.Error(err)
@ -250,6 +253,22 @@ func documentScenarios(t *testing.T, folder string, title string, scenarios []in
w.Flush()
}
func appendOperatorDocumentScenario(t *testing.T, title string, scenarios []expressionScenario) {
filename := fmt.Sprintf("doc/%v/%v.md", "operators", title)
f, err := os.OpenFile(filename, os.O_WRONLY|os.O_APPEND, fs.ModeAppend)
if err != nil {
t.Error(err)
return
}
defer f.Close()
w := bufio.NewWriter(f)
for _, s := range scenarios {
documentOperatorScenario(t, w, s)
}
w.Flush()
}
func documentOperatorScenarios(t *testing.T, title string, scenarios []expressionScenario) {
genericScenarios := make([]interface{}, len(scenarios))
for i, s := range scenarios {