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 09:06:53 UTC
[groovy] branch GROOVY_2_5_X updated (b2e1bf6 -> 3113198)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from b2e1bf6 refactor generics tests
new 3bdc29d test refactor
new 9840377 GROOVY-8651: Method override weaker access check does not fully account for package-private visibility
new 3113198 refactor: don't weaken method access during override
The 3 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:
.../groovy/classgen/ClassCompletionVerifier.java | 7 ++-
.../classes/methods/MethodOverridingTest.groovy | 64 ++++++++++++++++++++++
.../methods}/RepetitiveMethodTest.groovy | 26 +++++----
src/test/groovy/bugs/Groovy4607Bug.groovy | 53 ------------------
src/test/groovy/bugs/Groovy4922Bug.groovy | 5 +-
5 files changed, 85 insertions(+), 70 deletions(-)
create mode 100644 src/test/gls/classes/methods/MethodOverridingTest.groovy
rename src/test/gls/{ch08/s04 => classes/methods}/RepetitiveMethodTest.groovy (73%)
delete mode 100644 src/test/groovy/bugs/Groovy4607Bug.groovy
[groovy] 02/03: GROOVY-8651: Method override weaker access check
does not fully account for package-private visibility
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 9840377110d143f4f1267d2df638dbe70a47d862
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 02c82d4..bd01683 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
[groovy] 03/03: refactor: don't weaken method access during override
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 31131986f37a4e73841d6ada105216caefe513ff
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Apr 30 18:22:53 2019 +1000
refactor: don't weaken method access during override
---
src/test/groovy/bugs/Groovy4922Bug.groovy | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/test/groovy/bugs/Groovy4922Bug.groovy b/src/test/groovy/bugs/Groovy4922Bug.groovy
index 9be282d..1767f33 100644
--- a/src/test/groovy/bugs/Groovy4922Bug.groovy
+++ b/src/test/groovy/bugs/Groovy4922Bug.groovy
@@ -18,7 +18,6 @@
*/
package groovy.bugs
-
class Groovy4922Bug extends GroovyTestCase {
void testShouldNotThrowStackOverflow() {
assertScript """
@@ -34,7 +33,7 @@ class Groovy4922Bug extends GroovyTestCase {
}
}
class Groovy4922BugChild extends Groovy4922BugSupport {
- protected void someMethod(String parameter) {
+ void someMethod(String parameter) {
super.someMethod(parameter)
}
}
@@ -42,7 +41,5 @@ class Groovy4922Bug extends GroovyTestCase {
child.someMethod("value")
assert child.support == "value"
"""
-
}
}
-
[groovy] 01/03: test refactor
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3bdc29dcdbd39a00997eeca2f2d1bbb81b4c0284
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Apr 30 16:57:51 2019 +1000
test refactor
---
.../methods}/RepetitiveMethodTest.groovy | 26 +++++++++++++---------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/test/gls/ch08/s04/RepetitiveMethodTest.groovy b/src/test/gls/classes/methods/RepetitiveMethodTest.groovy
similarity index 73%
rename from src/test/gls/ch08/s04/RepetitiveMethodTest.groovy
rename to src/test/gls/classes/methods/RepetitiveMethodTest.groovy
index a583f3e..b724e2d 100644
--- a/src/test/gls/ch08/s04/RepetitiveMethodTest.groovy
+++ b/src/test/gls/classes/methods/RepetitiveMethodTest.groovy
@@ -16,30 +16,36 @@
* specific language governing permissions and limitations
* under the License.
*/
-package gls.ch08.s04
+package gls.classes.methods
import gls.CompilableTestSupport
class RepetitiveMethodTest extends CompilableTestSupport {
void testRepetitiveMethod() {
- def text = """
+ def message = shouldNotCompile('''
class A {
void foo() {}
void foo() {}
}
- """
- shouldNotCompile(text)
+ ''')
+ assert message.contains('duplicates another method of the same signature')
}
- void testRepetitiveMethodsCreationForBooleanProperties() {
- shouldCompile """
- class BoolTest {
+ void testRepetitiveMethodsAllowedForProperties() {
+ shouldCompile '''
+ class PropertyOverride {
boolean success
+ int num
+
boolean isSuccess() {
- return success
+ success
+ }
+
+ int getNum() {
+ num
}
}
- """
+ '''
}
-}
\ No newline at end of file
+}