diff --git a/pkg/yqlib/candidate_node.go b/pkg/yqlib/candidate_node.go index 51485f11..7dee6303 100644 --- a/pkg/yqlib/candidate_node.go +++ b/pkg/yqlib/candidate_node.go @@ -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 { diff --git a/pkg/yqlib/doc/operators/anchor-and-alias-operators.md b/pkg/yqlib/doc/operators/anchor-and-alias-operators.md index 14b0bef2..7053b53d 100644 --- a/pkg/yqlib/doc/operators/anchor-and-alias-operators.md +++ b/pkg/yqlib/doc/operators/anchor-and-alias-operators.md @@ -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 ``` diff --git a/pkg/yqlib/lib.go b/pkg/yqlib/lib.go index 15561930..748b3f5b 100644 --- a/pkg/yqlib/lib.go +++ b/pkg/yqlib/lib.go @@ -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 diff --git a/pkg/yqlib/operator_anchors_aliases.go b/pkg/yqlib/operator_anchors_aliases.go index 50744871..84c3418d 100644 --- a/pkg/yqlib/operator_anchors_aliases.go +++ b/pkg/yqlib/operator_anchors_aliases.go @@ -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 diff --git a/pkg/yqlib/operator_anchors_aliases_test.go b/pkg/yqlib/operator_anchors_aliases_test.go index c5b2f74b..e47de341 100644 --- a/pkg/yqlib/operator_anchors_aliases_test.go +++ b/pkg/yqlib/operator_anchors_aliases_test.go @@ -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 } diff --git a/pkg/yqlib/operators_test.go b/pkg/yqlib/operators_test.go index eef6da26..4c9d3d01 100644 --- a/pkg/yqlib/operators_test.go +++ b/pkg/yqlib/operators_test.go @@ -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 {