From 28a6590ca2fd09f323469d36c7378b179b71708d Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 10:59:36 +0100 Subject: [PATCH 01/58] Add `existingEntryBlockWithComment` test --- .../org/openrewrite/yaml/MergeYamlTest.java | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index fd8bba68811..4331951b1a4 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.yaml; +import lombok.Value; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -111,7 +112,6 @@ void existingMultipleEntryBlock() { ); } - @DocumentExample @Test void nonExistentBlock() { rewriteRun( @@ -1010,6 +1010,72 @@ void mergeSequenceMapChangeComplexMapping() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithComment() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # Some comment + """, + """ + spring: + application: + name: main # Some comment + description: a description + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # Some comment + name2: main + """, + """ + spring: + application: + name: main # Some comment + name2: main + description: a description + """ + ) + ); + } + @Test void mergeScalar() { rewriteRun( From b6b4c5a802cf6da5bca89eb515758be3b5220e50 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 10:59:45 +0100 Subject: [PATCH 02/58] Cleanup MergeYaml code --- .../java/org/openrewrite/yaml/MergeYaml.java | 9 ++--- .../openrewrite/yaml/MergeYamlVisitor.java | 34 +++++++++---------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 5361955065f..01de6b8fba4 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -96,7 +96,7 @@ public TreeVisitor getVisitor() { .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Yaml.Document doc = docs.getDocuments().get(0); - if(doc.getBlock() instanceof Yaml.Mapping) { + if (doc.getBlock() instanceof Yaml.Mapping) { Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { @@ -110,9 +110,10 @@ public TreeVisitor getVisitor() { @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { if ("$".equals(key)) { - return document.withBlock((Yaml.Block) new MergeYamlVisitor<>(document.getBlock(), yaml, - Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty).visitNonNull(document.getBlock(), - ctx, getCursor())); + return document.withBlock((Yaml.Block) + new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) + .visitNonNull(document.getBlock(), ctx, getCursor()) + ); } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 66a5f310506..8283d556a79 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -31,7 +31,7 @@ @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Yaml scope; + private final Yaml existing; private final Yaml incoming; private final boolean acceptTheirs; @@ -48,7 +48,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Yaml.Document doc = docs.getDocuments().get(0); - if(doc.getBlock() instanceof Yaml.Mapping) { + if (doc.getBlock() instanceof Yaml.Mapping) { Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { @@ -64,7 +64,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { - if (scope.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { + if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { return mergeScalar(existingScalar, (Yaml.Scalar) incoming); } return super.visitScalar(existingScalar, p); @@ -72,15 +72,15 @@ public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { - if (scope.isScope(existingSeq)) { + if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence - return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> { - Yaml.Block b = (Yaml.Block) new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, - acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) - .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)); - return existingSeqEntry.withBlock(b); - })); + return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> + existingSeqEntry.withBlock((Yaml.Block) + new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) + .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)) + ) + )); } else if (incoming instanceof Yaml.Sequence) { return mergeSequence(existingSeq, (Yaml.Sequence) incoming, p, getCursor()); } @@ -90,14 +90,14 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - if (scope.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } return super.visitMapping(existingMapping, p); } - private static boolean keyMatches(Yaml.Mapping.Entry e1, Yaml.Mapping.Entry e2) { - return e1.getKey().getValue().equals(e2.getKey().getValue()); + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { + return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { @@ -117,9 +117,9 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor List mutatedEntries = ListUtils.map(m1.getEntries(), existingEntry -> { for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { - return existingEntry.withValue((Yaml.Block) new MergeYamlVisitor<>(existingEntry.getValue(), - incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) - .visitNonNull(existingEntry.getValue(), p, new Cursor(cursor, existingEntry))); + return existingEntry.withValue((Yaml.Block) + new MergeYamlVisitor<>(existingEntry.getValue(), incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) + .visitNonNull(existingEntry.getValue(), p, new Cursor(cursor, existingEntry))); } } return existingEntry; @@ -176,7 +176,7 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); - if(e1 == existingEntry) { + if (e1 == existingEntry) { // Made no change, no need to consider the entry "mutated" //noinspection DataFlowIssue return null; From 9445c84e44c2d2ac4e7fc213aab15db31be43636 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 16:02:13 +0100 Subject: [PATCH 03/58] Little progress... --- .../java/org/openrewrite/yaml/MergeYaml.java | 8 +- .../openrewrite/yaml/MergeYamlVisitor.java | 65 +++++++++++++++++ .../org/openrewrite/yaml/MergeYamlTest.java | 73 ++++++++++++++++++- 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 01de6b8fba4..0e52c7737d5 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -110,10 +110,16 @@ public TreeVisitor getVisitor() { @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { if ("$".equals(key)) { - return document.withBlock((Yaml.Block) + Yaml.Document d = document.withBlock((Yaml.Block) new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) .visitNonNull(document.getBlock(), ctx, getCursor()) ); + + if (getCursor().getMessage("RemovePrefix", false)) { + return d.withEnd(d.getEnd().withPrefix("")); + } + + return d; } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 8283d556a79..5a7bef07e80 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -125,6 +125,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return existingEntry; }); + int x = mutatedEntries.size(); mutatedEntries = ListUtils.concatAll(mutatedEntries, ListUtils.map(m2.getEntries(), incomingEntry -> { for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { @@ -136,6 +137,70 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } return incomingEntry; })); + boolean hasNewElements = x < mutatedEntries.size(); + + if (hasNewElements) { + System.out.println(">>>>"); + + Cursor currCursor = getCursor(); + Cursor c = cursor.dropParentWhile(it -> { + if (it instanceof Yaml.Document) { + return false; + } + + if (it instanceof Yaml.Mapping) { + boolean xx = ((Yaml.Mapping) it).getEntries().size() == 1; + + if (!xx) { + + } + + return xx; + } + + return true; + }); + + if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { + Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); + String comment = ""; + + if (c.getValue() instanceof Yaml.Document) { + comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); + + } + if (c.getValue() instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + + + int index = entries.size() - 1; + + if (currCursor.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) currCursor.getValue()).getEntries().size() == 1) { + for (int i = 0; i < entries.size(); i++) { + if (((Yaml.Mapping) entries.get(i).getValue()).getEntries().get(0).equals(((Yaml.Mapping) currCursor.getValue()).getEntries().get(0))) { + index = i + 1; + System.out.println(entries.size() == index); + break; + } + } + } + + // tenporary check + if (index < entries.size()) { + comment = entries.get(index).getPrefix().split("\n")[0]; + } + } + + + //mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + c.putMessage("RemovePrefix", true); + } + + + } + + //System.out.println((String) cursor.getMessage("RemovePrefix")); + return m1.withEntries(mutatedEntries); } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 4331951b1a4..23b4d87bb1a 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1030,12 +1030,12 @@ void existingEntryBlockWithComment() { """ spring: application: - name: main # Some comment + name: main # some comment """, """ spring: application: - name: main # Some comment + name: main # some comment description: a description """ ) @@ -1045,6 +1045,75 @@ void existingEntryBlockWithComment() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 3: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else + 2: old desc # Some comment 5 + D2: # Some comment 6 + 1: old description # Some comment 7 + D3: # Some comment 8 + 1: old text # Some comment 9 + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else + 2: old desc # Some comment 5 + 3: new desc + D2: # Some comment 6 + 1: old description # Some comment 7 + 2: new description + D3: # Some comment 8 + 1: old text # Some comment 9 + 2: new text + """ + ) + ); + } + + /* VAN: + spring: # Some comment + application: # Some comment 2 + name: main # Some comment 3 + + NAAR + + spring:> # Some comment + # Some comment 2 + # Some comment 3 + spec.recipe(new MergeYaml( "$", From ddcf04d6fa37caacc8fc888e5c681286e36d4ac9 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 16:22:18 +0100 Subject: [PATCH 04/58] Little progress... --- .../openrewrite/yaml/MergeYamlVisitor.java | 10 +--- .../org/openrewrite/yaml/MergeYamlTest.java | 58 ++++++++++++++++++- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 5a7bef07e80..3bd263f9936 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -149,13 +149,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (it instanceof Yaml.Mapping) { - boolean xx = ((Yaml.Mapping) it).getEntries().size() == 1; - - if (!xx) { - - } - - return xx; + return ((Yaml.Mapping) it).getEntries().size() == 1; } return true; @@ -192,7 +186,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } - //mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 23b4d87bb1a..2227c12b479 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1045,6 +1045,62 @@ void existingEntryBlockWithComment() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 4: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: + C: + D: + 1: something else + 2: something else + 3: old desc + D2: + 1: old description + D3: + 1: old text + """, + """ + A: # Some comment + B: + C: + D: + 1: something else + 2: something else + 3: old desc + 4: new desc + D2: + 1: old description + 2: new description + D3: + 1: old text + 2: new text + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentVariant2() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1113,7 +1169,7 @@ void existingEntryBlockWithCommentVariant() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant2() { + void existingEntryBlockWithCommentVariant3() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", From a6a2d864385aec8276dc6c93f8929f8f1f38d887 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 09:11:39 +0100 Subject: [PATCH 05/58] improvement --- .../main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 3bd263f9936..7d443926036 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -166,12 +166,11 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); - int index = entries.size() - 1; - if (currCursor.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) currCursor.getValue()).getEntries().size() == 1) { + if (currCursor.getValue() instanceof Yaml.Mapping) { for (int i = 0; i < entries.size(); i++) { - if (((Yaml.Mapping) entries.get(i).getValue()).getEntries().get(0).equals(((Yaml.Mapping) currCursor.getValue()).getEntries().get(0))) { + if (entries.get(i).getValue().equals(currCursor.getValue())) { index = i + 1; System.out.println(entries.size() == index); break; @@ -179,7 +178,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } - // tenporary check + // temporary check if (index < entries.size()) { comment = entries.get(index).getPrefix().split("\n")[0]; } From 3aa8750b77fbc43baa9395d05424a794cfce6cc4 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 09:28:42 +0100 Subject: [PATCH 06/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 7d443926036..da99d5e572a 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -140,8 +140,6 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor boolean hasNewElements = x < mutatedEntries.size(); if (hasNewElements) { - System.out.println(">>>>"); - Cursor currCursor = getCursor(); Cursor c = cursor.dropParentWhile(it -> { if (it instanceof Yaml.Document) { @@ -149,12 +147,22 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (it instanceof Yaml.Mapping) { - return ((Yaml.Mapping) it).getEntries().size() == 1; + List entries = ((Yaml.Mapping) it).getEntries(); + + // direct parent -> child with last member should search further upwards + if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { + return true; + } + return entries.size() == 1; } return true; }); + // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; + System.out.println(">>>"); + System.out.println(c); + if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; @@ -165,23 +173,21 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + int index = /*entries.size() */- 1; - int index = entries.size() - 1; - - if (currCursor.getValue() instanceof Yaml.Mapping) { + //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed for (int i = 0; i < entries.size(); i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { index = i + 1; - System.out.println(entries.size() == index); break; } } - } + // } // temporary check - if (index < entries.size()) { + // if (index < entries.size()) { comment = entries.get(index).getPrefix().split("\n")[0]; - } + // } } From d95f3348d6bebf4f15b4d70562847b147e330ff7 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 10:58:38 +0100 Subject: [PATCH 07/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index da99d5e572a..27389cb0251 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -139,8 +139,9 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor })); boolean hasNewElements = x < mutatedEntries.size(); + Cursor currCursor = getCursor(); + if (hasNewElements) { - Cursor currCursor = getCursor(); Cursor c = cursor.dropParentWhile(it -> { if (it instanceof Yaml.Document) { return false; @@ -194,11 +195,36 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } + } + /* System.out.println("----"); + System.out.println((Boolean) cursor.getMessage("RemovePrefix", null)); + System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); + System.out.println("----");*/ + if (cursor.getMessage("RemovePrefix", false)) { + System.out.println("Waarom niet "); } - //System.out.println((String) cursor.getMessage("RemovePrefix")); + for (int i = 0; i < m1.getEntries().size(); i++) { + if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && + mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && + ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { + System.out.println("yawel!!"); + + // temporary if + if ((i + 1) < mutatedEntries.size()) { + System.out.println("en goo......"); + // maybe do this in a more immutable way instead of a `set action`. + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); + } + } + + + if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { + + } + } return m1.withEntries(mutatedEntries); From 2145370444682233c46dc536c4ffb066130b0792 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 11:18:13 +0100 Subject: [PATCH 08/58] Fix null Yaml.Document.End objects --- .../java/org/openrewrite/yaml/YamlParser.java | 36 ++++++++++--------- .../java/org/openrewrite/yaml/tree/Yaml.java | 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java index 047c91789a8..3ec08feb969 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java @@ -50,6 +50,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.openrewrite.Tree.randomId; +import static org.openrewrite.marker.Markers.EMPTY; public class YamlParser implements org.openrewrite.Parser { private static final Pattern VARIABLE_PATTERN = Pattern.compile(":\\s*(@[^\n\r@]+@)"); @@ -82,8 +83,9 @@ public Stream parseInputs(Iterable sourceFiles, @Nullable Pat Yaml.Documents docs = (Yaml.Documents) sourceFile; // ensure there is always at least one Document, even in an empty yaml file if (docs.getDocuments().isEmpty()) { - return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", Markers.EMPTY, - false, new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), null))); + Yaml.Document.End end = new Yaml.Document.End(randomId(), "", EMPTY, false); + Yaml.Mapping mapping = new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null); + return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", EMPTY, false, mapping, end))); } return docs; } @@ -142,7 +144,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr documents.add(document.withEnd(new Yaml.Document.End( randomId(), fmt, - Markers.EMPTY, + EMPTY, ((DocumentEndEvent) event).getExplicit() ))); lastEnd = event.getEndMark().getIndex(); @@ -154,10 +156,10 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr document = new Yaml.Document( randomId(), fmt, - Markers.EMPTY, + EMPTY, ((DocumentStartEvent) event).getExplicit(), - new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), - null + new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", EMPTY, false) ); lastEnd = event.getEndMark().getIndex(); break; @@ -254,7 +256,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } break; } - Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, Markers.EMPTY, style, anchor, scalarValue); + Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, EMPTY, style, anchor, scalarValue); BlockBuilder builder = blockStack.isEmpty() ? null : blockStack.peek(); if (builder instanceof SequenceBuilder) { // Inline sequences like [1, 2] need to keep track of any whitespace between the element @@ -351,7 +353,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr throw new UnsupportedOperationException("Unknown anchor: " + alias.getAnchor()); } BlockBuilder builder = blockStack.peek(); - builder.push(new Yaml.Alias(randomId(), fmt, Markers.EMPTY, anchor)); + builder.push(new Yaml.Alias(randomId(), fmt, EMPTY, anchor)); lastEnd = event.getEndMark().getIndex(); break; } @@ -360,9 +362,9 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr if (document == null && !fmt.isEmpty()) { documents.add( new Yaml.Document( - randomId(), fmt, Markers.EMPTY, false, - new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) + randomId(), fmt, EMPTY, false, + new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", EMPTY, false) )); } break; @@ -372,7 +374,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } } - return new Yaml.Documents(randomId(), Markers.EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); + return new Yaml.Documents(randomId(), EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -408,7 +410,7 @@ private Yaml.Anchor buildYamlAnchor(FormatPreservingReader reader, int lastEnd, if (!isForScalar) { prefix = (prefixStart > -1 && eventPrefix.length() > prefixStart + 1) ? eventPrefix.substring(prefixStart + 1) : ""; } - return new Yaml.Anchor(randomId(), prefix, postFix.toString(), Markers.EMPTY, anchorKey); + return new Yaml.Anchor(randomId(), prefix, postFix.toString(), EMPTY, anchorKey); } private static int commentAwareIndexOf(char target, String s) { @@ -490,7 +492,7 @@ public void push(Yaml.Block block) { String entryPrefix = originalKeyPrefix.substring(entryPrefixStartIndex); String beforeMappingValueIndicator = keySuffix.substring(0, Math.max(commentAwareIndexOf(':', keySuffix), 0)); - entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, Markers.EMPTY, key, beforeMappingValueIndicator, block)); + entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, EMPTY, key, beforeMappingValueIndicator, block)); key = null; } } @@ -536,7 +538,7 @@ public void push(Yaml.Block block, @Nullable String commaPrefix) { entryPrefix = ""; blockPrefix = rawPrefix; } - entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, Markers.EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); + entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); } @Override @@ -566,7 +568,7 @@ private static class MappingWithPrefix extends Yaml.Mapping { private String prefix; public MappingWithPrefix(String prefix, @Nullable String startBracePrefix, List entries, @Nullable String endBracePrefix, @Nullable Anchor anchor) { - super(randomId(), Markers.EMPTY, startBracePrefix, entries, endBracePrefix, anchor); + super(randomId(), EMPTY, startBracePrefix, entries, endBracePrefix, anchor); this.prefix = prefix; } @@ -582,7 +584,7 @@ private static class SequenceWithPrefix extends Yaml.Sequence { private String prefix; public SequenceWithPrefix(String prefix, @Nullable String startBracketPrefix, List entries, @Nullable String endBracketPrefix, @Nullable Anchor anchor) { - super(randomId(), Markers.EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); + super(randomId(), EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); this.prefix = prefix; } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java index a92c0370e54..5245d9ab397 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java @@ -153,7 +153,7 @@ public Document copyPaste() { Markers.EMPTY, explicit, block.copyPaste(), - end == null ? null : end.copyPaste() + end.copyPaste() ); } From de92ccf6a4a57b0687d01bdacc3a0b2933018849 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 11:29:19 +0100 Subject: [PATCH 09/58] improvement --- .../java/org/openrewrite/yaml/MergeYamlVisitor.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 27389cb0251..63fb32eee31 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -170,25 +170,19 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor if (c.getValue() instanceof Yaml.Document) { comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); - } if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); - int index = /*entries.size() */- 1; //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - for (int i = 0; i < entries.size(); i++) { + //for (int i = 0; i < entries.size(); i++) { + for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { - index = i + 1; + comment = entries.get(i + 1).getPrefix().split("\n")[0]; break; } } // } - - // temporary check - // if (index < entries.size()) { - comment = entries.get(index).getPrefix().split("\n")[0]; - // } } From 97bffa9b52fdf656bee1b3c720ebf2a951e4c7fa Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 13:48:23 +0100 Subject: [PATCH 10/58] got it first working --- .../openrewrite/yaml/MergeYamlVisitor.java | 78 ++++++++++--------- .../org/openrewrite/yaml/MergeYamlTest.java | 4 + 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 63fb32eee31..64f24095e9c 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -21,13 +21,19 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; +import org.openrewrite.internal.StringUtils; import org.openrewrite.yaml.tree.Yaml; +import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; +import static org.openrewrite.internal.ListUtils.concatAll; +import static org.openrewrite.internal.ListUtils.map; +import static org.openrewrite.internal.StringUtils.isNotEmpty; + @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ @@ -63,9 +69,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean } @Override - public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { - if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { - return mergeScalar(existingScalar, (Yaml.Scalar) incoming); + public Yaml visitScalar(Scalar existingScalar, P p) { + if (existing.isScope(existingScalar) && incoming instanceof Scalar) { + return mergeScalar(existingScalar, (Scalar) incoming); } return super.visitScalar(existingScalar, p); } @@ -75,7 +81,7 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence - return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> + return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)) @@ -103,18 +109,18 @@ private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping. private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) + .map(e -> ((Scalar) e.getValue()).getValue()) .findAny(); return nameToAdd.map(nameToAddValue -> m1.getEntries().stream() .filter(e -> objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) + .map(e -> ((Scalar) e.getValue()).getValue()) .anyMatch(existingName -> existingName.equals(nameToAddValue))) .orElse(false); } private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { - List mutatedEntries = ListUtils.map(m1.getEntries(), existingEntry -> { + List mergedEntries = map(m1.getEntries(), existingEntry -> { for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) @@ -125,17 +131,18 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return existingEntry; }); - int x = mutatedEntries.size(); - mutatedEntries = ListUtils.concatAll(mutatedEntries, ListUtils.map(m2.getEntries(), incomingEntry -> { + int x = mergedEntries.size(); + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { - if (keyMatches(existingEntry, incomingEntry)) { + if (keyMatches(existingEntry, it)) { return null; } } - if (shouldAutoFormat) { - incomingEntry = autoFormat(incomingEntry, p, cursor); + // workaround: autoFormat put sometimes extra spaces before elements + if (!mergedEntries.isEmpty() && mergedEntries.get(0).getPrefix().contains("\n") && it.getValue() instanceof Scalar) { + return it.withPrefix("\n" + mergedEntries.get(0).getPrefix().split("\n")[1]); } - return incomingEntry; + return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); boolean hasNewElements = x < mutatedEntries.size(); @@ -175,17 +182,16 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor List entries = ((Yaml.Mapping) c.getValue()).getEntries(); //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - //for (int i = 0; i < entries.size(); i++) { - for (int i = 0; i < entries.size() - 1; i++) { - if (entries.get(i).getValue().equals(currCursor.getValue())) { - comment = entries.get(i + 1).getPrefix().split("\n")[0]; - break; - } + //for (int i = 0; i < entries.size(); i++) { + for (int i = 0; i < entries.size() - 1; i++) { + if (entries.get(i).getValue().equals(currCursor.getValue())) { + comment = entries.get(i + 1).getPrefix().split("\n")[0]; + break; } - // } + } + // } } - mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } @@ -208,16 +214,15 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor // temporary if if ((i + 1) < mutatedEntries.size()) { - System.out.println("en goo......"); - // maybe do this in a more immutable way instead of a `set action`. + System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } } - if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { + /*if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { - } + }*/ } @@ -229,17 +234,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Yaml.Scalar); + boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Scalar); if (isSequenceOfScalars) { List incomingEntries = new ArrayList<>(s2.getEntries()); nextEntry: for (Yaml.Sequence.Entry entry : s1.getEntries()) { - if (entry.getBlock() instanceof Yaml.Scalar) { - String existingScalar = ((Yaml.Scalar) entry.getBlock()).getValue(); + if (entry.getBlock() instanceof Scalar) { + String existingScalar = ((Scalar) entry.getBlock()).getValue(); for (Yaml.Sequence.Entry incomingEntry : incomingEntries) { - if (((Yaml.Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { + if (((Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { incomingEntries.remove(incomingEntry); continue nextEntry; } @@ -247,14 +252,13 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - return s1.withEntries(ListUtils.concatAll(s1.getEntries(), - ListUtils.map(incomingEntries, incomingEntry -> autoFormat(incomingEntry, p, cursor)))); + return s1.withEntries(concatAll(s1.getEntries(), map(incomingEntries, it -> autoFormat(it, p, cursor)))); } else { if (objectIdentifyingProperty == null) { // No identifier set to match entries on, so cannot continue return s1; } else { - List mutatedEntries = ListUtils.map(s2.getEntries(), entry -> { + List mutatedEntries = map(s2.getEntries(), entry -> { Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); @@ -262,7 +266,6 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { // Made no change, no need to consider the entry "mutated" - //noinspection DataFlowIssue return null; } return e1; @@ -274,10 +277,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - List entries = ListUtils.concatAll( - s1.getEntries().stream().filter(entry -> !mutatedEntries.contains(entry)) - .collect(Collectors.toList()), - ListUtils.map(mutatedEntries, entry -> autoFormat(entry, p, cursor))); + List entries = concatAll( + s1.getEntries().stream().filter(entry -> !mutatedEntries.contains(entry)).collect(Collectors.toList()), + map(mutatedEntries, entry -> autoFormat(entry, p, cursor))); if (entries.size() != s1.getEntries().size()) { return s1.withEntries(entries); @@ -292,7 +294,7 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private Yaml.Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { + private Scalar mergeScalar(Scalar y1, Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); return !s1.equals(s2) && !acceptTheirs ? y1.withValue(s2) : y1; diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 2227c12b479..d0031fc775c 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1059,6 +1059,8 @@ void existingEntryBlockWithCommentVariant() { 2: new description D3: 2: new text + 3: more new text + E: description """, false, null, @@ -1093,6 +1095,8 @@ void existingEntryBlockWithCommentVariant() { D3: 1: old text 2: new text + 3: more new text + E: description """ ) ); From 0317b774b75c0d9a13b534505a49dea864893123 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 15:29:18 +0100 Subject: [PATCH 11/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 37 ++++-- .../org/openrewrite/yaml/MergeYamlTest.java | 122 +++++++++++------- 2 files changed, 100 insertions(+), 59 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 64f24095e9c..28137d7c327 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.openrewrite.internal.ListUtils.concatAll; @@ -37,6 +38,9 @@ @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ + + private final Pattern linebreakPattern = Pattern.compile("\\R"); + private final Yaml existing; private final Yaml incoming; private final boolean acceptTheirs; @@ -139,8 +143,12 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && mergedEntries.get(0).getPrefix().contains("\n") && it.getValue() instanceof Scalar) { - return it.withPrefix("\n" + mergedEntries.get(0).getPrefix().split("\n")[1]); + if (!mergedEntries.isEmpty() && linebreakPattern.matcher(mergedEntries.get(0).getPrefix()).find() && it.getValue() instanceof Scalar) { + String[] parts = linebreakPattern.split(mergedEntries.get(0).getPrefix()); + if (parts.length > 1) { // why? + return it.withPrefix("\n" + parts[1]); + } + } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); @@ -149,22 +157,26 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor Cursor currCursor = getCursor(); if (hasNewElements) { - Cursor c = cursor.dropParentWhile(it -> { + /*if (currCursor.getValue() instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) currCursor.getValue()).getEntries(); + System.out.println("d"); + }*/ + + Cursor c = getCursor().dropParentUntil(it -> { if (it instanceof Yaml.Document) { - return false; + return true; } if (it instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) it).getEntries(); - - // direct parent -> child with last member should search further upwards + // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { - return true; + return false; } - return entries.size() == 1; + return entries.size() > 1; } - return true; + return false; }); // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; @@ -189,6 +201,13 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor break; } } + if (comment.isEmpty() && linebreakPattern.matcher(entries.get(entries.size() - 1).getPrefix()).find()) { + String[] parts = linebreakPattern.split(entries.get(entries.size() - 1).getPrefix()); + if (parts.length > 0) { // why??? + comment = entries.get(entries.size() - 1).getPrefix().split("\n")[0]; + } + } + // // } } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index d0031fc775c..2c3e0d0053f 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.yaml; -import lombok.Value; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -112,6 +111,7 @@ void existingMultipleEntryBlock() { ); } + @DocumentExample @Test void nonExistentBlock() { rewriteRun( @@ -1012,39 +1012,7 @@ void mergeSequenceMapChangeComplexMapping() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithComment() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - spring: - application: - description: a description - """, - false, - null, - null - )), - yaml( - """ - spring: - application: - name: main # some comment - """, - """ - spring: - application: - name: main # some comment - description: a description - """ - ) - ); - } - - @Issue("https://github.com/openrewrite/rewrite/issues/2218") - @Test - void existingEntryBlockWithCommentVariant() { + void existingEntryBlockWithCommentAtFirstLine() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1104,7 +1072,39 @@ void existingEntryBlockWithCommentVariant() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant2() { + void existingEntryBlockWithCommentAtLastLine() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # some comment + """, + """ + spring: + application: + name: main # some comment + description: a description + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentAllOverThePlace() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1156,24 +1156,46 @@ void existingEntryBlockWithCommentVariant2() { ); } - /* VAN: - spring: # Some comment - application: # Some comment 2 - name: main # Some comment 3 - - NAAR - - spring:> # Some comment - # Some comment 2 - # Some comment 3 - spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + 2: new desc + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + 1: old desc # Some comment 4 + D: whatever + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + 1: old desc # Some comment 4 + 2: new desc + D: whatever + """ + ) + ); + } @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant3() { + void existingEntryBlockWithCommentNotAtLastLine() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", From 643bfd1eb685ed546c59de6b31ff0f024d90d809 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 15:47:17 +0100 Subject: [PATCH 12/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 28137d7c327..22d7870f5ef 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -21,7 +21,6 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; -import org.openrewrite.internal.StringUtils; import org.openrewrite.yaml.tree.Yaml; import org.openrewrite.yaml.tree.Yaml.Scalar; @@ -33,13 +32,12 @@ import static org.openrewrite.internal.ListUtils.concatAll; import static org.openrewrite.internal.ListUtils.map; -import static org.openrewrite.internal.StringUtils.isNotEmpty; @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Pattern linebreakPattern = Pattern.compile("\\R"); + private final Pattern LINE_BREAK = Pattern.compile("\\R"); private final Yaml existing; private final Yaml incoming; @@ -143,12 +141,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && linebreakPattern.matcher(mergedEntries.get(0).getPrefix()).find() && it.getValue() instanceof Scalar) { - String[] parts = linebreakPattern.split(mergedEntries.get(0).getPrefix()); - if (parts.length > 1) { // why? - return it.withPrefix("\n" + parts[1]); - } - + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreakPieces(mergedEntries.get(0), 2)) { + return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); @@ -201,11 +195,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor break; } } - if (comment.isEmpty() && linebreakPattern.matcher(entries.get(entries.size() - 1).getPrefix()).find()) { - String[] parts = linebreakPattern.split(entries.get(entries.size() - 1).getPrefix()); - if (parts.length > 0) { // why??? - comment = entries.get(entries.size() - 1).getPrefix().split("\n")[0]; - } + if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { + comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } // // } @@ -313,6 +304,16 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } + private boolean hasLineBreakPieces(Yaml.Mapping.Entry entry, int atLeast) { + boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); + return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); + } + + private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + String[] parts = LINE_BREAK.split(entry.getPrefix()); + return parts.length > index ? parts[index] : ""; + } + private Scalar mergeScalar(Scalar y1, Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); From 237270e71a97c8e512af9f24f924c8af9fdb104b Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 16:36:03 +0100 Subject: [PATCH 13/58] why?? --- .../openrewrite/yaml/MergeYamlVisitor.java | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 22d7870f5ef..b26b10ad218 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -30,8 +30,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import static org.openrewrite.internal.ListUtils.concatAll; -import static org.openrewrite.internal.ListUtils.map; +import static org.openrewrite.internal.ListUtils.*; @AllArgsConstructor @RequiredArgsConstructor @@ -72,6 +71,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Scalar existingScalar, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingScalar) && incoming instanceof Scalar) { return mergeScalar(existingScalar, (Scalar) incoming); } @@ -80,6 +80,8 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence @@ -98,12 +100,24 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { + System.out.println("<<< visitMapping"); + if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { + System.out.println("ja!!"); + } + System.out.println(getCursor()); + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } return super.visitMapping(existingMapping, p); } + @Override + public Yaml visitMappingEntry(Yaml.Mapping.Entry entry, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + return super.visitMappingEntry(entry, p); + } + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } @@ -173,10 +187,6 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return false; }); - // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>>"); - System.out.println(c); - if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; @@ -203,6 +213,12 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + + + + // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; + System.out.println(">>>"); + System.out.println(c); c.putMessage("RemovePrefix", true); } } @@ -212,21 +228,23 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); System.out.println("----");*/ - if (cursor.getMessage("RemovePrefix", false)) { - System.out.println("Waarom niet "); - } - for (int i = 0; i < m1.getEntries().size(); i++) { if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { System.out.println("yawel!!"); - // temporary if if ((i + 1) < mutatedEntries.size()) { System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } + + //Entry s = mutatedEntries.get(i).getValue().withPrefix("Whatever"); + /*Yaml.Mapping xdx = (Yaml.Mapping) mutatedEntries.get(i).getValue(); + Yaml.Mapping xxxx= xdx.withEntries(mapLast(xdx.getEntries(), it -> it.withPrefix("boom\n"))); + + mutatedEntries.set(i, mutatedEntries.get(i).withValue(xxxx));*/ + } From bd350d8c3840ca73a12998302470ec690031fa64 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 10:30:54 +0100 Subject: [PATCH 14/58] Add more logging --- .../main/java/org/openrewrite/yaml/MergeYaml.java | 2 ++ .../org/openrewrite/yaml/MergeYamlVisitor.java | 14 ++++++++------ .../java/org/openrewrite/yaml/YamlVisitor.java | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 0e52c7737d5..168127c9e63 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -116,6 +116,8 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) ); if (getCursor().getMessage("RemovePrefix", false)) { + System.out.println("|||| after visit document"); + System.out.println(System.identityHashCode(getCursor())); return d.withEnd(d.getEnd().withPrefix("")); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index b26b10ad218..9dc1d28ba60 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -100,12 +100,13 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - System.out.println("<<< visitMapping"); + //System.out.println("<<< visitMapping"); if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { - System.out.println("ja!!"); + System.out.println(System.identityHashCode(getCursor())); + System.out.println(getCursor()); + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); } - System.out.println(getCursor()); - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } @@ -217,9 +218,10 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>>"); - System.out.println(c); + System.out.println(">>> RemovePrefix"); + System.out.println(System.identityHashCode(c)); c.putMessage("RemovePrefix", true); + System.out.println(c); } } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java index 63b5ecb5069..7434e66a542 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java @@ -66,11 +66,13 @@ public Y2 autoFormat(Y2 y, @Nullable Yaml stopAfter, P p, Curs } public Yaml visitDocuments(Yaml.Documents documents, P p) { + System.out.println("visitDocumentSSSSSS"); return documents.withDocuments(ListUtils.map(documents.getDocuments(), d -> visitAndCast(d, p))) .withMarkers(visitMarkers(documents.getMarkers(), p)); } public Yaml visitDocument(Yaml.Document document, P p) { + System.out.println("visitDocument"); return document.withBlock((Yaml.Block) visit(document.getBlock(), p)) .withMarkers(visitMarkers(document.getMarkers(), p)); } From cf6e9de146cf5cf2ad7f65cc0ed24568cba231fd Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 15:07:21 +0100 Subject: [PATCH 15/58] Got comment working --- .../java/org/openrewrite/yaml/MergeYaml.java | 2 - .../openrewrite/yaml/MergeYamlVisitor.java | 117 ++++++------------ .../org/openrewrite/yaml/YamlVisitor.java | 2 - 3 files changed, 38 insertions(+), 83 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 168127c9e63..0e52c7737d5 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -116,8 +116,6 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) ); if (getCursor().getMessage("RemovePrefix", false)) { - System.out.println("|||| after visit document"); - System.out.println(System.identityHashCode(getCursor())); return d.withEnd(d.getEnd().withPrefix("")); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 9dc1d28ba60..3359bac0799 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -22,6 +22,9 @@ import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; import org.openrewrite.yaml.tree.Yaml; +import org.openrewrite.yaml.tree.Yaml.Document; +import org.openrewrite.yaml.tree.Yaml.Mapping; +import org.openrewrite.yaml.tree.Yaml.Mapping.Entry; import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; @@ -54,9 +57,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(Yaml.Documents.class::cast) .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping - Yaml.Document doc = docs.getDocuments().get(0); - if (doc.getBlock() instanceof Yaml.Mapping) { - Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); + Document doc = docs.getDocuments().get(0); + if (doc.getBlock() instanceof Mapping) { + Mapping m = (Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { Yaml.Sequence s = (Yaml.Sequence) doc.getBlock(); @@ -71,7 +74,6 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Scalar existingScalar, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingScalar) && incoming instanceof Scalar) { return mergeScalar(existingScalar, (Scalar) incoming); } @@ -80,10 +82,8 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - if (existing.isScope(existingSeq)) { - if (incoming instanceof Yaml.Mapping) { + if (incoming instanceof Mapping) { // Distribute the incoming mapping to each entry in the sequence return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) @@ -99,31 +99,25 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { } @Override - public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - //System.out.println("<<< visitMapping"); - if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { - System.out.println(System.identityHashCode(getCursor())); - System.out.println(getCursor()); - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - } + public Yaml visitMapping(Mapping existingMapping, P p) { + if (existing.isScope(existingMapping) && incoming instanceof Mapping) { + Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); - if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { - return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); + if (getCursor().getMessage("RemovePrefix", false)) { + List entries = ((Mapping) getCursor().getValue()).getEntries(); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); + } + + return mapping; } return super.visitMapping(existingMapping, p); } - @Override - public Yaml visitMappingEntry(Yaml.Mapping.Entry entry, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - return super.visitMappingEntry(entry, p); - } - - private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { + private static boolean keyMatches(@Nullable Entry e1, @Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } - private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { + private boolean keyMatches(Mapping m1, Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) .map(e -> ((Scalar) e.getValue()).getValue()) @@ -136,9 +130,9 @@ private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { .orElse(false); } - private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { - List mergedEntries = map(m1.getEntries(), existingEntry -> { - for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { + private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { + List mergedEntries = map(m1.getEntries(), existingEntry -> { + for (Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) new MergeYamlVisitor<>(existingEntry.getValue(), incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) @@ -149,8 +143,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor }); int x = mergedEntries.size(); - List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { - for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { + for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { return null; } @@ -166,18 +160,13 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor Cursor currCursor = getCursor(); if (hasNewElements) { - /*if (currCursor.getValue() instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) currCursor.getValue()).getEntries(); - System.out.println("d"); - }*/ - Cursor c = getCursor().dropParentUntil(it -> { - if (it instanceof Yaml.Document) { + if (it instanceof Document) { return true; } - if (it instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) it).getEntries(); + if (it instanceof Mapping) { + List entries = ((Mapping) it).getEntries(); // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { return false; @@ -188,71 +177,41 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return false; }); - if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { - Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); + if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { + Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; - if (c.getValue() instanceof Yaml.Document) { - comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); + if (c.getValue() instanceof Document) { + comment = ((Document) c.getValue()).getEnd().getPrefix(); } - if (c.getValue() instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + if (c.getValue() instanceof Mapping) { + List entries = ((Mapping) c.getValue()).getEntries(); - //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - //for (int i = 0; i < entries.size(); i++) { for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { - comment = entries.get(i + 1).getPrefix().split("\n")[0]; + comment = grabPartLineBreak(entries.get(i + 1), 0); break; } } if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } - // - // } } mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); - - - - // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>> RemovePrefix"); - System.out.println(System.identityHashCode(c)); c.putMessage("RemovePrefix", true); - System.out.println(c); } } - /* System.out.println("----"); - System.out.println((Boolean) cursor.getMessage("RemovePrefix", null)); - System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); - System.out.println("----");*/ - + //Works only for direct children, needs a better way (with cursor messages probably) for (int i = 0; i < m1.getEntries().size(); i++) { if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { - System.out.println("yawel!!"); - if ((i + 1) < mutatedEntries.size()) { - System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } - - //Entry s = mutatedEntries.get(i).getValue().withPrefix("Whatever"); - /*Yaml.Mapping xdx = (Yaml.Mapping) mutatedEntries.get(i).getValue(); - Yaml.Mapping xxxx= xdx.withEntries(mapLast(xdx.getEntries(), it -> it.withPrefix("boom\n"))); - - mutatedEntries.set(i, mutatedEntries.get(i).withValue(xxxx));*/ - } - - - /*if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { - - }*/ } @@ -289,9 +248,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } else { List mutatedEntries = map(s2.getEntries(), entry -> { - Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); + Mapping incomingMapping = (Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { - Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); + Mapping existingMapping = (Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { @@ -324,12 +283,12 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreakPieces(Yaml.Mapping.Entry entry, int atLeast) { + private boolean hasLineBreakPieces(Entry entry, int atLeast) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); } - private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + private String grabPartLineBreak(Entry entry, int index) { String[] parts = LINE_BREAK.split(entry.getPrefix()); return parts.length > index ? parts[index] : ""; } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java index 7434e66a542..63b5ecb5069 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java @@ -66,13 +66,11 @@ public Y2 autoFormat(Y2 y, @Nullable Yaml stopAfter, P p, Curs } public Yaml visitDocuments(Yaml.Documents documents, P p) { - System.out.println("visitDocumentSSSSSS"); return documents.withDocuments(ListUtils.map(documents.getDocuments(), d -> visitAndCast(d, p))) .withMarkers(visitMarkers(documents.getMarkers(), p)); } public Yaml visitDocument(Yaml.Document document, P p) { - System.out.println("visitDocument"); return document.withBlock((Yaml.Block) visit(document.getBlock(), p)) .withMarkers(visitMarkers(document.getMarkers(), p)); } From 2c3f52fc0ff8c4807e7ed2e12611a9d8ad54c8e3 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 15:56:23 +0100 Subject: [PATCH 16/58] Improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 3359bac0799..a50a1443f06 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -142,7 +142,6 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return existingEntry; }); - int x = mergedEntries.size(); List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { @@ -155,11 +154,8 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); - boolean hasNewElements = x < mutatedEntries.size(); - Cursor currCursor = getCursor(); - - if (hasNewElements) { + if (m1.getEntries().size() < mutatedEntries.size()) { Cursor c = getCursor().dropParentUntil(it -> { if (it instanceof Document) { return true; @@ -168,7 +164,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { if (it instanceof Mapping) { List entries = ((Mapping) it).getEntries(); // last member should search further upwards until two entries are found - if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { + if (entries.get(entries.size() - 1).equals(getCursor().getParentOrThrow().getValue())) { return false; } return entries.size() > 1; @@ -178,17 +174,15 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { }); if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { - Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); - } - if (c.getValue() instanceof Mapping) { + } else { List entries = ((Mapping) c.getValue()).getEntries(); for (int i = 0; i < entries.size() - 1; i++) { - if (entries.get(i).getValue().equals(currCursor.getValue())) { + if (entries.get(i).getValue().equals(getCursor().getValue())) { comment = grabPartLineBreak(entries.get(i + 1), 0); break; } @@ -198,26 +192,26 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } } - mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); c.putMessage("RemovePrefix", true); } } - //Works only for direct children, needs a better way (with cursor messages probably) - for (int i = 0; i < m1.getEntries().size(); i++) { - if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && - mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && - ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { - if ((i + 1) < mutatedEntries.size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); - } - } - } - + removePrefixDirectChildren(m1.getEntries(), mutatedEntries); return m1.withEntries(mutatedEntries); } + private void removePrefixDirectChildren(List m1Entries, List mutatedEntries) { + for (int i = 0; i < m1Entries.size() - 1; i++) { + if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && + ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); + } + } + } + private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cursor cursor) { if (acceptTheirs) { return s1; From 4bc691b7c480297c8bc2ebbf87e6f6623cb294e7 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 16:02:11 +0100 Subject: [PATCH 17/58] Improvement --- .../main/java/org/openrewrite/yaml/MergeYaml.java | 8 ++------ .../java/org/openrewrite/yaml/MergeYamlVisitor.java | 13 +++++++------ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 0e52c7737d5..3b1bf9c1355 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -84,6 +84,7 @@ public String getDescription() { } final static String FOUND_MATCHING_ELEMENT = "FOUND_MATCHING_ELEMENT"; + final static String REMOVE_PREFIX = "REMOVE_PREFIX"; @Override public TreeVisitor getVisitor() { @@ -114,12 +115,7 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) .visitNonNull(document.getBlock(), ctx, getCursor()) ); - - if (getCursor().getMessage("RemovePrefix", false)) { - return d.withEnd(d.getEnd().withPrefix("")); - } - - return d; + return getCursor().getMessage(REMOVE_PREFIX, false) ? d.withEnd(d.getEnd().withPrefix("")) : d; } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index a50a1443f06..bd6c937782f 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -34,6 +34,7 @@ import java.util.stream.Collectors; import static org.openrewrite.internal.ListUtils.*; +import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @AllArgsConstructor @RequiredArgsConstructor @@ -103,7 +104,7 @@ public Yaml visitMapping(Mapping existingMapping, P p) { if (existing.isScope(existingMapping) && incoming instanceof Mapping) { Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); - if (getCursor().getMessage("RemovePrefix", false)) { + if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Mapping) getCursor().getValue()).getEntries(); return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); } @@ -149,7 +150,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreakPieces(mergedEntries.get(0), 2)) { + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreak(mergedEntries.get(0), 2)) { return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; @@ -187,14 +188,14 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { break; } } - if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { + if (comment.isEmpty() && hasLineBreak(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } } Entry last = mutatedEntries.get(mutatedEntries.size() - 1); mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); - c.putMessage("RemovePrefix", true); + c.putMessage(REMOVE_PREFIX, true); } } @@ -277,9 +278,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreakPieces(Entry entry, int atLeast) { + private boolean hasLineBreak(Entry entry, int atLeastParts) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); - return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); + return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); } private String grabPartLineBreak(Entry entry, int index) { From de2a316f0ce4bc07575cd6e4e8815bfb62098046 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 16:23:03 +0100 Subject: [PATCH 18/58] Found a non-working test case... --- .../org/openrewrite/yaml/MergeYamlTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 2c3e0d0053f..4126d052873 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1156,6 +1156,59 @@ void existingEntryBlockWithCommentAllOverThePlace() { ); } + @Test + void fixThisTestAsWell() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 3: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else # Some comment 5 + 2: old desc + D2: # Some comment 6 + 1: old description # Some comment 7 + D3: # Some comment 8 + 1: old text # Some comment 9 + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else # Some comment 5 + 2: old desc + 3: new desc + D2: # Some comment 6 + 1: old description # Some comment 7 + 2: new description + D3: # Some comment 8 + 1: old text # Some comment 9 + 2: new text + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentAllOverThePlaceWEGG() { From 5b141fc14d11f4b0b024831d3b54d988b6b43f4d Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 08:52:55 +0100 Subject: [PATCH 19/58] Fix non-working test case... --- .../openrewrite/yaml/MergeYamlVisitor.java | 18 +++-- .../org/openrewrite/yaml/MergeYamlTest.java | 74 +++++-------------- 2 files changed, 28 insertions(+), 64 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index bd6c937782f..76386782bf9 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -175,36 +175,40 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { }); if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { - String comment = ""; + String comment = null; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); } else { List entries = ((Mapping) c.getValue()).getEntries(); + // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(getCursor().getValue())) { comment = grabPartLineBreak(entries.get(i + 1), 0); break; } } - if (comment.isEmpty() && hasLineBreak(entries.get(entries.size() - 1), 1)) { + // or retrieve it for last item from next element (could potentially be much higher in the tree) + if (comment == null && hasLineBreak(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } } - Entry last = mutatedEntries.get(mutatedEntries.size() - 1); - mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); - c.putMessage(REMOVE_PREFIX, true); + if (comment != null) { + Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); + c.putMessage(REMOVE_PREFIX, true); + } } } - removePrefixDirectChildren(m1.getEntries(), mutatedEntries); + removePrefixForDirectChildren(m1.getEntries(), mutatedEntries); return m1.withEntries(mutatedEntries); } - private void removePrefixDirectChildren(List m1Entries, List mutatedEntries) { + private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 4126d052873..87168f724c7 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1116,8 +1116,10 @@ void existingEntryBlockWithCommentAllOverThePlace() { D: 3: new desc D2: - 2: new description + 4: d D3: + 2: new description + D4: 2: new text """, false, @@ -1132,9 +1134,13 @@ void existingEntryBlockWithCommentAllOverThePlace() { D: # Some comment 4 1: something else 2: old desc # Some comment 5 - D2: # Some comment 6 + D2: + 1: a + 2: b + 3: c + D3: # Some comment 6 1: old description # Some comment 7 - D3: # Some comment 8 + D4: # Some comment 8 1: old text # Some comment 9 """, """ @@ -1145,63 +1151,15 @@ void existingEntryBlockWithCommentAllOverThePlace() { 1: something else 2: old desc # Some comment 5 3: new desc - D2: # Some comment 6 - 1: old description # Some comment 7 - 2: new description - D3: # Some comment 8 - 1: old text # Some comment 9 - 2: new text - """ - ) - ); - } - - @Test - void fixThisTestAsWell() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - A: - B: - C: - D: - 3: new desc D2: - 2: new description - D3: - 2: new text - """, - false, - null, - null - )), - yaml( - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 - 1: something else # Some comment 5 - 2: old desc - D2: # Some comment 6 - 1: old description # Some comment 7 - D3: # Some comment 8 - 1: old text # Some comment 9 - """, - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 - 1: something else # Some comment 5 - 2: old desc - 3: new desc - D2: # Some comment 6 + 1: a + 2: b + 3: c + 4: d + D3: # Some comment 6 1: old description # Some comment 7 2: new description - D3: # Some comment 8 + D4: # Some comment 8 1: old text # Some comment 9 2: new text """ @@ -1231,6 +1189,7 @@ void existingEntryBlockWithCommentAllOverThePlaceWEGG() { A: # Some comment B: # Some comment 2 C: # Some comment 3 + BOE: JE 1: old desc # Some comment 4 D: whatever """, @@ -1238,6 +1197,7 @@ void existingEntryBlockWithCommentAllOverThePlaceWEGG() { A: # Some comment B: # Some comment 2 C: # Some comment 3 + BOE: JE 1: old desc # Some comment 4 2: new desc D: whatever From 0eca879dd63788be7a6f0f0f9395495617d0e769 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 08:56:21 +0100 Subject: [PATCH 20/58] Remove test no longer needed --- .../org/openrewrite/yaml/MergeYamlTest.java | 43 +------------------ 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 87168f724c7..655973286fc 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1136,7 +1136,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { 2: old desc # Some comment 5 D2: 1: a - 2: b + 2: b # Some comment 10 3: c D3: # Some comment 6 1: old description # Some comment 7 @@ -1153,7 +1153,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { 3: new desc D2: 1: a - 2: b + 2: b # Some comment 10 3: c 4: d D3: # Some comment 6 @@ -1167,45 +1167,6 @@ void existingEntryBlockWithCommentAllOverThePlace() { ); } - @Issue("https://github.com/openrewrite/rewrite/issues/2218") - @Test - void existingEntryBlockWithCommentAllOverThePlaceWEGG() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - A: - B: - C: - 2: new desc - """, - false, - null, - null - )), - yaml( - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - BOE: JE - 1: old desc # Some comment 4 - D: whatever - """, - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - BOE: JE - 1: old desc # Some comment 4 - 2: new desc - D: whatever - """ - ) - ); - } - @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentNotAtLastLine() { From 5b37a93b5e2eb0ea1c2d92ccfc6524d6e2f22d33 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 09:21:32 +0100 Subject: [PATCH 21/58] Improvement --- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 76386782bf9..fa030f78bb7 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -40,7 +40,7 @@ @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Pattern LINE_BREAK = Pattern.compile("\\R"); + private static final Pattern LINE_BREAK = Pattern.compile("\\R"); private final Yaml existing; private final Yaml incoming; @@ -143,7 +143,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return existingEntry; }); - List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), it -> { for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { return null; From 17a0059d39bd34bfd653e4626bf2f86d0ddba1de Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 10:44:51 +0100 Subject: [PATCH 22/58] Dont replace comments when cursor is a root level --- rewrite-core/src/main/java/org/openrewrite/Cursor.java | 2 +- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/Cursor.java b/rewrite-core/src/main/java/org/openrewrite/Cursor.java index 3be9d7c15a3..7ed54841f73 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Cursor.java +++ b/rewrite-core/src/main/java/org/openrewrite/Cursor.java @@ -57,7 +57,7 @@ public Cursor getRoot() { /** * @return true if this cursor is the root of the tree, false otherwise */ - final boolean isRoot() { + final public boolean isRoot() { return ROOT_VALUE.equals(value); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index fa030f78bb7..9b5f75b543d 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -33,6 +33,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static org.openrewrite.Cursor.ROOT_VALUE; import static org.openrewrite.internal.ListUtils.*; import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @@ -156,9 +157,9 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); - if (m1.getEntries().size() < mutatedEntries.size()) { + if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { Cursor c = getCursor().dropParentUntil(it -> { - if (it instanceof Document) { + if (ROOT_VALUE.equals(it) || it instanceof Document) { return true; } From 34b5f9d5af90516384131703ebec4005d4a6763d Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 13:51:32 +0100 Subject: [PATCH 23/58] Put Yaml prefixes back --- .../openrewrite/yaml/MergeYamlVisitor.java | 77 ++++++++++--------- .../java/org/openrewrite/yaml/YamlParser.java | 37 +++++---- 2 files changed, 58 insertions(+), 56 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 9b5f75b543d..aee420576e6 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -60,8 +60,8 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Document doc = docs.getDocuments().get(0); - if (doc.getBlock() instanceof Mapping) { - Mapping m = (Mapping) doc.getBlock(); + if (doc.getBlock() instanceof Yaml.Mapping) { + Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { Yaml.Sequence s = (Yaml.Sequence) doc.getBlock(); @@ -75,9 +75,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean } @Override - public Yaml visitScalar(Scalar existingScalar, P p) { - if (existing.isScope(existingScalar) && incoming instanceof Scalar) { - return mergeScalar(existingScalar, (Scalar) incoming); + public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { + if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { + return mergeScalar(existingScalar, (Yaml.Scalar) incoming); } return super.visitScalar(existingScalar, p); } @@ -85,7 +85,7 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { if (existing.isScope(existingSeq)) { - if (incoming instanceof Mapping) { + if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) @@ -101,12 +101,12 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { } @Override - public Yaml visitMapping(Mapping existingMapping, P p) { - if (existing.isScope(existingMapping) && incoming instanceof Mapping) { - Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); + public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { + Yaml.Mapping mapping = mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); if (getCursor().getMessage(REMOVE_PREFIX, false)) { - List entries = ((Mapping) getCursor().getValue()).getEntries(); + List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); } @@ -115,25 +115,25 @@ public Yaml visitMapping(Mapping existingMapping, P p) { return super.visitMapping(existingMapping, p); } - private static boolean keyMatches(@Nullable Entry e1, @Nullable Entry e2) { + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } - private boolean keyMatches(Mapping m1, Mapping m2) { + private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Scalar) e.getValue()).getValue()) + .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) .findAny(); return nameToAdd.map(nameToAddValue -> m1.getEntries().stream() .filter(e -> objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Scalar) e.getValue()).getValue()) + .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) .anyMatch(existingName -> existingName.equals(nameToAddValue))) .orElse(false); } - private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { - List mergedEntries = map(m1.getEntries(), existingEntry -> { + private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { + List mergedEntries = map(m1.getEntries(), existingEntry -> { for (Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) @@ -150,11 +150,14 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return null; } } - // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); + // workaround: autoFormat cannot handle new inserted values very well + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && MergeYamlVisitor.this.hasLineBreak(mergedEntries.get(0), 2)) { + String prefix = MergeYamlVisitor.this.grabPartLineBreak(mergedEntries.get(0), 1); + String newValue = ((Scalar) it.getValue()).getValue().replaceAll("\\R", prefix + "\n"); + return it.withPrefix("\n" + prefix) + .withValue(((Scalar) it.getValue()).withValue(newValue)); } - return shouldAutoFormat ? autoFormat(it, p, cursor) : it; + return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { @@ -163,8 +166,8 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return true; } - if (it instanceof Mapping) { - List entries = ((Mapping) it).getEntries(); + if (it instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) it).getEntries(); // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(getCursor().getParentOrThrow().getValue())) { return false; @@ -175,13 +178,13 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return false; }); - if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { + if (c.getValue() instanceof Document || c.getValue() instanceof Yaml.Mapping) { String comment = null; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); } else { - List entries = ((Mapping) c.getValue()).getEntries(); + List entries = ((Yaml.Mapping) c.getValue()).getEntries(); // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { @@ -197,7 +200,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } if (comment != null) { - Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + Yaml.Mapping.Entry last = mutatedEntries.get(mutatedEntries.size() - 1); mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); c.putMessage(REMOVE_PREFIX, true); } @@ -209,10 +212,10 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return m1.withEntries(mutatedEntries); } - private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { + private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { for (int i = 0; i < m1Entries.size() - 1; i++) { - if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && - ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { + if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && + ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); } } @@ -223,17 +226,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Scalar); + boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Yaml.Scalar); if (isSequenceOfScalars) { List incomingEntries = new ArrayList<>(s2.getEntries()); nextEntry: for (Yaml.Sequence.Entry entry : s1.getEntries()) { - if (entry.getBlock() instanceof Scalar) { - String existingScalar = ((Scalar) entry.getBlock()).getValue(); + if (entry.getBlock() instanceof Yaml.Scalar) { + String existingScalar = ((Yaml.Scalar) entry.getBlock()).getValue(); for (Yaml.Sequence.Entry incomingEntry : incomingEntries) { - if (((Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { + if (((Yaml.Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { incomingEntries.remove(incomingEntry); continue nextEntry; } @@ -248,9 +251,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } else { List mutatedEntries = map(s2.getEntries(), entry -> { - Mapping incomingMapping = (Mapping) entry.getBlock(); + Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { - Mapping existingMapping = (Mapping) existingEntry.getBlock(); + Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { @@ -283,17 +286,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreak(Entry entry, int atLeastParts) { + private boolean hasLineBreak(Yaml.Mapping.Entry entry, int atLeastParts) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); } - private String grabPartLineBreak(Entry entry, int index) { + private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { String[] parts = LINE_BREAK.split(entry.getPrefix()); return parts.length > index ? parts[index] : ""; } - private Scalar mergeScalar(Scalar y1, Scalar y2) { + private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); return !s1.equals(s2) && !acceptTheirs ? y1.withValue(s2) : y1; diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java index 3ec08feb969..ca7685f37ba 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java @@ -50,7 +50,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.openrewrite.Tree.randomId; -import static org.openrewrite.marker.Markers.EMPTY; public class YamlParser implements org.openrewrite.Parser { private static final Pattern VARIABLE_PATTERN = Pattern.compile(":\\s*(@[^\n\r@]+@)"); @@ -83,9 +82,9 @@ public Stream parseInputs(Iterable sourceFiles, @Nullable Pat Yaml.Documents docs = (Yaml.Documents) sourceFile; // ensure there is always at least one Document, even in an empty yaml file if (docs.getDocuments().isEmpty()) { - Yaml.Document.End end = new Yaml.Document.End(randomId(), "", EMPTY, false); - Yaml.Mapping mapping = new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null); - return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", EMPTY, false, mapping, end))); + Yaml.Document.End end = new Yaml.Document.End(randomId(), "", Markers.EMPTY, false); + Yaml.Mapping mapping = new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null); + return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", Markers.EMPTY, false, mapping, end))); } return docs; } @@ -144,7 +143,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr documents.add(document.withEnd(new Yaml.Document.End( randomId(), fmt, - EMPTY, + Markers.EMPTY, ((DocumentEndEvent) event).getExplicit() ))); lastEnd = event.getEndMark().getIndex(); @@ -156,10 +155,10 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr document = new Yaml.Document( randomId(), fmt, - EMPTY, + Markers.EMPTY, ((DocumentStartEvent) event).getExplicit(), - new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", EMPTY, false) + new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) ); lastEnd = event.getEndMark().getIndex(); break; @@ -256,7 +255,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } break; } - Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, EMPTY, style, anchor, scalarValue); + Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, Markers.EMPTY, style, anchor, scalarValue); BlockBuilder builder = blockStack.isEmpty() ? null : blockStack.peek(); if (builder instanceof SequenceBuilder) { // Inline sequences like [1, 2] need to keep track of any whitespace between the element @@ -353,7 +352,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr throw new UnsupportedOperationException("Unknown anchor: " + alias.getAnchor()); } BlockBuilder builder = blockStack.peek(); - builder.push(new Yaml.Alias(randomId(), fmt, EMPTY, anchor)); + builder.push(new Yaml.Alias(randomId(), fmt, Markers.EMPTY, anchor)); lastEnd = event.getEndMark().getIndex(); break; } @@ -362,9 +361,9 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr if (document == null && !fmt.isEmpty()) { documents.add( new Yaml.Document( - randomId(), fmt, EMPTY, false, - new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", EMPTY, false) + randomId(), fmt, Markers.EMPTY, false, + new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) )); } break; @@ -374,7 +373,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } } - return new Yaml.Documents(randomId(), EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); + return new Yaml.Documents(randomId(), Markers.EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -410,7 +409,7 @@ private Yaml.Anchor buildYamlAnchor(FormatPreservingReader reader, int lastEnd, if (!isForScalar) { prefix = (prefixStart > -1 && eventPrefix.length() > prefixStart + 1) ? eventPrefix.substring(prefixStart + 1) : ""; } - return new Yaml.Anchor(randomId(), prefix, postFix.toString(), EMPTY, anchorKey); + return new Yaml.Anchor(randomId(), prefix, postFix.toString(), Markers.EMPTY, anchorKey); } private static int commentAwareIndexOf(char target, String s) { @@ -492,7 +491,7 @@ public void push(Yaml.Block block) { String entryPrefix = originalKeyPrefix.substring(entryPrefixStartIndex); String beforeMappingValueIndicator = keySuffix.substring(0, Math.max(commentAwareIndexOf(':', keySuffix), 0)); - entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, EMPTY, key, beforeMappingValueIndicator, block)); + entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, Markers.EMPTY, key, beforeMappingValueIndicator, block)); key = null; } } @@ -538,7 +537,7 @@ public void push(Yaml.Block block, @Nullable String commaPrefix) { entryPrefix = ""; blockPrefix = rawPrefix; } - entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); + entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, Markers.EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); } @Override @@ -568,7 +567,7 @@ private static class MappingWithPrefix extends Yaml.Mapping { private String prefix; public MappingWithPrefix(String prefix, @Nullable String startBracePrefix, List entries, @Nullable String endBracePrefix, @Nullable Anchor anchor) { - super(randomId(), EMPTY, startBracePrefix, entries, endBracePrefix, anchor); + super(randomId(), Markers.EMPTY, startBracePrefix, entries, endBracePrefix, anchor); this.prefix = prefix; } @@ -584,7 +583,7 @@ private static class SequenceWithPrefix extends Yaml.Sequence { private String prefix; public SequenceWithPrefix(String prefix, @Nullable String startBracketPrefix, List entries, @Nullable String endBracketPrefix, @Nullable Anchor anchor) { - super(randomId(), EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); + super(randomId(), Markers.EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); this.prefix = prefix; } From 1381e9f152841621b1c0cd762cffb89fe669caaf Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 14:18:34 +0100 Subject: [PATCH 24/58] More comment, more difficult --- .../openrewrite/yaml/MergeYamlVisitor.java | 7 +-- .../org/openrewrite/yaml/MergeYamlTest.java | 52 ++++++++++--------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index aee420576e6..8f4c0a21ead 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -151,11 +151,8 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } } // workaround: autoFormat cannot handle new inserted values very well - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && MergeYamlVisitor.this.hasLineBreak(mergedEntries.get(0), 2)) { - String prefix = MergeYamlVisitor.this.grabPartLineBreak(mergedEntries.get(0), 1); - String newValue = ((Scalar) it.getValue()).getValue().replaceAll("\\R", prefix + "\n"); - return it.withPrefix("\n" + prefix) - .withValue(((Scalar) it.getValue()).withValue(newValue)); + if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { + return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 655973286fc..03b3cb0ca1b 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1036,7 +1036,7 @@ void existingEntryBlockWithCommentAtFirstLine() { )), yaml( """ - A: # Some comment + A: # Comment untouched B: C: D: @@ -1049,7 +1049,7 @@ void existingEntryBlockWithCommentAtFirstLine() { 1: old text """, """ - A: # Some comment + A: # Comment untouched B: C: D: @@ -1090,12 +1090,12 @@ void existingEntryBlockWithCommentAtLastLine() { """ spring: application: - name: main # some comment + name: main # Comment moved from root to previous element """, """ spring: application: - name: main # some comment + name: main # Comment moved from root to previous element description: a description """ ) @@ -1128,39 +1128,43 @@ void existingEntryBlockWithCommentAllOverThePlace() { )), yaml( """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 + A: # Comment untouched 1 + B: # Comment untouched 2 + C: # Comment untouched 3 + D: # Comment untouched 4 1: something else - 2: old desc # Some comment 5 + 2: old desc # Comment moved from next block to previous element 1 D2: 1: a - 2: b # Some comment 10 + # Above comment untouched 1 + 2: b # Comment untouched 5 3: c - D3: # Some comment 6 - 1: old description # Some comment 7 - D4: # Some comment 8 - 1: old text # Some comment 9 + # Above comment untouched 2 + D3: # Comment untouched 6 + 1: old description # Comment moved from next block to previous element 2 + D4: # Comment untouched 7 + 1: old text # Comment moved from root to previous element """, """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 + A: # Comment untouched 1 + B: # Comment untouched 2 + C: # Comment untouched 3 + D: # Comment untouched 4 1: something else - 2: old desc # Some comment 5 + 2: old desc # Comment moved from next block to previous element 1 3: new desc D2: 1: a - 2: b # Some comment 10 + # Above comment untouched 1 + 2: b # Comment untouched 5 3: c 4: d - D3: # Some comment 6 - 1: old description # Some comment 7 + # Above comment untouched 2 + D3: # Comment untouched 6 + 1: old description # Comment moved from next block to previous element 2 2: new description - D4: # Some comment 8 - 1: old text # Some comment 9 + D4: # Comment untouched 7 + 1: old text # Comment moved from root to previous element 2: new text """ ) From 6c30223d62205a2c523382fdedffc628c0cea52b Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:30:13 +0100 Subject: [PATCH 25/58] Improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 23 +++++++++++-------- .../org/openrewrite/yaml/MergeYamlTest.java | 14 +++++------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 8f4c0a21ead..d54c7f98e1c 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -28,6 +28,7 @@ import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.regex.Pattern; @@ -107,7 +108,7 @@ public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); - return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); } return mapping; @@ -152,7 +153,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } // workaround: autoFormat cannot handle new inserted values very well if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); + return it.withPrefix("\n" + grabAfterFirstLineBreak(mergedEntries.get(0))); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); @@ -186,13 +187,13 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(getCursor().getValue())) { - comment = grabPartLineBreak(entries.get(i + 1), 0); + comment = grabBeforeFirstLineBreak(entries.get(i + 1)); break; } } // or retrieve it for last item from next element (could potentially be much higher in the tree) if (comment == null && hasLineBreak(entries.get(entries.size() - 1), 1)) { - comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); + comment = grabBeforeFirstLineBreak(entries.get(entries.size() - 1)); } } @@ -213,7 +214,7 @@ private void removePrefixForDirectChildren(List m1Entries, L for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); } } } @@ -284,13 +285,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } private boolean hasLineBreak(Yaml.Mapping.Entry entry, int atLeastParts) { - boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); - return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); + return LINE_BREAK.matcher(entry.getPrefix()).find() && LINE_BREAK.split(entry.getPrefix()).length >= atLeastParts; } - private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + private String grabBeforeFirstLineBreak(Yaml.Mapping.Entry entry) { String[] parts = LINE_BREAK.split(entry.getPrefix()); - return parts.length > index ? parts[index] : ""; + return parts.length > 0 ? parts[0] : ""; + } + + private String grabAfterFirstLineBreak(Yaml.Mapping.Entry entry) { + String[] parts = LINE_BREAK.split(entry.getPrefix()); + return parts.length > 1 ? String.join("\n", Arrays.copyOfRange(parts, 1, parts.length)) : ""; } private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 03b3cb0ca1b..df4ba6bbead 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1104,7 +1104,7 @@ void existingEntryBlockWithCommentAtLastLine() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentAllOverThePlace() { + void existingEntryBlockWithCommentsAllOverThePlace() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1133,7 +1133,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { C: # Comment untouched 3 D: # Comment untouched 4 1: something else - 2: old desc # Comment moved from next block to previous element 1 + 2: old desc # Comment moved from prefix D2 to prefix D->3 D2: 1: a # Above comment untouched 1 @@ -1141,9 +1141,9 @@ void existingEntryBlockWithCommentAllOverThePlace() { 3: c # Above comment untouched 2 D3: # Comment untouched 6 - 1: old description # Comment moved from next block to previous element 2 + 1: old description # Comment moved from prefix D4 to prefix D3->2 D4: # Comment untouched 7 - 1: old text # Comment moved from root to previous element + 1: old text # Comment moved from end document to prefix D4->2 """, """ A: # Comment untouched 1 @@ -1151,7 +1151,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { C: # Comment untouched 3 D: # Comment untouched 4 1: something else - 2: old desc # Comment moved from next block to previous element 1 + 2: old desc # Comment moved from prefix D2 to prefix D->3 3: new desc D2: 1: a @@ -1161,10 +1161,10 @@ void existingEntryBlockWithCommentAllOverThePlace() { 4: d # Above comment untouched 2 D3: # Comment untouched 6 - 1: old description # Comment moved from next block to previous element 2 + 1: old description # Comment moved from prefix D4 to prefix D3->2 2: new description D4: # Comment untouched 7 - 1: old text # Comment moved from root to previous element + 1: old text # Comment moved from end document to prefix D4->2 2: new text """ ) From 21f09cae1dcd3ae204ec78db0d0e43858e719cf5 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:45:28 +0100 Subject: [PATCH 26/58] Improvement of test --- .../org/openrewrite/yaml/MergeYamlTest.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index df4ba6bbead..0e6caad72d6 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1134,14 +1134,16 @@ void existingEntryBlockWithCommentsAllOverThePlace() { D: # Comment untouched 4 1: something else 2: old desc # Comment moved from prefix D2 to prefix D->3 + # This is also part of prefix D2, but should NOT be moved to D->3 D2: 1: a - # Above comment untouched 1 - 2: b # Comment untouched 5 + # Comment above tag untouched 1 + 2: b # Comment with a lot of spaces untouched 5 3: c - # Above comment untouched 2 + # Comment above tag untouched 2 + # with multilines D3: # Comment untouched 6 - 1: old description # Comment moved from prefix D4 to prefix D3->2 + 1: old description # Comment with a lot of spaces moved from prefix D4 to prefix D3->2 D4: # Comment untouched 7 1: old text # Comment moved from end document to prefix D4->2 """, @@ -1153,15 +1155,17 @@ void existingEntryBlockWithCommentsAllOverThePlace() { 1: something else 2: old desc # Comment moved from prefix D2 to prefix D->3 3: new desc + # This is also part of prefix D2, but should NOT be moved to D->3 D2: 1: a - # Above comment untouched 1 - 2: b # Comment untouched 5 + # Comment above tag untouched 1 + 2: b # Comment with a lot of spaces untouched 5 3: c 4: d - # Above comment untouched 2 + # Comment above tag untouched 2 + # with multilines D3: # Comment untouched 6 - 1: old description # Comment moved from prefix D4 to prefix D3->2 + 1: old description # Comment with a lot of spaces moved from prefix D4 to prefix D3->2 2: new description D4: # Comment untouched 7 1: old text # Comment moved from end document to prefix D4->2 From 6beb8467f045b462b2464819d074a267ca158f17 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:53:33 +0100 Subject: [PATCH 27/58] Use System.lineSeparator() --- .../main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index d54c7f98e1c..67b62bdb333 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -34,6 +34,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static java.lang.System.lineSeparator; import static org.openrewrite.Cursor.ROOT_VALUE; import static org.openrewrite.internal.ListUtils.*; import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @@ -108,7 +109,7 @@ public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); - return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); } return mapping; @@ -153,7 +154,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } // workaround: autoFormat cannot handle new inserted values very well if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabAfterFirstLineBreak(mergedEntries.get(0))); + return it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(mergedEntries.get(0))); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); @@ -214,7 +215,7 @@ private void removePrefixForDirectChildren(List m1Entries, L for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix(lineSeparator() + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); } } } @@ -295,7 +296,7 @@ private String grabBeforeFirstLineBreak(Yaml.Mapping.Entry entry) { private String grabAfterFirstLineBreak(Yaml.Mapping.Entry entry) { String[] parts = LINE_BREAK.split(entry.getPrefix()); - return parts.length > 1 ? String.join("\n", Arrays.copyOfRange(parts, 1, parts.length)) : ""; + return parts.length > 1 ? String.join(lineSeparator(), Arrays.copyOfRange(parts, 1, parts.length)) : ""; } private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { From 0f7e1b0b529827dafe96d5da7414d187b84b280b Mon Sep 17 00:00:00 2001 From: lingenj Date: Wed, 20 Nov 2024 09:17:39 +0100 Subject: [PATCH 28/58] improvement --- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 67b62bdb333..142e09876d4 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -156,7 +156,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { return it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(mergedEntries.get(0))); } - return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; + return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { From bec84ea98889c3053413dc40dcca979716d63254 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 10:59:36 +0100 Subject: [PATCH 29/58] Add `existingEntryBlockWithComment` test --- .../org/openrewrite/yaml/MergeYamlTest.java | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index fd8bba68811..4331951b1a4 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -15,6 +15,7 @@ */ package org.openrewrite.yaml; +import lombok.Value; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -111,7 +112,6 @@ void existingMultipleEntryBlock() { ); } - @DocumentExample @Test void nonExistentBlock() { rewriteRun( @@ -1010,6 +1010,72 @@ void mergeSequenceMapChangeComplexMapping() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithComment() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # Some comment + """, + """ + spring: + application: + name: main # Some comment + description: a description + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # Some comment + name2: main + """, + """ + spring: + application: + name: main # Some comment + name2: main + description: a description + """ + ) + ); + } + @Test void mergeScalar() { rewriteRun( From b43cbc015ffa49fcd5943cb809a7a9b5654bd4d9 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 10:59:45 +0100 Subject: [PATCH 30/58] Cleanup MergeYaml code --- .../java/org/openrewrite/yaml/MergeYaml.java | 9 ++--- .../openrewrite/yaml/MergeYamlVisitor.java | 34 +++++++++---------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 5361955065f..01de6b8fba4 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -96,7 +96,7 @@ public TreeVisitor getVisitor() { .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Yaml.Document doc = docs.getDocuments().get(0); - if(doc.getBlock() instanceof Yaml.Mapping) { + if (doc.getBlock() instanceof Yaml.Mapping) { Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { @@ -110,9 +110,10 @@ public TreeVisitor getVisitor() { @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { if ("$".equals(key)) { - return document.withBlock((Yaml.Block) new MergeYamlVisitor<>(document.getBlock(), yaml, - Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty).visitNonNull(document.getBlock(), - ctx, getCursor())); + return document.withBlock((Yaml.Block) + new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) + .visitNonNull(document.getBlock(), ctx, getCursor()) + ); } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 66a5f310506..8283d556a79 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -31,7 +31,7 @@ @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Yaml scope; + private final Yaml existing; private final Yaml incoming; private final boolean acceptTheirs; @@ -48,7 +48,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Yaml.Document doc = docs.getDocuments().get(0); - if(doc.getBlock() instanceof Yaml.Mapping) { + if (doc.getBlock() instanceof Yaml.Mapping) { Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { @@ -64,7 +64,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { - if (scope.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { + if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { return mergeScalar(existingScalar, (Yaml.Scalar) incoming); } return super.visitScalar(existingScalar, p); @@ -72,15 +72,15 @@ public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { - if (scope.isScope(existingSeq)) { + if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence - return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> { - Yaml.Block b = (Yaml.Block) new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, - acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) - .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)); - return existingSeqEntry.withBlock(b); - })); + return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> + existingSeqEntry.withBlock((Yaml.Block) + new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) + .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)) + ) + )); } else if (incoming instanceof Yaml.Sequence) { return mergeSequence(existingSeq, (Yaml.Sequence) incoming, p, getCursor()); } @@ -90,14 +90,14 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - if (scope.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } return super.visitMapping(existingMapping, p); } - private static boolean keyMatches(Yaml.Mapping.Entry e1, Yaml.Mapping.Entry e2) { - return e1.getKey().getValue().equals(e2.getKey().getValue()); + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { + return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { @@ -117,9 +117,9 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor List mutatedEntries = ListUtils.map(m1.getEntries(), existingEntry -> { for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { - return existingEntry.withValue((Yaml.Block) new MergeYamlVisitor<>(existingEntry.getValue(), - incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) - .visitNonNull(existingEntry.getValue(), p, new Cursor(cursor, existingEntry))); + return existingEntry.withValue((Yaml.Block) + new MergeYamlVisitor<>(existingEntry.getValue(), incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) + .visitNonNull(existingEntry.getValue(), p, new Cursor(cursor, existingEntry))); } } return existingEntry; @@ -176,7 +176,7 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); - if(e1 == existingEntry) { + if (e1 == existingEntry) { // Made no change, no need to consider the entry "mutated" //noinspection DataFlowIssue return null; From 44a26ffd8752930ee646476febbf8f5149a9bf01 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 16:02:13 +0100 Subject: [PATCH 31/58] Little progress... --- .../java/org/openrewrite/yaml/MergeYaml.java | 8 +- .../openrewrite/yaml/MergeYamlVisitor.java | 65 +++++++++++++++++ .../org/openrewrite/yaml/MergeYamlTest.java | 73 ++++++++++++++++++- 3 files changed, 143 insertions(+), 3 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 01de6b8fba4..0e52c7737d5 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -110,10 +110,16 @@ public TreeVisitor getVisitor() { @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { if ("$".equals(key)) { - return document.withBlock((Yaml.Block) + Yaml.Document d = document.withBlock((Yaml.Block) new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) .visitNonNull(document.getBlock(), ctx, getCursor()) ); + + if (getCursor().getMessage("RemovePrefix", false)) { + return d.withEnd(d.getEnd().withPrefix("")); + } + + return d; } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 8283d556a79..5a7bef07e80 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -125,6 +125,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return existingEntry; }); + int x = mutatedEntries.size(); mutatedEntries = ListUtils.concatAll(mutatedEntries, ListUtils.map(m2.getEntries(), incomingEntry -> { for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { @@ -136,6 +137,70 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } return incomingEntry; })); + boolean hasNewElements = x < mutatedEntries.size(); + + if (hasNewElements) { + System.out.println(">>>>"); + + Cursor currCursor = getCursor(); + Cursor c = cursor.dropParentWhile(it -> { + if (it instanceof Yaml.Document) { + return false; + } + + if (it instanceof Yaml.Mapping) { + boolean xx = ((Yaml.Mapping) it).getEntries().size() == 1; + + if (!xx) { + + } + + return xx; + } + + return true; + }); + + if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { + Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); + String comment = ""; + + if (c.getValue() instanceof Yaml.Document) { + comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); + + } + if (c.getValue() instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + + + int index = entries.size() - 1; + + if (currCursor.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) currCursor.getValue()).getEntries().size() == 1) { + for (int i = 0; i < entries.size(); i++) { + if (((Yaml.Mapping) entries.get(i).getValue()).getEntries().get(0).equals(((Yaml.Mapping) currCursor.getValue()).getEntries().get(0))) { + index = i + 1; + System.out.println(entries.size() == index); + break; + } + } + } + + // tenporary check + if (index < entries.size()) { + comment = entries.get(index).getPrefix().split("\n")[0]; + } + } + + + //mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + c.putMessage("RemovePrefix", true); + } + + + } + + //System.out.println((String) cursor.getMessage("RemovePrefix")); + return m1.withEntries(mutatedEntries); } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 4331951b1a4..23b4d87bb1a 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1030,12 +1030,12 @@ void existingEntryBlockWithComment() { """ spring: application: - name: main # Some comment + name: main # some comment """, """ spring: application: - name: main # Some comment + name: main # some comment description: a description """ ) @@ -1045,6 +1045,75 @@ void existingEntryBlockWithComment() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 3: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else + 2: old desc # Some comment 5 + D2: # Some comment 6 + 1: old description # Some comment 7 + D3: # Some comment 8 + 1: old text # Some comment 9 + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else + 2: old desc # Some comment 5 + 3: new desc + D2: # Some comment 6 + 1: old description # Some comment 7 + 2: new description + D3: # Some comment 8 + 1: old text # Some comment 9 + 2: new text + """ + ) + ); + } + + /* VAN: + spring: # Some comment + application: # Some comment 2 + name: main # Some comment 3 + + NAAR + + spring:> # Some comment + # Some comment 2 + # Some comment 3 + spec.recipe(new MergeYaml( "$", From 993426ad45cae57e873b7291d8cc1ea26450e941 Mon Sep 17 00:00:00 2001 From: lingenj Date: Thu, 14 Nov 2024 16:22:18 +0100 Subject: [PATCH 32/58] Little progress... --- .../openrewrite/yaml/MergeYamlVisitor.java | 10 +--- .../org/openrewrite/yaml/MergeYamlTest.java | 58 ++++++++++++++++++- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 5a7bef07e80..3bd263f9936 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -149,13 +149,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (it instanceof Yaml.Mapping) { - boolean xx = ((Yaml.Mapping) it).getEntries().size() == 1; - - if (!xx) { - - } - - return xx; + return ((Yaml.Mapping) it).getEntries().size() == 1; } return true; @@ -192,7 +186,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } - //mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 23b4d87bb1a..2227c12b479 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1045,6 +1045,62 @@ void existingEntryBlockWithComment() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentVariant() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 4: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: + C: + D: + 1: something else + 2: something else + 3: old desc + D2: + 1: old description + D3: + 1: old text + """, + """ + A: # Some comment + B: + C: + D: + 1: something else + 2: something else + 3: old desc + 4: new desc + D2: + 1: old description + 2: new description + D3: + 1: old text + 2: new text + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentVariant2() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1113,7 +1169,7 @@ void existingEntryBlockWithCommentVariant() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant2() { + void existingEntryBlockWithCommentVariant3() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", From efb880dbdb90ff0fd10ddd666d5a4b9d852fae79 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 09:11:39 +0100 Subject: [PATCH 33/58] improvement --- .../main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 3bd263f9936..7d443926036 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -166,12 +166,11 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); - int index = entries.size() - 1; - if (currCursor.getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) currCursor.getValue()).getEntries().size() == 1) { + if (currCursor.getValue() instanceof Yaml.Mapping) { for (int i = 0; i < entries.size(); i++) { - if (((Yaml.Mapping) entries.get(i).getValue()).getEntries().get(0).equals(((Yaml.Mapping) currCursor.getValue()).getEntries().get(0))) { + if (entries.get(i).getValue().equals(currCursor.getValue())) { index = i + 1; System.out.println(entries.size() == index); break; @@ -179,7 +178,7 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } - // tenporary check + // temporary check if (index < entries.size()) { comment = entries.get(index).getPrefix().split("\n")[0]; } From cf2fe00edb48e1e55fc75eba8000f68eb983e750 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 09:28:42 +0100 Subject: [PATCH 34/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 7d443926036..da99d5e572a 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -140,8 +140,6 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor boolean hasNewElements = x < mutatedEntries.size(); if (hasNewElements) { - System.out.println(">>>>"); - Cursor currCursor = getCursor(); Cursor c = cursor.dropParentWhile(it -> { if (it instanceof Yaml.Document) { @@ -149,12 +147,22 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (it instanceof Yaml.Mapping) { - return ((Yaml.Mapping) it).getEntries().size() == 1; + List entries = ((Yaml.Mapping) it).getEntries(); + + // direct parent -> child with last member should search further upwards + if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { + return true; + } + return entries.size() == 1; } return true; }); + // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; + System.out.println(">>>"); + System.out.println(c); + if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; @@ -165,23 +173,21 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + int index = /*entries.size() */- 1; - int index = entries.size() - 1; - - if (currCursor.getValue() instanceof Yaml.Mapping) { + //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed for (int i = 0; i < entries.size(); i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { index = i + 1; - System.out.println(entries.size() == index); break; } } - } + // } // temporary check - if (index < entries.size()) { + // if (index < entries.size()) { comment = entries.get(index).getPrefix().split("\n")[0]; - } + // } } From 8d5888225511af08517ec702228a97cc04553e66 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 10:58:38 +0100 Subject: [PATCH 35/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index da99d5e572a..27389cb0251 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -139,8 +139,9 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor })); boolean hasNewElements = x < mutatedEntries.size(); + Cursor currCursor = getCursor(); + if (hasNewElements) { - Cursor currCursor = getCursor(); Cursor c = cursor.dropParentWhile(it -> { if (it instanceof Yaml.Document) { return false; @@ -194,11 +195,36 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } + } + /* System.out.println("----"); + System.out.println((Boolean) cursor.getMessage("RemovePrefix", null)); + System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); + System.out.println("----");*/ + if (cursor.getMessage("RemovePrefix", false)) { + System.out.println("Waarom niet "); } - //System.out.println((String) cursor.getMessage("RemovePrefix")); + for (int i = 0; i < m1.getEntries().size(); i++) { + if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && + mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && + ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { + System.out.println("yawel!!"); + + // temporary if + if ((i + 1) < mutatedEntries.size()) { + System.out.println("en goo......"); + // maybe do this in a more immutable way instead of a `set action`. + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); + } + } + + + if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { + + } + } return m1.withEntries(mutatedEntries); From 1f5e5ac1c2531f0876d9a9fc649ee084133fcb7d Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 11:18:13 +0100 Subject: [PATCH 36/58] Fix null Yaml.Document.End objects --- .../java/org/openrewrite/yaml/YamlParser.java | 36 ++++++++++--------- .../java/org/openrewrite/yaml/tree/Yaml.java | 2 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java index 047c91789a8..3ec08feb969 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java @@ -50,6 +50,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.openrewrite.Tree.randomId; +import static org.openrewrite.marker.Markers.EMPTY; public class YamlParser implements org.openrewrite.Parser { private static final Pattern VARIABLE_PATTERN = Pattern.compile(":\\s*(@[^\n\r@]+@)"); @@ -82,8 +83,9 @@ public Stream parseInputs(Iterable sourceFiles, @Nullable Pat Yaml.Documents docs = (Yaml.Documents) sourceFile; // ensure there is always at least one Document, even in an empty yaml file if (docs.getDocuments().isEmpty()) { - return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", Markers.EMPTY, - false, new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), null))); + Yaml.Document.End end = new Yaml.Document.End(randomId(), "", EMPTY, false); + Yaml.Mapping mapping = new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null); + return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", EMPTY, false, mapping, end))); } return docs; } @@ -142,7 +144,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr documents.add(document.withEnd(new Yaml.Document.End( randomId(), fmt, - Markers.EMPTY, + EMPTY, ((DocumentEndEvent) event).getExplicit() ))); lastEnd = event.getEndMark().getIndex(); @@ -154,10 +156,10 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr document = new Yaml.Document( randomId(), fmt, - Markers.EMPTY, + EMPTY, ((DocumentStartEvent) event).getExplicit(), - new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), - null + new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", EMPTY, false) ); lastEnd = event.getEndMark().getIndex(); break; @@ -254,7 +256,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } break; } - Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, Markers.EMPTY, style, anchor, scalarValue); + Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, EMPTY, style, anchor, scalarValue); BlockBuilder builder = blockStack.isEmpty() ? null : blockStack.peek(); if (builder instanceof SequenceBuilder) { // Inline sequences like [1, 2] need to keep track of any whitespace between the element @@ -351,7 +353,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr throw new UnsupportedOperationException("Unknown anchor: " + alias.getAnchor()); } BlockBuilder builder = blockStack.peek(); - builder.push(new Yaml.Alias(randomId(), fmt, Markers.EMPTY, anchor)); + builder.push(new Yaml.Alias(randomId(), fmt, EMPTY, anchor)); lastEnd = event.getEndMark().getIndex(); break; } @@ -360,9 +362,9 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr if (document == null && !fmt.isEmpty()) { documents.add( new Yaml.Document( - randomId(), fmt, Markers.EMPTY, false, - new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) + randomId(), fmt, EMPTY, false, + new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", EMPTY, false) )); } break; @@ -372,7 +374,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } } - return new Yaml.Documents(randomId(), Markers.EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); + return new Yaml.Documents(randomId(), EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -408,7 +410,7 @@ private Yaml.Anchor buildYamlAnchor(FormatPreservingReader reader, int lastEnd, if (!isForScalar) { prefix = (prefixStart > -1 && eventPrefix.length() > prefixStart + 1) ? eventPrefix.substring(prefixStart + 1) : ""; } - return new Yaml.Anchor(randomId(), prefix, postFix.toString(), Markers.EMPTY, anchorKey); + return new Yaml.Anchor(randomId(), prefix, postFix.toString(), EMPTY, anchorKey); } private static int commentAwareIndexOf(char target, String s) { @@ -490,7 +492,7 @@ public void push(Yaml.Block block) { String entryPrefix = originalKeyPrefix.substring(entryPrefixStartIndex); String beforeMappingValueIndicator = keySuffix.substring(0, Math.max(commentAwareIndexOf(':', keySuffix), 0)); - entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, Markers.EMPTY, key, beforeMappingValueIndicator, block)); + entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, EMPTY, key, beforeMappingValueIndicator, block)); key = null; } } @@ -536,7 +538,7 @@ public void push(Yaml.Block block, @Nullable String commaPrefix) { entryPrefix = ""; blockPrefix = rawPrefix; } - entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, Markers.EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); + entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); } @Override @@ -566,7 +568,7 @@ private static class MappingWithPrefix extends Yaml.Mapping { private String prefix; public MappingWithPrefix(String prefix, @Nullable String startBracePrefix, List entries, @Nullable String endBracePrefix, @Nullable Anchor anchor) { - super(randomId(), Markers.EMPTY, startBracePrefix, entries, endBracePrefix, anchor); + super(randomId(), EMPTY, startBracePrefix, entries, endBracePrefix, anchor); this.prefix = prefix; } @@ -582,7 +584,7 @@ private static class SequenceWithPrefix extends Yaml.Sequence { private String prefix; public SequenceWithPrefix(String prefix, @Nullable String startBracketPrefix, List entries, @Nullable String endBracketPrefix, @Nullable Anchor anchor) { - super(randomId(), Markers.EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); + super(randomId(), EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); this.prefix = prefix; } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java index a92c0370e54..5245d9ab397 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/tree/Yaml.java @@ -153,7 +153,7 @@ public Document copyPaste() { Markers.EMPTY, explicit, block.copyPaste(), - end == null ? null : end.copyPaste() + end.copyPaste() ); } From 6597ec8163a7aab9c8ba762f8ed4a4f69e73cabb Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 11:29:19 +0100 Subject: [PATCH 37/58] improvement --- .../java/org/openrewrite/yaml/MergeYamlVisitor.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 27389cb0251..63fb32eee31 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -170,25 +170,19 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor if (c.getValue() instanceof Yaml.Document) { comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); - } if (c.getValue() instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) c.getValue()).getEntries(); - int index = /*entries.size() */- 1; //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - for (int i = 0; i < entries.size(); i++) { + //for (int i = 0; i < entries.size(); i++) { + for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { - index = i + 1; + comment = entries.get(i + 1).getPrefix().split("\n")[0]; break; } } // } - - // temporary check - // if (index < entries.size()) { - comment = entries.get(index).getPrefix().split("\n")[0]; - // } } From b51d7dc944691f35840bec55119de064dbbf3fcf Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 13:48:23 +0100 Subject: [PATCH 38/58] got it first working --- .../openrewrite/yaml/MergeYamlVisitor.java | 78 ++++++++++--------- .../org/openrewrite/yaml/MergeYamlTest.java | 4 + 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 63fb32eee31..64f24095e9c 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -21,13 +21,19 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; +import org.openrewrite.internal.StringUtils; import org.openrewrite.yaml.tree.Yaml; +import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; +import static org.openrewrite.internal.ListUtils.concatAll; +import static org.openrewrite.internal.ListUtils.map; +import static org.openrewrite.internal.StringUtils.isNotEmpty; + @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ @@ -63,9 +69,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean } @Override - public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { - if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { - return mergeScalar(existingScalar, (Yaml.Scalar) incoming); + public Yaml visitScalar(Scalar existingScalar, P p) { + if (existing.isScope(existingScalar) && incoming instanceof Scalar) { + return mergeScalar(existingScalar, (Scalar) incoming); } return super.visitScalar(existingScalar, p); } @@ -75,7 +81,7 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence - return existingSeq.withEntries(ListUtils.map(existingSeq.getEntries(), (i, existingSeqEntry) -> + return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) new MergeYamlVisitor<>(existingSeqEntry.getBlock(), incoming, acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) .visitNonNull(existingSeqEntry.getBlock(), p, new Cursor(getCursor(), existingSeqEntry)) @@ -103,18 +109,18 @@ private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping. private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) + .map(e -> ((Scalar) e.getValue()).getValue()) .findAny(); return nameToAdd.map(nameToAddValue -> m1.getEntries().stream() .filter(e -> objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) + .map(e -> ((Scalar) e.getValue()).getValue()) .anyMatch(existingName -> existingName.equals(nameToAddValue))) .orElse(false); } private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { - List mutatedEntries = ListUtils.map(m1.getEntries(), existingEntry -> { + List mergedEntries = map(m1.getEntries(), existingEntry -> { for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) @@ -125,17 +131,18 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return existingEntry; }); - int x = mutatedEntries.size(); - mutatedEntries = ListUtils.concatAll(mutatedEntries, ListUtils.map(m2.getEntries(), incomingEntry -> { + int x = mergedEntries.size(); + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { - if (keyMatches(existingEntry, incomingEntry)) { + if (keyMatches(existingEntry, it)) { return null; } } - if (shouldAutoFormat) { - incomingEntry = autoFormat(incomingEntry, p, cursor); + // workaround: autoFormat put sometimes extra spaces before elements + if (!mergedEntries.isEmpty() && mergedEntries.get(0).getPrefix().contains("\n") && it.getValue() instanceof Scalar) { + return it.withPrefix("\n" + mergedEntries.get(0).getPrefix().split("\n")[1]); } - return incomingEntry; + return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); boolean hasNewElements = x < mutatedEntries.size(); @@ -175,17 +182,16 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor List entries = ((Yaml.Mapping) c.getValue()).getEntries(); //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - //for (int i = 0; i < entries.size(); i++) { - for (int i = 0; i < entries.size() - 1; i++) { - if (entries.get(i).getValue().equals(currCursor.getValue())) { - comment = entries.get(i + 1).getPrefix().split("\n")[0]; - break; - } + //for (int i = 0; i < entries.size(); i++) { + for (int i = 0; i < entries.size() - 1; i++) { + if (entries.get(i).getValue().equals(currCursor.getValue())) { + comment = entries.get(i + 1).getPrefix().split("\n")[0]; + break; } - // } + } + // } } - mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); c.putMessage("RemovePrefix", true); } @@ -208,16 +214,15 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor // temporary if if ((i + 1) < mutatedEntries.size()) { - System.out.println("en goo......"); - // maybe do this in a more immutable way instead of a `set action`. + System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } } - if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { + /*if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { - } + }*/ } @@ -229,17 +234,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Yaml.Scalar); + boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Scalar); if (isSequenceOfScalars) { List incomingEntries = new ArrayList<>(s2.getEntries()); nextEntry: for (Yaml.Sequence.Entry entry : s1.getEntries()) { - if (entry.getBlock() instanceof Yaml.Scalar) { - String existingScalar = ((Yaml.Scalar) entry.getBlock()).getValue(); + if (entry.getBlock() instanceof Scalar) { + String existingScalar = ((Scalar) entry.getBlock()).getValue(); for (Yaml.Sequence.Entry incomingEntry : incomingEntries) { - if (((Yaml.Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { + if (((Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { incomingEntries.remove(incomingEntry); continue nextEntry; } @@ -247,14 +252,13 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - return s1.withEntries(ListUtils.concatAll(s1.getEntries(), - ListUtils.map(incomingEntries, incomingEntry -> autoFormat(incomingEntry, p, cursor)))); + return s1.withEntries(concatAll(s1.getEntries(), map(incomingEntries, it -> autoFormat(it, p, cursor)))); } else { if (objectIdentifyingProperty == null) { // No identifier set to match entries on, so cannot continue return s1; } else { - List mutatedEntries = ListUtils.map(s2.getEntries(), entry -> { + List mutatedEntries = map(s2.getEntries(), entry -> { Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); @@ -262,7 +266,6 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { // Made no change, no need to consider the entry "mutated" - //noinspection DataFlowIssue return null; } return e1; @@ -274,10 +277,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - List entries = ListUtils.concatAll( - s1.getEntries().stream().filter(entry -> !mutatedEntries.contains(entry)) - .collect(Collectors.toList()), - ListUtils.map(mutatedEntries, entry -> autoFormat(entry, p, cursor))); + List entries = concatAll( + s1.getEntries().stream().filter(entry -> !mutatedEntries.contains(entry)).collect(Collectors.toList()), + map(mutatedEntries, entry -> autoFormat(entry, p, cursor))); if (entries.size() != s1.getEntries().size()) { return s1.withEntries(entries); @@ -292,7 +294,7 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private Yaml.Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { + private Scalar mergeScalar(Scalar y1, Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); return !s1.equals(s2) && !acceptTheirs ? y1.withValue(s2) : y1; diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 2227c12b479..d0031fc775c 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1059,6 +1059,8 @@ void existingEntryBlockWithCommentVariant() { 2: new description D3: 2: new text + 3: more new text + E: description """, false, null, @@ -1093,6 +1095,8 @@ void existingEntryBlockWithCommentVariant() { D3: 1: old text 2: new text + 3: more new text + E: description """ ) ); From 66cdfd22e561521b87a329f28f8aa8a7b5310570 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 15:29:18 +0100 Subject: [PATCH 39/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 37 ++++-- .../org/openrewrite/yaml/MergeYamlTest.java | 122 +++++++++++------- 2 files changed, 100 insertions(+), 59 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 64f24095e9c..28137d7c327 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.openrewrite.internal.ListUtils.concatAll; @@ -37,6 +38,9 @@ @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ + + private final Pattern linebreakPattern = Pattern.compile("\\R"); + private final Yaml existing; private final Yaml incoming; private final boolean acceptTheirs; @@ -139,8 +143,12 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && mergedEntries.get(0).getPrefix().contains("\n") && it.getValue() instanceof Scalar) { - return it.withPrefix("\n" + mergedEntries.get(0).getPrefix().split("\n")[1]); + if (!mergedEntries.isEmpty() && linebreakPattern.matcher(mergedEntries.get(0).getPrefix()).find() && it.getValue() instanceof Scalar) { + String[] parts = linebreakPattern.split(mergedEntries.get(0).getPrefix()); + if (parts.length > 1) { // why? + return it.withPrefix("\n" + parts[1]); + } + } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); @@ -149,22 +157,26 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor Cursor currCursor = getCursor(); if (hasNewElements) { - Cursor c = cursor.dropParentWhile(it -> { + /*if (currCursor.getValue() instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) currCursor.getValue()).getEntries(); + System.out.println("d"); + }*/ + + Cursor c = getCursor().dropParentUntil(it -> { if (it instanceof Yaml.Document) { - return false; + return true; } if (it instanceof Yaml.Mapping) { List entries = ((Yaml.Mapping) it).getEntries(); - - // direct parent -> child with last member should search further upwards + // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { - return true; + return false; } - return entries.size() == 1; + return entries.size() > 1; } - return true; + return false; }); // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; @@ -189,6 +201,13 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor break; } } + if (comment.isEmpty() && linebreakPattern.matcher(entries.get(entries.size() - 1).getPrefix()).find()) { + String[] parts = linebreakPattern.split(entries.get(entries.size() - 1).getPrefix()); + if (parts.length > 0) { // why??? + comment = entries.get(entries.size() - 1).getPrefix().split("\n")[0]; + } + } + // // } } diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index d0031fc775c..2c3e0d0053f 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -15,7 +15,6 @@ */ package org.openrewrite.yaml; -import lombok.Value; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; import org.openrewrite.Issue; @@ -112,6 +111,7 @@ void existingMultipleEntryBlock() { ); } + @DocumentExample @Test void nonExistentBlock() { rewriteRun( @@ -1012,39 +1012,7 @@ void mergeSequenceMapChangeComplexMapping() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithComment() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - spring: - application: - description: a description - """, - false, - null, - null - )), - yaml( - """ - spring: - application: - name: main # some comment - """, - """ - spring: - application: - name: main # some comment - description: a description - """ - ) - ); - } - - @Issue("https://github.com/openrewrite/rewrite/issues/2218") - @Test - void existingEntryBlockWithCommentVariant() { + void existingEntryBlockWithCommentAtFirstLine() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1104,7 +1072,39 @@ void existingEntryBlockWithCommentVariant() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant2() { + void existingEntryBlockWithCommentAtLastLine() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + spring: + application: + description: a description + """, + false, + null, + null + )), + yaml( + """ + spring: + application: + name: main # some comment + """, + """ + spring: + application: + name: main # some comment + description: a description + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/2218") + @Test + void existingEntryBlockWithCommentAllOverThePlace() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1156,24 +1156,46 @@ void existingEntryBlockWithCommentVariant2() { ); } - /* VAN: - spring: # Some comment - application: # Some comment 2 - name: main # Some comment 3 - - NAAR - - spring:> # Some comment - # Some comment 2 - # Some comment 3 - spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + 2: new desc + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + 1: old desc # Some comment 4 + D: whatever + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + 1: old desc # Some comment 4 + 2: new desc + D: whatever + """ + ) + ); + } @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentVariant3() { + void existingEntryBlockWithCommentNotAtLastLine() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", From 4ffc9fb7aec6ebee382c7559bf7f819b254d1010 Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 15:47:17 +0100 Subject: [PATCH 40/58] improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 28137d7c327..22d7870f5ef 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -21,7 +21,6 @@ import org.jspecify.annotations.Nullable; import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; -import org.openrewrite.internal.StringUtils; import org.openrewrite.yaml.tree.Yaml; import org.openrewrite.yaml.tree.Yaml.Scalar; @@ -33,13 +32,12 @@ import static org.openrewrite.internal.ListUtils.concatAll; import static org.openrewrite.internal.ListUtils.map; -import static org.openrewrite.internal.StringUtils.isNotEmpty; @AllArgsConstructor @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Pattern linebreakPattern = Pattern.compile("\\R"); + private final Pattern LINE_BREAK = Pattern.compile("\\R"); private final Yaml existing; private final Yaml incoming; @@ -143,12 +141,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && linebreakPattern.matcher(mergedEntries.get(0).getPrefix()).find() && it.getValue() instanceof Scalar) { - String[] parts = linebreakPattern.split(mergedEntries.get(0).getPrefix()); - if (parts.length > 1) { // why? - return it.withPrefix("\n" + parts[1]); - } - + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreakPieces(mergedEntries.get(0), 2)) { + return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); @@ -201,11 +195,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor break; } } - if (comment.isEmpty() && linebreakPattern.matcher(entries.get(entries.size() - 1).getPrefix()).find()) { - String[] parts = linebreakPattern.split(entries.get(entries.size() - 1).getPrefix()); - if (parts.length > 0) { // why??? - comment = entries.get(entries.size() - 1).getPrefix().split("\n")[0]; - } + if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { + comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } // // } @@ -313,6 +304,16 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } + private boolean hasLineBreakPieces(Yaml.Mapping.Entry entry, int atLeast) { + boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); + return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); + } + + private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + String[] parts = LINE_BREAK.split(entry.getPrefix()); + return parts.length > index ? parts[index] : ""; + } + private Scalar mergeScalar(Scalar y1, Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); From 9537d4f7e5de52c5216910d752e46017de24e15e Mon Sep 17 00:00:00 2001 From: lingenj Date: Fri, 15 Nov 2024 16:36:03 +0100 Subject: [PATCH 41/58] why?? --- .../openrewrite/yaml/MergeYamlVisitor.java | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 22d7870f5ef..b26b10ad218 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -30,8 +30,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import static org.openrewrite.internal.ListUtils.concatAll; -import static org.openrewrite.internal.ListUtils.map; +import static org.openrewrite.internal.ListUtils.*; @AllArgsConstructor @RequiredArgsConstructor @@ -72,6 +71,7 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Scalar existingScalar, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingScalar) && incoming instanceof Scalar) { return mergeScalar(existingScalar, (Scalar) incoming); } @@ -80,6 +80,8 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + if (existing.isScope(existingSeq)) { if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence @@ -98,12 +100,24 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { + System.out.println("<<< visitMapping"); + if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { + System.out.println("ja!!"); + } + System.out.println(getCursor()); + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } return super.visitMapping(existingMapping, p); } + @Override + public Yaml visitMappingEntry(Yaml.Mapping.Entry entry, P p) { + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + return super.visitMappingEntry(entry, p); + } + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } @@ -173,10 +187,6 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return false; }); - // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>>"); - System.out.println(c); - if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; @@ -203,6 +213,12 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor } mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + + + + // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; + System.out.println(">>>"); + System.out.println(c); c.putMessage("RemovePrefix", true); } } @@ -212,21 +228,23 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); System.out.println("----");*/ - if (cursor.getMessage("RemovePrefix", false)) { - System.out.println("Waarom niet "); - } - for (int i = 0; i < m1.getEntries().size(); i++) { if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { System.out.println("yawel!!"); - // temporary if if ((i + 1) < mutatedEntries.size()) { System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } + + //Entry s = mutatedEntries.get(i).getValue().withPrefix("Whatever"); + /*Yaml.Mapping xdx = (Yaml.Mapping) mutatedEntries.get(i).getValue(); + Yaml.Mapping xxxx= xdx.withEntries(mapLast(xdx.getEntries(), it -> it.withPrefix("boom\n"))); + + mutatedEntries.set(i, mutatedEntries.get(i).withValue(xxxx));*/ + } From 4161e15e6f5409fb1000586396459b4350635783 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 10:30:54 +0100 Subject: [PATCH 42/58] Add more logging --- .../main/java/org/openrewrite/yaml/MergeYaml.java | 2 ++ .../org/openrewrite/yaml/MergeYamlVisitor.java | 14 ++++++++------ .../java/org/openrewrite/yaml/YamlVisitor.java | 2 ++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 0e52c7737d5..168127c9e63 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -116,6 +116,8 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) ); if (getCursor().getMessage("RemovePrefix", false)) { + System.out.println("|||| after visit document"); + System.out.println(System.identityHashCode(getCursor())); return d.withEnd(d.getEnd().withPrefix("")); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index b26b10ad218..9dc1d28ba60 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -100,12 +100,13 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { @Override public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - System.out.println("<<< visitMapping"); + //System.out.println("<<< visitMapping"); if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { - System.out.println("ja!!"); + System.out.println(System.identityHashCode(getCursor())); + System.out.println(getCursor()); + System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); } - System.out.println(getCursor()); - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); } @@ -217,9 +218,10 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>>"); - System.out.println(c); + System.out.println(">>> RemovePrefix"); + System.out.println(System.identityHashCode(c)); c.putMessage("RemovePrefix", true); + System.out.println(c); } } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java index 63b5ecb5069..7434e66a542 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java @@ -66,11 +66,13 @@ public Y2 autoFormat(Y2 y, @Nullable Yaml stopAfter, P p, Curs } public Yaml visitDocuments(Yaml.Documents documents, P p) { + System.out.println("visitDocumentSSSSSS"); return documents.withDocuments(ListUtils.map(documents.getDocuments(), d -> visitAndCast(d, p))) .withMarkers(visitMarkers(documents.getMarkers(), p)); } public Yaml visitDocument(Yaml.Document document, P p) { + System.out.println("visitDocument"); return document.withBlock((Yaml.Block) visit(document.getBlock(), p)) .withMarkers(visitMarkers(document.getMarkers(), p)); } From be1ea5cb92fdfe1b56bc7b281d4d6a360074479b Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 15:07:21 +0100 Subject: [PATCH 43/58] Got comment working --- .../java/org/openrewrite/yaml/MergeYaml.java | 2 - .../openrewrite/yaml/MergeYamlVisitor.java | 117 ++++++------------ .../org/openrewrite/yaml/YamlVisitor.java | 2 - 3 files changed, 38 insertions(+), 83 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 168127c9e63..0e52c7737d5 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -116,8 +116,6 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) ); if (getCursor().getMessage("RemovePrefix", false)) { - System.out.println("|||| after visit document"); - System.out.println(System.identityHashCode(getCursor())); return d.withEnd(d.getEnd().withPrefix("")); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 9dc1d28ba60..3359bac0799 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -22,6 +22,9 @@ import org.openrewrite.Cursor; import org.openrewrite.internal.ListUtils; import org.openrewrite.yaml.tree.Yaml; +import org.openrewrite.yaml.tree.Yaml.Document; +import org.openrewrite.yaml.tree.Yaml.Mapping; +import org.openrewrite.yaml.tree.Yaml.Mapping.Entry; import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; @@ -54,9 +57,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(Yaml.Documents.class::cast) .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping - Yaml.Document doc = docs.getDocuments().get(0); - if (doc.getBlock() instanceof Yaml.Mapping) { - Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); + Document doc = docs.getDocuments().get(0); + if (doc.getBlock() instanceof Mapping) { + Mapping m = (Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { Yaml.Sequence s = (Yaml.Sequence) doc.getBlock(); @@ -71,7 +74,6 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean @Override public Yaml visitScalar(Scalar existingScalar, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); if (existing.isScope(existingScalar) && incoming instanceof Scalar) { return mergeScalar(existingScalar, (Scalar) incoming); } @@ -80,10 +82,8 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - if (existing.isScope(existingSeq)) { - if (incoming instanceof Yaml.Mapping) { + if (incoming instanceof Mapping) { // Distribute the incoming mapping to each entry in the sequence return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) @@ -99,31 +99,25 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { } @Override - public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { - //System.out.println("<<< visitMapping"); - if (getCursor().toString().equals("Cursor{Mapping->Document->Documents->root}")) { - System.out.println(System.identityHashCode(getCursor())); - System.out.println(getCursor()); - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - } + public Yaml visitMapping(Mapping existingMapping, P p) { + if (existing.isScope(existingMapping) && incoming instanceof Mapping) { + Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); - if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { - return mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); + if (getCursor().getMessage("RemovePrefix", false)) { + List entries = ((Mapping) getCursor().getValue()).getEntries(); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); + } + + return mapping; } return super.visitMapping(existingMapping, p); } - @Override - public Yaml visitMappingEntry(Yaml.Mapping.Entry entry, P p) { - System.out.println((boolean) (getCursor().getMessage("RemovePrefix", false))); - return super.visitMappingEntry(entry, p); - } - - private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { + private static boolean keyMatches(@Nullable Entry e1, @Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } - private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { + private boolean keyMatches(Mapping m1, Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) .map(e -> ((Scalar) e.getValue()).getValue()) @@ -136,9 +130,9 @@ private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { .orElse(false); } - private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { - List mergedEntries = map(m1.getEntries(), existingEntry -> { - for (Yaml.Mapping.Entry incomingEntry : m2.getEntries()) { + private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { + List mergedEntries = map(m1.getEntries(), existingEntry -> { + for (Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) new MergeYamlVisitor<>(existingEntry.getValue(), incomingEntry.getValue(), acceptTheirs, objectIdentifyingProperty, shouldAutoFormat) @@ -149,8 +143,8 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor }); int x = mergedEntries.size(); - List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { - for (Yaml.Mapping.Entry existingEntry : m1.getEntries()) { + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { + for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { return null; } @@ -166,18 +160,13 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor Cursor currCursor = getCursor(); if (hasNewElements) { - /*if (currCursor.getValue() instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) currCursor.getValue()).getEntries(); - System.out.println("d"); - }*/ - Cursor c = getCursor().dropParentUntil(it -> { - if (it instanceof Yaml.Document) { + if (it instanceof Document) { return true; } - if (it instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) it).getEntries(); + if (it instanceof Mapping) { + List entries = ((Mapping) it).getEntries(); // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { return false; @@ -188,71 +177,41 @@ private Yaml.Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor return false; }); - if (c.getValue() instanceof Yaml.Document || c.getValue() instanceof Yaml.Mapping) { - Yaml.Mapping.Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); + if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { + Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; - if (c.getValue() instanceof Yaml.Document) { - comment = ((Yaml.Document) c.getValue()).getEnd().getPrefix(); + if (c.getValue() instanceof Document) { + comment = ((Document) c.getValue()).getEnd().getPrefix(); } - if (c.getValue() instanceof Yaml.Mapping) { - List entries = ((Yaml.Mapping) c.getValue()).getEntries(); + if (c.getValue() instanceof Mapping) { + List entries = ((Mapping) c.getValue()).getEntries(); - //if (currCursor.getValue() instanceof Yaml.Mapping) { // the `if` can be possible be removed - //for (int i = 0; i < entries.size(); i++) { for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(currCursor.getValue())) { - comment = entries.get(i + 1).getPrefix().split("\n")[0]; + comment = grabPartLineBreak(entries.get(i + 1), 0); break; } } if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } - // - // } } mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); - - - - // int index2 = ((Yaml.Mapping) currCursor.getValue()).getEntries().size() -1; - System.out.println(">>> RemovePrefix"); - System.out.println(System.identityHashCode(c)); c.putMessage("RemovePrefix", true); - System.out.println(c); } } - /* System.out.println("----"); - System.out.println((Boolean) cursor.getMessage("RemovePrefix", null)); - System.out.println((Boolean) currCursor.getMessage("RemovePrefix", null)); - System.out.println("----");*/ - + //Works only for direct children, needs a better way (with cursor messages probably) for (int i = 0; i < m1.getEntries().size(); i++) { if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { - System.out.println("yawel!!"); - if ((i + 1) < mutatedEntries.size()) { - System.out.println("en go......"); mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); } - - //Entry s = mutatedEntries.get(i).getValue().withPrefix("Whatever"); - /*Yaml.Mapping xdx = (Yaml.Mapping) mutatedEntries.get(i).getValue(); - Yaml.Mapping xxxx= xdx.withEntries(mapLast(xdx.getEntries(), it -> it.withPrefix("boom\n"))); - - mutatedEntries.set(i, mutatedEntries.get(i).withValue(xxxx));*/ - } - - - /*if(!m1.getEntries().get(i).equals(mutatedEntries.get(i))) { - - }*/ } @@ -289,9 +248,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } else { List mutatedEntries = map(s2.getEntries(), entry -> { - Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); + Mapping incomingMapping = (Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { - Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); + Mapping existingMapping = (Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { @@ -324,12 +283,12 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreakPieces(Yaml.Mapping.Entry entry, int atLeast) { + private boolean hasLineBreakPieces(Entry entry, int atLeast) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); } - private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + private String grabPartLineBreak(Entry entry, int index) { String[] parts = LINE_BREAK.split(entry.getPrefix()); return parts.length > index ? parts[index] : ""; } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java index 7434e66a542..63b5ecb5069 100755 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlVisitor.java @@ -66,13 +66,11 @@ public Y2 autoFormat(Y2 y, @Nullable Yaml stopAfter, P p, Curs } public Yaml visitDocuments(Yaml.Documents documents, P p) { - System.out.println("visitDocumentSSSSSS"); return documents.withDocuments(ListUtils.map(documents.getDocuments(), d -> visitAndCast(d, p))) .withMarkers(visitMarkers(documents.getMarkers(), p)); } public Yaml visitDocument(Yaml.Document document, P p) { - System.out.println("visitDocument"); return document.withBlock((Yaml.Block) visit(document.getBlock(), p)) .withMarkers(visitMarkers(document.getMarkers(), p)); } From 5437ae975c780bd6397cbb41e8cdc0de7695408d Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 15:56:23 +0100 Subject: [PATCH 44/58] Improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 3359bac0799..a50a1443f06 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -142,7 +142,6 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return existingEntry; }); - int x = mergedEntries.size(); List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { @@ -155,11 +154,8 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); - boolean hasNewElements = x < mutatedEntries.size(); - Cursor currCursor = getCursor(); - - if (hasNewElements) { + if (m1.getEntries().size() < mutatedEntries.size()) { Cursor c = getCursor().dropParentUntil(it -> { if (it instanceof Document) { return true; @@ -168,7 +164,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { if (it instanceof Mapping) { List entries = ((Mapping) it).getEntries(); // last member should search further upwards until two entries are found - if (entries.get(entries.size() - 1).equals(currCursor.getParentOrThrow().getValue())) { + if (entries.get(entries.size() - 1).equals(getCursor().getParentOrThrow().getValue())) { return false; } return entries.size() > 1; @@ -178,17 +174,15 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { }); if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { - Entry lastEntry = mutatedEntries.get(mutatedEntries.size() - 1); String comment = ""; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); - } - if (c.getValue() instanceof Mapping) { + } else { List entries = ((Mapping) c.getValue()).getEntries(); for (int i = 0; i < entries.size() - 1; i++) { - if (entries.get(i).getValue().equals(currCursor.getValue())) { + if (entries.get(i).getValue().equals(getCursor().getValue())) { comment = grabPartLineBreak(entries.get(i + 1), 0); break; } @@ -198,26 +192,26 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } } - mutatedEntries.set(mutatedEntries.size() - 1, lastEntry.withPrefix(comment + lastEntry.getPrefix())); + Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); c.putMessage("RemovePrefix", true); } } - //Works only for direct children, needs a better way (with cursor messages probably) - for (int i = 0; i < m1.getEntries().size(); i++) { - if (m1.getEntries().get(i).getValue() instanceof Yaml.Mapping && - mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && - ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size() > ((Yaml.Mapping) m1.getEntries().get(i).getValue()).getEntries().size()) { - if ((i + 1) < mutatedEntries.size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + mutatedEntries.get(i + 1).getPrefix().split("\n")[1])); - } - } - } - + removePrefixDirectChildren(m1.getEntries(), mutatedEntries); return m1.withEntries(mutatedEntries); } + private void removePrefixDirectChildren(List m1Entries, List mutatedEntries) { + for (int i = 0; i < m1Entries.size() - 1; i++) { + if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && + ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); + } + } + } + private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cursor cursor) { if (acceptTheirs) { return s1; From 7089d03adb752f78b8c977aacf176af3b2abe97c Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 16:02:11 +0100 Subject: [PATCH 45/58] Improvement --- .../main/java/org/openrewrite/yaml/MergeYaml.java | 8 ++------ .../java/org/openrewrite/yaml/MergeYamlVisitor.java | 13 +++++++------ 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java index 0e52c7737d5..3b1bf9c1355 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -84,6 +84,7 @@ public String getDescription() { } final static String FOUND_MATCHING_ELEMENT = "FOUND_MATCHING_ELEMENT"; + final static String REMOVE_PREFIX = "REMOVE_PREFIX"; @Override public TreeVisitor getVisitor() { @@ -114,12 +115,7 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty) .visitNonNull(document.getBlock(), ctx, getCursor()) ); - - if (getCursor().getMessage("RemovePrefix", false)) { - return d.withEnd(d.getEnd().withPrefix("")); - } - - return d; + return getCursor().getMessage(REMOVE_PREFIX, false) ? d.withEnd(d.getEnd().withPrefix("")) : d; } Yaml.Document d = super.visitDocument(document, ctx); if (d == document && !getCursor().getMessage(FOUND_MATCHING_ELEMENT, false)) { diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index a50a1443f06..bd6c937782f 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -34,6 +34,7 @@ import java.util.stream.Collectors; import static org.openrewrite.internal.ListUtils.*; +import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @AllArgsConstructor @RequiredArgsConstructor @@ -103,7 +104,7 @@ public Yaml visitMapping(Mapping existingMapping, P p) { if (existing.isScope(existingMapping) && incoming instanceof Mapping) { Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); - if (getCursor().getMessage("RemovePrefix", false)) { + if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Mapping) getCursor().getValue()).getEntries(); return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); } @@ -149,7 +150,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } } // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreakPieces(mergedEntries.get(0), 2)) { + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreak(mergedEntries.get(0), 2)) { return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? autoFormat(it, p, cursor) : it; @@ -187,14 +188,14 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { break; } } - if (comment.isEmpty() && hasLineBreakPieces(entries.get(entries.size() - 1), 1)) { + if (comment.isEmpty() && hasLineBreak(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } } Entry last = mutatedEntries.get(mutatedEntries.size() - 1); mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); - c.putMessage("RemovePrefix", true); + c.putMessage(REMOVE_PREFIX, true); } } @@ -277,9 +278,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreakPieces(Entry entry, int atLeast) { + private boolean hasLineBreak(Entry entry, int atLeastParts) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); - return a && !grabPartLineBreak(entry, atLeast - 1).isEmpty(); + return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); } private String grabPartLineBreak(Entry entry, int index) { From 08d2f01cf9830abfa1396f577d54269ef4fa9783 Mon Sep 17 00:00:00 2001 From: lingenj Date: Mon, 18 Nov 2024 16:23:03 +0100 Subject: [PATCH 46/58] Found a non-working test case... --- .../org/openrewrite/yaml/MergeYamlTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 2c3e0d0053f..4126d052873 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1156,6 +1156,59 @@ void existingEntryBlockWithCommentAllOverThePlace() { ); } + @Test + void fixThisTestAsWell() { + rewriteRun( + spec -> spec.recipe(new MergeYaml( + "$", + //language=yaml + """ + A: + B: + C: + D: + 3: new desc + D2: + 2: new description + D3: + 2: new text + """, + false, + null, + null + )), + yaml( + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else # Some comment 5 + 2: old desc + D2: # Some comment 6 + 1: old description # Some comment 7 + D3: # Some comment 8 + 1: old text # Some comment 9 + """, + """ + A: # Some comment + B: # Some comment 2 + C: # Some comment 3 + D: # Some comment 4 + 1: something else # Some comment 5 + 2: old desc + 3: new desc + D2: # Some comment 6 + 1: old description # Some comment 7 + 2: new description + D3: # Some comment 8 + 1: old text # Some comment 9 + 2: new text + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentAllOverThePlaceWEGG() { From 43fa06a4d0f2006e373c2adf8fc1d54129d8cd25 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 08:52:55 +0100 Subject: [PATCH 47/58] Fix non-working test case... --- .../openrewrite/yaml/MergeYamlVisitor.java | 18 +++-- .../org/openrewrite/yaml/MergeYamlTest.java | 74 +++++-------------- 2 files changed, 28 insertions(+), 64 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index bd6c937782f..76386782bf9 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -175,36 +175,40 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { }); if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { - String comment = ""; + String comment = null; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); } else { List entries = ((Mapping) c.getValue()).getEntries(); + // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(getCursor().getValue())) { comment = grabPartLineBreak(entries.get(i + 1), 0); break; } } - if (comment.isEmpty() && hasLineBreak(entries.get(entries.size() - 1), 1)) { + // or retrieve it for last item from next element (could potentially be much higher in the tree) + if (comment == null && hasLineBreak(entries.get(entries.size() - 1), 1)) { comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); } } - Entry last = mutatedEntries.get(mutatedEntries.size() - 1); - mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); - c.putMessage(REMOVE_PREFIX, true); + if (comment != null) { + Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); + c.putMessage(REMOVE_PREFIX, true); + } } } - removePrefixDirectChildren(m1.getEntries(), mutatedEntries); + removePrefixForDirectChildren(m1.getEntries(), mutatedEntries); return m1.withEntries(mutatedEntries); } - private void removePrefixDirectChildren(List m1Entries, List mutatedEntries) { + private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 4126d052873..87168f724c7 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1116,8 +1116,10 @@ void existingEntryBlockWithCommentAllOverThePlace() { D: 3: new desc D2: - 2: new description + 4: d D3: + 2: new description + D4: 2: new text """, false, @@ -1132,9 +1134,13 @@ void existingEntryBlockWithCommentAllOverThePlace() { D: # Some comment 4 1: something else 2: old desc # Some comment 5 - D2: # Some comment 6 + D2: + 1: a + 2: b + 3: c + D3: # Some comment 6 1: old description # Some comment 7 - D3: # Some comment 8 + D4: # Some comment 8 1: old text # Some comment 9 """, """ @@ -1145,63 +1151,15 @@ void existingEntryBlockWithCommentAllOverThePlace() { 1: something else 2: old desc # Some comment 5 3: new desc - D2: # Some comment 6 - 1: old description # Some comment 7 - 2: new description - D3: # Some comment 8 - 1: old text # Some comment 9 - 2: new text - """ - ) - ); - } - - @Test - void fixThisTestAsWell() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - A: - B: - C: - D: - 3: new desc D2: - 2: new description - D3: - 2: new text - """, - false, - null, - null - )), - yaml( - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 - 1: something else # Some comment 5 - 2: old desc - D2: # Some comment 6 - 1: old description # Some comment 7 - D3: # Some comment 8 - 1: old text # Some comment 9 - """, - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 - 1: something else # Some comment 5 - 2: old desc - 3: new desc - D2: # Some comment 6 + 1: a + 2: b + 3: c + 4: d + D3: # Some comment 6 1: old description # Some comment 7 2: new description - D3: # Some comment 8 + D4: # Some comment 8 1: old text # Some comment 9 2: new text """ @@ -1231,6 +1189,7 @@ void existingEntryBlockWithCommentAllOverThePlaceWEGG() { A: # Some comment B: # Some comment 2 C: # Some comment 3 + BOE: JE 1: old desc # Some comment 4 D: whatever """, @@ -1238,6 +1197,7 @@ void existingEntryBlockWithCommentAllOverThePlaceWEGG() { A: # Some comment B: # Some comment 2 C: # Some comment 3 + BOE: JE 1: old desc # Some comment 4 2: new desc D: whatever From 48d993c824e9df28286f2e5eee9514dbc60fecec Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 08:56:21 +0100 Subject: [PATCH 48/58] Remove test no longer needed --- .../org/openrewrite/yaml/MergeYamlTest.java | 43 +------------------ 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 87168f724c7..655973286fc 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1136,7 +1136,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { 2: old desc # Some comment 5 D2: 1: a - 2: b + 2: b # Some comment 10 3: c D3: # Some comment 6 1: old description # Some comment 7 @@ -1153,7 +1153,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { 3: new desc D2: 1: a - 2: b + 2: b # Some comment 10 3: c 4: d D3: # Some comment 6 @@ -1167,45 +1167,6 @@ void existingEntryBlockWithCommentAllOverThePlace() { ); } - @Issue("https://github.com/openrewrite/rewrite/issues/2218") - @Test - void existingEntryBlockWithCommentAllOverThePlaceWEGG() { - rewriteRun( - spec -> spec.recipe(new MergeYaml( - "$", - //language=yaml - """ - A: - B: - C: - 2: new desc - """, - false, - null, - null - )), - yaml( - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - BOE: JE - 1: old desc # Some comment 4 - D: whatever - """, - """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - BOE: JE - 1: old desc # Some comment 4 - 2: new desc - D: whatever - """ - ) - ); - } - @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test void existingEntryBlockWithCommentNotAtLastLine() { From 019ab7d90b7194818de2b323dd581f69b44d9d22 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 09:21:32 +0100 Subject: [PATCH 49/58] Improvement --- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 76386782bf9..fa030f78bb7 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -40,7 +40,7 @@ @RequiredArgsConstructor public class MergeYamlVisitor

extends YamlVisitor

{ - private final Pattern LINE_BREAK = Pattern.compile("\\R"); + private static final Pattern LINE_BREAK = Pattern.compile("\\R"); private final Yaml existing; private final Yaml incoming; @@ -143,7 +143,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return existingEntry; }); - List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), (i, it) -> { + List mutatedEntries = concatAll(mergedEntries, map(m2.getEntries(), it -> { for (Entry existingEntry : m1.getEntries()) { if (keyMatches(existingEntry, it)) { return null; From 60b60c01c3a817c55be592ee9755f4c28421568c Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 10:44:51 +0100 Subject: [PATCH 50/58] Dont replace comments when cursor is a root level --- rewrite-core/src/main/java/org/openrewrite/Cursor.java | 2 +- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/Cursor.java b/rewrite-core/src/main/java/org/openrewrite/Cursor.java index 3be9d7c15a3..7ed54841f73 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Cursor.java +++ b/rewrite-core/src/main/java/org/openrewrite/Cursor.java @@ -57,7 +57,7 @@ public Cursor getRoot() { /** * @return true if this cursor is the root of the tree, false otherwise */ - final boolean isRoot() { + final public boolean isRoot() { return ROOT_VALUE.equals(value); } diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index fa030f78bb7..9b5f75b543d 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -33,6 +33,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static org.openrewrite.Cursor.ROOT_VALUE; import static org.openrewrite.internal.ListUtils.*; import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @@ -156,9 +157,9 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); - if (m1.getEntries().size() < mutatedEntries.size()) { + if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { Cursor c = getCursor().dropParentUntil(it -> { - if (it instanceof Document) { + if (ROOT_VALUE.equals(it) || it instanceof Document) { return true; } From 324712ced6dd6ab459b5b2e64984842344f6b69b Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 13:51:32 +0100 Subject: [PATCH 51/58] Put Yaml prefixes back --- .../openrewrite/yaml/MergeYamlVisitor.java | 77 ++++++++++--------- .../java/org/openrewrite/yaml/YamlParser.java | 37 +++++---- 2 files changed, 58 insertions(+), 56 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 9b5f75b543d..aee420576e6 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -60,8 +60,8 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean .map(docs -> { // Any comments will have been put on the parent Document node, preserve by copying to the mapping Document doc = docs.getDocuments().get(0); - if (doc.getBlock() instanceof Mapping) { - Mapping m = (Mapping) doc.getBlock(); + if (doc.getBlock() instanceof Yaml.Mapping) { + Yaml.Mapping m = (Yaml.Mapping) doc.getBlock(); return m.withEntries(ListUtils.mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); } else if (doc.getBlock() instanceof Yaml.Sequence) { Yaml.Sequence s = (Yaml.Sequence) doc.getBlock(); @@ -75,9 +75,9 @@ public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean } @Override - public Yaml visitScalar(Scalar existingScalar, P p) { - if (existing.isScope(existingScalar) && incoming instanceof Scalar) { - return mergeScalar(existingScalar, (Scalar) incoming); + public Yaml visitScalar(Yaml.Scalar existingScalar, P p) { + if (existing.isScope(existingScalar) && incoming instanceof Yaml.Scalar) { + return mergeScalar(existingScalar, (Yaml.Scalar) incoming); } return super.visitScalar(existingScalar, p); } @@ -85,7 +85,7 @@ public Yaml visitScalar(Scalar existingScalar, P p) { @Override public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { if (existing.isScope(existingSeq)) { - if (incoming instanceof Mapping) { + if (incoming instanceof Yaml.Mapping) { // Distribute the incoming mapping to each entry in the sequence return existingSeq.withEntries(map(existingSeq.getEntries(), (i, existingSeqEntry) -> existingSeqEntry.withBlock((Yaml.Block) @@ -101,12 +101,12 @@ public Yaml visitSequence(Yaml.Sequence existingSeq, P p) { } @Override - public Yaml visitMapping(Mapping existingMapping, P p) { - if (existing.isScope(existingMapping) && incoming instanceof Mapping) { - Mapping mapping = mergeMapping(existingMapping, (Mapping) incoming, p, getCursor()); + public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { + if (existing.isScope(existingMapping) && incoming instanceof Yaml.Mapping) { + Yaml.Mapping mapping = mergeMapping(existingMapping, (Yaml.Mapping) incoming, p, getCursor()); if (getCursor().getMessage(REMOVE_PREFIX, false)) { - List entries = ((Mapping) getCursor().getValue()).getEntries(); + List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); } @@ -115,25 +115,25 @@ public Yaml visitMapping(Mapping existingMapping, P p) { return super.visitMapping(existingMapping, p); } - private static boolean keyMatches(@Nullable Entry e1, @Nullable Entry e2) { + private static boolean keyMatches(Yaml.Mapping.@Nullable Entry e1, Yaml.Mapping.@Nullable Entry e2) { return e1 != null && e2 != null && e1.getKey().getValue().equals(e2.getKey().getValue()); } - private boolean keyMatches(Mapping m1, Mapping m2) { + private boolean keyMatches(Yaml.Mapping m1, Yaml.Mapping m2) { Optional nameToAdd = m2.getEntries().stream() .filter(e -> objectIdentifyingProperty != null && objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Scalar) e.getValue()).getValue()) + .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) .findAny(); return nameToAdd.map(nameToAddValue -> m1.getEntries().stream() .filter(e -> objectIdentifyingProperty.equals(e.getKey().getValue())) - .map(e -> ((Scalar) e.getValue()).getValue()) + .map(e -> ((Yaml.Scalar) e.getValue()).getValue()) .anyMatch(existingName -> existingName.equals(nameToAddValue))) .orElse(false); } - private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { - List mergedEntries = map(m1.getEntries(), existingEntry -> { + private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor cursor) { + List mergedEntries = map(m1.getEntries(), existingEntry -> { for (Entry incomingEntry : m2.getEntries()) { if (keyMatches(existingEntry, incomingEntry)) { return existingEntry.withValue((Yaml.Block) @@ -150,11 +150,14 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return null; } } - // workaround: autoFormat put sometimes extra spaces before elements - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); + // workaround: autoFormat cannot handle new inserted values very well + if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && MergeYamlVisitor.this.hasLineBreak(mergedEntries.get(0), 2)) { + String prefix = MergeYamlVisitor.this.grabPartLineBreak(mergedEntries.get(0), 1); + String newValue = ((Scalar) it.getValue()).getValue().replaceAll("\\R", prefix + "\n"); + return it.withPrefix("\n" + prefix) + .withValue(((Scalar) it.getValue()).withValue(newValue)); } - return shouldAutoFormat ? autoFormat(it, p, cursor) : it; + return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { @@ -163,8 +166,8 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return true; } - if (it instanceof Mapping) { - List entries = ((Mapping) it).getEntries(); + if (it instanceof Yaml.Mapping) { + List entries = ((Yaml.Mapping) it).getEntries(); // last member should search further upwards until two entries are found if (entries.get(entries.size() - 1).equals(getCursor().getParentOrThrow().getValue())) { return false; @@ -175,13 +178,13 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return false; }); - if (c.getValue() instanceof Document || c.getValue() instanceof Mapping) { + if (c.getValue() instanceof Document || c.getValue() instanceof Yaml.Mapping) { String comment = null; if (c.getValue() instanceof Document) { comment = ((Document) c.getValue()).getEnd().getPrefix(); } else { - List entries = ((Mapping) c.getValue()).getEntries(); + List entries = ((Yaml.Mapping) c.getValue()).getEntries(); // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { @@ -197,7 +200,7 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { } if (comment != null) { - Entry last = mutatedEntries.get(mutatedEntries.size() - 1); + Yaml.Mapping.Entry last = mutatedEntries.get(mutatedEntries.size() - 1); mutatedEntries.set(mutatedEntries.size() - 1, last.withPrefix(comment + last.getPrefix())); c.putMessage(REMOVE_PREFIX, true); } @@ -209,10 +212,10 @@ private Mapping mergeMapping(Mapping m1, Mapping m2, P p, Cursor cursor) { return m1.withEntries(mutatedEntries); } - private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { + private void removePrefixForDirectChildren(List m1Entries, List mutatedEntries) { for (int i = 0; i < m1Entries.size() - 1; i++) { - if (m1Entries.get(i).getValue() instanceof Mapping && mutatedEntries.get(i).getValue() instanceof Mapping && - ((Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { + if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && + ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); } } @@ -223,17 +226,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } - boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Scalar); + boolean isSequenceOfScalars = s2.getEntries().stream().allMatch(entry -> entry.getBlock() instanceof Yaml.Scalar); if (isSequenceOfScalars) { List incomingEntries = new ArrayList<>(s2.getEntries()); nextEntry: for (Yaml.Sequence.Entry entry : s1.getEntries()) { - if (entry.getBlock() instanceof Scalar) { - String existingScalar = ((Scalar) entry.getBlock()).getValue(); + if (entry.getBlock() instanceof Yaml.Scalar) { + String existingScalar = ((Yaml.Scalar) entry.getBlock()).getValue(); for (Yaml.Sequence.Entry incomingEntry : incomingEntries) { - if (((Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { + if (((Yaml.Scalar) incomingEntry.getBlock()).getValue().equals(existingScalar)) { incomingEntries.remove(incomingEntry); continue nextEntry; } @@ -248,9 +251,9 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur return s1; } else { List mutatedEntries = map(s2.getEntries(), entry -> { - Mapping incomingMapping = (Mapping) entry.getBlock(); + Yaml.Mapping incomingMapping = (Yaml.Mapping) entry.getBlock(); for (Yaml.Sequence.Entry existingEntry : s1.getEntries()) { - Mapping existingMapping = (Mapping) existingEntry.getBlock(); + Yaml.Mapping existingMapping = (Yaml.Mapping) existingEntry.getBlock(); if (keyMatches(existingMapping, incomingMapping)) { Yaml.Sequence.Entry e1 = existingEntry.withBlock(mergeMapping(existingMapping, incomingMapping, p, cursor)); if (e1 == existingEntry) { @@ -283,17 +286,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } } - private boolean hasLineBreak(Entry entry, int atLeastParts) { + private boolean hasLineBreak(Yaml.Mapping.Entry entry, int atLeastParts) { boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); } - private String grabPartLineBreak(Entry entry, int index) { + private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { String[] parts = LINE_BREAK.split(entry.getPrefix()); return parts.length > index ? parts[index] : ""; } - private Scalar mergeScalar(Scalar y1, Scalar y2) { + private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { String s1 = y1.getValue(); String s2 = y2.getValue(); return !s1.equals(s2) && !acceptTheirs ? y1.withValue(s2) : y1; diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java index 3ec08feb969..ca7685f37ba 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/YamlParser.java @@ -50,7 +50,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.openrewrite.Tree.randomId; -import static org.openrewrite.marker.Markers.EMPTY; public class YamlParser implements org.openrewrite.Parser { private static final Pattern VARIABLE_PATTERN = Pattern.compile(":\\s*(@[^\n\r@]+@)"); @@ -83,9 +82,9 @@ public Stream parseInputs(Iterable sourceFiles, @Nullable Pat Yaml.Documents docs = (Yaml.Documents) sourceFile; // ensure there is always at least one Document, even in an empty yaml file if (docs.getDocuments().isEmpty()) { - Yaml.Document.End end = new Yaml.Document.End(randomId(), "", EMPTY, false); - Yaml.Mapping mapping = new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null); - return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", EMPTY, false, mapping, end))); + Yaml.Document.End end = new Yaml.Document.End(randomId(), "", Markers.EMPTY, false); + Yaml.Mapping mapping = new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null); + return docs.withDocuments(singletonList(new Yaml.Document(randomId(), "", Markers.EMPTY, false, mapping, end))); } return docs; } @@ -144,7 +143,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr documents.add(document.withEnd(new Yaml.Document.End( randomId(), fmt, - EMPTY, + Markers.EMPTY, ((DocumentEndEvent) event).getExplicit() ))); lastEnd = event.getEndMark().getIndex(); @@ -156,10 +155,10 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr document = new Yaml.Document( randomId(), fmt, - EMPTY, + Markers.EMPTY, ((DocumentStartEvent) event).getExplicit(), - new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", EMPTY, false) + new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) ); lastEnd = event.getEndMark().getIndex(); break; @@ -256,7 +255,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } break; } - Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, EMPTY, style, anchor, scalarValue); + Yaml.Scalar finalScalar = new Yaml.Scalar(randomId(), fmt, Markers.EMPTY, style, anchor, scalarValue); BlockBuilder builder = blockStack.isEmpty() ? null : blockStack.peek(); if (builder instanceof SequenceBuilder) { // Inline sequences like [1, 2] need to keep track of any whitespace between the element @@ -353,7 +352,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr throw new UnsupportedOperationException("Unknown anchor: " + alias.getAnchor()); } BlockBuilder builder = blockStack.peek(); - builder.push(new Yaml.Alias(randomId(), fmt, EMPTY, anchor)); + builder.push(new Yaml.Alias(randomId(), fmt, Markers.EMPTY, anchor)); lastEnd = event.getEndMark().getIndex(); break; } @@ -362,9 +361,9 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr if (document == null && !fmt.isEmpty()) { documents.add( new Yaml.Document( - randomId(), fmt, EMPTY, false, - new Yaml.Mapping(randomId(), EMPTY, null, emptyList(), null, null), - new Yaml.Document.End(randomId(), "", EMPTY, false) + randomId(), fmt, Markers.EMPTY, false, + new Yaml.Mapping(randomId(), Markers.EMPTY, null, emptyList(), null, null), + new Yaml.Document.End(randomId(), "", Markers.EMPTY, false) )); } break; @@ -374,7 +373,7 @@ private Yaml.Documents parseFromInput(Path sourceFile, EncodingDetectingInputStr } } - return new Yaml.Documents(randomId(), EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); + return new Yaml.Documents(randomId(), Markers.EMPTY, sourceFile, FileAttributes.fromPath(sourceFile), source.getCharset().name(), source.isCharsetBomMarked(), null, documents); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -410,7 +409,7 @@ private Yaml.Anchor buildYamlAnchor(FormatPreservingReader reader, int lastEnd, if (!isForScalar) { prefix = (prefixStart > -1 && eventPrefix.length() > prefixStart + 1) ? eventPrefix.substring(prefixStart + 1) : ""; } - return new Yaml.Anchor(randomId(), prefix, postFix.toString(), EMPTY, anchorKey); + return new Yaml.Anchor(randomId(), prefix, postFix.toString(), Markers.EMPTY, anchorKey); } private static int commentAwareIndexOf(char target, String s) { @@ -492,7 +491,7 @@ public void push(Yaml.Block block) { String entryPrefix = originalKeyPrefix.substring(entryPrefixStartIndex); String beforeMappingValueIndicator = keySuffix.substring(0, Math.max(commentAwareIndexOf(':', keySuffix), 0)); - entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, EMPTY, key, beforeMappingValueIndicator, block)); + entries.add(new Yaml.Mapping.Entry(randomId(), entryPrefix, Markers.EMPTY, key, beforeMappingValueIndicator, block)); key = null; } } @@ -538,7 +537,7 @@ public void push(Yaml.Block block, @Nullable String commaPrefix) { entryPrefix = ""; blockPrefix = rawPrefix; } - entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); + entries.add(new Yaml.Sequence.Entry(randomId(), entryPrefix, Markers.EMPTY, block.withPrefix(blockPrefix), hasDash, commaPrefix)); } @Override @@ -568,7 +567,7 @@ private static class MappingWithPrefix extends Yaml.Mapping { private String prefix; public MappingWithPrefix(String prefix, @Nullable String startBracePrefix, List entries, @Nullable String endBracePrefix, @Nullable Anchor anchor) { - super(randomId(), EMPTY, startBracePrefix, entries, endBracePrefix, anchor); + super(randomId(), Markers.EMPTY, startBracePrefix, entries, endBracePrefix, anchor); this.prefix = prefix; } @@ -584,7 +583,7 @@ private static class SequenceWithPrefix extends Yaml.Sequence { private String prefix; public SequenceWithPrefix(String prefix, @Nullable String startBracketPrefix, List entries, @Nullable String endBracketPrefix, @Nullable Anchor anchor) { - super(randomId(), EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); + super(randomId(), Markers.EMPTY, startBracketPrefix, entries, endBracketPrefix, anchor); this.prefix = prefix; } From 93d6bf922e8c45cbac7cbeb8ab289d5f7418016d Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 14:18:34 +0100 Subject: [PATCH 52/58] More comment, more difficult --- .../openrewrite/yaml/MergeYamlVisitor.java | 7 +-- .../org/openrewrite/yaml/MergeYamlTest.java | 52 ++++++++++--------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index aee420576e6..8f4c0a21ead 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -151,11 +151,8 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } } // workaround: autoFormat cannot handle new inserted values very well - if (!mergedEntries.isEmpty() && it.getValue() instanceof Scalar && MergeYamlVisitor.this.hasLineBreak(mergedEntries.get(0), 2)) { - String prefix = MergeYamlVisitor.this.grabPartLineBreak(mergedEntries.get(0), 1); - String newValue = ((Scalar) it.getValue()).getValue().replaceAll("\\R", prefix + "\n"); - return it.withPrefix("\n" + prefix) - .withValue(((Scalar) it.getValue()).withValue(newValue)); + if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { + return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 655973286fc..03b3cb0ca1b 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1036,7 +1036,7 @@ void existingEntryBlockWithCommentAtFirstLine() { )), yaml( """ - A: # Some comment + A: # Comment untouched B: C: D: @@ -1049,7 +1049,7 @@ void existingEntryBlockWithCommentAtFirstLine() { 1: old text """, """ - A: # Some comment + A: # Comment untouched B: C: D: @@ -1090,12 +1090,12 @@ void existingEntryBlockWithCommentAtLastLine() { """ spring: application: - name: main # some comment + name: main # Comment moved from root to previous element """, """ spring: application: - name: main # some comment + name: main # Comment moved from root to previous element description: a description """ ) @@ -1128,39 +1128,43 @@ void existingEntryBlockWithCommentAllOverThePlace() { )), yaml( """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 + A: # Comment untouched 1 + B: # Comment untouched 2 + C: # Comment untouched 3 + D: # Comment untouched 4 1: something else - 2: old desc # Some comment 5 + 2: old desc # Comment moved from next block to previous element 1 D2: 1: a - 2: b # Some comment 10 + # Above comment untouched 1 + 2: b # Comment untouched 5 3: c - D3: # Some comment 6 - 1: old description # Some comment 7 - D4: # Some comment 8 - 1: old text # Some comment 9 + # Above comment untouched 2 + D3: # Comment untouched 6 + 1: old description # Comment moved from next block to previous element 2 + D4: # Comment untouched 7 + 1: old text # Comment moved from root to previous element """, """ - A: # Some comment - B: # Some comment 2 - C: # Some comment 3 - D: # Some comment 4 + A: # Comment untouched 1 + B: # Comment untouched 2 + C: # Comment untouched 3 + D: # Comment untouched 4 1: something else - 2: old desc # Some comment 5 + 2: old desc # Comment moved from next block to previous element 1 3: new desc D2: 1: a - 2: b # Some comment 10 + # Above comment untouched 1 + 2: b # Comment untouched 5 3: c 4: d - D3: # Some comment 6 - 1: old description # Some comment 7 + # Above comment untouched 2 + D3: # Comment untouched 6 + 1: old description # Comment moved from next block to previous element 2 2: new description - D4: # Some comment 8 - 1: old text # Some comment 9 + D4: # Comment untouched 7 + 1: old text # Comment moved from root to previous element 2: new text """ ) From 57fe8268010780f95926f7a1200db35249415128 Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:30:13 +0100 Subject: [PATCH 53/58] Improvement --- .../openrewrite/yaml/MergeYamlVisitor.java | 23 +++++++++++-------- .../org/openrewrite/yaml/MergeYamlTest.java | 14 +++++------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 8f4c0a21ead..d54c7f98e1c 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -28,6 +28,7 @@ import org.openrewrite.yaml.tree.Yaml.Scalar; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Optional; import java.util.regex.Pattern; @@ -107,7 +108,7 @@ public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); - return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabPartLineBreak(entries.get(entries.size() - 1), 1)))); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); } return mapping; @@ -152,7 +153,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } // workaround: autoFormat cannot handle new inserted values very well if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabPartLineBreak(mergedEntries.get(0), 1)); + return it.withPrefix("\n" + grabAfterFirstLineBreak(mergedEntries.get(0))); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); @@ -186,13 +187,13 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso // get comment from next element in same mapping block for (int i = 0; i < entries.size() - 1; i++) { if (entries.get(i).getValue().equals(getCursor().getValue())) { - comment = grabPartLineBreak(entries.get(i + 1), 0); + comment = grabBeforeFirstLineBreak(entries.get(i + 1)); break; } } // or retrieve it for last item from next element (could potentially be much higher in the tree) if (comment == null && hasLineBreak(entries.get(entries.size() - 1), 1)) { - comment = grabPartLineBreak(entries.get(entries.size() - 1), 0); + comment = grabBeforeFirstLineBreak(entries.get(entries.size() - 1)); } } @@ -213,7 +214,7 @@ private void removePrefixForDirectChildren(List m1Entries, L for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabPartLineBreak(mutatedEntries.get(i + 1), 1))); + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); } } } @@ -284,13 +285,17 @@ private Yaml.Sequence mergeSequence(Yaml.Sequence s1, Yaml.Sequence s2, P p, Cur } private boolean hasLineBreak(Yaml.Mapping.Entry entry, int atLeastParts) { - boolean a = LINE_BREAK.matcher(entry.getPrefix()).find(); - return a && !grabPartLineBreak(entry, atLeastParts - 1).isEmpty(); + return LINE_BREAK.matcher(entry.getPrefix()).find() && LINE_BREAK.split(entry.getPrefix()).length >= atLeastParts; } - private String grabPartLineBreak(Yaml.Mapping.Entry entry, int index) { + private String grabBeforeFirstLineBreak(Yaml.Mapping.Entry entry) { String[] parts = LINE_BREAK.split(entry.getPrefix()); - return parts.length > index ? parts[index] : ""; + return parts.length > 0 ? parts[0] : ""; + } + + private String grabAfterFirstLineBreak(Yaml.Mapping.Entry entry) { + String[] parts = LINE_BREAK.split(entry.getPrefix()); + return parts.length > 1 ? String.join("\n", Arrays.copyOfRange(parts, 1, parts.length)) : ""; } private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index 03b3cb0ca1b..df4ba6bbead 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1104,7 +1104,7 @@ void existingEntryBlockWithCommentAtLastLine() { @Issue("https://github.com/openrewrite/rewrite/issues/2218") @Test - void existingEntryBlockWithCommentAllOverThePlace() { + void existingEntryBlockWithCommentsAllOverThePlace() { rewriteRun( spec -> spec.recipe(new MergeYaml( "$", @@ -1133,7 +1133,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { C: # Comment untouched 3 D: # Comment untouched 4 1: something else - 2: old desc # Comment moved from next block to previous element 1 + 2: old desc # Comment moved from prefix D2 to prefix D->3 D2: 1: a # Above comment untouched 1 @@ -1141,9 +1141,9 @@ void existingEntryBlockWithCommentAllOverThePlace() { 3: c # Above comment untouched 2 D3: # Comment untouched 6 - 1: old description # Comment moved from next block to previous element 2 + 1: old description # Comment moved from prefix D4 to prefix D3->2 D4: # Comment untouched 7 - 1: old text # Comment moved from root to previous element + 1: old text # Comment moved from end document to prefix D4->2 """, """ A: # Comment untouched 1 @@ -1151,7 +1151,7 @@ void existingEntryBlockWithCommentAllOverThePlace() { C: # Comment untouched 3 D: # Comment untouched 4 1: something else - 2: old desc # Comment moved from next block to previous element 1 + 2: old desc # Comment moved from prefix D2 to prefix D->3 3: new desc D2: 1: a @@ -1161,10 +1161,10 @@ void existingEntryBlockWithCommentAllOverThePlace() { 4: d # Above comment untouched 2 D3: # Comment untouched 6 - 1: old description # Comment moved from next block to previous element 2 + 1: old description # Comment moved from prefix D4 to prefix D3->2 2: new description D4: # Comment untouched 7 - 1: old text # Comment moved from root to previous element + 1: old text # Comment moved from end document to prefix D4->2 2: new text """ ) From 1067c48e5bb1d1fb1f80db7eef4bce3d828e1d1a Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:45:28 +0100 Subject: [PATCH 54/58] Improvement of test --- .../org/openrewrite/yaml/MergeYamlTest.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java index df4ba6bbead..0e6caad72d6 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -1134,14 +1134,16 @@ void existingEntryBlockWithCommentsAllOverThePlace() { D: # Comment untouched 4 1: something else 2: old desc # Comment moved from prefix D2 to prefix D->3 + # This is also part of prefix D2, but should NOT be moved to D->3 D2: 1: a - # Above comment untouched 1 - 2: b # Comment untouched 5 + # Comment above tag untouched 1 + 2: b # Comment with a lot of spaces untouched 5 3: c - # Above comment untouched 2 + # Comment above tag untouched 2 + # with multilines D3: # Comment untouched 6 - 1: old description # Comment moved from prefix D4 to prefix D3->2 + 1: old description # Comment with a lot of spaces moved from prefix D4 to prefix D3->2 D4: # Comment untouched 7 1: old text # Comment moved from end document to prefix D4->2 """, @@ -1153,15 +1155,17 @@ void existingEntryBlockWithCommentsAllOverThePlace() { 1: something else 2: old desc # Comment moved from prefix D2 to prefix D->3 3: new desc + # This is also part of prefix D2, but should NOT be moved to D->3 D2: 1: a - # Above comment untouched 1 - 2: b # Comment untouched 5 + # Comment above tag untouched 1 + 2: b # Comment with a lot of spaces untouched 5 3: c 4: d - # Above comment untouched 2 + # Comment above tag untouched 2 + # with multilines D3: # Comment untouched 6 - 1: old description # Comment moved from prefix D4 to prefix D3->2 + 1: old description # Comment with a lot of spaces moved from prefix D4 to prefix D3->2 2: new description D4: # Comment untouched 7 1: old text # Comment moved from end document to prefix D4->2 From d21afa94fcb41cb9cfcc1b51de7419c17cc447ab Mon Sep 17 00:00:00 2001 From: lingenj Date: Tue, 19 Nov 2024 15:53:33 +0100 Subject: [PATCH 55/58] Use System.lineSeparator() --- .../main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index d54c7f98e1c..67b62bdb333 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -34,6 +34,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; +import static java.lang.System.lineSeparator; import static org.openrewrite.Cursor.ROOT_VALUE; import static org.openrewrite.internal.ListUtils.*; import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; @@ -108,7 +109,7 @@ public Yaml visitMapping(Yaml.Mapping existingMapping, P p) { if (getCursor().getMessage(REMOVE_PREFIX, false)) { List entries = ((Yaml.Mapping) getCursor().getValue()).getEntries(); - return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix("\n" + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); + return mapping.withEntries(mapLast(mapping.getEntries(), it -> it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(entries.get(entries.size() - 1))))); } return mapping; @@ -153,7 +154,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso } // workaround: autoFormat cannot handle new inserted values very well if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { - return it.withPrefix("\n" + grabAfterFirstLineBreak(mergedEntries.get(0))); + return it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(mergedEntries.get(0))); } return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; })); @@ -214,7 +215,7 @@ private void removePrefixForDirectChildren(List m1Entries, L for (int i = 0; i < m1Entries.size() - 1; i++) { if (m1Entries.get(i).getValue() instanceof Yaml.Mapping && mutatedEntries.get(i).getValue() instanceof Yaml.Mapping && ((Yaml.Mapping) m1Entries.get(i).getValue()).getEntries().size() < ((Yaml.Mapping) mutatedEntries.get(i).getValue()).getEntries().size()) { - mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix("\n" + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); + mutatedEntries.set(i + 1, mutatedEntries.get(i + 1).withPrefix(lineSeparator() + grabAfterFirstLineBreak(mutatedEntries.get(i + 1)))); } } } @@ -295,7 +296,7 @@ private String grabBeforeFirstLineBreak(Yaml.Mapping.Entry entry) { private String grabAfterFirstLineBreak(Yaml.Mapping.Entry entry) { String[] parts = LINE_BREAK.split(entry.getPrefix()); - return parts.length > 1 ? String.join("\n", Arrays.copyOfRange(parts, 1, parts.length)) : ""; + return parts.length > 1 ? String.join(lineSeparator(), Arrays.copyOfRange(parts, 1, parts.length)) : ""; } private Scalar mergeScalar(Yaml.Scalar y1, Yaml.Scalar y2) { From 35948c795a9feaed3c5a78e0e77fe81fa3c506f9 Mon Sep 17 00:00:00 2001 From: lingenj Date: Wed, 20 Nov 2024 09:17:39 +0100 Subject: [PATCH 56/58] improvement --- .../src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 67b62bdb333..142e09876d4 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -156,7 +156,7 @@ private Mapping mergeMapping(Yaml.Mapping m1, Yaml.Mapping m2, P p, Cursor curso if (!mergedEntries.isEmpty() && it.getValue() instanceof Yaml.Scalar && hasLineBreak(mergedEntries.get(0), 2)) { return it.withPrefix(lineSeparator() + grabAfterFirstLineBreak(mergedEntries.get(0))); } - return shouldAutoFormat ? MergeYamlVisitor.this.autoFormat(it, p, cursor) : it; + return shouldAutoFormat ? autoFormat(it, p, cursor) : it; })); if (m1.getEntries().size() < mutatedEntries.size() && !getCursor().isRoot()) { From 7454f5ea39a8af6a587d75ce4b7c690715e78e9a Mon Sep 17 00:00:00 2001 From: Sam Snyder Date: Fri, 22 Nov 2024 00:46:15 -0800 Subject: [PATCH 57/58] Update rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java Co-authored-by: Jacob van Lingen --- .../java/org/openrewrite/yaml/MergeYamlVisitor.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java index 142e09876d4..0ae6d731f0c 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -38,7 +38,18 @@ import static org.openrewrite.Cursor.ROOT_VALUE; import static org.openrewrite.internal.ListUtils.*; import static org.openrewrite.yaml.MergeYaml.REMOVE_PREFIX; - +/** + * Visitor class to merge two yaml files. + * + * @implNote Loops recursively through the documents, for every part a new MergeYamlVisitor instance will be created. + * As inline comments are put on the prefix of the next element (which can be an item very much higher in the tree), + * the following solutions are chosen to merge the comments as well: + *