More XPath matching fixes

- paths with multiple occurrences of // - // preceding @Attribute
openrewrite · Sep 28, 2024 · d86f21c · d86f21c
1 parent 9f796c5
commit d86f21c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 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
@@ -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,36 +167,32 @@ 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),
                             blankPartTag.getName(),
                             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
@@ -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(