You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by pa...@apache.org on 2015/05/26 12:54:24 UTC

incubator-groovy git commit: GROOVY-7433: API inconsistency between takeWhile, dropWhile and collectReplacements for CharSequences

Repository: incubator-groovy
Updated Branches:
  refs/heads/master ed1b7d3ae -> 08f8cff0d


GROOVY-7433: API inconsistency between takeWhile, dropWhile and collectReplacements for CharSequences


Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/08f8cff0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/08f8cff0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/08f8cff0

Branch: refs/heads/master
Commit: 08f8cff0d7a54ee3ca2f5a6c789867033e39c88f
Parents: ed1b7d3
Author: Paul King <pa...@asert.com.au>
Authored: Tue May 26 20:53:53 2015 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Tue May 26 20:53:53 2015 +1000

----------------------------------------------------------------------
 .../groovy/runtime/StringGroovyMethods.java     | 109 +++++++++++++------
 .../typehandling/DefaultTypeTransformation.java |   7 +-
 src/test/groovy/GroovyMethodsTest.groovy        |   7 +-
 ...ingGMClosureParamTypeInferenceSTCTest.groovy |  14 ++-
 4 files changed, 88 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/08f8cff0/src/main/org/codehaus/groovy/runtime/StringGroovyMethods.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/StringGroovyMethods.java b/src/main/org/codehaus/groovy/runtime/StringGroovyMethods.java
index 324670f..c0830a4 100644
--- a/src/main/org/codehaus/groovy/runtime/StringGroovyMethods.java
+++ b/src/main/org/codehaus/groovy/runtime/StringGroovyMethods.java
@@ -26,6 +26,7 @@ import groovy.lang.Range;
 
 import groovy.transform.stc.ClosureParams;
 import groovy.transform.stc.FromString;
+import groovy.transform.stc.PickFirstResolver;
 import groovy.transform.stc.SimpleType;
 import org.codehaus.groovy.runtime.callsite.BooleanClosureWrapper;
 import org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation;
@@ -51,6 +52,7 @@ import java.util.regex.Pattern;
 
 import static org.codehaus.groovy.runtime.DefaultGroovyMethods.callClosureForLine;
 import static org.codehaus.groovy.runtime.DefaultGroovyMethods.each;
+import static org.codehaus.groovy.runtime.DefaultGroovyMethods.join;
 
 /**
  * This class defines new groovy methods which appear on String-related JDK
@@ -501,7 +503,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      * </pre>
      *
      * @param self the original CharSequence
-     * @param num the number of characters to drop from this iterator
+     * @param num the number of characters to drop from this String
      * @return a CharSequence consisting of all characters except the first <code>num</code> ones,
      *         or else an empty String, if this CharSequence has less than <code>num</code> characters.
      * @since 1.8.1
@@ -520,7 +522,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      * A GString variant of the equivalent CharSequence method.
      *
      * @param self the original GString
-     * @param num the number of characters to drop from this iterator
+     * @param num the number of characters to drop from this GString
      * @return a String consisting of all characters except the first <code>num</code> ones,
      *         or else an empty String, if the toString() of this GString has less than <code>num</code> characters.
      * @see #drop(CharSequence, int)
@@ -550,18 +552,10 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      *         evaluates to true for each element dropped from the front of the CharSequence
      * @since 2.0.0
      */
-    public static CharSequence dropWhile(CharSequence self, @ClosureParams(value=SimpleType.class, options="char") Closure condition) {
-        int num = 0;
-        BooleanClosureWrapper bcw = new BooleanClosureWrapper(condition);
-        while (num < self.length()) {
-            char value = self.charAt(num);
-            if (bcw.call(value)) {
-                num += 1;
-            } else {
-                break;
-            }
-        }
-        return drop(self, num);
+    @SuppressWarnings("unchecked")
+    public static String dropWhile(CharSequence self, @ClosureParams(value=FromString.class, conflictResolutionStrategy=PickFirstResolver.class, options={"String", "Character"}) Closure condition) {
+        Iterator selfIter = hasCharacterArg(condition) ? new CharacterIterator(self) : new StringIterator(self);
+        return join(DefaultGroovyMethods.dropWhile(selfIter, condition), "");
     }
 
     /**
@@ -575,8 +569,54 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      * @see #dropWhile(CharSequence, groovy.lang.Closure)
      * @since 2.3.7
      */
-    public static String dropWhile(GString self, @ClosureParams(value=SimpleType.class, options="char") Closure condition) {
-        return dropWhile(self.toString(), condition).toString();
+    public static String dropWhile(GString self, @ClosureParams(value=FromString.class, conflictResolutionStrategy=PickFirstResolver.class, options={"String", "Character"}) Closure condition) {
+        return dropWhile(self.toString(), condition);
+    }
+
+    private static final class CharacterIterator implements Iterator<Character> {
+        private final CharSequence delegate;
+        private int length;
+        private int index;
+
+        public CharacterIterator(CharSequence delegate) {
+            this.delegate = delegate;
+            length = delegate.length();
+        }
+
+        public boolean hasNext() {
+            return index < length;
+        }
+
+        public Character next() {
+            return delegate.charAt(index++);
+        }
+
+        public void remove() {
+            throw new UnsupportedOperationException("Remove not supported for CharSequence iterators");
+        }
+    }
+
+    private static final class StringIterator implements Iterator<String> {
+        private final CharSequence delegate;
+        private int length;
+        private int index;
+
+        public StringIterator(CharSequence delegate) {
+            this.delegate = delegate;
+            length = delegate.length();
+        }
+
+        public boolean hasNext() {
+            return index < length;
+        }
+
+        public String next() {
+            return Character.toString(delegate.charAt(index++));
+        }
+
+        public void remove() {
+            throw new UnsupportedOperationException("Remove not supported for CharSequence iterators");
+        }
     }
 
     /**
@@ -592,7 +632,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      * @since 1.8.2
      */
     public static <T> T eachLine(CharSequence self, @ClosureParams(value=FromString.class, options={"String","String,Integer"}) Closure<T> closure) throws IOException {
-        return eachLine(self.toString(), 0, closure);
+        return eachLine((CharSequence)self.toString(), 0, closure);
     }
 
     /**
@@ -611,7 +651,7 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
     public static <T> T eachLine(CharSequence self, int firstLine, @ClosureParams(value=FromString.class, options={"String","String,Integer"}) Closure<T> closure) throws IOException {
         int count = firstLine;
         T result = null;
-        for (String line : readLines(self.toString())) {
+        for (String line : readLines((CharSequence)self.toString())) {
             result = callClosureForLine(closure, line, count);
             count++;
         }
@@ -644,6 +684,9 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      * <p>
      * <pre class="groovyTestCase">
      * assert "Groovy".collectReplacements{ it == 'o' ? '_O_' : null } == 'Gr_O__O_vy'
+     * assert "Groovy".collectReplacements{ it.equalsIgnoreCase('O') ? '_O_' : null } == 'Gr_O__O_vy'
+     * assert "Groovy".collectReplacements{ char c -> c == 'o' ? '_O_' : null } == 'Gr_O__O_vy'
+     * assert "Groovy".collectReplacements{ Character c -> c == 'o' ? '_O_' : null } == 'Gr_O__O_vy'
      * assert "B&W".collectReplacements{ it == '&' ? '&amp;' : null } == 'B&amp;W'
      * </pre>
      *
@@ -652,13 +695,13 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      *         have been replaced with the corresponding replacements
      *         as determined by the {@code transform} Closure.
      */
-    public static String collectReplacements(String orig, @ClosureParams(value=SimpleType.class, options="char") Closure<String> transform) {
+    public static String collectReplacements(String orig, @ClosureParams(value=FromString.class, conflictResolutionStrategy=PickFirstResolver.class, options={"String", "Character"}) Closure<String> transform) {
         if (orig == null) return orig;
 
         StringBuilder sb = null; // lazy create for edge-case efficiency
         for (int i = 0, len = orig.length(); i < len; i++) {
             final char ch = orig.charAt(i);
-            final String replacement = transform.call(ch);
+            final String replacement = transform.call(hasCharacterArg(transform) ? ch : Character.toString(ch));
 
             if (replacement != null) {
                 // output differs from input; we write to our local buffer
@@ -676,6 +719,12 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
         return sb == null ? orig : sb.toString();
     }
 
+    private static boolean hasCharacterArg(Closure c) {
+        if (c.getMaximumNumberOfParameters() != 1) return false;
+        String typeName = c.getParameterTypes()[0].getName();
+        return typeName.equals("char") || typeName.equals("java.lang.Character");
+    }
+
     /**
      * Process each regex group matched substring of the given CharSequence. If the closure
      * parameter takes one argument, an array with all match groups is passed to it.
@@ -3180,18 +3229,10 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      *         element passed to the given closure evaluates to true
      * @since 2.0.0
      */
-    public static CharSequence takeWhile(CharSequence self, @ClosureParams(value=SimpleType.class, options="char") Closure condition) {
-        int num = 0;
-        BooleanClosureWrapper bcw = new BooleanClosureWrapper(condition);
-        while (num < self.length()) {
-            char value = self.charAt(num);
-            if (bcw.call(value)) {
-                num += 1;
-            } else {
-                break;
-            }
-        }
-        return take(self, num);
+    @SuppressWarnings("unchecked")
+    public static String takeWhile(CharSequence self, @ClosureParams(value=FromString.class, conflictResolutionStrategy=PickFirstResolver.class, options={"String", "Character"}) Closure condition) {
+        Iterator selfIter = hasCharacterArg(condition) ? new CharacterIterator(self) : new StringIterator(self);
+        return join(DefaultGroovyMethods.takeWhile(selfIter, condition), "");
     }
 
     /**
@@ -3203,8 +3244,8 @@ public class StringGroovyMethods extends DefaultGroovyMethodsSupport {
      *         element passed to the given closure evaluates to true
      * @since 2.3.7
      */
-    public static String takeWhile(GString self, @ClosureParams(value=SimpleType.class, options="char") Closure condition) {
-        return (String) takeWhile(self.toString(), condition);
+    public static String takeWhile(GString self, @ClosureParams(value=FromString.class, conflictResolutionStrategy=PickFirstResolver.class, options={"String", "Character"}) Closure condition) {
+        return takeWhile(self.toString(), condition);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/08f8cff0/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
index 1742330..7213ecb 100644
--- a/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
+++ b/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java
@@ -450,11 +450,8 @@ public class DefaultTypeTransformation {
             method.call(adapter);
             return adapter.asList();
         }
-        else if (value instanceof String) {
-            return StringGroovyMethods.toList((String) value);
-        }
-        else if (value instanceof GString) {
-            return StringGroovyMethods.toList(value.toString());
+        else if (value instanceof String || value instanceof GString) {
+            return StringGroovyMethods.toList((CharSequence) value);
         }
         else if (value instanceof File) {
             try {

http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/08f8cff0/src/test/groovy/GroovyMethodsTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/GroovyMethodsTest.groovy b/src/test/groovy/GroovyMethodsTest.groovy
index b32db93..074a964 100644
--- a/src/test/groovy/GroovyMethodsTest.groovy
+++ b/src/test/groovy/GroovyMethodsTest.groovy
@@ -1508,10 +1508,9 @@ class GroovyMethodsTest extends GroovyTestCase {
                      new StringBuffer( 'groovy' ),
                      new StringBuilder( 'groovy' ) ]
         data.each {
-            // Need toString() as CharBuffer.subSequence returns a java.nio.StringCharBuffer
-            assert it.takeWhile{ it == '' }.toString() == ''
-            assert it.takeWhile{ it != 'v' }.toString() == 'groo'
-            assert it.takeWhile{ it }.toString() == 'groovy'
+            assert it.takeWhile{ it == '' } == ''
+            assert it.takeWhile{ it != 'v' } == 'groo'
+            assert it.takeWhile{ it } == 'groovy'
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/08f8cff0/src/test/groovy/transform/stc/StringGMClosureParamTypeInferenceSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/StringGMClosureParamTypeInferenceSTCTest.groovy b/src/test/groovy/transform/stc/StringGMClosureParamTypeInferenceSTCTest.groovy
index 9afc706..365dcf0 100644
--- a/src/test/groovy/transform/stc/StringGMClosureParamTypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/StringGMClosureParamTypeInferenceSTCTest.groovy
@@ -28,16 +28,18 @@ import groovy.transform.NotYetImplemented
 class StringGMClosureParamTypeInferenceSTCTest extends StaticTypeCheckingTestCase {
     void testCollectReplacements() {
         assertScript '''
-            assert "Groovy".collectReplacements { c -> String.valueOf(c.toUpperCase()) } == 'GROOVY'
+            assert "Groovy".collectReplacements { s -> s.equalsIgnoreCase('O') ? s.toUpperCase() : s } == 'GrOOvy'
         '''
     }
 
     void testDropWhile() {
         assertScript '''
             def text = "Groovy"
-            assert text.dropWhile{ it < (char)'Z' } == 'roovy\'
-            assert text.dropWhile{ it != (char)'v' } == 'vy\'
-'''
+            assert text.dropWhile{ it.charAt(0) < (char)'Z' } == 'roovy'
+            assert text.dropWhile{ !it.equalsIgnoreCase('V') } == 'vy'
+            assert text.dropWhile{ char it -> it < (char)'Z' } == 'roovy'
+            assert text.dropWhile{ Character it -> it != (char)'v' } == 'vy'
+        '''
     }
 
     void testEachLine() {
@@ -179,8 +181,8 @@ text.splitEachLine('([,:])') { a -> println a[0].toUpperCase() }
 
     void testTakeWhileOnCharSeq() {
         assertScript '''
-            String foo(CharSequence cs) { cs.takeWhile { it < (char) 'j' }}
+            String foo(CharSequence cs) { cs.takeWhile { it.charAt(0) < (char) 'j' }}
             assert foo("abcdefghijklmnopqrstuvwxyz") == 'abcdefghi'
-'''
+        '''
     }
 }