GROOVY-11627: SC: disable mixed public and private overloads checking

Author: eric-milles

src/main/java/groovy/lang/MetaClassImpl.java

@@ -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();

src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java

@@ -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;
@@ -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);
@@ -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())) {

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"
+        '''
+    }
 }

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