Skip to content

Commit

Permalink
GROOVY-2582, GROOVY-11549: no bridge method for interface default method
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed Feb 16, 2025
1 parent 6a3d942 commit e484ac5
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 59 deletions.
31 changes: 25 additions & 6 deletions src/main/java/org/codehaus/groovy/classgen/Verifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -1416,10 +1416,31 @@ public static long getTimestamp(final Class<?> clazz) {
}

protected void addCovariantMethods(final ClassNode classNode) {
// unimplemented abstract methods from interfaces
Map<String, MethodNode> absInterfaceMethods = ClassNodeUtils.getDeclaredMethodsFromInterfaces(classNode);
Map<String, MethodNode> allInterfaceMethods = new HashMap<>(absInterfaceMethods);
ClassNodeUtils.addDeclaredMethodsFromAllInterfaces(classNode, allInterfaceMethods);
Map<String, MethodNode> absInterfaceMethods = new HashMap<>();
Map<String, MethodNode> allInterfaceMethods = new HashMap<>();
Set<ClassNode> allInterfaces = getAllInterfaces(classNode);
allInterfaces.remove(classNode);

for (ClassNode in : allInterfaces) {
for (MethodNode mn : in.getMethods()) {
// interfaces may have private/static methods
if (mn.isPrivate() || mn.isStatic()) continue;

if (mn.isAbstract()) {
allInterfaceMethods.putIfAbsent(mn.getTypeDescriptor(), mn);
} else { // GROOVY-11549: default method replaces an abstract
mn = allInterfaceMethods.put(mn.getTypeDescriptor(), mn);
if (mn != null && !mn.isAbstract())
allInterfaceMethods.put(mn.getTypeDescriptor(), mn);
}
}
}

for (Map.Entry<String, MethodNode> entry : allInterfaceMethods.entrySet()) {
if (entry.getValue().isAbstract()/* || entry.getValue().isDefault()*/) {
absInterfaceMethods.putIfAbsent(entry.getKey(), entry.getValue());
}
}

List<MethodNode> declaredMethods = new ArrayList<>(classNode.getMethods());
for (Iterator<MethodNode> methodsIterator = declaredMethods.iterator(); methodsIterator.hasNext(); ) {
Expand All @@ -1434,8 +1455,6 @@ protected void addCovariantMethods(final ClassNode classNode) {
throw new RuntimeParserException("The method " + m.getName() + " should be public as it implements the corresponding method from interface " + interfaceMethod.getDeclaringClass(), sourceOf(m));
}
}
// interfaces may have private/static methods in JDK9
absInterfaceMethods.values().removeIf(interfaceMethod -> interfaceMethod.isPrivate() || interfaceMethod.isStatic());

Map<String, MethodNode> methodsToAdd = new HashMap<>();
Map<String, ClassNode > genericsSpec = Collections.emptyMap();
Expand Down
26 changes: 12 additions & 14 deletions src/test/gls/invocation/CovariantReturnTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -146,20 +146,18 @@ final class CovariantReturnTest {
assert c.foo() == ""
'''

// basically the same as above, but with an example from an error report
// Properties has a method "String getProperty(String)", this class
// is also a GroovyObject, meaning a "Object getProperty(String)" method
// should be implemented. But this method should not be the usual automatically
// added getProperty, but a bridge to the getProperty method provided by Properties
assertScript '''
class Configuration extends java.util.Properties {
}
assert Configuration.declaredMethods.count{ it.name=="getProperty" } == 1
def conf = new Configuration()
conf.setProperty("a","b")
// the following assert would fail if standard getProperty method was added by the compiler
assert conf.getProperty("a") == "b"
// an example from an error report
// Properties has a method "String getProperty(String)"
// GroovyObject has a default method "Object getProperty(String)"
assertScript '''import static groovy.test.GroovyAssert.shouldFail
class C extends Properties { }
shouldFail(NoSuchMethodException) {
C.getDeclaredMethod("getProperty")
}
def c = new C()
c.setProperty("a", "b")
assert c.getProperty("a") == "b" // fails if standard method added by the compiler
'''

assertScript '''
Expand Down
62 changes: 23 additions & 39 deletions src/test/groovy/bugs/Groovy4116Bug.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -21,47 +21,31 @@ package groovy.bugs
import groovy.test.GroovyTestCase
import org.codehaus.groovy.control.MultipleCompilationErrorsException

class Groovy4116Bug extends GroovyTestCase {
void testAnInterfaceMethodNotImplementedPublic() {
try {
new GroovyShell().parse """
class C4116 implements I4116 {
protected foo() {}
}
interface I4116 {
def foo()
}
final class Groovy4116Bug extends GroovyTestCase {

def c = new C4116()
c.foo()
"""
fail('The compilation should have failed as the interface method is not implemented as public')
} catch (MultipleCompilationErrorsException e) {
def syntaxError = e.errorCollector.getSyntaxError(0)
assert syntaxError.message.contains('The method foo should be public as it implements the corresponding method from interface I4116')
}
void testAnInterfaceMethodNotImplementedPublic() {
def err = shouldFail MultipleCompilationErrorsException, '''
class C4116 implements I4116 {
protected foo() {}
}
interface I4116 {
def foo()
}
'''
assert err =~ /The method foo should be public as it implements the corresponding method from interface I4116/
}

void testAnInterfaceMethodNotImplementedPublicV2SuperClassInterface() {
try {
new GroovyShell().parse """
abstract class S4116V2 implements I4116V2 { }
class C4116V2 extends S4116V2 {
protected foo() {}
}
interface I4116V2 {
def foo()
}
def c = new C4116V2()
c.foo()
"""
fail('The compilation should have failed as the interface method is not implemented as public')
} catch (MultipleCompilationErrorsException e) {
def syntaxError = e.errorCollector.getSyntaxError(0)
assert syntaxError.message.contains('The method foo should be public as it implements the corresponding method from interface I4116')
}
def err = shouldFail MultipleCompilationErrorsException, '''
abstract class A4116 implements I4116 {
}
class C4116 extends A4116 {
protected foo() {}
}
interface I4116 {
def foo()
}
'''
assert err =~ /The method foo should be public as it implements the corresponding method from interface I4116/
}
}
}

0 comments on commit e484ac5

Please sign in to comment.