This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch GROOVY-12133
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY-12133 by this push:
new 77c85a20f5 Minor tweak
77c85a20f5 is described below
commit 77c85a20f5514278115dbb3b3ac854e1e04f7ac1
Author: Daniel Sun <[email protected]>
AuthorDate: Sat Jul 18 00:39:05 2026 +0900
Minor tweak
---
src/main/java/groovy/util/regex/BalancedGroup.java | 48 ++++++----
.../groovy/runtime/StringGroovyMethods.java | 2 -
src/test/groovy/bugs/Groovy12133.groovy | 43 ---------
.../util/regex/BalancedGroupConstructorTest.groovy | 100 +++++++++++++++++++++
4 files changed, 132 insertions(+), 61 deletions(-)
diff --git a/src/main/java/groovy/util/regex/BalancedGroup.java
b/src/main/java/groovy/util/regex/BalancedGroup.java
index 772c807061..bbf51d687b 100644
--- a/src/main/java/groovy/util/regex/BalancedGroup.java
+++ b/src/main/java/groovy/util/regex/BalancedGroup.java
@@ -49,8 +49,11 @@ import java.util.regex.Pattern;
* knowing both the balancing capture and the open/close match positions).
* </p>
* <p>
- * Parent links are set when a node is constructed as a child of another node;
- * root nodes have a {@code null} parent.
+ * Parent links and nesting depth are wired when a node is attached as a child
+ * of another node during construction; root nodes have a {@code null} parent
+ * and depth {@code 0}. Prefer {@link #find} (or the GDK methods on
+ * {@link CharSequence}) as the public entry point — constructors are
+ * package-private because they perform one-shot parent wiring.
* </p>
*
* @since 6.0.0
@@ -79,14 +82,14 @@ public final class BalancedGroup {
private final int fullEnd;
private final List<BalancedGroup> children;
private BalancedGroup parent;
+ /** Nesting depth; set once when this node is attached as a child (O(1)
{@link #getDepth()}). */
+ private int depth;
/**
- * Constructs a node with the given matched text and children.
- * Offsets are treated as relative to {@code matchedString}
- * ({@code start = 0}, {@code end = matchedString.length()}, and the same
- * for the full span). Prefer
- * {@link #BalancedGroup(String, int, int, int, int, List)} when absolute
- * indices in a source string are known.
+ * Package-private constructor: builds a node with offsets relative to
+ * {@code matchedString} ({@code start = 0}, {@code end =
matchedString.length()},
+ * and the same for the full span). Prefer {@link #find} for public use.
+ * Wires each child's parent link (one-shot: a child may not already have
a parent).
*
* @param matchedString the text captured for this group (never {@code
null})
* @param children immediate nested groups, or {@code null}/empty for
a leaf
@@ -94,12 +97,14 @@ public final class BalancedGroup {
* {@code children} contains {@code null}
* @throws IllegalArgumentException if any child already has a parent
*/
- public BalancedGroup(String matchedString, List<BalancedGroup> children) {
+ BalancedGroup(String matchedString, List<BalancedGroup> children) {
this(matchedString, 0, lengthOf(matchedString), 0,
lengthOf(matchedString), children);
}
/**
- * Constructs a node with matched text, absolute source offsets, and
children.
+ * Package-private constructor: builds a node with absolute source offsets
and children.
+ * Prefer {@link #find} for public use. Wires each child's parent link and
depth
+ * (one-shot: a child may not already have a parent).
*
* @param matchedString the text captured for this group (never {@code
null});
* may include or exclude boundary delimiters
depending on
@@ -113,8 +118,8 @@ public final class BalancedGroup {
* {@code children} contains {@code null}
* @throws IllegalArgumentException if ranges are invalid or a child
already has a parent
*/
- public BalancedGroup(String matchedString, int start, int end, int
fullStart, int fullEnd,
- List<BalancedGroup> children) {
+ BalancedGroup(String matchedString, int start, int end, int fullStart, int
fullEnd,
+ List<BalancedGroup> children) {
this.matchedString = Objects.requireNonNull(matchedString,
"matchedString");
if (start < 0 || end < start) {
throw new IllegalArgumentException("invalid match range: [" +
start + ", " + end + ")");
@@ -139,10 +144,24 @@ public final class BalancedGroup {
throw new IllegalArgumentException("child BalancedGroup
already has a parent");
}
child.parent = this;
+ // Bottom-up assembly: this node may later be attached under a
grandparent,
+ // so depth is assigned (and cascaded) here and again when we
are attached.
+ child.assignDepth(this.depth + 1);
}
}
}
+ /**
+ * Sets this node's depth and cascades to descendants. Invoked only during
the
+ * one-shot parent-wiring phase of construction (tree is not yet
published).
+ */
+ private void assignDepth(int newDepth) {
+ this.depth = newDepth;
+ for (BalancedGroup child : children) {
+ child.assignDepth(newDepth + 1);
+ }
+ }
+
private static int lengthOf(String s) {
return s == null ? 0 : s.length();
}
@@ -371,14 +390,11 @@ public final class BalancedGroup {
/**
* Nesting depth of this node (0 for a root).
+ * Computed once when the parent link is wired; O(1).
*
* @return the number of ancestors
*/
public int getDepth() {
- int depth = 0;
- for (BalancedGroup p = parent; p != null; p = p.parent) {
- depth++;
- }
return depth;
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
b/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
index 2883e12748..4d21889a3c 100644
--- a/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
@@ -4539,5 +4539,3 @@ public class StringGroovyMethods extends
DefaultGroovyMethodsSupport {
return BalancedGroup.find(self, openRegex, closeRegex, options);
}
}
-
-
diff --git a/src/test/groovy/bugs/Groovy12133.groovy
b/src/test/groovy/bugs/Groovy12133.groovy
index 46d44a236e..d1d2dddf53 100644
--- a/src/test/groovy/bugs/Groovy12133.groovy
+++ b/src/test/groovy/bugs/Groovy12133.groovy
@@ -99,23 +99,6 @@ final class Groovy12133 {
assertThrows(UnsupportedOperationException, () ->
leaf.children.add(null))
}
- @Test
- void testConstructorWiresParentAndRejectsReparenting() {
- BalancedGroup child = new BalancedGroup('(E)', null)
- BalancedGroup parent = new BalancedGroup('(D(E))', [child])
-
- assertSame(parent, child.parent)
- assertEquals(1, parent.children.size())
- assertSame(child, parent.children[0])
- assertEquals(0, child.start)
- assertEquals(3, child.end)
-
- assertThrows(IllegalArgumentException, () -> new
BalancedGroup('other', [child]))
- assertThrows(NullPointerException, () -> new BalancedGroup(null, null))
- assertThrows(IllegalArgumentException, () -> new BalancedGroup('ab',
0, 1, 0, 1, null)) // length mismatch
- assertThrows(IllegalArgumentException, () -> new BalancedGroup('a',
-1, 1, 0, 1, null))
- }
-
@Test
void testDanglingOpenRescuesCompletedChildren() {
// .NET strict (?(Open)(?!)) would fail the whole match; we salvage
closed spans.
@@ -422,32 +405,6 @@ final class Groovy12133 {
BalancedGroup.MatchOptions.defaults().withIgnoreRegex('[')))
}
- @Test
- void testConstructorRejectsInvalidFullRangeAndEndBeforeStart() {
- assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 3,
1, 0, 1, null))
- assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 0,
1, 3, 1, null))
- assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 0,
1, -1, 0, null))
- assertThrows(NullPointerException, () -> new BalancedGroup('a', 0, 1,
0, 1, [null]))
- }
-
- @Test
- void testConstructorAcceptsEmptyListAndAbsoluteOffsets() {
- BalancedGroup leaf = new BalancedGroup('leaf', [])
- assertTrue(leaf.children.isEmpty())
- assertEquals(0, leaf.start)
- assertEquals(4, leaf.end)
- assertEquals(0, leaf.fullStart)
- assertEquals(4, leaf.fullEnd)
-
- BalancedGroup absolute = new BalancedGroup('xy', 10, 12, 8, 15, null)
- assertEquals('xy', absolute.matchedString)
- assertEquals(10, absolute.start)
- assertEquals(12, absolute.end)
- assertEquals(8, absolute.fullStart)
- assertEquals(15, absolute.fullEnd)
- assertEquals(2, absolute.length)
- }
-
@Test
void testMatchOptionsWithersPreserveOtherFields() {
def base = BalancedGroup.MatchOptions.defaults()
diff --git
a/src/test/groovy/groovy/util/regex/BalancedGroupConstructorTest.groovy
b/src/test/groovy/groovy/util/regex/BalancedGroupConstructorTest.groovy
new file mode 100644
index 0000000000..66dc682259
--- /dev/null
+++ b/src/test/groovy/groovy/util/regex/BalancedGroupConstructorTest.groovy
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.util.regex
+
+import org.junit.jupiter.api.Test
+
+import static org.junit.jupiter.api.Assertions.assertEquals
+import static org.junit.jupiter.api.Assertions.assertSame
+import static org.junit.jupiter.api.Assertions.assertThrows
+import static org.junit.jupiter.api.Assertions.assertTrue
+
+/**
+ * Package-private constructor contracts for {@link BalancedGroup}.
+ * Lives in the same package so tests can exercise the package-private
+ * constructors without widening them to public API.
+ *
+ * User-facing behaviour is covered by {@code bugs.Groovy12133}.
+ */
+final class BalancedGroupConstructorTest {
+
+ @Test
+ void testConstructorWiresParentAndRejectsReparenting() {
+ BalancedGroup child = new BalancedGroup('(E)', null)
+ BalancedGroup parent = new BalancedGroup('(D(E))', [child])
+
+ assertSame(parent, child.parent)
+ assertEquals(1, parent.children.size())
+ assertSame(child, parent.children[0])
+ assertEquals(0, child.start)
+ assertEquals(3, child.end)
+ assertEquals(0, parent.depth)
+ assertEquals(1, child.depth)
+
+ assertThrows(IllegalArgumentException, () -> new
BalancedGroup('other', [child]))
+ assertThrows(NullPointerException, () -> new BalancedGroup(null, null))
+ assertThrows(IllegalArgumentException, () -> new BalancedGroup('ab',
0, 1, 0, 1, null)) // length mismatch
+ assertThrows(IllegalArgumentException, () -> new BalancedGroup('a',
-1, 1, 0, 1, null))
+ }
+
+ @Test
+ void testConstructorRejectsInvalidFullRangeAndEndBeforeStart() {
+ assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 3,
1, 0, 1, null))
+ assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 0,
1, 3, 1, null))
+ assertThrows(IllegalArgumentException, () -> new BalancedGroup('a', 0,
1, -1, 0, null))
+ assertThrows(NullPointerException, () -> new BalancedGroup('a', 0, 1,
0, 1, [null]))
+ }
+
+ @Test
+ void testConstructorAcceptsEmptyListAndAbsoluteOffsets() {
+ BalancedGroup leaf = new BalancedGroup('leaf', [])
+ assertTrue(leaf.children.isEmpty())
+ assertEquals(0, leaf.start)
+ assertEquals(4, leaf.end)
+ assertEquals(0, leaf.fullStart)
+ assertEquals(4, leaf.fullEnd)
+ assertEquals(0, leaf.depth)
+
+ BalancedGroup absolute = new BalancedGroup('xy', 10, 12, 8, 15, null)
+ assertEquals('xy', absolute.matchedString)
+ assertEquals(10, absolute.start)
+ assertEquals(12, absolute.end)
+ assertEquals(8, absolute.fullStart)
+ assertEquals(15, absolute.fullEnd)
+ assertEquals(2, absolute.length)
+ assertEquals(0, absolute.depth)
+ }
+
+ @Test
+ void testDepthCascadesWhenParentIsLaterAttached() {
+ // Mirrors bottom-up assembly in BalancedGroup.find: inner nodes are
+ // constructed first (depth relative to a temporary root), then the
+ // outer node attaches them and must recompute descendant depths.
+ BalancedGroup grandChild = new BalancedGroup('(E)', null)
+ BalancedGroup child = new BalancedGroup('(D(E))', [grandChild])
+ assertEquals(1, grandChild.depth)
+
+ BalancedGroup root = new BalancedGroup('(A(D(E)))', [child])
+ assertEquals(0, root.depth)
+ assertEquals(1, child.depth)
+ assertEquals(2, grandChild.depth)
+ assertSame(root, child.parent)
+ assertSame(child, grandChild.parent)
+ }
+}