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
commit 3359ce94e8daf20cf7c64a34b6743aa7b6cf1c30 Author: Daniel Sun <[email protected]> AuthorDate: Sat Jul 18 00:39:05 2026 +0900 Minor tweak --- src/main/java/groovy/util/regex/BalancedGroup.java | 49 ++++++---- .../groovy/runtime/StringGroovyMethods.java | 2 - src/test/groovy/bugs/Groovy12133.groovy | 43 --------- .../util/regex/BalancedGroupConstructorTest.groovy | 100 +++++++++++++++++++++ 4 files changed, 132 insertions(+), 62 deletions(-) diff --git a/src/main/java/groovy/util/regex/BalancedGroup.java b/src/main/java/groovy/util/regex/BalancedGroup.java index 772c807061..3c73705203 100644 --- a/src/main/java/groovy/util/regex/BalancedGroup.java +++ b/src/main/java/groovy/util/regex/BalancedGroup.java @@ -20,7 +20,6 @@ package groovy.util.regex; import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Collections; import java.util.Deque; import java.util.LinkedHashMap; import java.util.List; @@ -49,8 +48,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 +81,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 +96,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 +117,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 +143,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 +389,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) + } +}
