Flag for fixed list merge key traverse override behavior,

and fix traversing map with merge key that would override local key (completes #2110 fix)

pkg/yqlib/doc/operators/anchor-and-alias-operators.md

@@ -96,6 +96,38 @@ y: 2
 x: 1
 ```
 
+## Override with local key
+like https://yaml.org/type/merge.html, but with x: 1 before the merge key. This is legacy behavior, see --yaml-fix-merge-anchor-to-spec
+
+Given a sample.yml file of:
+```yaml
+- &CENTER
+  x: 1
+  y: 2
+- &LEFT
+  x: 0
+  y: 2
+- &BIG
+  r: 10
+- &SMALL
+  r: 1
+- x: 1
+  !!merge <<:
+    - *BIG
+    - *LEFT
+    - *SMALL
+```
+then
+```bash
+yq '.[4] | explode(.)' sample.yml
+```
+will output
+```yaml
+x: 0
+r: 10
+y: 2
+```
+
 ## Get anchor
 Given a sample.yml file of:
 ```yaml

pkg/yqlib/doc/operators/traverse-read.md

@@ -304,6 +304,8 @@ foo_a
 ```
 
 ## Traversing merge anchors with override
+This is legacy behavior, see --yaml-fix-merge-anchor-to-spec
+
 Given a sample.yml file of:
 ```yaml
 foo: &foo
@@ -399,7 +401,7 @@ foobar_thing
 ```
 
 ## Traversing merge anchor lists
-Note that the keys earlier in the merge anchors sequence override later ones
+Note that the later merge anchors override previous, but this is legacy behavior, see --yaml-fix-merge-anchor-to-spec
 
 Given a sample.yml file of:
 ```yaml
@@ -428,10 +430,12 @@ yq '.foobarList.thing' sample.yml
 ```
 will output
 ```yaml
-foo_thing
+bar_thing
 ```
 
 ## Splatting merge anchor lists
+With legacy override behavior, see --yaml-fix-merge-anchor-to-spec
+
 Given a sample.yml file of:
 ```yaml
 foo: &foo
@@ -460,9 +464,9 @@ yq '.foobarList[]' sample.yml
 will output
 ```yaml
 bar_b
-foo_thing
-foobarList_c
 foo_a
+bar_thing
+foobarList_c
 ```
 
 ## Select multiple indices

pkg/yqlib/operator_anchors_aliases.go

@@ -3,6 +3,7 @@ package yqlib
 import (
 	"container/list"
 	"fmt"
+	"slices"
 )
 
 func assignAliasOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) {
@@ -237,9 +238,13 @@ func applyMergeAnchor(node *CandidateNode, merge *CandidateNode, mergeIndex int,
 		return applyMergeAnchorMap(node, merge, mergeIndex, inline, newContent)
 	case SequenceNode:
 		log.Debugf("a merge list!")
-		// Earlier keys take precedence
+		// With FixMergeAnchorToSpec, we rely on overrideEntry to reject duplicates
-		for index := len(merge.Content) - 1; index >= 0; index = index - 1 {
+		content := slices.All(merge.Content)
-			childValue := merge.Content[index]
+		if !ConfiguredYamlPreferences.FixMergeAnchorToSpec {
+			// Even without FixMergeAnchorToSpec, this already gave preference to earlier keys
+			content = slices.Backward(merge.Content)
+		}
+		for _, childValue := range content {
 			childInline := inline
 			if childValue.Kind == AliasNode {
 				childInline = false
@@ -303,8 +308,9 @@ func overrideEntry(node *CandidateNode, key *CandidateNode, value *CandidateNode
 		keyNode := newEl.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")
+			log.Debugf("overriding new content")
 			if !ConfiguredYamlPreferences.FixMergeAnchorToSpec {
+				//TODO This also fires in when an earlier element in a list merge anchor overwrites a later element, which *is* to the spec
 				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
 			}

pkg/yqlib/operator_anchors_aliases_test.go

@@ -122,6 +122,7 @@ var anchorOperatorScenarios = []expressionScenario{
 		expression:     ".[4] | explode(.)",
 		expected:       []string{"D0, P[4], (!!map)::r: 10\nx: 1\ny: 2\n"},
 	},
+	//TODO The following 2 tests warn about overwriting [3].r not being to spec while they shouldn't
 	{
 		description:    "Override",
 		subdescription: "see https://yaml.org/type/merge.html",
@@ -129,6 +130,15 @@ var anchorOperatorScenarios = []expressionScenario{
 		expression:     ".[4] | explode(.)",
 		expected:       []string{"D0, P[4], (!!map)::r: 10\ny: 2\nx: 1\n"},
 	},
+	// Correctly warns about overwriting [4].x
+	{
+		description: "Override with local key",
+		subdescription: "like https://yaml.org/type/merge.html, but with x: 1 before the merge key. " +
+			"This is legacy behavior, see --yaml-fix-merge-anchor-to-spec",
+		document:   specDocument + "- x: 1\n  << : [ *BIG, *LEFT, *SMALL ]\n",
+		expression: ".[4] | explode(.)",
+		expected:   []string{"D0, P[4], (!!map)::x: 0\nr: 10\ny: 2\n"},
+	},
 	{
 		description: "Get anchor",
 		document:    `a: &billyBob cat`,

pkg/yqlib/operator_traverse_path.go

@@ -3,7 +3,6 @@ package yqlib
 import (
 	"container/list"
 	"fmt"
-
 	"github.com/elliotchance/orderedmap"
 )
 
@@ -280,11 +279,39 @@ func doTraverseMap(newMatches *orderedmap.OrderedMap, node *CandidateNode, wante
 			log.Debug("MATCHED")
 			if prefs.IncludeMapKeys {
 				log.Debug("including key")
-				newMatches.Set(key.GetKey(), key)
+				keyName := key.GetKey()
+				if newMatches.Has(keyName) {
+					if ConfiguredYamlPreferences.FixMergeAnchorToSpec {
+						log.Debug("not overwriting existing key")
+					} else {
+						log.Warning(
+							"--yaml-fix-merge-anchor-to-spec is false; "+
+								"causing the merge anchor to override the existing key at %v which isn't to the yaml spec. "+
+								"This flag will default to true in late 2025.", key.GetNicePath())
+						log.Debug("overwriting existing key")
+						newMatches.Set(keyName, key)
+					}
+				} else {
+					newMatches.Set(keyName, key)
+				}
 			}
 			if !prefs.DontIncludeMapValues {
 				log.Debug("including value")
-				newMatches.Set(value.GetKey(), value)
+				valueName := value.GetKey()
+				if newMatches.Has(valueName) {
+					if ConfiguredYamlPreferences.FixMergeAnchorToSpec {
+						log.Debug("not overwriting existing value")
+					} else {
+						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.", key.GetNicePath())
+						log.Debug("overwriting existing value")
+						newMatches.Set(valueName, value)
+					}
+				} else {
+					newMatches.Set(valueName, value)
+				}
 			}
 		}
 	}
@@ -300,9 +327,7 @@ func traverseMergeAnchor(newMatches *orderedmap.OrderedMap, merge *CandidateNode
 	case MappingNode:
 		return doTraverseMap(newMatches, merge, wantedKey, prefs, splat)
 	case SequenceNode:
-		// Earlier keys take precedence
+		for _, childValue := range merge.Content {
-		for index := len(merge.Content) - 1; index >= 0; index = index - 1 {
-			childValue := merge.Content[index]
 			if childValue.Kind == AliasNode {
 				childValue = childValue.Alias
 			}

pkg/yqlib/operator_traverse_path_test.go

@@ -25,6 +25,26 @@ foobar:
   thing: foobar_thing
 `
 
+var fixedTraversePathOperatorScenarios = []expressionScenario{
+	{
+		description:    "Traversing merge anchor lists",
+		subdescription: "Note that the keys earlier in the merge anchors sequence override later ones",
+		document:       mergeDocSample,
+		expression:     `.foobarList.thing`,
+		expected: []string{
+			"D0, P[foo thing], (!!str)::foo_thing\n",
+		},
+	},
+	{
+		description: "Traversing merge anchors with override",
+		document:    mergeDocSample,
+		expression:  `.foobar.c`,
+		expected: []string{
+			"D0, P[foobar c], (!!str)::foobar_c\n",
+		},
+	},
+}
+
 var traversePathOperatorScenarios = []expressionScenario{
 	{
 		skipDoc:     true,
@@ -400,9 +420,10 @@ var traversePathOperatorScenarios = []expressionScenario{
 		},
 	},
 	{
-		description: "Traversing merge anchors with override",
+		description:    "Traversing merge anchors with override",
-		document:    mergeDocSample,
+		subdescription: "This is legacy behavior, see --yaml-fix-merge-anchor-to-spec",
-		expression:  `.foobar.c`,
+		document:       mergeDocSample,
+		expression:     `.foobar.c`,
 		expected: []string{
 			"D0, P[foo c], (!!str)::foo_c\n",
 		},
@@ -442,12 +463,13 @@ var traversePathOperatorScenarios = []expressionScenario{
 		},
 	},
 	{
-		description:    "Traversing merge anchor lists",
+		description: "Traversing merge anchor lists",
-		subdescription: "Note that the keys earlier in the merge anchors sequence override later ones",
+		subdescription: "Note that the later merge anchors override previous, " +
-		document:       mergeDocSample,
+			"but this is legacy behavior, see --yaml-fix-merge-anchor-to-spec",
-		expression:     `.foobarList.thing`,
+		document:   mergeDocSample,
+		expression: `.foobarList.thing`,
 		expected: []string{
-			"D0, P[foo thing], (!!str)::foo_thing\n",
+			"D0, P[bar thing], (!!str)::bar_thing\n",
 		},
 	},
 	{
@@ -467,14 +489,15 @@ var traversePathOperatorScenarios = []expressionScenario{
 		},
 	},
 	{
-		description: "Splatting merge anchor lists",
+		description:    "Splatting merge anchor lists",
-		document:    mergeDocSample,
+		subdescription: "With legacy override behavior, see --yaml-fix-merge-anchor-to-spec",
-		expression:  `.foobarList[]`,
+		document:       mergeDocSample,
+		expression:     `.foobarList[]`,
 		expected: []string{
 			"D0, P[bar b], (!!str)::bar_b\n",
-			"D0, P[foo thing], (!!str)::foo_thing\n",
-			"D0, P[foobarList c], (!!str)::foobarList_c\n",
 			"D0, P[foo a], (!!str)::foo_a\n",
+			"D0, P[bar thing], (!!str)::bar_thing\n",
+			"D0, P[foobarList c], (!!str)::foobarList_c\n",
 		},
 	},
 	{
@@ -575,3 +598,11 @@ func TestTraversePathOperatorScenarios(t *testing.T) {
 	}
 	documentOperatorScenarios(t, "traverse-read", traversePathOperatorScenarios)
 }
+
+func TestTraversePathOperatorAlignedToSpecScenarios(t *testing.T) {
+	ConfiguredYamlPreferences.FixMergeAnchorToSpec = true
+	for _, tt := range fixedTraversePathOperatorScenarios {
+		testScenario(t, &tt)
+	}
+	ConfiguredYamlPreferences.FixMergeAnchorToSpec = false
+}