You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/04/30 08:02:21 UTC
[groovy] 02/02: GROOVY-8651: Method override weaker access check
does not fully account for package-private visibility
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 6578021bce3f2319c0fa6998fdc7a09d9a373d09
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Apr 30 18:02:11 2019 +1000
GROOVY-8651: Method override weaker access check does not fully account for package-private visibility
---
.../groovy/classgen/ClassCompletionVerifier.java | 7 ++-
.../classes/methods/MethodOverridingTest.groovy | 64 ++++++++++++++++++++++
src/test/groovy/bugs/Groovy4607Bug.groovy | 53 ------------------
3 files changed, 68 insertions(+), 56 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 7f2d933..f08cbbd 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -411,7 +411,7 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
msg.append(" in ");
msg.append(superMethod.getDeclaringClass().getName());
msg.append("; attempting to assign weaker access privileges; was ");
- msg.append(superMethod.isPublic() ? "public" : "protected");
+ msg.append(superMethod.isPublic() ? "public" : (superMethod.isProtected() ? "protected" : "package-private"));
addError(msg.toString(), method);
}
@@ -463,8 +463,9 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
for (MethodNode superMethod : cn.getSuperClass().getMethods(mn.getName())) {
Parameter[] superParams = superMethod.getParameters();
if (!hasEqualParameterTypes(params, superParams)) continue;
- if ((mn.isPrivate() && !superMethod.isPrivate()) ||
- (mn.isProtected() && superMethod.isPublic())) {
+ if ((mn.isPrivate() && !superMethod.isPrivate())
+ || (mn.isProtected() && !superMethod.isProtected() && !superMethod.isPrivate())
+ || (!mn.isPrivate() && !mn.isProtected() && !mn.isPublic() && (superMethod.isPublic() || superMethod.isProtected()))) {
addWeakerAccessError(cn, mn, params, superMethod);
return;
}
diff --git a/src/test/gls/classes/methods/MethodOverridingTest.groovy b/src/test/gls/classes/methods/MethodOverridingTest.groovy
new file mode 100644
index 0000000..182c071
--- /dev/null
+++ b/src/test/gls/classes/methods/MethodOverridingTest.groovy
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+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
+
+@RunWith(Parameterized)
+class MethodOverridingTest {
+ String baseVisibility, childVisibility
+
+ MethodOverridingTest(String baseVisibility, String childVisibility) {
+ this.baseVisibility = baseVisibility
+ this.childVisibility = childVisibility
+ }
+
+ @Parameterized.Parameters(name = '{1} should not override {0}')
+ static data() {
+ [
+ ['public', 'private'],
+ ['public', '@groovy.transform.PackageScope'],
+ ['public', 'protected'],
+ ['protected', 'private'],
+ ['protected', '@groovy.transform.PackageScope'],
+ ['@groovy.transform.PackageScope', 'private'],
+ ]*.toArray()
+ }
+
+ @Test
+ void 'weaker access must not override stronger'() {
+ def ex = shouldFail(CompilationFailedException,"""
+ abstract class Base {
+ $baseVisibility abstract myMethod()
+ }
+ 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/groovy/bugs/Groovy4607Bug.groovy b/src/test/groovy/bugs/Groovy4607Bug.groovy
deleted file mode 100644
index 0edd720..0000000
--- a/src/test/groovy/bugs/Groovy4607Bug.groovy
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package groovy.bugs
-
-import gls.CompilableTestSupport
-
-class Groovy4607Bug extends CompilableTestSupport {
- void testProtectedOverridingPublic() {
- def errorMsg = shouldNotCompile('''
- abstract class Super {
- public abstract myMethod()
- }
- class Child extends Super {
- protected myMethod() { true }
- }
- assert new Child() != null
- ''')
- assert errorMsg.contains('cannot override myMethod in Super')
- assert errorMsg.contains('attempting to assign weaker access privileges')
- assert errorMsg.contains('was public')
- }
-
- void testPrivateOverridingProtected() {
- def errorMsg = shouldNotCompile('''
- class Super {
- protected myMethod(int i, int j) { i + j }
- }
- class Child extends Super {
- private myMethod(int i, int j) { i - j }
- }
- assert new Child() != null
- ''')
- assert errorMsg.contains('cannot override myMethod in Super')
- assert errorMsg.contains('attempting to assign weaker access privileges')
- assert errorMsg.contains('was protected')
- }
-}
\ No newline at end of file