You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2019/11/15 18:19:22 UTC

[groovy] branch master updated: GROOVY-8955: prevent Matcher.find()'s side-effect on Matcher.asBoolean()

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new f0d1c9a  GROOVY-8955: prevent Matcher.find()'s side-effect on Matcher.asBoolean()
f0d1c9a is described below

commit f0d1c9abef1780c8f7088d002309f7384cdec6f2
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
---
 .../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 {
             }
         };
     }
-
 }