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 2a194544946..bd5e90a7408 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYaml.java @@ -15,23 +15,24 @@ */ package org.openrewrite.yaml; -import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.NoArgsConstructor; import lombok.Value; +import lombok.experimental.NonFinal; import org.intellij.lang.annotations.Language; import org.jspecify.annotations.Nullable; import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.yaml.tree.Yaml; +import java.util.Optional; + import static org.openrewrite.internal.StringUtils.isBlank; import static org.openrewrite.yaml.MergeYaml.InsertMode.*; @Value @EqualsAndHashCode(callSuper = false) @NoArgsConstructor(force = true) // TODO: remove with @deprecated constructor -@AllArgsConstructor // TODO: remove with @deprecated constructor public class MergeYaml extends Recipe { @Option(displayName = "Key path", description = "A [JsonPath](https://docs.openrewrite.org/reference/jsonpath-and-jsonpathmatcher-reference) expression used to find matching keys.", @@ -87,16 +88,36 @@ public MergeYaml(String key, @Language("yml") String yaml, @Nullable Boolean acc this(key, yaml, acceptTheirs, objectIdentifyingProperty, filePattern, null, insertProperty); } + public MergeYaml(String key, @Language("yml") String yaml, @Nullable Boolean acceptTheirs, @Nullable String objectIdentifyingProperty, @Nullable String filePattern, @Nullable InsertMode insertMode, @Nullable String insertProperty) { + this.key = key; + this.yaml = yaml; + this.acceptTheirs = acceptTheirs; + this.objectIdentifyingProperty = objectIdentifyingProperty; + this.filePattern = filePattern; + this.insertMode = insertMode; + this.insertProperty = insertProperty; + } + public enum InsertMode { Before, After, Last } + @Nullable + @NonFinal + transient Yaml incoming = null; + @Override public Validated validate() { + // parse here return super.validate() .and(Validated.test("yaml", "Must be valid YAML", - yaml, y -> new YamlParser().parse(yaml) - .findFirst() - .map(doc -> !((Yaml.Documents) doc).getDocuments().isEmpty()) - .orElse(false))) + yaml, y -> { + Optional firstDocument = MergeYaml.maybeParse(yaml); + if (firstDocument.isPresent()) { + incoming = MergeYaml.parse(yaml); + return true; + } else { + return false; + } + })) .and(Validated.test("insertProperty", "Insert property must be filed when `insert mode` is either `BeforeProperty` or `AfterProperty`.", insertProperty, s -> insertMode == null || insertMode == Last || !isBlank(s))); } @@ -120,31 +141,18 @@ public String getDescription() { @Override public TreeVisitor getVisitor() { + //assert incoming != null; // silence compiler warning, incoming cannot be null here. Otherwise, validate() would have failed. return Preconditions.check(new FindSourceFiles(filePattern), new YamlIsoVisitor() { final JsonPathMatcher matcher = new JsonPathMatcher(key); - final Yaml incoming = new YamlParser().parse(yaml) - .findFirst() - .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(); - 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(); - return s.withEntries(ListUtils.mapFirst(s.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); - } - return doc.getBlock().withPrefix(doc.getPrefix()); - }) - .orElseThrow(() -> new IllegalArgumentException("Could not parse as YAML")); - @Override public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) { + if (incoming == null) { + return super.visitDocument(document, ctx); + } if ("$".equals(key)) { Yaml.Document d = document.withBlock((Yaml.Block) - new MergeYamlVisitor<>(document.getBlock(), yaml, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty, insertMode, insertProperty) + new MergeYamlVisitor<>(document.getBlock(), incoming, Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty, insertMode, insertProperty) .visitNonNull(document.getBlock(), ctx, getCursor()) ); if (getCursor().getMessage(REMOVE_PREFIX, false)) { @@ -168,7 +176,7 @@ public Yaml.Document visitDocument(Yaml.Document document, ExecutionContext ctx) } // No matching element already exists, so it must be constructed //noinspection LanguageMismatch - return d.withBlock((Yaml.Block) new MergeYamlVisitor<>(d.getBlock(), snippet, + return d.withBlock((Yaml.Block) new MergeYamlVisitor<>(d.getBlock(), MergeYaml.parse(snippet), Boolean.TRUE.equals(acceptTheirs), objectIdentifyingProperty, insertMode, insertProperty).visitNonNull(d.getBlock(), ctx, getCursor())); } @@ -210,6 +218,9 @@ private String indent(String text) { @Override public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) { + if (incoming == null) { + return super.visitMapping(mapping, ctx); + } Yaml.Mapping m = super.visitMapping(mapping, ctx); if (matcher.matches(getCursor())) { getCursor().putMessageOnFirstEnclosing(Yaml.Document.class, FOUND_MATCHING_ELEMENT, true); @@ -221,6 +232,9 @@ public Yaml.Mapping visitMapping(Yaml.Mapping mapping, ExecutionContext ctx) { @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { + if (incoming == null) { + return super.visitMappingEntry(entry, ctx); + } if (matcher.matches(getCursor())) { getCursor().putMessageOnFirstEnclosing(Yaml.Document.class, FOUND_MATCHING_ELEMENT, true); Yaml.Block value = (Yaml.Block) new MergeYamlVisitor<>(entry.getValue(), incoming, @@ -236,6 +250,9 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC @Override public Yaml.Sequence visitSequence(Yaml.Sequence sequence, ExecutionContext ctx) { + if (incoming == null) { + return super.visitSequence(sequence, ctx); + } if (matcher.matches(getCursor().getParentOrThrow())) { getCursor().putMessageOnFirstEnclosing(Yaml.Document.class, FOUND_MATCHING_ELEMENT, true); return sequence.withEntries(ListUtils.map(sequence.getEntries(), @@ -247,4 +264,28 @@ public Yaml.Sequence visitSequence(Yaml.Sequence sequence, ExecutionContext ctx) } }); } + + static Optional maybeParse(@Language("yml") String yaml) { + return new YamlParser().parse(yaml) + .findFirst() + .filter(Yaml.Documents.class::isInstance) + .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(); + 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(); + return s.withEntries(ListUtils.mapFirst(s.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); + } + return doc.getBlock().withPrefix(doc.getPrefix()); + }); + } + + static Yaml parse(@Language("yml") String yaml) { + return maybeParse(yaml) + .orElseThrow(() -> new IllegalArgumentException("Could not parse as YAML")); + } } 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 86767d2e352..68a6c2a6752 100644 --- a/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java +++ b/rewrite-yaml/src/main/java/org/openrewrite/yaml/MergeYamlVisitor.java @@ -89,28 +89,12 @@ public MergeYamlVisitor(Yaml.Block block, Yaml incoming, boolean acceptTheirs, @ this.shouldAutoFormat = shouldAutoFormat; } + /** + * @deprecated Use {@link #MergeYamlVisitor(Yaml.Block, Yaml, boolean, String, boolean, InsertMode, String)} instead. + */ + @Deprecated public MergeYamlVisitor(Yaml scope, @Language("yml") String yamlString, boolean acceptTheirs, @Nullable String objectIdentifyingProperty, @Nullable InsertMode insertMode, @Nullable String insertProperty) { - this(scope, - new YamlParser().parse(yamlString) - .findFirst() - .map(Yaml.Documents.class::cast) - .map(docs -> { - // Any comments will have been put on the parent Yaml.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(); - return m.withEntries(mapFirst(m.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); - } else if (doc.getBlock() instanceof Yaml.Sequence) { - Yaml.Sequence s = (Yaml.Sequence) doc.getBlock(); - return s.withEntries(mapFirst(s.getEntries(), entry -> entry.withPrefix(doc.getPrefix()))); - } - return doc.getBlock().withPrefix(doc.getPrefix()); - }) - .orElseThrow(() -> new IllegalArgumentException("Could not parse as YAML")), - acceptTheirs, - objectIdentifyingProperty, - insertMode, - insertProperty); + this(scope, MergeYaml.parse(yamlString), acceptTheirs, objectIdentifyingProperty, insertMode, insertProperty); } @Override 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 2c0c455bf5a..a3ea08ccdc4 100644 --- a/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java +++ b/rewrite-yaml/src/test/java/org/openrewrite/yaml/MergeYamlTest.java @@ -20,6 +20,7 @@ import org.openrewrite.Issue; import org.openrewrite.test.RewriteTest; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.openrewrite.yaml.Assertions.yaml; import static org.openrewrite.yaml.MergeYaml.InsertMode.After; import static org.openrewrite.yaml.MergeYaml.InsertMode.Before; @@ -2820,4 +2821,23 @@ void preventKeysToBeAppendedToPreviousCommentIfManyLineBreaks() { ) ); } + + @Test + void invalidYaml() { + assertThrows(AssertionError.class, () -> rewriteRun( + spec -> spec + .recipe(new MergeYaml( + "$.some.object", + // language=yaml + """ + script: |-ParseError + """, + false, + "name", + null, + null, + null + )) + )); + } }