diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java index 8128be7cec1..340f1865aec 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddProfile.java @@ -101,7 +101,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (maybeProfile.isPresent()) { Xml.Tag profile = maybeProfile.get(); - t = (Xml.Tag) new RemoveContentVisitor(profile, false).visitNonNull(t, ctx, getCursor().getParentOrThrow()); + t = (Xml.Tag) new RemoveContentVisitor(profile, false, false).visitNonNull(t, ctx, getCursor().getParentOrThrow()); } Xml.Tag profileTag = Xml.Tag.build("\n" + diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/AddRepository.java b/rewrite-maven/src/main/java/org/openrewrite/maven/AddRepository.java index e42ac01d111..b18f9802db2 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/AddRepository.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/AddRepository.java @@ -170,7 +170,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { repositories = (Xml.Tag) new AddToTagVisitor<>(repo, Xml.Tag.build(assembleReleases())) .visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); } else { - repositories = (Xml.Tag) new RemoveContentVisitor<>(releases, true) + repositories = (Xml.Tag) new RemoveContentVisitor<>(releases, true, true) .visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); if (!isNoSnapshots()) { repositories = (Xml.Tag) new AddToTagVisitor<>(repo, Xml.Tag.build(assembleReleases())) @@ -184,7 +184,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (snapshots == null) { repositories = (Xml.Tag) new AddToTagVisitor<>(repo, Xml.Tag.build(assembleSnapshots())).visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); } else { - repositories = (Xml.Tag) new RemoveContentVisitor<>(snapshots, true).visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); + repositories = (Xml.Tag) new RemoveContentVisitor<>(snapshots, true, true).visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); if (!isNoSnapshots()) { repositories = (Xml.Tag) new AddToTagVisitor<>(repo, Xml.Tag.build(assembleSnapshots())).visitNonNull(repositories, ctx, getCursor().getParentOrThrow()); } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyClassifier.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyClassifier.java index 5f1afc788b1..68ee12c5f3a 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyClassifier.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyClassifier.java @@ -86,7 +86,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { Optional classifier = tag.getChild("classifier"); if (classifier.isPresent()) { if (newClassifier == null) { - doAfterVisit(new RemoveContentVisitor<>(classifier.get(), false)); + doAfterVisit(new RemoveContentVisitor<>(classifier.get(), false, true)); } else if (!newClassifier.equals(classifier.get().getValue().orElse(null))) { doAfterVisit(new ChangeTagValueVisitor<>(classifier.get(), newClassifier)); } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java index c0cb645d7c0..6b3b61157b4 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyGroupIdAndArtifactId.java @@ -200,7 +200,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { if (versionTagPresent) { // If the previous dependency had a version but the new artifact is managed, removed the version tag. if (!configuredToOverrideManageVersion && newDependencyManaged || (oldDependencyManaged && configuredToChangeManagedDependency)) { - t = (Xml.Tag) new RemoveContentVisitor<>(versionTag.get(), false).visit(t, ctx); + t = (Xml.Tag) new RemoveContentVisitor<>(versionTag.get(), false, true).visit(t, ctx); } else { // Otherwise, change the version to the new value. t = changeChildTagValue(t, "version", resolvedNewVersion, ctx); diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyScope.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyScope.java index 31fe5c32a95..db148d7507c 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyScope.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ChangeDependencyScope.java @@ -81,7 +81,7 @@ public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { Optional scope = tag.getChild("scope"); if (scope.isPresent()) { if (newScope == null) { - doAfterVisit(new RemoveContentVisitor<>(scope.get(), false)); + doAfterVisit(new RemoveContentVisitor<>(scope.get(), false, true)); } else if (!newScope.equals(scope.get().getValue().orElse(null))) { doAfterVisit(new ChangeTagValueVisitor<>(scope.get(), newScope)); } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/ManageDependencies.java b/rewrite-maven/src/main/java/org/openrewrite/maven/ManageDependencies.java index 1cf72265d5d..b9eaed6df3a 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/ManageDependencies.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/ManageDependencies.java @@ -169,7 +169,7 @@ public RemoveVersionTagVisitor(String groupPattern, String artifactPattern) { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (isDependencyTag() && isDependencyTag(groupPattern, artifactPattern)) { - tag.getChild("version").ifPresent(versionTag -> doAfterVisit(new RemoveContentVisitor<>(versionTag, false))); + tag.getChild("version").ifPresent(versionTag -> doAfterVisit(new RemoveContentVisitor<>(versionTag, false, true))); return tag; } return super.visitTag(tag, ctx); diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveDependency.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveDependency.java index fbcc52c1bd1..0d015f4417d 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveDependency.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveDependency.java @@ -80,7 +80,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Scope checkScope = scope != null ? Scope.fromName(scope) : null; ResolvedDependency dependency = findDependency(tag, checkScope); if (dependency != null) { - doAfterVisit(new RemoveContentVisitor<>(tag, true)); + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveManagedDependency.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveManagedDependency.java index e8bee845d20..f65fbf99269 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveManagedDependency.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveManagedDependency.java @@ -79,7 +79,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Scope checkScope = scope != null ? Scope.fromName(scope) : null; boolean isBomImport = tag.getChildValue("scope").map("import"::equalsIgnoreCase).orElse(false); if (isBomImport || findManagedDependency(tag, checkScope) != null) { - doAfterVisit(new RemoveContentVisitor<>(tag, true)); + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); } } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemovePlugin.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemovePlugin.java index 35e8430e1b8..d0910a48469 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/RemovePlugin.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RemovePlugin.java @@ -60,7 +60,7 @@ public TreeVisitor getVisitor() { @Override public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { for (Xml.Tag plugin : FindPlugin.find(document, groupId, artifactId)) { - doAfterVisit(new RemoveContentVisitor<>(plugin, true)); + doAfterVisit(new RemoveContentVisitor<>(plugin, true, true)); } return super.visitDocument(document, ctx); } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveProperty.java b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveProperty.java index 15e3e27dbd0..23f7b9f93ce 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveProperty.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/RemoveProperty.java @@ -52,7 +52,7 @@ private class RemovePropertyVisitor extends MavenVisitor { @Override public Xml visitTag(Xml.Tag tag, ExecutionContext ctx) { if (isPropertyTag() && propertyName.equals(tag.getName())) { - doAfterVisit(new RemoveContentVisitor<>(tag, true)); + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); maybeUpdateModel(); } return super.visitTag(tag, ctx); diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/cleanup/DependencyManagementDependencyRequiresVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/cleanup/DependencyManagementDependencyRequiresVersion.java index cf953246749..f852a6c35b1 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/cleanup/DependencyManagementDependencyRequiresVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/cleanup/DependencyManagementDependencyRequiresVersion.java @@ -40,7 +40,7 @@ public TreeVisitor getVisitor() { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (isManagedDependencyTag() && tag.getChildValue("version").orElse(null) == null) { - doAfterVisit(new RemoveContentVisitor<>(tag, true)); + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); } return super.visitTag(tag, ctx); } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/AddProfileTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/AddProfileTest.java index c1b1bfbc526..15c2b06468e 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/AddProfileTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/AddProfileTest.java @@ -131,6 +131,7 @@ void preExistingMatchingProfile() { artifact 1 + myprofile @@ -146,6 +147,7 @@ void preExistingMatchingProfile() { artifact 1 + myprofile diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java index 026c033956e..2501908550d 100644 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/RemovePropertyTest.java @@ -41,11 +41,11 @@ void removeProperty() { """ 4.0.0 - + com.mycompany.app my-app 1 - + a b @@ -55,11 +55,11 @@ void removeProperty() { """ 4.0.0 - + com.mycompany.app my-app 1 - + a @@ -83,11 +83,11 @@ void removeOnlyProperty() { """ 4.0.0 - + com.mycompany.app my-app 1 - + b @@ -96,7 +96,7 @@ void removeOnlyProperty() { """ 4.0.0 - + com.mycompany.app my-app 1 @@ -111,4 +111,110 @@ void removeOnlyProperty() { ) ); } + + @Test + void removePropertyWithComment() { + rewriteRun( + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + a + + b + + + """, + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + a + + + """, + sourceSpecs -> + sourceSpecs.afterRecipe(d -> { + MavenResolutionResult resolution = d.getMarkers().findFirst(MavenResolutionResult.class).orElseThrow(); + Map properties = resolution.getPom().getRequested().getProperties(); + assertThat(properties.get("a.version")).isEqualTo("a"); + assertThat(properties.get("bla.version")).isNull(); + }) + ) + ); + } + + @Test + void removePropertyWithCommentAndEmptyParents() { + rewriteRun( + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + + b + + + """, + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } + + @Test + void removePropertyWithTwoComments() { + rewriteRun( + pomXml( + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + + + + b + + + """, + """ + + 4.0.0 + + com.mycompany.app + my-app + 1 + + """ + ) + ); + } } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveContentVisitor.java b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveContentVisitor.java index e9822497e4b..372279fa2bd 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveContentVisitor.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveContentVisitor.java @@ -25,10 +25,12 @@ public class RemoveContentVisitor

extends XmlVisitor

{ private final Content scope; private final boolean removeEmptyAncestors; + private final boolean removePrecedingComment; - public RemoveContentVisitor(Content tag, boolean removeEmptyAncestors) { + public RemoveContentVisitor(Content tag, boolean removeEmptyAncestors, boolean removePrecedingComment) { this.scope = tag; this.removeEmptyAncestors = removeEmptyAncestors; + this.removePrecedingComment = removePrecedingComment; } @Override @@ -39,13 +41,18 @@ public Xml visitTag(Xml.Tag tag, P p) { for (Content content : t.getContent()) { if (scope.isScope(content)) { List contents = new ArrayList<>(t.getContent()); - contents.remove(content); + int indexOf = contents.indexOf(content); + contents.remove(indexOf); + + if (removePrecedingComment && 0 < indexOf && contents.get(indexOf - 1) instanceof Xml.Comment) { + doAfterVisit(new RemoveContentVisitor<>(contents.get(indexOf - 1), true, removePrecedingComment)); + } if (removeEmptyAncestors && contents.isEmpty() && t.getAttributes().isEmpty()) { if (getCursor().getParentOrThrow().getValue() instanceof Xml.Document) { return t.withContent(null).withClosing(null); } else { - doAfterVisit(new RemoveContentVisitor<>(t, true)); + doAfterVisit(new RemoveContentVisitor<>(t, true, removePrecedingComment)); } } else { return t.withContent(contents); diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveEmptyXmlTags.java b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveEmptyXmlTags.java index 2e59d0faf10..520022ec3fb 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveEmptyXmlTags.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveEmptyXmlTags.java @@ -39,7 +39,7 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); //noinspection ConstantValue if (t != null && (t.getContent() == null || t.getContent().isEmpty()) && t.getAttributes().isEmpty()) { - doAfterVisit(new RemoveContentVisitor<>(t, true)); + doAfterVisit(new RemoveContentVisitor<>(t, true, true)); } return t; } diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java index 0e856260342..7391675969c 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/RemoveXmlTag.java @@ -55,7 +55,7 @@ public TreeVisitor getVisitor() { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { if (xPathMatcher.matches(getCursor())) { - doAfterVisit(new RemoveContentVisitor<>(tag, true)); + doAfterVisit(new RemoveContentVisitor<>(tag, true, true)); } return super.visitTag(tag, ctx); } diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/RemoveContentTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/RemoveContentTest.java index ea5d4ee05e8..eb4f3171cb5 100755 --- a/rewrite-xml/src/test/java/org/openrewrite/xml/RemoveContentTest.java +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/RemoveContentTest.java @@ -34,7 +34,7 @@ void removeContent() { spec -> spec.recipe(toRecipe(() -> new XmlVisitor<>() { @Override public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { - doAfterVisit(new RemoveContentVisitor<>(requireNonNull(x.getRoot().getContent()).get(1), false)); + doAfterVisit(new RemoveContentVisitor<>(requireNonNull(x.getRoot().getContent()).get(1), false, true)); return super.visitDocument(x, ctx); } }).withMaxCycles(1)), @@ -61,7 +61,7 @@ void removeAncestorsThatBecomeEmpty() { @Override public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { doAfterVisit(new RemoveContentVisitor<>(requireNonNull(x.getRoot().getChildren()).get(1) - .getChildren().get(0).getChildren().get(0), true)); + .getChildren().get(0).getChildren().get(0), true, true)); return super.visitDocument(x, ctx); } }).withMaxCycles(1)), @@ -92,7 +92,7 @@ void rootChangedToEmptyTagIfLastRemainingTag() { @Override public Xml visitDocument(Xml.Document x, ExecutionContext ctx) { doAfterVisit(new RemoveContentVisitor<>(requireNonNull(x.getRoot().getChildren()).get(0) - .getChildren().get(0).getChildren().get(0), true)); + .getChildren().get(0).getChildren().get(0), true, true)); return super.visitDocument(x, ctx); } }).withMaxCycles(1)),