groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject groovy git commit: GROOVY-7883 Static compiler prefers private constructor over public if private matches be(closes #705)
Date Sat, 12 May 2018 07:40:48 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_5_X fc1b21fb1 -> cd2b75bac


GROOVY-7883 Static compiler prefers private constructor over public if private matches be(closes
#705)

(cherry picked from commit 3c96b54)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/cd2b75ba
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/cd2b75ba
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/cd2b75ba

Branch: refs/heads/GROOVY_2_5_X
Commit: cd2b75bacdb98cd9be8205ba8814c97f792e11ba
Parents: fc1b21f
Author: Daniel Sun <realbluesun@hotmail.com>
Authored: Sat May 12 15:06:11 2018 +0800
Committer: sunlan <sunlan@apache.org>
Committed: Sat May 12 15:40:41 2018 +0800

----------------------------------------------------------------------
 .../java/org/codehaus/groovy/ast/ClassNode.java |  16 ++
 .../stc/StaticTypeCheckingVisitor.java          |  58 ++++++
 .../sc/MethodCallsStaticCompilationTest.groovy  |   2 +-
 .../classgen/asm/sc/bugs/Groovy7883Bug.groovy   | 175 +++++++++++++++++++
 .../src/main/java/groovy/sql/Sql.java           |   2 +-
 5 files changed, 251 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/cd2b75ba/src/main/java/org/codehaus/groovy/ast/ClassNode.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
index ad3b3f4..1574d4d 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java
@@ -786,6 +786,22 @@ public class ClassNode extends AnnotatedNode implements Opcodes {
         return null;
     }
 
+    public List<ClassNode> getOuterClasses() {
+        if (!(this instanceof InnerClassNode)) {
+            return Collections.emptyList();
+        }
+
+        List<ClassNode> result = new LinkedList<>();
+        ClassNode outestClass = ((InnerClassNode) this).getOuterMostClass();
+        ClassNode cn = this;
+
+        do {
+            result.add(cn = cn.getOuterClass());
+        } while (!cn.equals(outestClass));
+
+        return result;
+    }
+
     /**
      * Adds a statement to the object initializer.
      *

http://git-wip-us.apache.org/repos/asf/groovy/blob/cd2b75ba/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
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 b8f3763..b600160 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -179,6 +179,7 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.isIntCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isLongCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isNumberCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound;
+import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
 import static org.codehaus.groovy.syntax.Types.ASSIGN;
 import static org.codehaus.groovy.syntax.Types.ASSIGNMENT_OPERATOR;
 import static org.codehaus.groovy.syntax.Types.COMPARE_EQUAL;
@@ -4289,6 +4290,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
 
         // lookup in DGM methods too
         findDGMMethodsByNameAndArguments(getTransformLoader(), receiver, name, args, methods);
+        methods = filterMethodsByVisibility(methods);
         List<MethodNode> chosen = chooseBestMethod(receiver, methods, args);
         if (!chosen.isEmpty()) return chosen;
 
@@ -4315,6 +4317,62 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
         return EMPTY_METHODNODE_LIST;
     }
 
+    private List<MethodNode> filterMethodsByVisibility(List<MethodNode> methods)
{
+        if (!asBoolean(methods)) {
+            return EMPTY_METHODNODE_LIST;
+        }
+
+        List<MethodNode> result = new LinkedList<>();
+
+        ClassNode enclosingClassNode = typeCheckingContext.getEnclosingClassNode();
+        boolean isEnclosingInnerClass = enclosingClassNode instanceof InnerClassNode;
+        List<ClassNode> outerClasses = enclosingClassNode.getOuterClasses();
+
+        outer:
+        for (MethodNode methodNode : methods) {
+            if (methodNode instanceof ExtensionMethodNode) {
+                result.add(methodNode);
+                continue;
+            }
+
+            ClassNode declaringClass = methodNode.getDeclaringClass();
+
+            if (isEnclosingInnerClass) {
+                for (ClassNode outerClass : outerClasses) {
+                    if (outerClass.isDerivedFrom(declaringClass)) {
+                        if (outerClass.equals(declaringClass)) {
+                            result.add(methodNode);
+                            continue outer;
+                        } else {
+                            if (methodNode.isPublic() || methodNode.isProtected()) {
+                                result.add(methodNode);
+                                continue outer;
+                            }
+                        }
+                    }
+                }
+            }
+
+            if (declaringClass instanceof InnerClassNode) {
+                if (declaringClass.getOuterClasses().contains(enclosingClassNode)) {
+                    result.add(methodNode);
+                    continue;
+                }
+            }
+
+            if (methodNode.isPrivate() && !enclosingClassNode.equals(declaringClass))
{
+                continue;
+            }
+            if (methodNode.isProtected() && !enclosingClassNode.isDerivedFrom(declaringClass))
{
+                continue;
+            }
+
+            result.add(methodNode);
+        }
+
+        return result;
+    }
+
     /**
      * Given a method name and a prefix, returns the name of the property that should be
looked up,
      * following the java beans rules. For example, "getName" would return "name", while

http://git-wip-us.apache.org/repos/asf/groovy/blob/cd2b75ba/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
index 95d2b08..b0266d2 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/MethodCallsStaticCompilationTest.groovy
@@ -177,7 +177,7 @@ import groovy.transform.TypeCheckingMode//import org.codehaus.groovy.classgen.as
             class Bar {
                 def foo() {new Foo()}
             }
-        ''', 'Cannot call private constructor'
+        ''', '[Static type checking] - Cannot find matching method Foo#<init>()'
     }
 
     // GROOVY-7063

http://git-wip-us.apache.org/repos/asf/groovy/blob/cd2b75ba/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
new file mode 100644
index 0000000..a40fe4e
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7883Bug.groovy
@@ -0,0 +1,175 @@
+/*
+ *  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 org.codehaus.groovy.classgen.asm.sc.bugs
+
+class Groovy7883Bug extends GroovyTestCase {
+    void testGroovy7883() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        void doIt() {
+            throw new AssertionError("abc")
+        }
+        
+        try {
+            doIt()
+            assert false: "should never reach here"
+        } catch (AssertionError e) {
+            assert 'abc' == e.message
+        }
+        '''
+    }
+
+    void test2() {
+        def errMsg = shouldFail '''
+        @groovy.transform.CompileStatic
+        class A {
+            private void doIt() {}
+        }
+        
+        @groovy.transform.CompileStatic
+        class B {
+            public void m() { new A().doIt() }
+        }
+        
+        new B().m()
+        '''
+
+        assert errMsg.contains('[Static type checking] - Cannot find matching method A#doIt()')
+    }
+
+    void test3() {
+        def errMsg = shouldFail '''
+        @groovy.transform.CompileStatic
+        class A {
+            protected void doIt() {}
+        }
+        
+        @groovy.transform.CompileStatic
+        class B {
+            public void m() { new A().doIt() }
+        }
+        
+        new B().m()
+        '''
+
+        assert errMsg.contains('[Static type checking] - Cannot find matching method A#doIt()')
+    }
+
+    void test4() {
+        def errMsg = shouldFail '''
+        @groovy.transform.CompileStatic
+        class A {
+            private void doIt() {}
+        }
+        
+        @groovy.transform.CompileStatic
+        class B extends A {
+            public void m() { doIt() }
+        }
+        
+        new B().m()
+        '''
+
+        assert errMsg.contains('[Static type checking] - Cannot find matching method B#doIt()')
+    }
+
+    /**
+     * ensure the filtering logic does not break any code
+     */
+    void test5() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        class A {
+            protected void doIt() { doIt2() }
+            private void doIt2() {}
+            public void doIt3() { doIt() }
+        }
+        
+        @groovy.transform.CompileStatic
+        class B extends A {
+            public void m() { doIt() }
+        }
+        
+        new B().m()
+        new B().doIt3()
+        '''
+    }
+
+    void test6() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        class A {
+            protected void doIt(ArrayList al) { doIt2(al) }
+            private void doIt2(List list, String x = "abc") {}
+        }
+        
+        @groovy.transform.CompileStatic
+        class B extends A {
+            public void m() { doIt(new ArrayList()) }
+        }
+        
+        new B().m()
+        '''
+    }
+
+    void test7() {
+        assertScript '''
+        @groovy.transform.CompileStatic
+        class A {
+            protected void doIt(ArrayList al) { doIt2(al) }
+            private void doIt2(List list, String x = "abc") {}
+            public void doIt3() { }
+        }
+        
+        @groovy.transform.CompileStatic
+        class B extends A {
+            class C {
+                public void m() {
+                    doIt(new ArrayList());
+                    doIt3();
+                }
+            }
+            
+        }
+        
+        assert true
+        '''
+    }
+
+    void test8() {
+        def errMsg = shouldFail  '''
+        @groovy.transform.CompileStatic
+        class A {
+            protected void doIt(ArrayList al) { doIt2(al) }
+            private void doIt2(List list, String x = "abc") {}
+        }
+        
+        @groovy.transform.CompileStatic
+        class B extends A {
+            class C {
+                public void m() { doIt2(new ArrayList()) }
+            }
+            
+        }
+        
+        '''
+
+        assert errMsg.contains('[Static type checking] - Cannot find matching method B$C#doIt2(java.util.ArrayList)')
+    }
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/cd2b75ba/subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java
----------------------------------------------------------------------
diff --git a/subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java b/subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java
index da61e9f..633c0a7 100644
--- a/subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java
+++ b/subprojects/groovy-sql/src/main/java/groovy/sql/Sql.java
@@ -3973,7 +3973,7 @@ public class Sql {
      * @return the resulting list of rows
      * @throws SQLException if a database error occurs
      */
-    protected List<GroovyRowResult> asList(String sql, ResultSet rs,
+    public List<GroovyRowResult> asList(String sql, ResultSet rs,
                                            @ClosureParams(value=SimpleType.class, options="java.sql.ResultSetMetaData")
Closure metaClosure) throws SQLException {
         return asList(sql, rs, 0, 0, metaClosure);
     }


Mime
View raw message