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 2020/07/18 18:57:42 UTC

[groovy] 01/01: GROOVY-5103: allow plain star imports to pick up static inner types

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

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

commit 31ae3ddaf6502478fdf3e8ee991c2e4c3f713705
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
    
    - 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()
         '''