Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 31 additions & 33 deletions src/main/java/groovy/lang/MetaClassImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -446,50 +446,48 @@ protected LinkedList<CachedClass> getSuperClasses() {
}

private void removeMultimethodsOverloadedWithPrivateMethods() {
MethodIndexAction mia = new MethodIndexAction() {
var mia = new MethodIndexAction() {
@Override
public boolean skipClass(final Class<?> clazz) {
return clazz == theClass;
}

@Override
public void methodNameAction(final Class<?> clazz, final MetaMethodIndex.Cache e) {
if (e.methods == null)
return;

boolean hasPrivate = false;
if (e.methods instanceof FastArray) {
FastArray methods = (FastArray) e.methods;
final int len = methods.size();
final Object[] data = methods.getArray();
for (int i = 0; i != len; ++i) {
public void methodNameAction(final Class<?> clazz, final MetaMethodIndex.Cache entry) {
if (hasPrivateInMethods(clazz, entry)) { // GROOVY-5193, etc.
// We have private methods for that name, so remove the
// multimethods. That is the same as in our index for
// super, so just copy the list from there. It is not
// possible to use a pointer here because the methods
// in the index for super are replaced later by MOP
// methods like super$5$foo
Object o = entry.methodsForSuper;
if (o instanceof FastArray) {
entry.methods = ((FastArray) o).copy();
} else {
entry.methods = o;
}
}
}

private boolean hasPrivateInMethods(final Class<?> clazz, final MetaMethodIndex.Cache entry) {
if (entry.methods instanceof FastArray) {
FastArray methods = (FastArray) entry.methods;
int size = methods.size();
var data = methods.getArray();
for (int i = 0; i != size; ++i) {
MetaMethod method = (MetaMethod) data[i];
if (method.isPrivate() && clazz == method.getDeclaringClass().getTheClass()) {
hasPrivate = true;
break;
if (method.isPrivate() && method.getDeclaringClass().getTheClass() == clazz) {
return true;
}
}
} else {
MetaMethod method = (MetaMethod) e.methods;
if (method.isPrivate() && clazz == method.getDeclaringClass().getTheClass()) {
hasPrivate = true;
} else if (entry.methods instanceof MetaMethod) {
MetaMethod method = (MetaMethod) entry.methods;
if (method.isPrivate() && method.getDeclaringClass().getTheClass() == clazz) {
return true;
}
}

if (!hasPrivate) return;

// We have private methods for that name, so remove the
// multimethods. That is the same as in our index for
// super, so just copy the list from there. It is not
// possible to use a pointer here, because the methods
// in the index for super are replaced later by MOP
// methods like super$5$foo
final Object o = e.methodsForSuper;
if (o instanceof FastArray) {
e.methods = ((FastArray) o).copy();
} else {
e.methods = o;
}
return false;
}
};
mia.iterate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import static org.codehaus.groovy.ast.tools.GenericsUtils.buildWildcardType;
import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.isStaticallyCompiled;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
Expand Down Expand Up @@ -538,8 +539,8 @@ public void visitMethod(final MethodNode node) {
checkOverloadingPrivateAndPublic(node);
}
checkMethodForIncorrectModifiers(node);
checkGenericsUsage(node, node.getParameters());
checkGenericsUsage(node, node.getReturnType());
checkGenericsUsage(node, node.getParameters());
for (Parameter param : node.getParameters()) {
if (ClassHelper.isPrimitiveVoid(param.getType())) {
addError("The " + getDescription(param) + " in " + getDescription(node) + " has invalid type void", param);
Expand Down Expand Up @@ -588,6 +589,7 @@ private void checkMethodForWeakerAccessPrivileges(final MethodNode mn, final Cla
}

private void checkOverloadingPrivateAndPublic(final MethodNode node) {
if (isStaticallyCompiled(currentClass)) return; // GROOVY-11627
boolean mixed = false;
if (node.isPublic()) {
for (MethodNode mn : currentClass.getDeclaredMethods(node.getName())) {
Expand Down
26 changes: 26 additions & 0 deletions src/test/groovy/bugs/Groovy5193.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package bugs
import org.codehaus.groovy.control.MultipleCompilationErrorsException
import org.junit.jupiter.api.Test

import static groovy.test.GroovyAssert.assertScript
import static groovy.test.GroovyAssert.shouldFail

final class Groovy5193 {
Expand Down Expand Up @@ -54,4 +55,29 @@ final class Groovy5193 {
'''
assert err.message.contains('Mixing private and public/protected methods of the same name causes multimethods to be disabled')
}

@Test
void testMixingMethodsWithPrivatePublicAccessInSameClass3() {
assertScript '''
class A {
def f(x) { g(x) }
def g(x) {"object case"}
}
class B extends A {
private g(String x) {"string case"}
}

def a = new A()
assert a.f(null) == "object case"
assert a.g(null) == "object case"
assert a.f("xx") == "object case"
assert a.g("xx") == "object case"

def b = new B()
assert b.f(null) == "object case"
assert b.g(null) == "object case"
assert b.f("xx") == "object case"
assert b.g("xx") == "string case"
'''
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1810,21 +1810,24 @@ final class TraitASTTransformationTest {
"""
}

// GROOVY-5193
// GROOVY-5193, GROOVY-11627
@Test
void testMixPrivatePublicMethodsOfSameName() {
def err = shouldFail shell, '''
assertScript shell, '''
@CompileStatic
trait T {
private String secret(String s) { s.toUpperCase() }
String secret() { 'public' }
String foo() { secret('secret') }
private String f(String s) { s.toUpperCase() }
public String f(Object o) { 'default value' }
public String f( ) { 'unknown value' }
def m() {
f('secret')
}
}
class C implements T {
}

assert new C().foo() == 'SECRET'
assert new C().m() == 'SECRET'
'''
assert err =~ 'Mixing private and public/protected methods of the same name causes multimethods to be disabled'
}

@Test
Expand Down