GROOVY-11305: STC: support for-each loop for implicitly-Iterable types

src/main/java/org/codehaus/groovy/runtime/DefaultGroovyMethods.java

@@ -8271,38 +8271,49 @@ public static boolean isWhitespace(Character self) {
     // iterator
 
     /**
-     * Attempts to create an Iterator for the given object by first
-     * converting it to a Collection.
+     * Creates an Iterator for the given Object by converting it to a Collection.
      *
-     * @param o an object
-     * @return an Iterator for the given Object.
+     * @param self an object
+     * @return an Iterator for the given Object
      * @see org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation#asCollection(java.lang.Object)
      * @since 1.0
      */
-    public static Iterator iterator(final Object o) {
-        if (o instanceof Iterator) return (Iterator)o;
-        return DefaultTypeTransformation.asCollection(o).iterator();
+    public static Iterator iterator(final Object self) {
+        if (self instanceof Iterator) return (Iterator) self;
+        return DefaultTypeTransformation.asCollection(self).iterator();
+    }
+
+    /**
+     * Supports 'duck-typing' when trying to get an Iterator for each element of
+     * a Collection, some of which may already be an Iterator.
+     *
+     * @param self an Iterator object
+     * @return the Iterator itself
+     * @since 1.5.0
+     */
+    public static <T> Iterator<T> iterator(final Iterator<T> self) {
+        return self;
     }
 
     /**
-     * Allows an Enumeration to behave like an Iterator.  Note that the
-     * {@link java.util.Iterator#remove() remove()} method is unsupported since the
-     * underlying Enumeration does not provide a mechanism for removing items.
+     * Allows an Enumeration to behave like an Iterable.  Note that the
+     * {@link java.util.Iterator#remove() remove()} method is unsupported since
+     * the underlying Enumeration doesn't provide a mechanism for removing items.
      *
-     * @param enumeration an Enumeration object
+     * @param self an Enumeration object
      * @return an Iterator for the given Enumeration
      * @since 1.0
      */
-    public static <T> Iterator<T> iterator(final Enumeration<T> enumeration) {
+    public static <T> Iterator<T> iterator(final Enumeration<T> self) {
         return new Iterator<T>() {
             @Override
             public boolean hasNext() {
-                return enumeration.hasMoreElements();
+                return self.hasMoreElements();
             }
 
             @Override
             public T next() {
-                return enumeration.nextElement();
+                return self.nextElement();
             }
 
             @Override
@@ -8313,15 +8324,12 @@ public void remove() {
     }
 
     /**
-     * An identity function for iterators, supporting 'duck-typing' when trying to get an
-     * iterator for each object within a collection, some of which may already be iterators.
+     * Returns an Iterator for the given Map's entry set.
      *
-     * @param self an iterator object
-     * @return itself
-     * @since 1.5.0
+     * @since 5.0.0
      */
-    public static <T> Iterator<T> iterator(Iterator<T> self) {
-        return self;
+    public static <K,V> Iterator<Map.Entry<K,V>> iterator(final Map<K,V> self) {
+        return self.entrySet().iterator();
     }
 
     //--------------------------------------------------------------------------

src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java

@@ -255,18 +255,12 @@ public void visitConstructorCallExpression(final ConstructorCallExpression call)
     @Override
     public void visitForLoop(final ForStatement statement) {
         super.visitForLoop(statement);
-        Expression collectionExpression = statement.getCollectionExpression();
+        var collectionExpression = statement.getCollectionExpression();
         if (!(collectionExpression instanceof ClosureListExpression)) {
-            ClassNode forLoopVariableType = statement.getVariableType();
-            ClassNode collectionType = getType(collectionExpression);
-            ClassNode componentType;
-            if (ClassHelper.isWrapperCharacter(ClassHelper.getWrapper(forLoopVariableType)) && ClassHelper.isStringType(collectionType)) {
-                // we allow auto-coercion here
-                componentType = forLoopVariableType;
-            } else {
-                componentType = inferLoopElementType(collectionType);
+            if (statement.getVariable().isDynamicTyped()) { // GROOVY-8169
+                ClassNode inferredType = getType(statement.getVariable());
+                statement.getVariable().setType(inferredType); // GROOVY-5640, GROOVY-5641
             }
-            statement.getVariable().setType(componentType);
         }
     }
 

src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java

@@ -2105,21 +2105,25 @@ public void visitForLoop(final ForStatement forLoop) {
             ClassNode collectionType = getType(collectionExpression);
             ClassNode forLoopVariableType = forLoop.getVariableType();
             ClassNode componentType;
-            if (isWrapperCharacter(getWrapper(forLoopVariableType)) && isStringType(collectionType)) {
-                // we allow auto-coercion here
-                componentType = forLoopVariableType;
+            if (isStringType(collectionType)) {
+                if (isWrapperCharacter(getWrapper(forLoopVariableType))) {
+                    // we support auto-coercion for char
+                    componentType = forLoopVariableType;
+                } else {
+                    componentType = STRING_TYPE;
+                }
             } else {
-                componentType = inferLoopElementType(collectionType);
-            }
-            if (getUnwrapper(componentType) == forLoopVariableType) {
-                // prefer primitive type over boxed type
-                componentType = forLoopVariableType;
+                componentType = inferComponentType(collectionType, null);
+                if (componentType == null) {
+                    componentType = OBJECT_TYPE;
+                } else if (getUnwrapper(componentType).equals(forLoopVariableType)) {
+                    componentType = forLoopVariableType; // prefer the primitive type
+                }
             }
             if (!checkCompatibleAssignmentTypes(forLoopVariableType, componentType)) {
                 addStaticTypeError("Cannot loop with element of type " + prettyPrintType(forLoopVariableType) + " with collection of type " + prettyPrintType(collectionType), forLoop);
             }
-            if (!isDynamicTyped(forLoopVariableType)) {
-                // user has specified a type, prefer it over the inferred type
+            if (!isDynamicTyped(forLoopVariableType)) { // user-supplied type
                 componentType = forLoopVariableType;
             }
             typeCheckingContext.controlStructureVariables.put(forLoop.getVariable(), componentType);
@@ -5785,11 +5789,11 @@ private static Map<GenericsTypeName, GenericsType> extractPlaceHolders(final Cla
         Map<GenericsTypeName, GenericsType> result = null;
         ClassNode[] todo;
         if (receiver instanceof UnionTypeClassNode) {
-            todo = ((UnionTypeClassNode) receiver).getDelegates();
+            todo = ((UnionTypeClassNode) receiver).getDelegates(); // GROOVY-7275
         } else {
             todo = new ClassNode[] {!isPrimitiveType(declaringClass) ? wrapTypeIfNecessary(receiver) : receiver};
         }
-        for (ClassNode type : todo) {
+out:    for (ClassNode type : todo) {
             for (ClassNode current = type; current != null; ) {
                 Map<GenericsTypeName, GenericsType> placeHolders = new HashMap<>();
                 if (current.isGenericsPlaceHolder())
@@ -5824,7 +5828,7 @@ else if (current.getGenericsTypes() != null
                 result = placeHolders;
 
                 // we are done if we are now in the declaring class
-                if (currentIsDeclaring) break;
+                if (currentIsDeclaring) break out;
 
                 current = getNextSuperClass(current, declaringClass);
                 if (current == null && isClassType(declaringClass)) {

src/test/groovy/transform/stc/LoopsSTCTest.groovy

@@ -114,21 +114,19 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
     void testForInLoopOnMap1() {
         assertScript '''
             @ASTTest(phase=INSTRUCTION_SELECTION, value={
-                lookup('forLoop').each {
-                    assert it instanceof org.codehaus.groovy.ast.stmt.ForStatement
-                    def collection = it.collectionExpression // MethodCallExpression
-                    def inft = collection.getNodeMetaData(INFERRED_TYPE)
-                    assert inft == make(Set)
-                    def entryInft = inft.genericsTypes[0].type
-                    assert entryInft == make(Map.Entry)
-                    assert entryInft.genericsTypes[0].type == STRING_TYPE
-                    assert entryInft.genericsTypes[1].type == Integer_TYPE
-                }
+                def loop = lookup('loop')[0]
+                assert loop instanceof org.codehaus.groovy.ast.stmt.ForStatement
+                def collectionType = loop.collectionExpression.getNodeMetaData(INFERRED_TYPE)
+                assert collectionType == SET_TYPE
+                def elementType = collectionType.genericsTypes[0].type
+                assert elementType == make(Map.Entry)
+                assert elementType.genericsTypes[0].type == STRING_TYPE
+                assert elementType.genericsTypes[1].type == Integer_TYPE
             })
             void test() {
                 def result = ""
                 def sum = 0
-                forLoop:
+                loop:
                 for ( Map.Entry<String, Integer> it in [a:1, b:3].entrySet() ) {
                    result += it.getKey()
                    sum += it.getValue()
@@ -187,6 +185,23 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-8169
+    void testForInLoopOnList2() {
+        assertScript '''
+            @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                def loop = lookup('loop')[0]
+                assert loop.variable.type == OBJECT_TYPE
+                assert loop.variable.originType == OBJECT_TYPE
+            })
+            void test() {
+                List strings = ['one', 'two']
+                loop:
+                for (Object s in strings) { }
+            }
+            test()
+        '''
+    }
+
     // GROOVY-8643
     void testForInLoopOnArray() {
         assertScript '''
@@ -295,6 +310,23 @@ class LoopsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
+    // GROOVY-11305
+    void testForInLoopOnNearlyIterable() {
+        assertScript '''
+            class C {
+                Iterator<String> iterator() {
+                    ['a','b','c'].iterator()
+                }
+            }
+            def list = []
+            def c = new C()
+            for (item in c) {
+                list.add(item.toUpperCase())
+            }
+            assert list == ['A','B','C']
+        '''
+    }
+
     // GROOVY-10651
     void testForInLoopOnRawTypeIterable() {
         assertScript '''