This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new dbcb40562d GROOVY-11758, GROOVY-11830: non-public super class method
shadowing
dbcb40562d is described below
commit dbcb40562d7c4dce30172bc5f0bc8f64db10589a
Author: Eric Milles <[email protected]>
AuthorDate: Sun Jan 4 18:26:00 2026 -0600
GROOVY-11758, GROOVY-11830: non-public super class method shadowing
4_0_x backport
---
.../groovy/classgen/ClassCompletionVerifier.java | 97 +++++++-----
src/test/groovy/groovy/InterfaceTest.groovy | 164 +++++++++++++++++++--
src/test/groovy/groovy/OverrideTest.groovy | 148 +++++++------------
3 files changed, 272 insertions(+), 137 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index a6745708d3..f68e2523c7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -412,9 +412,71 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
}
private void checkMethodsForWeakerAccess(final ClassNode cn) {
- for (MethodNode method : cn.getMethods()) {
- checkMethodForWeakerAccessPrivileges(method, cn);
+ for (MethodNode cnMethod : cn.getMethods()) {
+ if (!cnMethod.isPublic() && !cnMethod.isStatic()) {
+ sc: for (MethodNode scMethod :
cn.getSuperClass().getMethods(cnMethod.getName())) {
+ if (!scMethod.isStatic()
+ && !scMethod.isPrivate()
+ && (cnMethod.isPrivate()
+ || (cnMethod.isProtected() && scMethod.isPublic())
+ || (cnMethod.isPackageScope() &&
(scMethod.isPublic() || scMethod.isProtected())))) {
+ if
(ParameterUtils.parametersEqual(cnMethod.getParameters(),
scMethod.getParameters())) {
+ addWeakerAccessError(cn, cnMethod, scMethod);
+ break sc;
+ }
+ }
+ }
+ }
}
+
+ // Verifier: checks weaker access of cn's methods against abstract or
default interface methods
+
+ // GROOVY-11758, GROOVY-11830: check for non-public super class method
that hides public method
+ Map<String, MethodNode> interfaceMethods =
ClassNodeUtils.getDeclaredMethodsFromInterfaces(cn);
+ if (!interfaceMethods.isEmpty()) {
+ for (MethodNode cnMethod : cn.getMethods()) {
+ if (!cnMethod.isPrivate() && !cnMethod.isStatic()) {
+ interfaceMethods.remove(cnMethod.getTypeDescriptor());
+ }
+ }
+ for (MethodNode publicMethod : interfaceMethods.values()) {
+ for (MethodNode scMethod :
cn.getSuperClass().getMethods(publicMethod.getName())) {
+ if (!scMethod.isPublic() && !scMethod.isStatic() &&
(!scMethod.isPrivate() || !publicMethod.isDefault())
+ &&
ParameterUtils.parametersEqual(scMethod.getParameters(),
publicMethod.getParameters())) {
+ addWeakerAccessError2(cn, scMethod, publicMethod);
+ }
+ }
+ }
+ }
+ }
+
+ private void addWeakerAccessError(final ClassNode cn, final MethodNode
cnMethod, final MethodNode scMethod) {
+ StringBuilder msg = new StringBuilder();
+ msg.append(cnMethod.getName());
+ appendParamsDescription(cnMethod.getParameters(), msg);
+ msg.append(" in ");
+ msg.append(cn.getName());
+ msg.append(" cannot override ");
+ msg.append(scMethod.getName());
+ msg.append(" in ");
+ msg.append(scMethod.getDeclaringClass().getName());
+ msg.append("; attempting to assign weaker access privileges; was ");
+ msg.append(scMethod.isPublic() ? "public" : (scMethod.isProtected() ?
"protected" : "package-private"));
+
+ addError(msg.toString(), cnMethod);
+ }
+
+ private void addWeakerAccessError2(final ClassNode cn, final MethodNode
scMethod, final MethodNode ifMethod) {
+ StringBuilder msg = new StringBuilder();
+ msg.append(scMethod.isPrivate() ? "private" : (scMethod.isProtected()
? "protected" : "package-private"));
+ msg.append(" method ");
+ msg.append(scMethod.getName());
+ appendParamsDescription(scMethod.getParameters(), msg);
+ msg.append(" from ");
+ msg.append(scMethod.getDeclaringClass().getName());
+ msg.append(" cannot shadow the public method in ");
+ msg.append(ifMethod.getDeclaringClass().getName());
+ addError(msg.toString(), cn);
}
private void checkMethodsForOverridingFinal(final ClassNode cn) {
@@ -459,22 +521,6 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
msg.append(')');
}
- private void addWeakerAccessError(final ClassNode cn, final MethodNode
method, final Parameter[] parameters, final MethodNode superMethod) {
- StringBuilder msg = new StringBuilder();
- msg.append(method.getName());
- appendParamsDescription(parameters, msg);
- msg.append(" in ");
- msg.append(cn.getName());
- msg.append(" cannot override ");
- msg.append(superMethod.getName());
- msg.append(" in ");
- msg.append(superMethod.getDeclaringClass().getName());
- msg.append("; attempting to assign weaker access privileges; was ");
- msg.append(superMethod.isPublic() ? "public" :
(superMethod.isProtected() ? "protected" : "package-private"));
-
- addError(msg.toString(), method);
- }
-
@Override
public void visitMethod(final MethodNode node) {
inConstructor = false;
@@ -514,21 +560,6 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
}
}
- private void checkMethodForWeakerAccessPrivileges(final MethodNode mn,
final ClassNode cn) {
- if (mn.isPublic()) return;
- Parameter[] params = mn.getParameters();
- for (MethodNode superMethod :
cn.getSuperClass().getMethods(mn.getName())) {
- Parameter[] superParams = superMethod.getParameters();
- if (!ParameterUtils.parametersEqual(params, superParams)) continue;
- if ((mn.isPrivate() && !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;
- }
- }
- }
-
private void checkOverloadingPrivateAndPublic(final MethodNode node) {
if (node.isStaticConstructor()) return;
boolean hasPrivate = node.isPrivate();
diff --git a/src/test/groovy/groovy/InterfaceTest.groovy
b/src/test/groovy/groovy/InterfaceTest.groovy
index 50afb49569..d52cf36319 100644
--- a/src/test/groovy/groovy/InterfaceTest.groovy
+++ b/src/test/groovy/groovy/InterfaceTest.groovy
@@ -18,42 +18,186 @@
*/
package groovy
-import gls.CompilableTestSupport
+import groovy.test.NotYetImplemented
+import org.junit.Test
-final class InterfaceTest extends CompilableTestSupport {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
- void testGenericsInInterfaceMembers() {
- // control
- shouldCompile '''
+final class InterfaceTest {
+
+ @Test
+ void testGenericsInInterfaceMembers1() {
+ assertScript '''
interface I {
def <T> T m1(T x)
def <U extends CharSequence> U m2(U x)
def <V, W> V m3(W x)
def <N extends Number> void m4( )
}
+
+ print 'works'
+ '''
+ }
+
+ @Test
+ void testGenericsInInterfaceMembers2() {
+ shouldFail '''
+ interface I {
+ def <?> m(x)
+ }
+ '''
+ }
+
+ @Test
+ void testGenericsInInterfaceMembers3() {
+ shouldFail '''
+ interface I {
+ def <? extends CharSequence> m(x)
+ }
'''
- // erroneous
- shouldNotCompile 'interface I { def <?> m(x) }'
- shouldNotCompile 'interface I { def <? extends CharSequence> m(x) }'
}
// GROOVY-5106
+ @Test
void testReImplementsInterface1() {
def err = shouldFail '''
interface I<T> {}
interface J<T> extends I<T> {}
class X implements I<String>, J<Number> {}
'''
- assert err.contains('The interface I cannot be implemented more than
once with different arguments: I<java.lang.String> and I<java.lang.Number>')
+ assert err.message.contains('The interface I cannot be implemented
more than once with different arguments: I<java.lang.String> and
I<java.lang.Number>')
}
// GROOVY-5106
+ @Test
void testReImplementsInterface2() {
def err = shouldFail '''
interface I<T> {}
class X implements I<Number> {}
class Y extends X implements I<String> {}
'''
- assert err.contains('The interface I cannot be implemented more than
once with different arguments: I<java.lang.String> and I<java.lang.Number>')
+ assert err.message.contains('The interface I cannot be implemented
more than once with different arguments: I<java.lang.String> and
I<java.lang.Number>')
+ }
+
+ // GROOVY-11707
+ @NotYetImplemented @Test
+ void testReImplementsInterface3() {
+ assertScript '''
+ abstract class A implements Comparable {
+ }
+ abstract class B extends A implements Comparable {
+ }
+
+ print 'works'
+ '''
+ }
+
+ // GROOVY-11736
+ @Test
+ void testReImplementsInterface4() {
+ def err = shouldFail '''
+ abstract class A implements Comparable<Object> {
+ }
+ abstract class B extends A implements Comparable {
+ }
+ '''
+ assert err.message.contains('The interface Comparable cannot be
implemented more than once with different arguments: java.lang.Comparable and
java.lang.Comparable<java.lang.Object>')
+ }
+
+ // GROOVY-11803
+ @Test
+ void testDefaultInterfaceMethod1() {
+ assertScript '''
+ interface Foo {
+ default int barSize() {
+ return bar.size()
+ }
+ String getBar()
+ }
+ class Baz implements Foo {
+ final String bar = 'BAR'
+ }
+
+ assert new Baz().barSize() == 3
+ '''
+ }
+
+ // GROOVY-11803
+ @Test
+ void testDefaultInterfaceMethod2() {
+ assertScript '''
+ interface Foo {
+ default int barSize() {
+ return bar.size()
+ }
+ default String getBar() {
+ return 'fizzbuzz'
+ }
+ }
+ class Baz implements Foo {
+ }
+
+ assert new Baz().barSize() == 8
+ '''
+ }
+
+ // GROOVY-11548
+ @Test
+ void testDefaultInterfaceMethod3() {
+ assertScript '''
+ interface A {
+ default m() { 'A' }
+ }
+ class B {
+ public final m() { 'B' }
+ }
+ class C extends B implements A {
+ }
+
+ assert new C().m() == 'B'
+ '''
+ }
+
+ // GROOVY-11758, GROOVY-11830
+ @Test
+ void testSuperClassAndInterfaceMethod() {
+ for (spec in ['protected
final','protected','@PackageScope','private']) {
+ def err = shouldFail """import groovy.transform.*
+ interface A {
+ def m()
+ }
+ class B {
+ $spec def m() { 'B' }
+ }
+ class C extends B implements A {
+ }
+ """
+ spec = spec.split()[0]
+ if (spec == '@PackageScope') spec = 'package-private'
+ assert err =~ /$spec method m\(\) from B cannot shadow the public
method in A/
+ }
+ }
+
+ // GROOVY-11753
+ @Test
+ void testSuperClassCovariantOfParameterizedInterface() {
+ assertScript '''
+ class A extends B {
+ }
+ class B implements C<String> {
+ static class NestMate {
+ }
+ @Override
+ void p(String s) {
+ print(s)
+ }
+ }
+ interface C<T> {
+ void p(T t)
+ }
+
+ new A().p("")
+ '''
}
}
diff --git a/src/test/groovy/groovy/OverrideTest.groovy
b/src/test/groovy/groovy/OverrideTest.groovy
index a7699d7002..5eec323bd3 100644
--- a/src/test/groovy/groovy/OverrideTest.groovy
+++ b/src/test/groovy/groovy/OverrideTest.groovy
@@ -33,15 +33,13 @@ final class OverrideTest {
void methodTakeT(T t) { }
T methodMakeT() { return null }
}
-
interface Intf<U> {
def method4()
void method5(U u)
U method6()
}
-
- interface IntfString extends Intf<String> {}
-
+ interface IntfString extends Intf<String> {
+ }
class OverrideAnnotationTest extends Parent<Integer> implements
IntfString {
@Override method() {}
@Override void methodTakeT(Integer arg) {}
@@ -63,15 +61,13 @@ final class OverrideTest {
void methodTakeT(T t) { }
T methodMakeT() { return null }
}
-
interface Intf<U> {
def method4()
void method5(U u)
U method6()
}
-
- interface IntfString extends Intf<String> {}
-
+ interface IntfString extends Intf<String> {
+ }
class OverrideAnnotationTest extends Parent<Integer> implements
IntfString {
@Override method() {}
@Override void methodTakeT(arg) {}
@@ -80,11 +76,9 @@ final class OverrideTest {
@Override void method5(String arg) {}
@Override String method6() {}
}
-
- new OverrideAnnotationTest()
'''
- assert err.message.contains(/The return type of java.lang.Double
methodMakeT() in OverrideAnnotationTest is incompatible with java.lang.Integer
in Parent/)
- assert err.message.contains(/Method 'methodTakeT' from class
'OverrideAnnotationTest' does not override method from its superclass or
interfaces but is annotated with @Override./)
+ assert err.message.contains("The return type of java.lang.Double
methodMakeT() in OverrideAnnotationTest is incompatible with java.lang.Integer
in Parent")
+ assert err.message.contains("Method 'methodTakeT' from class
'OverrideAnnotationTest' does not override method from its superclass or
interfaces but is annotated with @Override")
}
@Test
@@ -93,9 +87,8 @@ final class OverrideTest {
interface Intf<U> {
def method()
}
-
- interface IntfString extends Intf<String> {}
-
+ interface IntfString extends Intf<String> {
+ }
class HasSpuriousMethod implements IntfString {
@Override method() {}
@Override someOtherMethod() {}
@@ -111,9 +104,8 @@ final class OverrideTest {
def method()
U method6()
}
-
- interface IntfString extends Intf<String> {}
-
+ interface IntfString extends Intf<String> {
+ }
class HasMethodWithBadReturnType implements IntfString {
@Override method() {}
@Override methodReturnsObject() {}
@@ -129,9 +121,8 @@ final class OverrideTest {
def method()
void method6(U u)
}
-
- interface IntfString extends Intf<String> {}
-
+ interface IntfString extends Intf<String> {
+ }
class HasMethodWithBadArgType implements IntfString {
@Override method() {}
@Override void methodTakesObject(arg) {}
@@ -140,99 +131,86 @@ final class OverrideTest {
assert err.message.contains("Method 'methodTakesObject' from class
'HasMethodWithBadArgType' does not override method from its superclass or
interfaces but is annotated with @Override.")
}
- // GROOVY-6654
@Test
void testCovariantParameterType1() {
assertScript '''
- class C<T> {
- void proc(T t) {}
- }
-
- class D extends C<String> {
- @Override
- void proc(String s) {}
+ class C implements Comparable<C> {
+ int index
+ int compareTo(C c) {
+ index <=> c.index
+ }
}
- def d = new D()
+ def one = new C(index:1)
+ def two = new C(index:2)
+ assert one < two
'''
}
- // GROOVY-10675
@Test
void testCovariantParameterType2() {
assertScript '''
- @FunctionalInterface
- interface A<I, O> {
- O apply(I in)
- }
- interface B<X, Y> extends A<X, Y> {
+ interface I<T> {
+ int handle(long n, T t)
}
- class C implements B<Number, String> {
- @Override String apply(Number n) { 'x' }
+ class C implements I<String> {
+ int handle(long n, String something) {
+ 1
+ }
}
- def result = new C().apply(42)
- assert result == 'x'
+ def c = new C()
+ assert c.handle(5,"hi") == 1
'''
}
- // GROOVY-7849
@Test
- void testCovariantArrayReturnType1() {
+ void testCovariantParameterType3() {
assertScript '''
- interface A {
- }
- interface B extends A {
- }
- interface I {
- A[] foo()
+ interface I<T> {
+ int testMethod(T t)
}
- class C implements I {
- B[] foo() { null }
+ class C implements I<Date> {
+ int testMethod(Date date) {}
+ int testMethod(Object obj) {}
}
- new C().foo()
+ assert C.declaredMethods.count{ it.name=="testMethod" } == 2
'''
}
- // GROOVY-7185
+ // GROOVY-6654
@Test
- void testCovariantArrayReturnType2() {
+ void testCovariantParameterType4() {
assertScript '''
- interface A<T> {
- T[] process();
- }
- class B implements A<String> {
- @Override
- public String[] process() {
- ['foo']
- }
+ class C<T> {
+ void proc(T t) {}
}
- class C extends B {
+ class D extends C<String> {
@Override
- String[] process() {
- super.process()
- }
+ void proc(String s) {}
}
- assert new C().process()[0] == 'foo'
+ def d = new D()
'''
}
- // GROOVY-11579
+ // GROOVY-10675
@Test
- void testCovariantBridgeReturnType() {
+ void testCovariantParameterType5() {
assertScript '''
- interface I<T> {
- T m()
+ @FunctionalInterface
+ interface A<I, O> {
+ O apply(I in)
}
- abstract class A {
- final String m() { 'A' }
+ interface B<X, Y> extends A<X, Y> {
}
- class C extends A implements I<String> {
+ class C implements B<Number, String> {
+ @Override String apply(Number n) { 'x' }
}
- assert new C().m() == 'A'
+ def result = new C().apply(42)
+ assert result == 'x'
'''
}
@@ -242,13 +220,12 @@ final class OverrideTest {
interface TemplatedInterface {
String execute(Map argument)
}
-
class TemplatedInterfaceImplementation implements
TemplatedInterface {
@Override
String execute(Map argument = [:]) {
- return null
}
}
+
new TemplatedInterfaceImplementation()
'''
}
@@ -259,31 +236,14 @@ final class OverrideTest {
interface TemplatedInterface {
String execute(Map argument)
}
-
class TemplatedInterfaceImplementation implements
TemplatedInterface {
@Override
String execute(Map argument, String foo = null) {
return foo
}
}
- new TemplatedInterfaceImplementation()
- '''
- }
- // GROOVY-11548
- @Test
- void testDefaultMethodDoesNotOverride() {
- assertScript '''
- class A {
- final func() { 'A' }
- }
- interface B {
- default func() { 'B' }
- }
- class C extends A implements B {
- }
-
- assert new C().func() == 'A'
+ new TemplatedInterfaceImplementation()
'''
}
}
