You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/02/22 09:22:48 UTC

[groovy] branch danielsun/tweak-resolving-further updated (090ac11 -> 3f81b9c)

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

sunlan pushed a change to branch danielsun/tweak-resolving-further
in repository https://gitbox.apache.org/repos/asf/groovy.git.


 discard 090ac11  GROOVY-9416: Avoid unnecessary looking up non default import classes when resolving types
     new 3f81b9c  GROOVY-9416: Avoid unnecessary looking up non default import classes when resolving types

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (090ac11)
            \
             N -- N -- N   refs/heads/danielsun/tweak-resolving-further (3f81b9c)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 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:
 .../org/codehaus/groovy/vmplugin/v9/Java9.java     | 17 ++++++--
 src/test/groovy/bugs/Groovy2558Bug.groovy          |  1 +
 .../codehaus/groovy/runtime/FileAppendTest.groovy  |  1 +
 .../groovy/runtime/WriterAppendTest.groovy         |  1 +
 .../groovy/vmplugin/v9/ClassFinderTest.groovy      | 45 ++--------------------
 5 files changed, 20 insertions(+), 45 deletions(-)


[groovy] 01/01: GROOVY-9416: Avoid unnecessary looking up non default import classes when resolving types

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

sunlan pushed a commit to branch danielsun/tweak-resolving-further
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 3f81b9ceb17f6317808b947352572a8377884514
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Feb 22 17:22:36 2020 +0800

    GROOVY-9416: Avoid unnecessary looking up non default import classes when resolving types
---
 .../codehaus/groovy/control/ResolveVisitor.java    |  5 ++-
 .../org/codehaus/groovy/vmplugin/VMPlugin.java     | 12 ++++++
 .../org/codehaus/groovy/vmplugin/v9/Java9.java     | 24 +++++++++--
 src/test/groovy/bugs/Groovy2558Bug.groovy          |  1 +
 .../codehaus/groovy/runtime/FileAppendTest.groovy  |  1 +
 .../groovy/runtime/WriterAppendTest.groovy         |  1 +
 .../groovy/vmplugin/v9/ClassFinderTest.groovy      | 49 ++++------------------
 7 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index 4395dc8..de43cf9 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -621,9 +621,10 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
                 }
             }
 
-            if (resolveFromDefaultImports(type, DEFAULT_IMPORTS)) {
+            if (VMPluginFactory.getPlugin().resolveFromDefaultImports(this, type)) {
                 return true;
             }
+
             if (BIGINTEGER_STR.equals(typeName)) {
                 type.setRedirect(ClassHelper.BigInteger_TYPE);
                 return true;
@@ -642,7 +643,7 @@ public class ResolveVisitor extends ClassCodeExpressionTransformer {
         DEFAULT_IMPORT_CLASS_AND_PACKAGES_CACHE.putAll(defaultImportClasses);
     }
 
-    protected boolean resolveFromDefaultImports(final ClassNode type, final String[] packagePrefixes) {
+    public boolean resolveFromDefaultImports(final ClassNode type, final String[] packagePrefixes) {
         String typeName = type.getName();
 
         for (String packagePrefix : packagePrefixes) {
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/VMPlugin.java b/src/main/java/org/codehaus/groovy/vmplugin/VMPlugin.java
index 7ef1065..7165bdd 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/VMPlugin.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/VMPlugin.java
@@ -23,6 +23,7 @@ import groovy.lang.MetaMethod;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.CompileUnit;
+import org.codehaus.groovy.control.ResolveVisitor;
 
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Method;
@@ -125,4 +126,15 @@ public interface VMPlugin {
     default Map<String, Set<String>> getDefaultImportClasses(String[] packageNames) {
         return Collections.emptyMap();
     }
+
+    /**
+     * Resolve types from default imported packages
+     *
+     * @param resolveVisitor the resolve visitor
+     * @param type the type to resolve
+     * @return resolved or not
+     */
+    default boolean resolveFromDefaultImports(ResolveVisitor resolveVisitor, final ClassNode type) {
+        return resolveVisitor.resolveFromDefaultImports(type, ResolveVisitor.DEFAULT_IMPORTS);
+    }
 }
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java b/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
index 60b354d..7c1ab80 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java
@@ -18,13 +18,15 @@
  */
 package org.codehaus.groovy.vmplugin.v9;
 
+import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyRuntimeException;
-import groovy.lang.GroovySystem;
 import groovy.lang.MetaClass;
 import groovy.lang.MetaMethod;
 import groovy.lang.Tuple;
 import groovy.lang.Tuple2;
 import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.control.ResolveVisitor;
 import org.codehaus.groovy.reflection.CachedClass;
 import org.codehaus.groovy.reflection.CachedMethod;
 import org.codehaus.groovy.reflection.ReflectionUtils;
@@ -81,10 +83,21 @@ public class Java9 extends Java8 {
         result.putAll(doFindClasses(URI.create("jrt:/modules/java.base/"), "java", javaPns));
 
         try {
-            URI gsLocation = DefaultGroovyMethods.getLocation(GroovySystem.class).toURI();
+            GroovyClassLoader groovyClassLoader = new GroovyClassLoader();
+
+            Class<?> groovySystemClass = groovyClassLoader.loadClass("groovy.lang.GroovySystem"); // java class
+            URI gsLocation = DefaultGroovyMethods.getLocation(groovySystemClass).toURI();
             result.putAll(doFindClasses(gsLocation, "groovy", groovyPns));
+
+            Class<?> grapeIvyClass = groovyClassLoader.loadClass("groovy.grape.GrapeIvy"); // groovy class
+            URI giLocation = DefaultGroovyMethods.getLocation(grapeIvyClass).toURI();
+            if (!giLocation.equals(gsLocation)) {
+                // in user production environment, Groovy core classes, e.g. `GroovySystem`(java class) and `GrapeIvy`(groovy class) are all packaged in the groovy core jar file,
+                // but in Groovy development environment, Groovy core classes are distributed in different directories
+                result.putAll(doFindClasses(giLocation, "groovy", groovyPns));
+            }
         } catch (Exception e) {
-            System.err.println("[WARNING] Failed to get default imported groovy classes: " + e.getMessage());
+            throw new GroovyBugError("[WARNING] Failed to get default imported groovy classes", e);
         }
 
         return result;
@@ -106,6 +119,11 @@ public class Java9 extends Java8 {
         return result;
     }
 
+    @Override
+    public boolean resolveFromDefaultImports(ResolveVisitor resolveVisitor, final ClassNode type) {
+        return false;
+    }
+
     private static class LookupHolder {
         private static final Method PRIVATE_LOOKUP;
         private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
diff --git a/src/test/groovy/bugs/Groovy2558Bug.groovy b/src/test/groovy/bugs/Groovy2558Bug.groovy
index 88abe83..a7490c3 100644
--- a/src/test/groovy/bugs/Groovy2558Bug.groovy
+++ b/src/test/groovy/bugs/Groovy2558Bug.groovy
@@ -19,6 +19,7 @@
 package groovy.bugs
 
 import groovy.test.GroovyTestCase
+import groovy.util.Person
 
 class Groovy2558Bug extends GroovyTestCase {
     void testMe () {
diff --git a/src/test/org/codehaus/groovy/runtime/FileAppendTest.groovy b/src/test/org/codehaus/groovy/runtime/FileAppendTest.groovy
index f67b240..154c0fb 100644
--- a/src/test/org/codehaus/groovy/runtime/FileAppendTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/FileAppendTest.groovy
@@ -19,6 +19,7 @@
 package org.codehaus.groovy.runtime
 
 import groovy.test.GroovyTestCase
+import groovy.lang.DummyGStringBase
 
 /**
  * Test File append and leftShift DGM methods
diff --git a/src/test/org/codehaus/groovy/runtime/WriterAppendTest.groovy b/src/test/org/codehaus/groovy/runtime/WriterAppendTest.groovy
index 387e55f..03e517e 100644
--- a/src/test/org/codehaus/groovy/runtime/WriterAppendTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/WriterAppendTest.groovy
@@ -19,6 +19,7 @@
 package org.codehaus.groovy.runtime
 
 import groovy.test.GroovyTestCase
+import groovy.lang.DummyGStringBase
 
 /**
  * Test Writer append and leftShift DGM methods
diff --git a/src/test/org/codehaus/groovy/vmplugin/v9/ClassFinderTest.groovy b/src/test/org/codehaus/groovy/vmplugin/v9/ClassFinderTest.groovy
index 37ee4b0..4f4f467 100644
--- a/src/test/org/codehaus/groovy/vmplugin/v9/ClassFinderTest.groovy
+++ b/src/test/org/codehaus/groovy/vmplugin/v9/ClassFinderTest.groovy
@@ -18,8 +18,8 @@
  */
 package org.codehaus.groovy.vmplugin.v9
 
+import groovy.grape.GrapeIvy
 import org.codehaus.groovy.control.ResolveVisitor
-import org.codehaus.groovy.runtime.DefaultGroovyMethods
 import org.codehaus.groovy.vmplugin.VMPluginFactory
 import org.junit.Test
 
@@ -41,11 +41,17 @@ class ClassFinderTest {
 
     @Test
     void findGroovyClass3() {
-        Map<String, Set<String>> result = ClassFinder.find(org.codehaus.groovy.control.ResolveVisitor.location.toURI(), "org/codehaus/groovy/control")
+        Map<String, Set<String>> result = ClassFinder.find(GroovySystem.location.toURI(), "org/codehaus/groovy/control")
         assert ["org/codehaus/groovy/control"] == result.get("ResolveVisitor")?.toList()
     }
 
     @Test
+    void findGroovyClass4() {
+        Map<String, Set<String>> result = ClassFinder.find(GrapeIvy.location.toURI(), "groovy/util")
+        assert ["groovy/util"] == result.get("ConfigSlurper")?.toList()
+    }
+
+    @Test
     void findGroovyClassRecursive() {
         Map<String, Set<String>> result = ClassFinder.find(GroovySystem.location.toURI(), "groovy/lang", true)
         assert ["groovy/lang"] == result.get("GString")?.toList()
@@ -102,44 +108,5 @@ class ClassFinderTest {
         Map<String, Set<String>> r1 = VMPluginFactory.getPlugin().getDefaultImportClasses(ResolveVisitor.DEFAULT_IMPORTS) as TreeMap<String, Set<String>>
 
         assert (ResolveVisitor.DEFAULT_IMPORTS as List).sort() == r1.values().stream().flatMap(e -> e.stream()).collect(Collectors.toSet()).sort()
-
-        def r2 = [:] as TreeMap
-        URI gsLocation = null;
-        try {
-            gsLocation = DefaultGroovyMethods.getLocation(GroovySystem.class).toURI();
-        } catch (URISyntaxException e) {
-            e.printStackTrace();
-        }
-
-        for (String prefix : ResolveVisitor.DEFAULT_IMPORTS) {
-            String pn = prefix.substring(0, prefix.length() - 1).replace('.', '/');
-
-            Map<String, Set<String>> map = Collections.emptyMap();
-            if (pn.startsWith("java/")) {
-                map = ClassFinder.find(URI.create("jrt:/modules/java.base/"), pn);
-            } else if (pn.startsWith("groovy/")) {
-                if (null != gsLocation) {
-                    map = ClassFinder.find(gsLocation, pn);
-                }
-            }
-
-            map = map.entrySet().stream()
-                    .collect(
-                            Collectors.toMap(
-                                    (Map.Entry entry) -> entry.getKey(),
-                                    (Map.Entry entry) -> entry.getValue().stream()
-                                            .map(e -> e.replace('/', '.') + ".")
-                                            .collect(Collectors.toSet())
-                            )
-                    );
-
-            r2.putAll(map);
-        }
-
-        assert r1.size() == r2.size()
-        assert r1.keySet() == r2.keySet()
-        r1.keySet().each { c1 ->
-            assert (r1.get(c1) as TreeSet) == (r2.get(c1) as TreeSet)
-        }
     }
 }