groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [groovy] 01/04: Allow protected override of package scoped methods
Date Tue, 22 Oct 2019 10:43:43 GMT
This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 224b606ff9f0bb89feb2727a5ab02f5e4109b31a
Author: Tobias Gesellchen <tobias@gesellix.de>
AuthorDate: Sat Oct 19 22:46:12 2019 +0200

    Allow protected override of package scoped methods
    
    Relates to GROOVY-8651
    
    Signed-off-by: Tobias Gesellchen <tobias@gesellix.de>
---
 .../groovy/classgen/ClassCompletionVerifier.java   | 10 +++---
 ...t.groovy => MethodOverridingAllowedTest.groovy} | 36 +++++++++++-----------
 ...st.groovy => MethodOverridingDeniedTest.groovy} |  4 +--
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 4102c28..432b356 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -466,7 +466,7 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
             Parameter[] superParams = superMethod.getParameters();
             if (!hasEqualParameterTypes(params, superParams)) continue;
             if ((mn.isPrivate() && !superMethod.isPrivate())
-                    || (mn.isProtected() && !superMethod.isProtected() &&
!superMethod.isPrivate())
+                    || (mn.isProtected() && !superMethod.isProtected() &&
!superMethod.isPackageScope() && !superMethod.isPrivate())
                     || (!mn.isPrivate() && !mn.isProtected() && !mn.isPublic()
&& (superMethod.isPublic() || superMethod.isProtected()))) {
                 addWeakerAccessError(cn, mn, params, superMethod);
                 return;
@@ -704,20 +704,20 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport
{
             checkGenericsUsage(ref, node);
         }
     }
-    
+
     private void checkGenericsUsage(ASTNode ref, Parameter[] params) {
         for (Parameter p : params) {
             checkGenericsUsage(ref, p.getType());
         }
     }
-    
+
     private void checkGenericsUsage(ASTNode ref, ClassNode node) {
         if (node.isArray()) {
             checkGenericsUsage(ref, node.getComponentType());
         } else if (!node.isRedirectNode() && node.isUsingGenerics()) {
-            addError(   
+            addError(
                     "A transform used a generics containing ClassNode "+ node + " " +
-                    "for "+getRefDescriptor(ref) + 
+                    "for "+getRefDescriptor(ref) +
                     "directly. You are not supposed to do this. " +
                     "Please create a new ClassNode referring to the old ClassNode " +
                     "and use the new ClassNode instead of the old one. Otherwise " +
diff --git a/src/test/gls/classes/methods/MethodOverridingTest.groovy b/src/test/gls/classes/methods/MethodOverridingAllowedTest.groovy
similarity index 55%
copy from src/test/gls/classes/methods/MethodOverridingTest.groovy
copy to src/test/gls/classes/methods/MethodOverridingAllowedTest.groovy
index 182c071..b244835 100644
--- a/src/test/gls/classes/methods/MethodOverridingTest.groovy
+++ b/src/test/gls/classes/methods/MethodOverridingAllowedTest.groovy
@@ -18,47 +18,47 @@
  */
 package gls.classes.methods
 
-import org.codehaus.groovy.control.CompilationFailedException
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.junit.runners.Parameterized
 
-import static groovy.test.GroovyAssert.shouldFail
+import static groovy.test.GroovyAssert.assertScript
 
 @RunWith(Parameterized)
-class MethodOverridingTest {
+class MethodOverridingAllowedTest {
     String baseVisibility, childVisibility
 
-    MethodOverridingTest(String baseVisibility, String childVisibility) {
+    MethodOverridingAllowedTest(String baseVisibility, String childVisibility) {
         this.baseVisibility = baseVisibility
         this.childVisibility = childVisibility
     }
 
-    @Parameterized.Parameters(name = '{1} should not override {0}')
+    @Parameterized.Parameters(name = '{1} may override {0}')
     static data() {
         [
-                ['public', 'private'],
-                ['public', '@groovy.transform.PackageScope'],
-                ['public', 'protected'],
-                ['protected', 'private'],
-                ['protected', '@groovy.transform.PackageScope'],
-                ['@groovy.transform.PackageScope', 'private'],
+            ['private', 'private'],
+            ['private', '@groovy.transform.PackageScope'],
+            ['private', 'protected'],
+            ['private', 'public'],
+            ['@groovy.transform.PackageScope', '@groovy.transform.PackageScope'],
+            ['@groovy.transform.PackageScope', 'protected'],
+            ['@groovy.transform.PackageScope', 'public'],
+            ['protected', 'protected'],
+            ['protected', 'public'],
+            ['public', 'public'],
         ]*.toArray()
     }
 
     @Test
-    void 'weaker access must not override stronger'() {
-        def ex = shouldFail(CompilationFailedException,"""
-            abstract class Base {
-                $baseVisibility abstract myMethod()
+    void 'stronger access may override weaker'() {
+        assertScript("""
+            class Base {
+                $baseVisibility myMethod() { true }
             }
             class Child extends Base {
                 $childVisibility myMethod() { true }
             }
             assert new Child() != null
         """)
-        assert ex.message.contains('cannot override myMethod in Base')
-        assert ex.message.contains('attempting to assign weaker access privileges')
-        assert ex.message.contains("was ${baseVisibility.contains('PackageScope') ? 'package-private'
: baseVisibility}")
     }
 }
diff --git a/src/test/gls/classes/methods/MethodOverridingTest.groovy b/src/test/gls/classes/methods/MethodOverridingDeniedTest.groovy
similarity index 95%
rename from src/test/gls/classes/methods/MethodOverridingTest.groovy
rename to src/test/gls/classes/methods/MethodOverridingDeniedTest.groovy
index 182c071..48da2d4 100644
--- a/src/test/gls/classes/methods/MethodOverridingTest.groovy
+++ b/src/test/gls/classes/methods/MethodOverridingDeniedTest.groovy
@@ -26,10 +26,10 @@ import org.junit.runners.Parameterized
 import static groovy.test.GroovyAssert.shouldFail
 
 @RunWith(Parameterized)
-class MethodOverridingTest {
+class MethodOverridingDeniedTest {
     String baseVisibility, childVisibility
 
-    MethodOverridingTest(String baseVisibility, String childVisibility) {
+    MethodOverridingDeniedTest(String baseVisibility, String childVisibility) {
         this.baseVisibility = baseVisibility
         this.childVisibility = childVisibility
     }


Mime
View raw message