This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 5011d46a82 GROOVY-11830: detect super class method clashes with
interface method
5011d46a82 is described below
commit 5011d46a822e5c805a7d8dac0c37dabe56fa5fc1
Author: Eric Milles <[email protected]>
AuthorDate: Sun Jan 4 15:29:09 2026 -0600
GROOVY-11830: detect super class method clashes with interface method
---
.../groovy/classgen/ClassCompletionVerifier.java | 9 +-
src/test/groovy/groovy/InterfaceTest.groovy | 124 +++++++++++++++------
src/test/groovy/groovy/OverrideTest.groovy | 69 ++++--------
3 files changed, 113 insertions(+), 89 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index b3c6c14dbc..e089815fa0 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -409,7 +409,7 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
// Verifier: checks weaker access of cn's methods against abstract or
default interface methods
- // GROOVY-11758: check for non-public final super class method that
shadows an interface method
+ // 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()) {
@@ -419,8 +419,8 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
}
for (MethodNode publicMethod : interfaceMethods.values()) {
for (MethodNode scMethod :
cn.getSuperClass().getMethods(publicMethod.getName())) {
- if (scMethod.isFinal() && !scMethod.isPublic() &&
!scMethod.isPrivate() && !scMethod.isStatic()
- &&
ParameterUtils.parametersEqual(scMethod.getParameters(),
publicMethod.getParameters())) {
+ if (!scMethod.isPublic() && !scMethod.isStatic() &&
(!scMethod.isPrivate() || !publicMethod.isDefault())
+ &&
ParameterUtils.parametersEqual(scMethod.getParameters(),
publicMethod.getParameters())) {
addWeakerAccessError2(cn, scMethod, publicMethod);
}
}
@@ -446,7 +446,8 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
private void addWeakerAccessError2(final ClassNode cn, final MethodNode
scMethod, final MethodNode ifMethod) {
StringBuilder msg = new StringBuilder();
- msg.append("inherited final method ");
+ msg.append(scMethod.isPrivate() ? "private" : (scMethod.isProtected()
? "protected" : "package-private"));
+ msg.append(" method ");
msg.append(scMethod.getName());
appendParamsDescription(scMethod.getParameters(), msg);
msg.append(" from ");
diff --git a/src/test/groovy/groovy/InterfaceTest.groovy
b/src/test/groovy/groovy/InterfaceTest.groovy
index 62641ccc19..8a7950cc16 100644
--- a/src/test/groovy/groovy/InterfaceTest.groovy
+++ b/src/test/groovy/groovy/InterfaceTest.groovy
@@ -18,56 +18,84 @@
*/
package groovy
-import gls.CompilableTestSupport
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
-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> (via X) and
I<java.lang.Number> (via J)')
+ assert err.message.contains('The interface I cannot be implemented
more than once with different arguments: I<java.lang.String> (via X) and
I<java.lang.Number> (via J)')
}
// 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> (via Y) and
I<java.lang.Number> (via X)')
+ assert err.message.contains('The interface I cannot be implemented
more than once with different arguments: I<java.lang.String> (via Y) and
I<java.lang.Number> (via X)')
}
// GROOVY-11707
+ @Test
void testReImplementsInterface3() {
- shouldCompile '''
+ 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> {
@@ -75,10 +103,11 @@ final class InterfaceTest extends CompilableTestSupport {
abstract class B extends A implements Comparable {
}
'''
- assert err.contains('The interface Comparable cannot be implemented
more than once with different arguments: java.lang.Comparable (via B) and
java.lang.Comparable<java.lang.Object> (via A)')
+ assert err.message.contains('The interface Comparable cannot be
implemented more than once with different arguments: java.lang.Comparable (via
B) and java.lang.Comparable<java.lang.Object> (via A)')
}
// GROOVY-11803
+ @Test
void testDefaultInterfaceMethod() {
assertScript '''
interface Foo {
@@ -93,25 +122,48 @@ final class InterfaceTest extends CompilableTestSupport {
assert new Baz().barSize() == 3
'''
- for (kind in ['default','private','static']) {
- assertScript """
- interface Foo {
- default int barSize() {
- return bar.size()
- }
- $kind String getBar() {
- return 'fizzbuzz'
- }
+ }
+
+ // GROOVY-11803
+ @ParameterizedTest
+ @ValueSource(strings=['default','private','static'])
+ void testDefaultInterfaceMethod2(String kind) {
+ assertScript """
+ interface Foo {
+ default int barSize() {
+ return bar.size()
}
- class Baz implements Foo {
+ $kind String getBar() {
+ return 'fizzbuzz'
}
+ }
+ class Baz implements Foo {
+ }
- assert new Baz().barSize() == 8
- """
- }
+ assert new Baz().barSize() == 8
+ """
+ }
+
+ // GROOVY-11548
+ @ParameterizedTest
+ @ValueSource(strings=['def','final','public'])
+ void testDefaultInterfaceMethod3(String spec) {
+ assertScript """
+ interface A {
+ default m() { 'A' }
+ }
+ class B {
+ $spec m() { 'B' }
+ }
+ class C extends B implements A {
+ }
+
+ assert new C().m() == 'B'
+ """
}
// GROOVY-10060
+ @Test
void testPrivateInterfaceMethod() {
assertScript '''
interface Foo {
@@ -120,18 +172,15 @@ final class InterfaceTest extends CompilableTestSupport {
default baz() { hello('Foo#baz') }
private hello(where) { "hello from $where"}
}
-
class Parent {
public bar() {
hello 'Parent#bar'
}
private hello(where) { "howdy from $where"}
}
-
class Impl1 extends Parent implements Foo {
def baz() { 'hi from Impl1#baz' }
}
-
class Impl2 extends Parent implements Foo {
}
@@ -147,6 +196,7 @@ final class InterfaceTest extends CompilableTestSupport {
}
// GROOVY-11237
+ @Test
void testPublicStaticInterfaceMethod() {
assertScript '''import static groovy.test.GroovyAssert.shouldFail
interface Foo {
@@ -168,23 +218,27 @@ final class InterfaceTest extends CompilableTestSupport {
'''
}
- // GROOVY-11758
- void testSuperClassFinalMethodAndInterfaceMethod() {
- def err = shouldNotCompile '''
+ // GROOVY-11758, GROOVY-11830
+ @ParameterizedTest
+ @ValueSource(strings=['protected
final','protected','@PackageScope','private'])
+ void testSuperClassAndInterfaceMethod(String spec) {
+ def err = shouldFail """import groovy.transform.*
interface A {
def m()
}
class B {
- protected final m() {
- }
+ $spec def m() { 'B' }
}
class C extends B implements A {
}
- '''
- assert err =~ /inherited final method m\(\) from B cannot shadow the
public method in 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 {
diff --git a/src/test/groovy/groovy/OverrideTest.groovy
b/src/test/groovy/groovy/OverrideTest.groovy
index dfc658f583..85fc76370a 100644
--- a/src/test/groovy/groovy/OverrideTest.groovy
+++ b/src/test/groovy/groovy/OverrideTest.groovy
@@ -18,7 +18,7 @@
*/
package groovy
-import org.junit.Test
+import org.junit.jupiter.api.Test
import static groovy.test.GroovyAssert.assertScript
import static groovy.test.GroovyAssert.shouldFail
@@ -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) {}
@@ -150,8 +141,8 @@ final class OverrideTest {
}
}
- one = new C(index:1)
- two = new C(index:2)
+ def one = new C(index:1)
+ def two = new C(index:2)
assert one < two
'''
}
@@ -162,14 +153,13 @@ final class OverrideTest {
interface I<T> {
int handle(long n, T t)
}
-
class C implements I<String> {
int handle(long n, String something) {
1
}
}
- c = new C()
+ def c = new C()
assert c.handle(5,"hi") == 1
'''
}
@@ -196,7 +186,6 @@ final class OverrideTest {
class C<T> {
void proc(T t) {}
}
-
class D extends C<String> {
@Override
void proc(String s) {}
@@ -237,7 +226,7 @@ final class OverrideTest {
}
}
'''
- assert err =~ /name clash: m\(I<java.lang.String>\) in class 'C' and
m\(I<java.lang.Object>\) in interface 'I' have the same erasure, yet neither
overrides the other./
+ assert err.message.contains("name clash: m(I<java.lang.String>) in
class 'C' and m(I<java.lang.Object>) in interface 'I' have the same erasure,
yet neither overrides the other")
}
@Test
@@ -246,13 +235,12 @@ final class OverrideTest {
interface TemplatedInterface {
String execute(Map argument)
}
-
class TemplatedInterfaceImplementation implements
TemplatedInterface {
@Override
String execute(Map argument = [:]) {
- return null
}
}
+
new TemplatedInterfaceImplementation()
'''
}
@@ -263,33 +251,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() {
- for (kind in ['def', 'final', 'public']) {
- assertScript """
- class A {
- $kind func() { 'A' }
- }
- interface B {
- default func() { 'B' }
- }
- class C extends A implements B {
- }
-
- assert new C().func() == 'A'
- """
- }
- }
}
