You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/07/18 22:14:17 UTC

[groovy] branch GROOVY_3_0_X updated (7984ab5 -> 5faf502)

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

paulk pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.


    from 7984ab5  GROOVY-7996: STC: error for mismatched closure resolve strategies (closes #1303)
     new 844530b  GROOVY-6977: add test case
     new c2e3b6a  GROOVY-5103: add test case
     new 5faf502  GROOVY-5103: allow plain star imports to pick up static inner types (closes #1318)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/codehaus/groovy/ast/ImportNode.java   |   6 +-
 .../codehaus/groovy/control/ResolveVisitor.java    |  78 ++++++-------
 src/test/gls/generics/GenericsUsageTest.groovy     | 122 ++++++++++++---------
 src/test/groovy/ImportTest.groovy                  |  52 ++++++---
 4 files changed, 144 insertions(+), 114 deletions(-)


[groovy] 01/03: GROOVY-6977: add test case

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 844530b81edf184855044f7abcbe1d5f5702278c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jul 18 11:17:36 2020 -0500

    GROOVY-6977: add test case
---
 src/test/gls/generics/GenericsUsageTest.groovy | 122 +++++++++++++++----------
 1 file changed, 72 insertions(+), 50 deletions(-)

diff --git a/src/test/gls/generics/GenericsUsageTest.groovy b/src/test/gls/generics/GenericsUsageTest.groovy
index 903cf89..214ab5d 100644
--- a/src/test/gls/generics/GenericsUsageTest.groovy
+++ b/src/test/gls/generics/GenericsUsageTest.groovy
@@ -42,25 +42,43 @@ final class GenericsUsageTest extends CompilableTestSupport {
     }
 
     void testCovariantReturn() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class A<T> {
-                T foo(T t) {1}
+                T foo(T t) { 1 }
             }
 
-            class B extends A<Long>{
-                String foo(Long l){"2"}
+            class B extends A<Long> {
+                String foo(Long l) { '2' }
             }
-        """
+        '''
 
-        assertScript """
+        // GROOVY-6977
+        assertScript '''
+            class A {
+              public <R> List<R> foo() {
+                List<R> list = new ArrayList<R>() {
+                    // ...
+                }
+                // ...
+                list
+              }
+            }
+
+            def longList = new A().<Long>foo()
+            assert longList != null
+            assert longList.empty
+        '''
+
+        assertScript '''
             class A<T> {
                 T foo(T t) {1}
             }
 
-            class B extends A<Long>{
-                Long foo(Long l){2}
+            class B extends A<Long> {
+                Long foo(Long l) { 2 }
             }
-            def b = new B();
+
+            def b = new B()
             try {
                 b.foo(new Object())
                 assert false
@@ -68,7 +86,7 @@ final class GenericsUsageTest extends CompilableTestSupport {
                 assert true
             }
             assert b.foo((Long) 1) == 2
-        """
+        '''
     }
 
     void testCovariantReturnWithInterface() {
@@ -230,45 +248,8 @@ final class GenericsUsageTest extends CompilableTestSupport {
         }
     }
 
-    private void shouldFailCompilationWithDefaultMessage(scriptText) {
-        shouldFailCompilationWithMessage scriptText, "Missing closing bracket '>' for generics types"
-    }
-
-    private void shouldFailCompilationWithMessage(scriptText, String errorMessage) {
-        shouldFailCompilationWithMessages(scriptText, [errorMessage])
-    }
-
-    private void shouldFailCompilationWithMessages(scriptText, List<String> errorMessages) {
-        try {
-            assertScript scriptText
-            fail("The script compilation should have failed as it contains generics errors, e.g. mis-matching generic brackets")
-        } catch (MultipleCompilationErrorsException mcee) {
-            def text = mcee.toString()
-            errorMessages.each {
-                assert text.contains(it)
-            }
-        }
-    }
-
-    private void shouldFailCompilationWithAnyMessage(scriptText, List<String> errorMessages) {
-        try {
-            assertScript scriptText
-            fail("The script compilation should have failed as it contains generics errors, e.g. mis-matching generic brackets")
-        } catch (MultipleCompilationErrorsException mcee) {
-            def text = mcee.toString()
-
-            for (errorMessage in errorMessages) {
-                if (text.contains(errorMessage)) {
-                    return
-                }
-            }
-
-            assert false, text + " can not match any expected error message: " + errorMessages
-        }
-    }
-
     // GROOVY-3975
-    void testGenericsInfoForClosureParameters() {
+    void testGenericsForClosureParameters() {
         def cl = { List<String> s -> }
         def type = cl.getClass().getMethod("call", List).genericParameterTypes[0]
         assert type.toString().contains("java.util.List<java.lang.String>")
@@ -277,7 +258,8 @@ final class GenericsUsageTest extends CompilableTestSupport {
         assert type.toString().contains("java.util.List<java.lang.String>")
     }
 
-    void testBoundedGenericsWithInheritanceGroovy4974() {
+    // GROOVY-4974
+    void testBoundedGenericsWithInheritance() {
         assertScript '''
             class TestGenerics {
                 static interface Z {}
@@ -300,7 +282,8 @@ final class GenericsUsageTest extends CompilableTestSupport {
         '''
     }
 
-    void testFriendlyErrorMessageForGenericsArityErrorsGroovy7865() {
+    // GROOVY-7865
+    void testFriendlyErrorMessageForGenericsArityErrors() {
         shouldFailCompilationWithMessages '''
             class MyList extends ArrayList<String, String> {}
         ''', ['(supplied with 2 type parameters)', 'which takes 1 parameter']
@@ -408,4 +391,43 @@ final class GenericsUsageTest extends CompilableTestSupport {
                 'The type String is not a valid substitute for the bounded parameter <T extends java.lang.Number>'
         ]
     }
+
+    //--------------------------------------------------------------------------
+
+    private void shouldFailCompilationWithDefaultMessage(scriptText) {
+        shouldFailCompilationWithMessage scriptText, "Missing closing bracket '>' for generics types"
+    }
+
+    private void shouldFailCompilationWithMessage(scriptText, String errorMessage) {
+        shouldFailCompilationWithMessages(scriptText, [errorMessage])
+    }
+
+    private void shouldFailCompilationWithMessages(scriptText, List<String> errorMessages) {
+        try {
+            assertScript scriptText
+            fail("The script compilation should have failed as it contains generics errors, e.g. mis-matching generic brackets")
+        } catch (MultipleCompilationErrorsException mcee) {
+            def text = mcee.toString()
+            errorMessages.each {
+                assert text.contains(it)
+            }
+        }
+    }
+
+    private void shouldFailCompilationWithAnyMessage(scriptText, List<String> errorMessages) {
+        try {
+            assertScript scriptText
+            fail("The script compilation should have failed as it contains generics errors, e.g. mis-matching generic brackets")
+        } catch (MultipleCompilationErrorsException mcee) {
+            def text = mcee.toString()
+
+            for (errorMessage in errorMessages) {
+                if (text.contains(errorMessage)) {
+                    return
+                }
+            }
+
+            assert false, text + " can not match any expected error message: " + errorMessages
+        }
+    }
 }


[groovy] 02/03: GROOVY-5103: add test case

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit c2e3b6a1af4e5a5eec041c4621397a50d6e84e37
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jul 18 12:26:30 2020 -0500

    GROOVY-5103: add test case
---
 src/test/groovy/ImportTest.groovy | 53 ++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/src/test/groovy/ImportTest.groovy b/src/test/groovy/ImportTest.groovy
index bece387..8031f61 100644
--- a/src/test/groovy/ImportTest.groovy
+++ b/src/test/groovy/ImportTest.groovy
@@ -18,28 +18,45 @@
  */
 package groovy
 
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-class ImportTest extends GroovyTestCase {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
 
+final class ImportTest {
+
+    @Test
     void testImportAll() {
-        def file = new File("foo.txt")
-        assert file instanceof File
-        assert file.getClass().name == "java.io.File"
+        assertScript '''
+            def file = new File('foo')
+            assert file instanceof File
+        '''
     }
-    
+
+    @Test
     void testImportByName() {
-        def x = [:]
-        assert x instanceof Map
-        /**
-         * For maps, map.getClass() should be used instead of map.class,
-         * when map has no member, named as "class"
-         */
-        assert x.getClass() != null
-        assert x.getClass().name.startsWith("java.util.")
-        
-        def y = [1, 2, 3]
-        assert y instanceof List
-        assert y.getClass().name.startsWith("java.util.")
+        assertScript '''
+            def map = [foo:'bar']
+            assert map instanceof Map
+        '''
+
+        assertScript '''
+            def list = [1, 2, 3]
+            assert list instanceof List
+        '''
+    }
+
+    @Test
+    void testImportStaticInnerClass() {
+        assertScript '''
+            import java.util.Map.Entry
+            Entry entry = [foo:'bar'].entrySet().first()
+        '''
+
+        // GROOVY-5103
+        shouldFail '''
+            import java.util.Map.*
+            Entry entry = [foo:'bar'].entrySet().first()
+        '''
     }
 }


[groovy] 03/03: GROOVY-5103: allow plain star imports to pick up static inner types (closes #1318)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 5faf5026ae465caf2f963655794c2157b23adf99
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jul 18 13:55:12 2020 -0500

    GROOVY-5103: allow plain star imports to pick up static inner types (closes #1318)
    
    - Java allows "import foo.Bar.*" to resolve "Baz" to "foo.Bar$Baz"
---
 .../java/org/codehaus/groovy/ast/ImportNode.java   |  6 +-
 .../codehaus/groovy/control/ResolveVisitor.java    | 78 +++++++++-------------
 src/test/groovy/ImportTest.groovy                  |  3 +-
 3 files changed, 39 insertions(+), 48 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/ImportNode.java b/src/main/java/org/codehaus/groovy/ast/ImportNode.java
index d5d3f50..406788f 100644
--- a/src/main/java/org/codehaus/groovy/ast/ImportNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ImportNode.java
@@ -25,7 +25,7 @@ import static java.util.Objects.requireNonNull;
  */
 public class ImportNode extends AnnotatedNode {
 
-    private final ClassNode type;
+    private ClassNode type;
     private final String alias;
     private final String fieldName;
     private final String packageName;
@@ -143,6 +143,10 @@ public class ImportNode extends AnnotatedNode {
         return type;
     }
 
+    public void setType(final ClassNode type) {
+        this.type = type;
+    }
+
     @Override
     public void visit(final GroovyCodeVisitor visitor) {
     }
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index dbaba62..3ae6a4f 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -727,11 +727,9 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
                 if (importNode != null && importNode != currImportNode) {
                     // static alias only for inner classes and must be at end of chain
                     ClassNode tmp = new ConstructedNestedClass(importNode.getType(), importNode.getFieldName());
-                    if (resolve(tmp, false, false, true)) {
-                        if ((tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
-                            type.setRedirect(tmp.redirect());
-                            return true;
-                        }
+                    if (resolve(tmp, false, false, true) && (tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
+                        type.setRedirect(tmp.redirect());
+                        return true;
                     }
                 }
             }
@@ -811,55 +809,47 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
                 // check package this class is defined in. The usage of ConstructedClassWithPackage here
                 // means, that the module package will not be involved when the
                 // compiler tries to find an inner class.
-                ConstructedClassWithPackage tmp =  new ConstructedClassWithPackage(module.getPackageName(), name);
+                ClassNode tmp =  new ConstructedClassWithPackage(module.getPackageName(), name);
                 if (resolve(tmp, false, false, false)) {
                     ambiguousClass(type, tmp, name);
-                    type.setRedirect(tmp.redirect());
                     return true;
                 }
             }
-
-            // check module static imports (for static inner classes)
+            // check static imports for static inner types
             for (ImportNode importNode : module.getStaticImports().values()) {
                 if (importNode.getFieldName().equals(name)) {
                     ClassNode tmp = new ConstructedNestedClass(importNode.getType(), name);
-                    if (resolve(tmp, false, false, true)) {
-                        if ((tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
-                            type.setRedirect(tmp.redirect());
-                            return true;
-                        }
+                    if (resolve(tmp, false, false, true) && (tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
+                        type.setRedirect(tmp.redirect());
+                        return true;
                     }
                 }
             }
-
-            // check module node import packages
-            for (ImportNode importNode : module.getStarImports()) {
-                String packagePrefix = importNode.getPackageName();
-                // We limit the inner class lookups here by using ConstructedClassWithPackage.
-                // This way only the name will change, the packagePrefix will
-                // not be included in the lookup. The case where the
-                // packagePrefix is really a class is handled elsewhere.
-                ConstructedClassWithPackage tmp = new ConstructedClassWithPackage(packagePrefix, name);
-                if (resolve(tmp, false, false, true)) {
+            for (ImportNode importNode : module.getStaticStarImports().values()) {
+                ClassNode tmp = new ConstructedNestedClass(importNode.getType(), name);
+                if (resolve(tmp, false, false, true) && (tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
                     ambiguousClass(type, tmp, name);
-                    type.setRedirect(tmp.redirect());
                     return true;
                 }
             }
-
-            // check for star imports (import static pkg.Outer.*) matching static inner classes
-            for (ImportNode importNode : module.getStaticStarImports().values()) {
-                ClassNode tmp = new ConstructedNestedClass(importNode.getType(), name);
-                if (resolve(tmp, false, false, true)) {
-                    if ((tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
+            // check star imports ("import foo.*" or "import foo.Bar.*")
+            for (ImportNode importNode : module.getStarImports()) {
+                if (importNode.getType() != null) {
+                    ClassNode tmp = new ConstructedNestedClass(importNode.getType(), name);
+                    if (resolve(tmp, false, false, true) && (tmp.getModifiers() & Opcodes.ACC_STATIC) != 0) {
+                        ambiguousClass(type, tmp, name);
+                        return true;
+                    }
+                } else {
+                    ClassNode tmp = new ConstructedClassWithPackage(importNode.getPackageName(), name);
+                    if (resolve(tmp, false, false, true)) {
                         ambiguousClass(type, tmp, name);
-                        type.setRedirect(tmp.redirect());
                         return true;
                     }
                 }
-
             }
         }
+
         return false;
     }
 
@@ -1450,19 +1440,17 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
                 currImportNode = null;
                 addError("unable to resolve class " + type.getName(), type);
             }
-            for (ImportNode importNode : module.getStaticStarImports().values()) {
-                ClassNode type = importNode.getType();
-                if (resolve(type, false, false, true)) continue;
-                // Maybe this type belongs in the same package as the node that is doing the
-                // static import. In that case, the package may not have been explicitly specified.
-                // Try with the node's package too. If still not found, revert to original type name.
-                if (type.getPackageName() == null && node.getPackageName() != null) {
-                    String oldTypeName = type.getName();
-                    type.setName(node.getPackageName() + "." + oldTypeName);
-                    if (resolve(type, false, false, true)) continue;
-                    type.setName(oldTypeName);
+            for (ImportNode importNode : module.getStarImports()) {
+                if (importNode.getLineNumber() > 0) {
+                    currImportNode = importNode;
+                    String importName = importNode.getPackageName();
+                    importName = importName.substring(0, importName.length()-1);
+                    ClassNode type = ClassHelper.makeWithoutCaching(importName);
+                    if (resolve(type, false, false, true)) {
+                        importNode.setType(type);
+                    }
+                    currImportNode = null;
                 }
-                addError("unable to resolve class " + type.getName(), type);
             }
             for (ImportNode importNode : module.getStaticImports().values()) {
                 ClassNode type = importNode.getType();
diff --git a/src/test/groovy/ImportTest.groovy b/src/test/groovy/ImportTest.groovy
index 8031f61..ab3750a 100644
--- a/src/test/groovy/ImportTest.groovy
+++ b/src/test/groovy/ImportTest.groovy
@@ -21,7 +21,6 @@ package groovy
 import org.junit.Test
 
 import static groovy.test.GroovyAssert.assertScript
-import static groovy.test.GroovyAssert.shouldFail
 
 final class ImportTest {
 
@@ -54,7 +53,7 @@ final class ImportTest {
         '''
 
         // GROOVY-5103
-        shouldFail '''
+        assertScript '''
             import java.util.Map.*
             Entry entry = [foo:'bar'].entrySet().first()
         '''