You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/07/21 21:30:04 UTC

[groovy] branch master updated: GROOVY-10700: STC: non-static (not just default) interface/trait methods

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

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 46cd195ced GROOVY-10700: STC: non-static (not just default) interface/trait methods
46cd195ced is described below

commit 46cd195cedce33d8f21c276c97a529512f771c9b
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jul 21 16:29:46 2022 -0500

    GROOVY-10700: STC: non-static (not just default) interface/trait methods
---
 .../groovy/ast/tools/WideningCategories.java       |  5 --
 .../transform/stc/StaticTypeCheckingVisitor.java   | 43 +++++-------
 src/test/groovy/bugs/Groovy10612.groovy            | 11 ++-
 src/test/groovy/bugs/Groovy10659.groovy            | 16 ++---
 src/test/groovy/bugs/Groovy10676.groovy            |  6 +-
 src/test/groovy/bugs/Groovy10700.groovy            | 82 ++++++++++++++++++++++
 6 files changed, 115 insertions(+), 48 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java b/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
index f07568d515..d68bc3cf5e 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/WideningCategories.java
@@ -20,7 +20,6 @@ package org.codehaus.groovy.ast.tools;
 
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.GenericsType;
-import org.codehaus.groovy.ast.MethodNode;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -554,10 +553,6 @@ public class WideningCategories {
             for (ClassNode anInterface : interfaces) {
                 usesGenerics |= anInterface.isUsingGenerics();
                 genericsTypesList.add(anInterface.getGenericsTypes());
-                for (MethodNode methodNode : anInterface.getMethods()) {
-                    MethodNode method = addMethod(methodNode.getName(), methodNode.getModifiers(), methodNode.getReturnType(), methodNode.getParameters(), methodNode.getExceptions(), methodNode.getCode());
-                    method.setDeclaringClass(anInterface); // important for static compilation!
-                }
             }
             setUsingGenerics(usesGenerics);
             if (usesGenerics) {
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index f56bc8eaf3..0b26ae18c9 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4787,9 +4787,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     /**
      * Returns methods defined for the specified receiver and adds "non-existing"
      * methods that will be generated afterwards by the compiler; for example if
-     * a method is using default values and the class node isn't compiled yet.
+     * a method is using default values and the class node is not compiled yet.
      *
-     * @param receiver the receiver where to find methods
+     * @param receiver the type to search for methods
      * @param name     the name of the methods to return
      * @return the methods that are defined on the receiver completed with stubs for future methods
      */
@@ -4806,24 +4806,29 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         }
 
         List<MethodNode> methods = receiver.getMethods(name);
-        if (receiver.isAbstract()) {
-            collectAllInterfaceMethodsByName(receiver, name, methods);
-        } else { // GROOVY-9890: always search for default methods
-            List<MethodNode> interfaceMethods = new ArrayList<>();
-            collectAllInterfaceMethodsByName(receiver, name, interfaceMethods);
-            interfaceMethods.stream().filter(mn -> mn.isDefault() || (mn.isPublic()
-                && !mn.isStatic() && !mn.isAbstract() && Traits.isTrait(mn.getDeclaringClass()))
-            ).forEach(methods::add);
+
+        // GROOVY-5166, GROOVY-9890, GROOVY-10700: non-static interface/trait methods
+        Set<ClassNode> done = new HashSet<>();
+        for (ClassNode next = receiver; next != null; next = next.getSuperClass()) {
+            done.add(next);
+            for (ClassNode face : next.getAllInterfaces()) {
+                if (done.add(face)) {
+                    for (MethodNode mn : face.getDeclaredMethods(name)) {
+                        if (mn.isPublic() && !mn.isStatic()) methods.add(mn);
+                    }
+                }
+            }
         }
 
         if (receiver.isInterface()) {
             methods.addAll(OBJECT_TYPE.getMethods(name));
         }
 
-        if (methods.isEmpty() || receiver.isResolved()) {
-            return methods;
+        if (!receiver.isResolved() && !methods.isEmpty()) {
+            methods = addGeneratedMethods(receiver, methods);
         }
-        return addGeneratedMethods(receiver, methods);
+
+        return methods;
     }
 
     private static List<MethodNode> addGeneratedMethods(final ClassNode receiver, final List<MethodNode> methods) {
@@ -5028,18 +5033,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return null;
     }
 
-    private static void collectAllInterfaceMethodsByName(final ClassNode type, final String name, final List<MethodNode> methods) {
-        Set<ClassNode> done = new LinkedHashSet<>();
-        for (ClassNode next = type; next != null; next = next.getSuperClass()) {
-            done.add(next);
-            for (ClassNode face : next.getAllInterfaces()) {
-                if (done.add(face)) {
-                    methods.addAll(face.getDeclaredMethods(name));
-                }
-            }
-        }
-    }
-
     protected ClassNode getType(final ASTNode node) {
         ClassNode type = node.getNodeMetaData(INFERRED_TYPE);
         if (type != null) {
diff --git a/src/test/groovy/bugs/Groovy10612.groovy b/src/test/groovy/bugs/Groovy10612.groovy
index 65c4b0908a..9516113538 100644
--- a/src/test/groovy/bugs/Groovy10612.groovy
+++ b/src/test/groovy/bugs/Groovy10612.groovy
@@ -16,15 +16,14 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-package bugs
+package groovy.bugs
 
-import groovy.transform.CompileStatic
 import org.junit.Test
 
 import static groovy.test.GroovyAssert.assertScript
 
-@CompileStatic
-class Groovy10612 {
+final class Groovy10612 {
+
     @Test
     void testStringConcatenationWithPlus() {
         assertScript '''
@@ -34,10 +33,10 @@ class Groovy10612 {
     }
 
     @Test
-    void testStringConcatenationWithPlus_CS() {
+    void testStringConcatenationWithPlusCS() {
         assertScript '''
             @groovy.transform.CompileStatic
-            def m() {
+            void m() {
                 assert '[a:1] is a map' == [a:1] + ' is a map'
                 assert '[a:1] is a map' == [a:1] + " is ${'a'} map"
             }
diff --git a/src/test/groovy/bugs/Groovy10659.groovy b/src/test/groovy/bugs/Groovy10659.groovy
index e3f3c3178c..0be9859816 100644
--- a/src/test/groovy/bugs/Groovy10659.groovy
+++ b/src/test/groovy/bugs/Groovy10659.groovy
@@ -16,40 +16,38 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-package bugs
+package groovy.bugs
 
-import groovy.transform.CompileStatic
 import org.junit.Test
 
 import static groovy.test.GroovyAssert.assertScript
 
-@CompileStatic
-class Groovy10659 {
+final class Groovy10659 {
     @Test
     void test() {
         assertScript '''
             def fun = { arg ->
                 return arg
             }
-            
+
             def r = fun (
                 x: 1,
                 [a: 1]: '2'
             )
             assert '[x:1, [a:1]:2]' == r.toString()
-            
+
             r = fun ([
                 x: 1,
                 [a: 1]: '2'
-            ]) 
+            ])
             assert '[x:1, [a:1]:2]' == r.toString()
-            
+
             r = fun (
                 x: 1,
                 [1, 2]: '2'
             )
             assert '[x:1, [1, 2]:2]' == r.toString()
-            
+
             r = fun (
                 x: 1,
                 [a: 1]: '2',
diff --git a/src/test/groovy/bugs/Groovy10676.groovy b/src/test/groovy/bugs/Groovy10676.groovy
index e5f22ee30b..6782550f58 100644
--- a/src/test/groovy/bugs/Groovy10676.groovy
+++ b/src/test/groovy/bugs/Groovy10676.groovy
@@ -16,18 +16,18 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-package bugs
+package groovy.bugs
 
 import org.codehaus.groovy.tools.ErrorReporter
 import org.junit.Test
 
-class Groovy10676 {
+final class Groovy10676 {
     @Test
     void test() {
         try {
             // error should be at column > 40, first line should be short
             new GroovyShell().parse('/*\r * some comment\r */\r           class class {}\r')
-        } catch(e) {
+        } catch (e) {
             Writer data = new StringWriter()
             new ErrorReporter(e, false).write(new PrintWriter(data))
             assert data.toString().contains("Unexpected input: 'class'")
diff --git a/src/test/groovy/bugs/Groovy10700.groovy b/src/test/groovy/bugs/Groovy10700.groovy
new file mode 100644
index 0000000000..7f55822375
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy10700.groovy
@@ -0,0 +1,82 @@
+/*
+ *  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 groovy.bugs
+
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy10700 {
+    @Test
+    void testDelegateAndInterface() {
+        def config = new CompilerConfiguration(targetDirectory: File.createTempDir())
+
+        def parentDir = File.createTempDir()
+        try {
+            new File(parentDir, 'p').mkdir()
+
+            def a = new File(parentDir, 'p/ABC.groovy')
+            a.write '''
+                package p
+
+                interface A {
+                    String decode(String s)
+                }
+
+                class B implements A {
+                    String decode(String s) { s }
+                }
+
+                class C implements A {
+                    @Delegate private final B b = new B()
+                }
+            '''
+            def b = new File(parentDir, 'p/D.groovy')
+            b.write '''
+                package p
+
+                class D extends C { // implements A
+                    @groovy.transform.TypeChecked
+                    void test() {
+                        def x = decode('string')
+                        assert x == 'string'
+                    }
+                }
+            '''
+
+            def loader = new GroovyClassLoader(this.class.classLoader)
+            def cu = new CompilationUnit(config, null, loader)
+            cu.addSources(a, b)
+            cu.compile()
+
+            def basePath = config.targetDirectory.absolutePath.replace('\\','/')
+            assertScript """
+                def loader = new GroovyClassLoader(this.class.classLoader)
+                loader.addClasspath('$basePath')
+                def d = loader.loadClass('p.D')
+                d.newInstance().test()
+            """
+        } finally {
+            config.targetDirectory.deleteDir()
+            parentDir.deleteDir()
+        }
+    }
+}