You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2019/11/17 08:54:09 UTC
[groovy] 14/18: GROOVY-8955: prevent Matcher.find()'s side-effect
on Matcher.asBoolean()
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 557f889d2eac60258be971b2b5738e23b7f14c49
Author: Szymon Stępniak <wo...@github.com>
AuthorDate: Fri Nov 15 11:05:48 2019 -0600
GROOVY-8955: prevent Matcher.find()'s side-effect on Matcher.asBoolean()
This closes #815
(cherry picked from commit f0d1c9abef1780c8f7088d002309f7384cdec6f2)
---
.../groovy/runtime/StringGroovyMethods.java | 34 +++++++-------
src/spec/doc/core-operators.adoc | 8 ++--
src/test/groovy/RegularExpressionsTest.groovy | 53 +++++++++++++++++-----
.../groovy/runtime/StringGroovyMethodsTest.java | 39 ++++++++++++++--
4 files changed, 98 insertions(+), 36 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java b/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
index 72b39f6..8500643 100644
--- a/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/runtime/StringGroovyMethods.java
@@ -78,16 +78,15 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
* A string is coerced to false if it is of length 0,
* and to true otherwise.
*
- * @param string the character sequence
+ * @param chars the character sequence
* @return the boolean value
* @since 1.7.0
*/
- public static boolean asBoolean(CharSequence string) {
- if (null == string) {
- return false;
+ public static boolean asBoolean(CharSequence chars) {
+ if (chars != null) {
+ return chars.length() > 0;
}
-
- return string.length() > 0;
+ return false;
}
/**
@@ -98,12 +97,11 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
* @since 1.7.0
*/
public static boolean asBoolean(Matcher matcher) {
- if (null == matcher) {
- return false;
+ if (matcher != null) {
+ RegexSupport.setLastMatcher(matcher);
+ return matcher.find(0); //GROOVY-8855
}
-
- RegexSupport.setLastMatcher(matcher);
- return matcher.find();
+ return false;
}
/**
@@ -917,8 +915,8 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
if (matcher.find()) {
if (hasGroup(matcher)) {
int count = matcher.groupCount();
- List groups = new ArrayList(count);
- for (int i = 0; i <= count; i++) {
+ List<String> groups = new ArrayList<>(count);
+ for (int i = 0; i <= count; i += 1) {
groups.add(matcher.group(i));
}
return InvokerHelper.toString(closure.call(groups));
@@ -1354,7 +1352,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
int counter = 0;
matcher.reset();
while (matcher.find()) {
- counter++;
+ counter += 1;
}
return counter;
}
@@ -1596,7 +1594,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
* @see java.util.regex.Matcher#group()
* @since 1.0
*/
- public static Iterator iterator(final Matcher matcher) {
+ public static Iterator iterator(Matcher matcher) {
matcher.reset();
return new Iterator() {
private boolean found /* = false */;
@@ -1627,7 +1625,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
// are we using groups?
// yes, so return the specified group as list
List<String> list = new ArrayList<String>(matcher.groupCount());
- for (int i = 0; i <= matcher.groupCount(); i++) {
+ for (int i = 0; i <= matcher.groupCount(); i += 1) {
list.add(matcher.group(i));
}
return list;
@@ -2464,13 +2462,13 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
matcher.reset();
} else if (idx > 0) {
matcher.reset();
- for (int i = 0; i < idx; i++) {
+ for (int i = 0; i < idx; i += 1) {
matcher.find();
}
} else if (idx < 0) {
matcher.reset();
idx += getCount(matcher);
- for (int i = 0; i < idx; i++) {
+ for (int i = 0; i < idx; i += 1) {
matcher.find();
}
}
diff --git a/src/spec/doc/core-operators.adoc b/src/spec/doc/core-operators.adoc
index e1a64f5..3ed328c 100644
--- a/src/spec/doc/core-operators.adoc
+++ b/src/spec/doc/core-operators.adoc
@@ -439,8 +439,7 @@ include::{projectdir}/src/spec/test/OperatorsTest.groovy[tags=pattern_op_variant
=== Find operator
-Alternatively to building a pattern, you can directly use the find operator `=~` to build a `java.util.regex.Matcher`
-instance:
+Alternatively to building a pattern, you can use the find operator `=~` to directly create a `java.util.regex.Matcher` instance:
[source,groovy]
----
@@ -448,10 +447,11 @@ include::{projectdir}/src/spec/test/OperatorsTest.groovy[tags=pattern_matcher_op
----
<1> `=~` creates a matcher against the `text` variable, using the pattern on the right hand side
<2> the return type of `=~` is a `Matcher`
-<3> equivalent to calling `if (!m.find())`
+<3> equivalent to calling `if (!m.find(0))`
Since a `Matcher` coerces to a `boolean` by calling its `find` method, the `=~` operator is consistent with the simple
-use of Perl's `=~` operator, when it appears as a predicate (in `if`, `while`, etc.).
+use of Perl's `=~` operator, when it appears as a predicate (in `if`, `?:`, etc.). When the intent is to iterate over
+matches of the specified pattern (in `while`, etc.) call `find()` directly on the matcher or use the `iterator` DGM.
=== Match operator
diff --git a/src/test/groovy/RegularExpressionsTest.groovy b/src/test/groovy/RegularExpressionsTest.groovy
index 45f93b1..d256f1c 100644
--- a/src/test/groovy/RegularExpressionsTest.groovy
+++ b/src/test/groovy/RegularExpressionsTest.groovy
@@ -18,22 +18,25 @@
*/
package groovy
-import groovy.test.GroovyTestCase
+import org.junit.Test
import java.util.regex.Matcher
import java.util.regex.Pattern
+import static groovy.test.GroovyAssert.shouldFail
+
/**
* Tests Groovy's regular expression syntax and DGM methods.
*/
-class RegularExpressionsTest extends GroovyTestCase {
+final class RegularExpressionsTest {
+ @Test
void testMatchOperator() {
assert "cheese" ==~ "cheese"
assert !("cheesecheese" ==~ "cheese")
}
- // The find operator is: =~
+ @Test // The find operator is: =~
void testFindOperator() {
assert "cheese" =~ "cheese"
@@ -46,14 +49,14 @@ class RegularExpressionsTest extends GroovyTestCase {
assert m instanceof Matcher
- while (m) {
+ while (m.find()) {
i = i + 1
}
assert i == 2
i = 0
m = "cheesecheese" =~ "e+"
- while (m) {
+ while (m.find()) {
i = i + 1
}
assert i == 4
@@ -65,7 +68,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert m.group() == "ee"
}
- // From the javadoc of the getAt() method
+ @Test // From the javadoc of the getAt() method
void testMatcherWithIntIndex() {
def p = /ab[d|f]/
def m = "abcabdabeabf" =~ p
@@ -98,6 +101,7 @@ class RegularExpressionsTest extends GroovyTestCase {
}
}
+ @Test
void testMatcherWithIndexAndRanges() {
def string = "cheesecheese"
def matcher = string =~ "e+"
@@ -117,6 +121,7 @@ class RegularExpressionsTest extends GroovyTestCase {
shouldFail { matcher[0, [1, 2]] }
}
+ @Test
void testMatcherIterator() {
def matcher = "cheesecheese" =~ "e+"
def iter = matcher.iterator()
@@ -154,6 +159,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert [["chee", "ch"], ["se", "s"], [" ple", " pl"], ["ase", "as"]] == matcher.collect { it }
}
+ @Test
void testMatcherEach() {
def count = 0
def result = []
@@ -183,7 +189,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert [["chee", "ch"], ["se", "s"], [" ple", " pl"], ["ase", "as"]] == result
}
- // Check consistency between each and collect
+ @Test // Check consistency between each and collect
void testMatcherEachVsCollect() {
def matcher = "cheese cheese" =~ "e+"
def result = []
@@ -210,6 +216,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert result == matcher.collect { a, g -> g }
}
+ @Test
void testSimplePattern() {
def pattern = ~"foo"
assert pattern instanceof Pattern
@@ -217,6 +224,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert !pattern.matcher("bar").matches()
}
+ @Test
void testMultiLinePattern() {
def pattern = ~"""foo"""
assert pattern instanceof Pattern
@@ -224,11 +232,12 @@ class RegularExpressionsTest extends GroovyTestCase {
assert !pattern.matcher("bar").matches()
}
+ @Test
void testPatternInAssertion() {
assert "foofoofoo" =~ ~"foo"
}
-
+ @Test
void testMatcherWithReplace() {
def matcher = "cheese-cheese" =~ "cheese"
def answer = matcher.replaceAll("edam")
@@ -238,6 +247,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert cheese == "nice cheese!"
}
+ @Test
void testGetLastMatcher() {
assert "cheese" ==~ "cheese"
assert Matcher.getLastMatcher().matches()
@@ -258,6 +268,7 @@ class RegularExpressionsTest extends GroovyTestCase {
}
}
+ @Test
void testRyhmeMatchGina() {
def myFairStringy = 'The rain in Spain stays mainly in the plain!'
// words, that end with 'ain': \b\w*ain\b
@@ -276,6 +287,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert found == 'rain Spain plain '
}
+ @Test
void testEachMatchWithPattern() {
def compiledPattern = ~/.at/
def result = []
@@ -283,6 +295,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert "cat sat hat" == result.join(" ")
}
+ @Test
void testPatternVersionsOfStringRegexMethods() {
def compiledPattern = ~/.at/
def s = "The cat sat on the hat"
@@ -291,6 +304,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert s.replaceAll(compiledPattern, 'x') == "The x x on the x"
}
+ @Test
void testFindOperatorCollect() {
def m = 'coffee' =~ /ee/
def result = ''
@@ -311,6 +325,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert m.collect { it }.join(',') == 'ee,ee'
}
+ @Test
void testIteration() {
def string = 'a:1 b:2 c:3'
def result = []
@@ -339,12 +354,14 @@ class RegularExpressionsTest extends GroovyTestCase {
assert numbers == '123'
}
+ @Test
void testFirst() {
assert "cheesecheese" =~ "cheese"
assert "cheesecheese" =~ /cheese/
assert "cheese" == /cheese/ /*they are both string syntaxes*/
}
+ @Test
void testSecond() {
// Let's create a regex Pattern
def pattern = ~/foo/
@@ -352,6 +369,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert pattern.matcher("foo").matches()
}
+ @Test
void testThird() {
// Let's create a Matcher
def matcher = "cheesecheese" =~ /cheese/
@@ -360,12 +378,14 @@ class RegularExpressionsTest extends GroovyTestCase {
assert answer == "edamedam"
}
+ @Test
void testFourth() {
// Let's do some replacement
def cheese = ("cheesecheese" =~ /cheese/).replaceFirst("nice")
assert cheese == "nicecheese"
}
+ @Test
void testFifth() {
def matcher = "\$abc." =~ "\\\$(.*)\\."
matcher.matches(); // must be invoked
@@ -374,6 +394,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert matcher[0][1] == "abc"
}
+ @Test
void testSixth() {
def matcher = "\$abc." =~ /\$(.*)\./ // no need to double-escape!
assert "\\\$(.*)\\." == /\$(.*)\./
@@ -383,6 +404,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert matcher[0][1] == "abc"
}
+ @Test
void testReplaceWithClosure() {
assert '1-FISH, two fish' == "one fish, two fish".replaceFirst(~/([a-z]{3})\s([a-z]{4})/) {
[one: 1, two: 2][it[1]] + '-' + it[2].toUpperCase()
@@ -392,6 +414,7 @@ class RegularExpressionsTest extends GroovyTestCase {
}
}
+ @Test
void testNoGroupMatcherAndGet() {
def p = /ab[d|f]/
def m = "abcabdabeabf" =~ p
@@ -404,6 +427,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert result == ['0:abd', '1:abf']
}
+ @Test
void testFindWithOneGroupAndGet() {
def p = /(?:ab([c|d|e|f]))/
def m = "abcabdabeabf" =~ p
@@ -416,6 +440,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert result == ['0:[abc, c]', '1:[abd, d]', '2:[abe, e]', '3:[abf, f]']
}
+ @Test
void testAnotherOneGroupMatcherAndGet() {
def m = "abcabdabeabfabxyzabx" =~ /(?:ab([d|x-z]+))/
def result = []
@@ -445,24 +470,28 @@ class RegularExpressionsTest extends GroovyTestCase {
assert '$\\23\\$\\45\\' == '$\\23\\$\\45\\'.replaceAll(p, c)
}
+ @Test
void testReplaceAllClosure() {
def p = /([^z]*)(z)/
def c = { all, m, d -> m }
replaceAllHelper(p, c)
}
+ @Test
void testReplaceAllClosureWithIt() {
def p = /([^z]*)(z)/
def c = { it[1] }
replaceAllHelper(p, c)
}
+ @Test
void testReplaceAllClosureWithObjectArray() {
def p = /([^z]*)(z)/
def c = { Object[] a -> a[1] }
replaceAllHelper(p, c)
}
+ @Test
void testFind() {
def p = /.ar/
assert null == 'foo foo baz'.find(p)
@@ -478,6 +507,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert 'bar' == 'foo bar baz'.find(compiledPattern)
}
+ @Test
void testFindClosureNoGroups() {
def p = /.ar/
def c = { match -> return "-$match-" }
@@ -491,6 +521,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert '-car-' == 'car'.find(compiledPattern, c)
}
+ @Test
void testFindClosureWithGroups() {
def AREA_CODE = /\d{3}/
def EXCHANGE = /\d{3}/
@@ -515,6 +546,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert "(888) 555-1212" == "bar 888-555-1212 foo".find(compiledPhonePattern, closureSingleVar)
}
+ @Test
void testFindAll() {
def p = /.at/
def compiledPattern = ~p
@@ -528,6 +560,7 @@ class RegularExpressionsTest extends GroovyTestCase {
assert ["+cat", "+sat", "+hat"] == orig.findAll(compiledPattern) { "+$it" }
}
+ @Test
void testFindAllWithGroups() {
def p = /(.)a(.)/
def compiledPattern = ~p
@@ -545,10 +578,9 @@ class RegularExpressionsTest extends GroovyTestCase {
assert ["c+cat+t", "s+sat+t", "h+hat+t"] == orig.findAll(compiledPattern, closureSingleVar)
}
+ @Test
void testMatchesPartially() {
-
def pattern = /\w+@\w+\.\w{2,}/
-
def useCases = [
"glaforge@gmail.com": true,
"glaforge" : true,
@@ -556,7 +588,6 @@ class RegularExpressionsTest extends GroovyTestCase {
"glaforge@" : true,
"glaforge@@" : false
]
-
useCases.each { String email, boolean bool ->
def matcher = email =~ pattern
diff --git a/src/test/org/codehaus/groovy/runtime/StringGroovyMethodsTest.java b/src/test/org/codehaus/groovy/runtime/StringGroovyMethodsTest.java
index 5e7fccd..e892ce4 100644
--- a/src/test/org/codehaus/groovy/runtime/StringGroovyMethodsTest.java
+++ b/src/test/org/codehaus/groovy/runtime/StringGroovyMethodsTest.java
@@ -19,16 +19,22 @@
package org.codehaus.groovy.runtime;
import groovy.lang.Closure;
-import groovy.test.GroovyTestCase;
+import org.junit.Test;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.Collections;
import java.util.List;
+import java.util.regex.Matcher;
import java.util.regex.Pattern;
-public class StringGroovyMethodsTest extends GroovyTestCase {
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+public final class StringGroovyMethodsTest {
+
+ @Test
public void testIncrementString() throws Exception {
String original = "z";
String answer = StringGroovyMethods.next(original);
@@ -37,6 +43,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertTrue(answer.compareTo(original) > 0);
}
+ @Test
public void testDecrementString() throws Exception {
String original = "a";
String answer = StringGroovyMethods.previous(original);
@@ -45,6 +52,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertTrue(ScriptBytecodeAdapter.compareLessThan(answer, original));
}
+ @Test
public void testToMethods() throws Exception {
assertEquals(StringGroovyMethods.toInteger("1"), new Integer(1));
assertEquals(StringGroovyMethods.toLong("1"), new Long(1));
@@ -61,6 +69,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(StringGroovyMethods.toBoolean("0"), Boolean.FALSE);
}
+ @Test
public void testIsMethods() throws Exception {
String intStr = "123";
String floatStr = "1.23E-1";
@@ -91,16 +100,33 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertFalse(StringGroovyMethods.isNumber(nonNumberStr));
}
+ @Test
+ public void testAsMethods() {
+ Pattern pattern = Pattern.compile("[a-z]+");
+ String correctInput = "abcde";
+ String incorrectInput = "123";
+ Matcher correctMatcher = pattern.matcher(correctInput);
+ Matcher incorrectMatcher = pattern.matcher(incorrectInput);
+
+ assertTrue(StringGroovyMethods.asBoolean(correctMatcher));
+ assertTrue(StringGroovyMethods.asBoolean(correctMatcher));
+ assertFalse(StringGroovyMethods.asBoolean(incorrectMatcher));
+ assertFalse(StringGroovyMethods.asBoolean(incorrectMatcher));
+ }
+
+ @Test
public void testStartsWithAny() {
assertTrue(StringGroovyMethods.startsWithAny("abcd", "ab", "ef"));
assertFalse(StringGroovyMethods.startsWithAny("abcd", "ef", "gh"));
}
+ @Test
public void testEndsWithAny() {
assertTrue(StringGroovyMethods.endsWithAny("abcd", "cd", "ef"));
assertFalse(StringGroovyMethods.endsWithAny("abcd", "ef", "gh"));
}
+ @Test
public void testIsBlank() {
assertTrue(StringGroovyMethods.isBlank(""));
assertTrue(StringGroovyMethods.isBlank(" "));
@@ -125,6 +151,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertFalse(StringGroovyMethods.isBlank("\tabc\t"));
}
+ @Test
public void testFindAllFromCharSequenceWithClosure() {
CharSequence charSequence = new StringBuilder().append("ABCD");
String regex = "(A)(B)(C)(D)";
@@ -134,6 +161,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindAllFromCharSequenceWithPatternAndClosure() {
CharSequence charSequence = new StringBuilder().append("ABCD");
Pattern pattern = Pattern.compile("(A)(B)(C)(D)");
@@ -143,6 +171,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindAllFromStringWithClosure() {
String string = "ABCD";
String regex = "(A)(B)(C)(D)";
@@ -152,6 +181,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindAllFromStringWithPatternAndClosure() {
String string = "ABCD";
Pattern pattern = Pattern.compile("(A)(B)(C)(D)");
@@ -161,6 +191,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindFromCharSequenceWithClosure() {
CharSequence charSequence = new StringBuilder().append("ABCD");
String regex = "(A)(B)(C)(D)";
@@ -170,6 +201,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindFromCharSequenceWithPatternAndClosure() {
CharSequence charSequence = new StringBuilder().append("ABCD");
Pattern pattern = Pattern.compile("(A)(B)(C)(D)");
@@ -179,6 +211,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindFromStringWithClosure() {
String string = "ABCD";
String regex = "(A)(B)(C)(D)";
@@ -188,6 +221,7 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
assertEquals(expectedResult, result);
}
+ @Test
public void testFindFromStringWithPatternAndClosure() {
String string = "ABCD";
Pattern pattern = Pattern.compile("(A)(B)(C)(D)");
@@ -211,5 +245,4 @@ public class StringGroovyMethodsTest extends GroovyTestCase {
}
};
}
-
}