GROOVY-5193: refactor checking

Author: eric-milles

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

@@ -533,8 +533,10 @@ public void visitMethod(final MethodNode node) {
         inConstructor = false;
         inStaticConstructor = node.isStaticConstructor();
         checkAbstractDeclaration(node);
-        checkRepetitiveMethod(node);
-        checkOverloadingPrivateAndPublic(node);
+        if (!inStaticConstructor) {
+            checkRepetitiveMethod(node);
+            checkOverloadingPrivateAndPublic(node);
+        }
         checkMethodForIncorrectModifiers(node);
         checkGenericsUsage(node, node.getParameters());
         checkGenericsUsage(node, node.getReturnType());
@@ -586,26 +588,28 @@ private void checkMethodForWeakerAccessPrivileges(final MethodNode mn, final Cla
     }
 
     private void checkOverloadingPrivateAndPublic(final MethodNode node) {
-        if (node.isStaticConstructor()) return;
-        boolean hasPrivate = node.isPrivate();
-        boolean hasPublic = node.isPublic();
-        for (MethodNode method : currentClass.getMethods(node.getName())) {
-            if (method == node) continue;
-            if (!method.getDeclaringClass().equals(node.getDeclaringClass())) continue;
-            if (method.isPublic() || method.isProtected()) {
-                hasPublic = true;
-            } else {
-                hasPrivate = true;
+        boolean mixed = false;
+        if (node.isPublic()) {
+            for (MethodNode mn : currentClass.getDeclaredMethods(node.getName())) {
+                if (mn != node && !(mn.isPublic() || mn.isProtected())) {
+                    mixed = true;
+                    break;
+                }
+            }
+        } else if (node.isPrivate()) {
+            for (MethodNode mn : currentClass.getDeclaredMethods(node.getName())) {
+                if (mn != node && (mn.isPublic() || mn.isProtected())) {
+                    mixed = true;
+                    break;
+                }
             }
-            if (hasPrivate && hasPublic) break;
         }
-        if (hasPrivate && hasPublic) {
+        if (mixed) { // GROOVY-5193
             addError("Mixing private and public/protected methods of the same name causes multimethods to be disabled and is forbidden to avoid surprising behaviour. Renaming the private methods will solve the problem.", node);
         }
     }
 
     private void checkRepetitiveMethod(final MethodNode node) {
-        if (node.isStaticConstructor()) return;
         for (MethodNode method : currentClass.getMethods(node.getName())) {
             if (method == node) continue;
             if (!method.getDeclaringClass().equals(node.getDeclaringClass())) continue;

src/test/groovy/bugs/Groovy5193.groovy

@@ -0,0 +1,57 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package bugs
+
+import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+final class Groovy5193 {
+
+    @Test
+    void testMixingMethodsWithPrivatePublicAccessInSameClass1() {
+        def err = shouldFail MultipleCompilationErrorsException, '''
+            class A5193 {
+                static main(args) {
+                }
+                public find(String id) {
+                }
+                private <T> T find(Class<T> type, String id, boolean suppressExceptions) {
+                }
+            }
+        '''
+        assert err.message.contains('Mixing private and public/protected methods of the same name causes multimethods to be disabled')
+    }
+
+    @Test
+    void testMixingMethodsWithPrivatePublicAccessInSameClass2() {
+        def err = shouldFail MultipleCompilationErrorsException, '''
+            class B5193 {
+                static main(args) {
+                }
+                public find(String id) {
+                }
+                private <T> T find(Class<T> type, String id, boolean suppressExceptions = true) {
+                }
+            }
+        '''
+        assert err.message.contains('Mixing private and public/protected methods of the same name causes multimethods to be disabled')
+    }
+}

src/test/groovy/bugs/Groovy5193Bug.groovy

@@ -1811,6 +1811,7 @@ final class TraitASTTransformationTest {
         """
     }
 
+    // GROOVY-5193
     @Test
     void testMixPrivatePublicMethodsOfSameName() {
         def err = shouldFail shell, '''
@@ -1822,8 +1823,7 @@ final class TraitASTTransformationTest {
             class C implements T {
             }
 
-            def c = new C()
-            assert c.foo() == 'SECRET'
+            assert new C().foo() == 'SECRET'
         '''
         assert err =~ 'Mixing private and public/protected methods of the same name causes multimethods to be disabled'
     }