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)
+    }
+}

Reply via email to