You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2026/08/16 11:50:17 UTC

(groovy) branch GROOVY-12264 created (now 4f5ea608bc)

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a change to branch GROOVY-12264
in repository https://gitbox.apache.org/repos/asf/groovy.git


      at 4f5ea608bc GROOVY-12264: Optimize the unrelated-default-method scan during class generation

This branch includes the following new commits:

     new 4f5ea608bc GROOVY-12264: Optimize the unrelated-default-method scan during class generation

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(groovy) 01/01: GROOVY-12264: Optimize the unrelated-default-method scan during class generation

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch GROOVY-12264
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 4f5ea608bcfd2ecd7a01fe80eb0d7e96ab7f78d3
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 16 20:30:19 2026 +0900

    GROOVY-12264: Optimize the unrelated-default-method scan during class generation
---
 .../java/org/codehaus/groovy/ast/MethodNode.java   |  5 +-
 .../org/codehaus/groovy/classgen/Verifier.java     | 90 ++++++++++++++--------
 src/test/groovy/bugs/Groovy10381.groovy            | 72 +++++++++++++++++
 3 files changed, 132 insertions(+), 35 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/MethodNode.java b/src/main/java/org/codehaus/groovy/ast/MethodNode.java
index 13497d9fdb..51c6e068de 100644
--- a/src/main/java/org/codehaus/groovy/ast/MethodNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/MethodNode.java
@@ -23,7 +23,6 @@ import org.codehaus.groovy.ast.stmt.Statement;
 
 import java.lang.reflect.Modifier;
 import java.util.List;
-import java.util.Optional;
 
 import static org.apache.groovy.ast.tools.ClassNodeUtils.formatTypeName;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptor;
@@ -284,8 +283,8 @@ public class MethodNode extends AnnotatedNode {
      * @return true if this method is a default interface method
      */
     public boolean isDefault() {
-        return (modifiers & (ACC_ABSTRACT | ACC_PUBLIC | ACC_STATIC)) == ACC_PUBLIC &&
-            Optional.ofNullable(getDeclaringClass()).filter(ClassNode::isInterface).isPresent();
+        return (modifiers & (ACC_ABSTRACT | ACC_PUBLIC | ACC_STATIC)) == ACC_PUBLIC
+            && getDeclaringClass() != null && getDeclaringClass().isInterface();
     }
 
     /**
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 1faac2d52a..a88ad087e7 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -88,7 +88,6 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.IntFunction;
-import java.util.stream.Collectors;
 
 import static java.lang.reflect.Modifier.isFinal;
 import static java.lang.reflect.Modifier.isPrivate;
@@ -318,40 +317,67 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         checkFinalVariables(node);
     }
 
-    private static void checkForDuplicateDefaultMethods(ClassNode node) {
+    /**
+     * Rejects unrelated inherited default methods (JLS 8.4.8 / 9.4.1).
+     * Uses each interface's own methods; {@code getAllDeclaredMethods()} would
+     * rebuild a hierarchy map per type and revisit inherited defaults.
+     */
+    private static void checkForDuplicateDefaultMethods(final ClassNode node) {
         if (node.getInterfaces().length < 2) return;
 
-        Map<String, MethodNode> defaultMethods = new HashMap<>(8);
-        Set<String> declared = node.getAllDeclaredMethods().stream()
-            .filter(m -> !m.isDefault())
-            .map(MethodNodeUtils::methodDescriptorWithoutReturnType)
-            .collect(Collectors.toSet());
-        node.getAllInterfaces().stream()
-                .flatMap(i -> i.getAllDeclaredMethods().stream())
-                .filter(MethodNode::isDefault)
-                .forEach(m -> {
-                    String signature = MethodNodeUtils.methodDescriptorWithoutReturnType(m);
-                    if (declared.contains(signature)) {
-                        return;
-                    }
-                    MethodNode existing = defaultMethods.get(signature);
-                    if (existing == null) {
-                        defaultMethods.put(signature, m);
-                        return;
-                    }
+        Map<String, MethodNode> defaultMethods = null;
+        Set<String> declared = null;
+        for (ClassNode iface : node.getAllInterfaces()) {
+            for (MethodNode method : iface.getMethods()) {
+                if (!method.isDefault()) continue;
 
-                    ClassNode existingDeclaringClass = existing.getDeclaringClass();
-                    ClassNode currentDeclaringClass = m.getDeclaringClass();
-                    if (!(existingDeclaringClass.equals(currentDeclaringClass)
-                            || existingDeclaringClass.implementsInterface(currentDeclaringClass)
-                            || currentDeclaringClass.implementsInterface(existingDeclaringClass))) {
-                        throw new RuntimeParserException(
-                                (node.isInterface() ? "interface" : "class") +  " " + node.getName()
-                                        + " inherits unrelated defaults for " + MethodNodeUtils.methodDescriptor(m, true)
-                                        + " from types " + existingDeclaringClass.getName()
-                                        + " and " + currentDeclaringClass.getName(), node);
-                    }
-                });
+                String signature = MethodNodeUtils.methodDescriptorWithoutReturnType(method);
+                if (declared != null && declared.contains(signature)) continue;
+
+                if (defaultMethods == null) {
+                    defaultMethods = new HashMap<>(8);
+                }
+                MethodNode existing = defaultMethods.putIfAbsent(signature, method);
+                if (existing == null) continue;
+
+                ClassNode existingDeclaringClass = existing.getDeclaringClass();
+                ClassNode currentDeclaringClass = method.getDeclaringClass();
+                if (existingDeclaringClass.equals(currentDeclaringClass)
+                        || existingDeclaringClass.implementsInterface(currentDeclaringClass)
+                        || currentDeclaringClass.implementsInterface(existingDeclaringClass)) {
+                    continue;
+                }
+
+                if (declared == null) {
+                    declared = collectNonDefaultMethodSignatures(node);
+                    if (declared.contains(signature)) continue;
+                }
+
+                throw new RuntimeParserException(
+                        (node.isInterface() ? "interface" : "class") +  " " + node.getName()
+                                + " inherits unrelated defaults for " + MethodNodeUtils.methodDescriptor(method, true)
+                                + " from types " + existingDeclaringClass.getName()
+                                + " and " + currentDeclaringClass.getName(), node);
+            }
+        }
+    }
+
+    /**
+     * Signatures of methods declared on {@code node} or a superclass. These hide
+     * inherited interface defaults; abstracts on other interfaces do not.
+     */
+    private static Set<String> collectNonDefaultMethodSignatures(final ClassNode node) {
+        Set<String> declared = null;
+        for (ClassNode cn = node; cn != null && !isObjectType(cn); cn = cn.getSuperClass()) {
+            for (MethodNode method : cn.getMethods()) {
+                if (method.isDefault()) continue;
+                if (declared == null) {
+                    declared = new HashSet<>();
+                }
+                declared.add(MethodNodeUtils.methodDescriptorWithoutReturnType(method));
+            }
+        }
+        return declared == null ? Collections.emptySet() : declared;
     }
 
     private static final String[] INVALID_COMPONENTS = {"clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"};
diff --git a/src/test/groovy/bugs/Groovy10381.groovy b/src/test/groovy/bugs/Groovy10381.groovy
index 2190d74014..e6e852d01e 100644
--- a/src/test/groovy/bugs/Groovy10381.groovy
+++ b/src/test/groovy/bugs/Groovy10381.groovy
@@ -58,6 +58,78 @@ final class Groovy10381 {
         '''
     }
 
+    @Test
+    void testDuplicateDefaultMethods_superclassProvidesMethod() {
+        assertScript '''
+            interface A {
+              default String m(String n) { n.toLowerCase() }
+            }
+            interface B {
+              default String m(String n) { n.toUpperCase() }
+            }
+            class Super {
+              String m(String n) { n }
+            }
+            class C extends Super implements A, B {
+            }
+            assert new C().m('Hi') == 'Hi'
+        '''
+    }
+
+    @Test
+    void testDuplicateDefaultMethods_interfaceProvidesOwnDefault() {
+        assertScript '''
+            interface A {
+              default String m(String n) { n.toLowerCase() }
+            }
+            interface B {
+              default String m(String n) { n.toUpperCase() }
+            }
+            interface C extends A, B {
+              default String m(String n) { n }
+            }
+            class D implements C {
+            }
+            assert new D().m('Hi') == 'Hi'
+        '''
+    }
+
+    @Test
+    void testDuplicateDefaultMethods_moreSpecificDefaultWins() {
+        assertScript '''
+            interface Base {
+              default String m(String n) { n }
+            }
+            interface A extends Base {
+              default String m(String n) { n.toLowerCase() }
+            }
+            interface B extends Base {
+            }
+            class C implements A, B {
+            }
+            assert new C().m('Hi') == 'hi'
+        '''
+    }
+
+    @Test
+    void testDuplicateDefaultMethods_interfaceRedeclaresAbstract() {
+        assertScript '''
+            interface A {
+              default String m(String n) { n.toLowerCase() }
+            }
+            interface B {
+              default String m(String n) { n.toUpperCase() }
+            }
+            interface C extends A, B {
+              String m(String n)
+            }
+            class D implements C {
+              String m(String n) { n }
+            }
+            assert new D().m('Hi') == 'Hi'
+        '''
+    }
+
     @Test
     void testDuplicateDefaultMethodsFromGroovyClasses_implements1() {
         def err = shouldFail '''