groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jwagenleit...@apache.org
Subject [2/2] groovy git commit: GROOVY-8509: SC error call to protected method from same package (closes #707)
Date Fri, 18 May 2018 14:22:03 GMT
GROOVY-8509: SC error call to protected method from same package (closes #707)

Also includes a change to the fix introduced by GROOVY-7883 (b1d1232770aa)
to prevent filtering protected methods in the StaticTypeCheckingVisitor if
caller is in the same package. From the same commit, removes the test
Groovy7883Bug#test3 because it should compile and the new test
introduced for GROOVY-8509 in MethodCallsStaticCompilation takes it place.


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8291749b
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8291749b
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8291749b

Branch: refs/heads/GROOVY_2_6_X
Commit: 8291749bbb473a6cad38712ffc9fdb553fcfb9ac
Parents: edf0cfd
Author: John Wagenleitner <jwagenleitner@apache.org>
Authored: Wed May 16 08:34:48 2018 -0700
Committer: John Wagenleitner <jwagenleitner@apache.org>
Committed: Fri May 18 07:20:22 2018 -0700

----------------------------------------------------------------------
 .../apache/groovy/ast/tools/ClassNodeUtils.java   | 16 ++++++++++++++++
 .../classgen/asm/sc/StaticInvocationWriter.java   |  2 ++
 .../transform/stc/StaticTypeCheckingVisitor.java  | 13 +++++--------
 .../sc/MethodCallsStaticCompilationTest.groovy    | 18 ++++++++++++++++++
 .../classgen/asm/sc/bugs/Groovy7883Bug.groovy     | 18 ------------------
 5 files changed, 41 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/8291749b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
index 51cab66..0e1a30a 100644
--- a/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
+++ b/src/main/java/org/apache/groovy/ast/tools/ClassNodeUtils.java
@@ -310,4 +310,20 @@ public class ClassNodeUtils {
         }
         return false;
     }
+
+    /**
+     * Determine if the given ClassNode values have the same package name.
+     *
+     * @param first a ClassNode
+     * @param second a ClassNode
+     * @return true if both given nodes have the same package name
+     * @throws NullPointerException if either of the given nodes are null
+     */
+    public static boolean samePackageName(ClassNode first, ClassNode second) {
+        String firstPackage = first.getPackageName();
+        String secondPackage = second.getPackageName();
+        return (firstPackage == secondPackage)
+                || (firstPackage != null && firstPackage.equals(secondPackage));
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/8291749b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 6917573..962ca3e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -71,6 +71,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
 import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.OBJECT_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
@@ -343,6 +344,7 @@ public class StaticInvocationWriter extends InvocationWriter {
                     isThisOrSuper = ((VariableExpression) receiver).isThisExpression() ||
((VariableExpression) receiver).isSuperExpression();
                 }
                 if (!implicitThis && !isThisOrSuper
+                        && !samePackageName(node, classNode)
                         && StaticTypeCheckingSupport.implementsInterfaceOrIsSubclassOf(node,target.getDeclaringClass()))
{
                     ASTNode src = receiver==null?args:receiver;
                     controller.getSourceUnit().addError(

http://git-wip-us.apache.org/repos/asf/groovy/blob/8291749b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 86e6718..5369ffe 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -122,6 +122,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static org.apache.groovy.ast.tools.ClassNodeUtils.samePackageName;
 import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.BigInteger_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.Boolean_TYPE;
@@ -4474,10 +4475,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
             if (methodNode.isPrivate() && !enclosingClassNode.equals(declaringClass))
{
                 continue;
             }
-            if (methodNode.isProtected() && !enclosingClassNode.isDerivedFrom(declaringClass))
{
+            if (methodNode.isProtected()
+                    && !enclosingClassNode.isDerivedFrom(declaringClass)
+                    && !samePackageName(enclosingClassNode, declaringClass)) {
                 continue;
             }
-            if (methodNode.isPackageScope() && !getPackageName(enclosingClassNode).equals(getPackageName(declaringClass)))
{
+            if (methodNode.isPackageScope() && !samePackageName(enclosingClassNode,
declaringClass)) {
                 continue;
             }
 
@@ -4487,12 +4490,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         return result;
     }
 
-    private static String getPackageName(ClassNode cn) {
-        String name = cn.getPackageName();
-
-        return null == name ? "" : name;
-    }
-
     /**
      * Given a method name and a prefix, returns the name of the property that should be
looked up,
      * following the java beans rules. For example, "getName" would return "name", while

http://git-wip-us.apache.org/repos/asf/groovy/blob/8291749b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
index b0266d2..40d6cd0 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
@@ -259,6 +259,24 @@ import groovy.transform.TypeCheckingMode//import org.codehaus.groovy.classgen.as
         '''
     }
 
+    //GROOVY-8509
+    void testProtectedCallFromClassInSamePackage() {
+        assertScript '''
+            package org.foo
+
+            class A {
+                protected A() {}
+                protected int m() { 123 }
+            }
+            class B {
+                int test() {
+                    new A().m()
+                }
+            }
+            assert new B().test() == 123
+        '''
+    }
+
     public static class Base {
         protected int foo() {
             123

http://git-wip-us.apache.org/repos/asf/groovy/blob/8291749b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
index a40fe4e..94d287f 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
@@ -53,24 +53,6 @@ class Groovy7883Bug extends GroovyTestCase {
         assert errMsg.contains('[Static type checking] - Cannot find matching method A#doIt()')
     }
 
-    void test3() {
-        def errMsg = shouldFail '''
-        @groovy.transform.CompileStatic
-        class A {
-            protected void doIt() {}
-        }
-        
-        @groovy.transform.CompileStatic
-        class B {
-            public void m() { new A().doIt() }
-        }
-        
-        new B().m()
-        '''
-
-        assert errMsg.contains('[Static type checking] - Cannot find matching method A#doIt()')
-    }
-
     void test4() {
         def errMsg = shouldFail '''
         @groovy.transform.CompileStatic


Mime
View raw message