You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2018/05/18 14:18:40 UTC
[2/2] groovy git commit: GROOVY-8509: SC error call to protected
method from same package (closes #707)
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/a3d97de4
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/a3d97de4
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/a3d97de4
Branch: refs/heads/master
Commit: a3d97de472d71f412c2fb245e7dff1ab7e6594b5
Parents: c7ae717
Author: John Wagenleitner <jw...@apache.org>
Authored: Wed May 16 08:34:48 2018 -0700
Committer: John Wagenleitner <jw...@apache.org>
Committed: Fri May 18 07:14:46 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/a3d97de4/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/a3d97de4/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 b44c544..6f4d948 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/a3d97de4/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 5750d7c..de8af20 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/a3d97de4/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/a3d97de4/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