Fix some issues around merge key/anchor handling

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

@@ -92,8 +92,8 @@ yq '.[4] | explode(.)' sample.yml
 will output
 ```yaml
 r: 10
-x: 1
 y: 2
+x: 1
 ```
 
 ## Get anchor
@@ -293,8 +293,8 @@ bar:
 foobarList:
   b: bar_b
   thing: foo_thing
-  c: foobarList_c
   a: foo_a
+  c: foobarList_c
 foobar:
   c: foo_c
   a: foo_a

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

@@ -399,7 +399,7 @@ foobar_thing
 ```
 
 ## Traversing merge anchor lists
-Note that the later merge anchors override previous
+Note that the keys earlier in the merge anchors sequence override later ones
 
 Given a sample.yml file of:
 ```yaml
@@ -428,7 +428,7 @@ yq '.foobarList.thing' sample.yml
 ```
 will output
 ```yaml
-bar_thing
+foo_thing
 ```
 
 ## Splatting merge anchor lists
@@ -460,9 +460,9 @@ yq '.foobarList[]' sample.yml
 will output
 ```yaml
 bar_b
-foo_a
-bar_thing
+foo_thing
 foobarList_c
+foo_a
 ```
 
 ## Select multiple indices

pkg/yqlib/operator_anchors_aliases.go

@@ -147,27 +147,15 @@ func reconstructAliasedMap(node *CandidateNode, context Context) error {
 		keyNode := node.Content[index]
 		valueNode := node.Content[index+1]
 		log.Debugf("traversing %v", keyNode.Value)
-		if keyNode.Value != "<<" {
-			err := overrideEntry(node, keyNode, valueNode, index, context.ChildContext(newContent))
+		if keyNode.Tag != "!!merge" {
+			err := overrideEntry(node, keyNode, valueNode, index, true, context.ChildContext(newContent))
 			if err != nil {
 				return err
 			}
 		} else {
-			if valueNode.Kind == SequenceNode {
-				log.Debugf("an alias merge list!")
-				for index := len(valueNode.Content) - 1; index >= 0; index = index - 1 {
-					aliasNode := valueNode.Content[index]
-					err := applyAlias(node, aliasNode.Alias, index, context.ChildContext(newContent))
-					if err != nil {
-						return err
-					}
-				}
-			} else {
-				log.Debugf("an alias merge!")
-				err := applyAlias(node, valueNode.Alias, index, context.ChildContext(newContent))
-				if err != nil {
-					return err
-				}
+			err := applyMergeAnchor(node, valueNode, index, context.ChildContext(newContent))
+			if err != nil {
+				return err
 			}
 		}
 	}
@@ -208,7 +196,7 @@ func explodeNode(node *CandidateNode, context Context) error {
 		hasAlias := false
 		for index := 0; index < len(node.Content); index = index + 2 {
 			keyNode := node.Content[index]
-			if keyNode.Value == "<<" {
+			if keyNode.Tag == "!!merge" {
 				hasAlias = true
 				break
 			}
@@ -237,33 +225,77 @@ func explodeNode(node *CandidateNode, context Context) error {
 	}
 }
 
-func applyAlias(node *CandidateNode, alias *CandidateNode, aliasIndex int, newContent Context) error {
-	log.Debug("alias is nil ?")
-	if alias == nil {
+func applyMergeAnchor(node *CandidateNode, merge *CandidateNode, mergeIndex int, newContent Context) error {
+	inline := true
+	if merge.Kind == AliasNode {
+		inline = false
+		merge = merge.Alias
+	}
+	switch merge.Kind {
+	case MappingNode:
+		log.Debugf("a merge map!")
+		return applyMergeAnchorMap(node, merge, mergeIndex, inline, newContent)
+	case SequenceNode:
+		log.Debugf("a merge list!")
+		// Earlier keys take precedence
+		for index := len(merge.Content) - 1; index >= 0; index = index - 1 {
+			childValue := merge.Content[index]
+			childInline := inline
+			if childValue.Kind == AliasNode {
+				childInline = false
+				childValue = childValue.Alias
+			}
+			if childValue.Kind != MappingNode {
+				return fmt.Errorf(
+					"can only use merge anchors with maps (!!map) or sequences (!!seq) of maps, but got sequence containing %v",
+					childValue.Tag)
+			}
+			err := applyMergeAnchorMap(node, childValue, mergeIndex, childInline, newContent)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	default:
+		return fmt.Errorf("can only use merge anchors with maps (!!map) or sequences (!!seq) of maps, but got %v", merge.Tag)
+	}
+}
+
+func applyMergeAnchorMap(node *CandidateNode, mergeMap *CandidateNode, mergeIndex int, explode bool, newContent Context) error {
+	if mergeMap == nil {
+		log.Debug("merge map is nil")
 		return nil
 	}
-	log.Debug("alias: %v", NodeToString(alias))
-	if alias.Kind != MappingNode {
-		return fmt.Errorf("merge anchor only supports maps, got %v instead", alias.Tag)
+	log.Debug("merge map: %v", NodeToString(mergeMap))
+	if mergeMap.Kind != MappingNode {
+		return fmt.Errorf("applyMergeAnchorMap expects !!map, got %v instead", mergeMap.Tag)
 	}
-	for index := 0; index < len(alias.Content); index = index + 2 {
-		keyNode := alias.Content[index]
-		log.Debugf("applying alias key %v", keyNode.Value)
-		valueNode := alias.Content[index+1]
-		err := overrideEntry(node, keyNode, valueNode, aliasIndex, newContent)
+
+	if explode {
+		err := explodeNode(mergeMap, newContent)
+		if err != nil {
+			return err
+		}
+	}
+
+	for index := 0; index < len(mergeMap.Content); index = index + 2 {
+		keyNode := mergeMap.Content[index]
+		log.Debugf("applying merge map key %v", keyNode.Value)
+		valueNode := mergeMap.Content[index+1]
+		err := overrideEntry(node, keyNode, valueNode, mergeIndex, explode, newContent)
 		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func overrideEntry(node *CandidateNode, key *CandidateNode, value *CandidateNode, startIndex int, newContent Context) error {
-
-	err := explodeNode(value, newContent)
-
-	if err != nil {
-		return err
+func overrideEntry(node *CandidateNode, key *CandidateNode, value *CandidateNode, startIndex int, explode bool, newContent Context) error {
+	if explode {
+		err := explodeNode(value, newContent)
+		if err != nil {
+			return err
+		}
 	}
 
 	for newEl := newContent.MatchingNodes.Front(); newEl != nil; newEl = newEl.Next() {
@@ -287,9 +319,11 @@ func overrideEntry(node *CandidateNode, key *CandidateNode, value *CandidateNode
 		}
 	}
 
-	err = explodeNode(key, newContent)
-	if err != nil {
-		return err
+	if explode {
+		err := explodeNode(key, newContent)
+		if err != nil {
+			return err
+		}
 	}
 	log.Debugf("adding %v:%v", key.Value, value.Value)
 	newContent.MatchingNodes.PushBack(key)

pkg/yqlib/operator_anchors_aliases_test.go

@@ -45,8 +45,8 @@ bar:
 foobarList:
     b: bar_b
     thing: foo_thing
-    c: foobarList_c
     a: foo_a
+    c: foobarList_c
 foobar:
     c: foo_c
     a: foo_a
@@ -58,7 +58,7 @@ var anchorOperatorScenarios = []expressionScenario{
 		skipDoc:       true,
 		description:   "merge anchor not map",
 		document:      "a: &a\n  - 0\nc:\n  <<: [*a]\n",
-		expectedError: "merge anchor only supports maps, got !!seq instead",
+		expectedError: "can only use merge anchors with maps (!!map) or sequences (!!seq) of maps, but got sequence containing !!seq",
 		expression:    "explode(.)",
 	},
 	{
@@ -80,7 +80,7 @@ var anchorOperatorScenarios = []expressionScenario{
 		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"},
+		expected:       []string{"D0, P[4], (!!map)::r: 10\ny: 2\nx: 1\n"},
 	},
 	{
 		description: "Get anchor",
@@ -254,7 +254,7 @@ var anchorOperatorScenarios = []expressionScenario{
 		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[foobarList], (!!map)::{b: bar_b, thing: foo_thing, a: foo_a, c: foobarList_c}\n",
 			"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
 		},
 	},
@@ -264,7 +264,7 @@ var anchorOperatorScenarios = []expressionScenario{
 		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[foobarList], (!!map)::{b: bar_b, thing: foo_thing, a: foo_a, c: foobarList_c}\n",
 			"D0, P[foobar], (!!map)::{c: foo_c, a: foo_a, thing: foobar_thing}\n",
 		},
 	},
@@ -283,6 +283,66 @@ var anchorOperatorScenarios = []expressionScenario{
 		expression:     `.thingOne |= explode(.) * {"value": false}`,
 		expected:       []string{expectedUpdatedArrayRef},
 	},
+	{ // Merge anchor with inline map
+		skipDoc:    true,
+		document:   `{<<: {a: 42}}`,
+		expression: `explode(.)`,
+		expected: []string{
+			"D0, P[], (!!map)::{a: 42}\n",
+		},
+	},
+	{ // Merge anchor with sequence with inline map
+		skipDoc:    true,
+		document:   `{<<: [{a: 42}]}`,
+		expression: `explode(.)`,
+		expected: []string{
+			"D0, P[], (!!map)::{a: 42}\n",
+		},
+	},
+	{ // Merge anchor with aliased sequence with inline map
+		skipDoc:    true,
+		document:   `{s: &s [{a: 42}], m: {<<: *s}}`,
+		expression: `.m | explode(.)`,
+		expected: []string{
+			"D0, P[m], (!!map)::{a: 42}\n",
+		},
+	},
+	{ // Exploding merge anchor should not explode neighbors
+		skipDoc: true,
+		// b must not be exploded, as `r: *a` will become invalid
+		document:   `{b: &b {a: &a 42}, r: *a, c: {<<: *b}}`,
+		expression: `explode(.c)`,
+		expected: []string{
+			"D0, P[], (!!map)::{b: &b {a: &a 42}, r: *a, c: {a: &a 42}}\n",
+		},
+	},
+	{ // Exploding sequence merge anchor should not explode neighbors
+		skipDoc: true,
+		// b must not be exploded, as `r: *a` will become invalid
+		document:   `{b: &b {a: &a 42}, r: *a, c: {<<: [*b]}}`,
+		expression: `explode(.c)`,
+		expected: []string{
+			"D0, P[], (!!map)::{b: &b {a: &a 42}, r: *a, c: {a: &a 42}}\n",
+		},
+	},
+	{ // Exploding inline merge anchor
+		skipDoc: true,
+		// `<<` map must be exploded, otherwise `c: *b` will become invalid
+		document:   `{a: {b: &b 42}, <<: {c: *b}}`,
+		expression: `explode(.)`,
+		expected: []string{
+			"D0, P[], (!!map)::{a: {b: 42}, c: 42}\n",
+		},
+	},
+	{ // Exploding inline merge anchor in sequence
+		skipDoc: true,
+		// `<<` map must be exploded, otherwise `c: *b` will become invalid
+		document:   `{a: {b: &b 42}, <<: [{c: *b}]}`,
+		expression: `explode(.)`,
+		expected: []string{
+			"D0, P[], (!!map)::{a: {b: 42}, c: 42}\n",
+		},
+	},
 }
 
 func TestAnchorAliasOperatorScenarios(t *testing.T) {

pkg/yqlib/operator_traverse_path.go

@@ -292,22 +292,36 @@ func doTraverseMap(newMatches *orderedmap.OrderedMap, node *CandidateNode, wante
 	return nil
 }
 
-func traverseMergeAnchor(newMatches *orderedmap.OrderedMap, value *CandidateNode, wantedKey string, prefs traversePreferences, splat bool) error {
-	switch value.Kind {
-	case AliasNode:
-		if value.Alias.Kind != MappingNode {
-			return fmt.Errorf("can only use merge anchors with maps (!!map), but got %v", value.Alias.Tag)
-		}
-		return doTraverseMap(newMatches, value.Alias, wantedKey, prefs, splat)
+func traverseMergeAnchor(newMatches *orderedmap.OrderedMap, merge *CandidateNode, wantedKey string, prefs traversePreferences, splat bool) error {
+	if merge.Kind == AliasNode {
+		merge = merge.Alias
+	}
+	switch merge.Kind {
+	case MappingNode:
+		return doTraverseMap(newMatches, merge, wantedKey, prefs, splat)
 	case SequenceNode:
-		for _, childValue := range value.Content {
-			err := traverseMergeAnchor(newMatches, childValue, wantedKey, prefs, splat)
+		// Earlier keys take precedence
+		for index := len(merge.Content) - 1; index >= 0; index = index - 1 {
+			childValue := merge.Content[index]
+			if childValue.Kind == AliasNode {
+				childValue = childValue.Alias
+			}
+			if childValue.Kind != MappingNode {
+				log.Debugf(
+					"can only use merge anchors with maps (!!map) or sequences (!!seq) of maps, but got sequence containing %v",
+					childValue.Tag)
+				return nil
+			}
+			err := doTraverseMap(newMatches, childValue, wantedKey, prefs, splat)
 			if err != nil {
 				return err
 			}
 		}
+		return nil
+	default:
+		log.Debugf("can only use merge anchors with maps (!!map) or sequences (!!seq) of maps, but got %v", merge.Tag)
+		return nil
 	}
-	return nil
 }
 
 func traverseArray(candidate *CandidateNode, operation *Operation, prefs traversePreferences) (*list.List, error) {