This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11627 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 85a4689585d5fad00d44d274010565bef922cef0 Author: Eric Milles <[email protected]> AuthorDate: Wed May 14 15:21:07 2025 -0500 GROOVY-11627: SC: disable mixed public and private overloads checking --- src/main/java/groovy/lang/MetaClassImpl.java | 64 +++++++++++----------- .../groovy/classgen/ClassCompletionVerifier.java | 4 +- src/test/groovy/bugs/Groovy5193.groovy | 26 +++++++++ .../traitx/TraitASTTransformationTest.groovy | 17 +++--- 4 files changed, 70 insertions(+), 41 deletions(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index 6f5c181fd6..445c018675 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -446,50 +446,48 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { } 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(); diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java index 2414ba05f3..5cf32d125e 100644 --- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java @@ -74,6 +74,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics; 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; @@ -538,8 +539,8 @@ out: for (ClassNode sc : superTypes) { 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); @@ -588,6 +589,7 @@ out: for (ClassNode sc : superTypes) { } 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())) { diff --git a/src/test/groovy/bugs/Groovy5193.groovy b/src/test/groovy/bugs/Groovy5193.groovy index d0284b5ae3..9ec5883a4d 100644 --- a/src/test/groovy/bugs/Groovy5193.groovy +++ b/src/test/groovy/bugs/Groovy5193.groovy @@ -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 { @@ -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" + ''' + } } diff --git a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy index e0bfd2b087..56b58bdf15 100644 --- a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy @@ -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
