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 10:00:46 UTC

[groovy] branch danielsun/tweak-resolving-further updated (c9665c6 -> f01d3a3)

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 c9665c6  GROOVY-9416: Avoid unnecessary looking up non default import classes when resolving types
     new f01d3a3  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   (c9665c6)
            \
             N -- N -- N   refs/heads/danielsun/tweak-resolving-further (f01d3a3)

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:
 src/test/groovy/lang/ScriptPrintTest.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


[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 f01d3a3e7c488503b91881f17471c06e6fb5fff5
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Feb 22 18:00:33 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/GroovyMethodsTest.groovy           |  1 +
 src/test/groovy/PropertyTest.groovy                |  1 +
 src/test/groovy/bugs/Groovy2558Bug.groovy          |  1 +
 src/test/groovy/lang/ScriptPrintTest.java          |  2 +-
 .../operator/MyColorOperatorOverloadingTest.groovy |  1 +
 .../codehaus/groovy/runtime/FileAppendTest.groovy  |  1 +
 .../groovy/runtime/WriterAppendTest.groovy         |  1 +
 .../groovy/vmplugin/v9/ClassFinderTest.groovy      | 49 ++++------------------
 .../groovy/groovy/swing/GroovySwingTestCase.groovy |  1 +
 .../groovy/groovy/swing/SwingBuilderTest.groovy    |  1 +
 13 files changed, 53 insertions(+), 47 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/GroovyMethodsTest.groovy b/src/test/groovy/GroovyMethodsTest.groovy
index 0c9d5d3..adcb7d7 100644
--- a/src/test/groovy/GroovyMethodsTest.groovy
+++ b/src/test/groovy/GroovyMethodsTest.groovy
@@ -24,6 +24,7 @@ import java.awt.Dimension
 import java.nio.CharBuffer
 import java.util.concurrent.LinkedBlockingQueue
 import org.codehaus.groovy.util.StringUtil
+import groovy.util.HeadlessTestSupport
 
 /** 
  * Tests various GDK methods
diff --git a/src/test/groovy/PropertyTest.groovy b/src/test/groovy/PropertyTest.groovy
index 39dafd0..48c3647 100644
--- a/src/test/groovy/PropertyTest.groovy
+++ b/src/test/groovy/PropertyTest.groovy
@@ -19,6 +19,7 @@
 package groovy
 
 import groovy.test.GroovyTestCase
+import groovy.util.HeadlessTestSupport
 
 /**
  * Tests the use of properties in Groovy
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/groovy/lang/ScriptPrintTest.java b/src/test/groovy/lang/ScriptPrintTest.java
index dde96c5..a846348 100644
--- a/src/test/groovy/lang/ScriptPrintTest.java
+++ b/src/test/groovy/lang/ScriptPrintTest.java
@@ -24,7 +24,7 @@ public class ScriptPrintTest extends TestSupport {
 
     public void testScriptWithCustomPrintln() throws Exception {
         assertScript(
-                "out = new MockWriter(); println(); assert out.output == 'println()', 'value of output is: ' + out.output\n"
+                "out = new groovy.lang.MockWriter(); println(); assert out.output == 'println()', 'value of output is: ' + out.output\n"
                         + "print('hey'); assert out.output == 'print(hey)' , 'value is: ' + out.output\n"
                         + "println('hey'); assert out.output == 'println(hey)', 'value is: ' + out.output\n");
     }
diff --git a/src/test/groovy/operator/MyColorOperatorOverloadingTest.groovy b/src/test/groovy/operator/MyColorOperatorOverloadingTest.groovy
index 3b34fe8..7451dd0 100644
--- a/src/test/groovy/operator/MyColorOperatorOverloadingTest.groovy
+++ b/src/test/groovy/operator/MyColorOperatorOverloadingTest.groovy
@@ -21,6 +21,7 @@ package groovy.operator
 import groovy.test.GroovyTestCase
 
 import static java.awt.Color.*
+import groovy.util.HeadlessTestSupport
 
 class MyColorOperatorOverloadingTest extends GroovyTestCase {
     void testAll() {
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)
-        }
     }
 }
diff --git a/subprojects/groovy-swing/src/test/groovy/groovy/swing/GroovySwingTestCase.groovy b/subprojects/groovy-swing/src/test/groovy/groovy/swing/GroovySwingTestCase.groovy
index 043a6fb..ccf063a 100644
--- a/subprojects/groovy-swing/src/test/groovy/groovy/swing/GroovySwingTestCase.groovy
+++ b/subprojects/groovy-swing/src/test/groovy/groovy/swing/GroovySwingTestCase.groovy
@@ -21,6 +21,7 @@ package groovy.swing
 import groovy.test.GroovyTestCase
 
 import javax.swing.SwingUtilities
+import groovy.util.HeadlessTestSupport
 
 abstract class GroovySwingTestCase extends GroovyTestCase {
 
diff --git a/subprojects/groovy-swing/src/test/groovy/groovy/swing/SwingBuilderTest.groovy b/subprojects/groovy-swing/src/test/groovy/groovy/swing/SwingBuilderTest.groovy
index 61537dd..910874b 100644
--- a/subprojects/groovy-swing/src/test/groovy/groovy/swing/SwingBuilderTest.groovy
+++ b/subprojects/groovy-swing/src/test/groovy/groovy/swing/SwingBuilderTest.groovy
@@ -31,6 +31,7 @@ import javax.swing.text.DateFormatter
 import javax.swing.text.NumberFormatter
 import java.awt.*
 import javax.swing.*
+import groovy.util.HeadlessTestSupport
 
 class SwingBuilderTest extends GroovySwingTestCase {