From d86f21c2fab3282dea94b7acfdfd622d68c320f6 Mon Sep 17 00:00:00 2001 From: Didier Loiseau Date: Sat, 28 Sep 2024 16:19:43 +0200 Subject: [PATCH] More XPath matching fixes - paths with multiple occurrences of // - // preceding @attribute --- .../org/openrewrite/xml/XPathMatcher.java | 27 ++++++++++++------- .../org/openrewrite/xml/XPathMatcherTest.java | 20 ++++++++++++-- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/rewrite-xml/src/main/java/org/openrewrite/xml/XPathMatcher.java b/rewrite-xml/src/main/java/org/openrewrite/xml/XPathMatcher.java index f1493b63474..c3fbd8fe165 100644 --- a/rewrite-xml/src/main/java/org/openrewrite/xml/XPathMatcher.java +++ b/rewrite-xml/src/main/java/org/openrewrite/xml/XPathMatcher.java @@ -48,12 +48,14 @@ public class XPathMatcher { private final boolean startsWithSlash; private final boolean startsWithDoubleSlash; private final String[] parts; + private final long tagMatchingParts; public XPathMatcher(String expression) { this.expression = expression; startsWithSlash = expression.startsWith("/"); startsWithDoubleSlash = expression.startsWith("//"); parts = splitOnXPathSeparator(expression.substring(startsWithDoubleSlash ? 2 : startsWithSlash ? 1 : 0)); + tagMatchingParts = Arrays.stream(parts).filter(part -> !part.isEmpty() && !part.startsWith("@")).count(); } private String[] splitOnXPathSeparator(String input) { @@ -165,25 +167,19 @@ public boolean matches(Cursor cursor) { int blankPartIndex = Arrays.asList(parts).indexOf(""); int doubleSlashIndex = expression.indexOf("//"); - if (path.size() > blankPartIndex && path.size() >= parts.length - (parts[parts.length - 1].startsWith("@") ? 2 : 1)) { - String newExpression; + if (path.size() > blankPartIndex && path.size() >= tagMatchingParts) { Xml.Tag blankPartTag = path.get(blankPartIndex); String part = parts[blankPartIndex + 1]; Matcher matcher = ELEMENT_WITH_CONDITION_PATTERN.matcher(part); if (matcher.matches() ? matchesElementWithConditionFunction(matcher, blankPartTag, cursor) != null : Objects.equals(blankPartTag.getName(), part)) { - newExpression = String.format( - "%s/%s", - expression.substring(0, doubleSlashIndex), - expression.substring(doubleSlashIndex + 2) - ); - if (new XPathMatcher(newExpression).matches(cursor)) { + if (matchesWithoutDoubleSlashesAt(cursor, doubleSlashIndex)) { return true; } // fall-through: maybe we can skip this element and match further down } - newExpression = String.format( + String newExpression = String.format( // the // here allows to skip several levels of nested elements "%s/%s//%s", expression.substring(0, doubleSlashIndex), @@ -191,10 +187,12 @@ public boolean matches(Cursor cursor) { expression.substring(doubleSlashIndex + 2) ); return new XPathMatcher(newExpression).matches(cursor); + } else if (path.size() == tagMatchingParts) { + return matchesWithoutDoubleSlashesAt(cursor, doubleSlashIndex); } } - if (parts.length > path.size() + 1) { + if (tagMatchingParts > path.size()) { return false; } @@ -236,6 +234,15 @@ public boolean matches(Cursor cursor) { } } + private boolean matchesWithoutDoubleSlashesAt(Cursor cursor, int doubleSlashIndex) { + String newExpression = String.format( + "%s/%s", + expression.substring(0, doubleSlashIndex), + expression.substring(doubleSlashIndex + 2) + ); + return new XPathMatcher(newExpression).matches(cursor); + } + /** * Checks that the given {@code tag} matches the XPath part represented by {@code matcher}. * diff --git a/rewrite-xml/src/test/java/org/openrewrite/xml/XPathMatcherTest.java b/rewrite-xml/src/test/java/org/openrewrite/xml/XPathMatcherTest.java index a9822673a0c..31b5363800b 100755 --- a/rewrite-xml/src/test/java/org/openrewrite/xml/XPathMatcherTest.java +++ b/rewrite-xml/src/test/java/org/openrewrite/xml/XPathMatcherTest.java @@ -118,7 +118,13 @@ void matchAbsolute() { assertThat(match("/dependencies/dependency", xmlDoc)).isTrue(); assertThat(match("/dependencies/*/artifactId", xmlDoc)).isTrue(); assertThat(match("/dependencies/*", xmlDoc)).isTrue(); + assertThat(match("/dependencies//dependency", xmlDoc)).isTrue(); + assertThat(match("/dependencies//dependency//groupId", xmlDoc)).isTrue(); + + // negative matches assertThat(match("/dependencies/dne", xmlDoc)).isFalse(); + assertThat(match("/dependencies//dne", xmlDoc)).isFalse(); + assertThat(match("/dependencies//dependency//dne", xmlDoc)).isFalse(); } @Test @@ -127,6 +133,17 @@ void matchAbsoluteAttribute() { assertThat(match("/dependencies/dependency/artifactId/@scope", xmlDoc)).isTrue(); assertThat(match("/dependencies/dependency/artifactId/@*", xmlDoc)).isTrue(); assertThat(match("/dependencies/dependency/groupId/@*", xmlDoc)).isFalse(); + assertThat(match("/dependencies//dependency//@scope", xmlDoc)).isTrue(); + assertThat(match("/dependencies//dependency//artifactId//@scope", xmlDoc)).isTrue(); + assertThat(match("/dependencies//dependency//@*", xmlDoc)).isTrue(); + assertThat(match("/dependencies//dependency//artifactId//@*", xmlDoc)).isTrue(); + + // negative matches + assertThat(match("/dependencies/dependency/artifactId/@dne", xmlDoc)).isFalse(); + assertThat(match("/dependencies/dependency/artifactId/@dne", xmlDoc)).isFalse(); + assertThat(match("/dependencies//dependency//@dne", xmlDoc)).isFalse(); + assertThat(match("/dependencies//dependency//artifactId//@dne", xmlDoc)).isFalse(); + } @Test @@ -136,8 +153,6 @@ void matchRelative() { assertThat(match("//dependency", xmlDoc)).isTrue(); assertThat(match("dependency/*", xmlDoc)).isTrue(); assertThat(match("dne", xmlDoc)).isFalse(); - assertThat(match("/dependencies//dependency", xmlDoc)).isTrue(); - assertThat(match("/dependencies//dependency/groupId", xmlDoc)).isTrue(); } @Test @@ -211,6 +226,7 @@ void matchPom() { // skip 2+ levels with // assertThat(match("/project/build//plugin/configuration/source", pomXml2)).isTrue(); assertThat(match("/project//configuration/source", pomXml2)).isTrue(); + assertThat(match("/project//plugin//source", pomXml2)).isTrue(); } private final SourceFile attributeXml = new XmlParser().parse(