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 2019/08/15 07:13:38 UTC

[groovy] branch master updated: GROOVY-9215: Incorrect compile time access error is raised when using… (#992)

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

sunlan 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 5865a07  GROOVY-9215: Incorrect compile time access error is raised when using… (#992)
5865a07 is described below

commit 5865a07f4537717c6f7d2600ae1fbf30a2e5db87
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Thu Aug 15 15:13:32 2019 +0800

    GROOVY-9215: Incorrect compile time access error is raised when using… (#992)
    
    * GROOVY-9215: Incorrect compile time access error is raised when using @CompileStatic and/or @TypeChecked
    
    * Trivial refactoring
    
    * Distinct `CompileDynamic`, `CompileStatic` and `TypeChecked`
---
 .../groovy/transform/ASTTransformationVisitor.java |  44 +++++-
 src/test/groovy/bugs/Groovy9215Bug.groovy          | 150 +++++++++++++++++++++
 2 files changed, 192 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
index 696ea5f..c356811 100644
--- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
@@ -19,12 +19,15 @@
 package org.codehaus.groovy.transform;
 
 import groovy.lang.GroovyClassLoader;
+import groovy.lang.Tuple;
+import groovy.lang.Tuple3;
 import groovy.transform.CompilationUnitAware;
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
 import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.classgen.GeneratorContext;
 import org.codehaus.groovy.control.ASTTransformationsContext;
 import org.codehaus.groovy.control.CompilationFailedException;
@@ -154,15 +157,52 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport {
      *
      * @param node the node to be processed
      */
-    public void visitAnnotations(AnnotatedNode node) {
+    public void visitAnnotations(final AnnotatedNode node) {
         super.visitAnnotations(node);
-        for (AnnotationNode annotation : node.getAnnotations()) {
+        for (AnnotationNode annotation : distinctAnnotations(node)) {
             if (transforms.containsKey(annotation)) {
                 targetNodes.add(new ASTNode[]{annotation, node});
             }
         }
     }
 
+    private static final Tuple3<String, String, String> COMPILEDYNAMIC_AND_COMPILESTATIC_AND_TYPECHECKED =
+            Tuple.tuple("groovy.transform.CompileDynamic", "groovy.transform.CompileStatic", "groovy.transform.TypeChecked");
+
+    // GROOVY-9215
+    // `StaticTypeCheckingVisitor` visits multi-times because `node` has duplicated `CompileStatic` and `TypeChecked`
+    // If annotation with higher priority appears, annotation with lower priority will be ignored
+    // Priority: CompileDynamic > CompileStatic > TypeChecked
+    private List<AnnotationNode> distinctAnnotations(AnnotatedNode node) {
+        List<AnnotationNode> result = new LinkedList<>();
+        AnnotationNode resultAnnotationNode = null;
+        int resultIndex = -1;
+
+        for (AnnotationNode annotationNode : node.getAnnotations()) {
+            int index = COMPILEDYNAMIC_AND_COMPILESTATIC_AND_TYPECHECKED.indexOf(annotationNode.getClassNode().getName());
+            if (-1 != index) {
+                if (1 == index) { // CompileStatic
+                    Expression value = annotationNode.getMember("value");
+                    if (null != value && "groovy.transform.TypeCheckingMode.SKIP".equals(value.getText())) {
+                        index = 0; // `CompileStatic` with "SKIP" `value` is actually `CompileDynamic`
+                    }
+                }
+
+                if (null == resultAnnotationNode || index < resultIndex) {
+                    resultAnnotationNode = annotationNode;
+                    resultIndex = index;
+                }
+                continue;
+            }
+            result.add(annotationNode);
+        }
+
+        if (null != resultAnnotationNode) result.add(resultAnnotationNode);
+
+        return result;
+    }
+
+
     public static void addPhaseOperations(final CompilationUnit compilationUnit) {
         final ASTTransformationsContext context = compilationUnit.getASTTransformationsContext();
         addGlobalTransforms(context);
diff --git a/src/test/groovy/bugs/Groovy9215Bug.groovy b/src/test/groovy/bugs/Groovy9215Bug.groovy
new file mode 100644
index 0000000..d534515
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9215Bug.groovy
@@ -0,0 +1,150 @@
+/*
+ *  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
+
+class Groovy9215Bug extends GroovyTestCase {
+    void testDuplicatedAnnotations1() {
+        assertScript '''
+            import groovy.transform.CompileStatic
+            import groovy.transform.TypeChecked
+            
+            @TypeChecked
+            @CompileStatic
+            class Data {
+                void getThing(Closure c){
+                    c("hello")
+                }
+            }
+            
+            @CompileStatic
+            @CompileStatic
+            class Op {
+                public Data d = new Data()
+            
+                void aFunc(Closure c){
+                    c()
+                }
+            
+                void broken() {
+                    aFunc({
+                        d.getThing({ String res ->
+                            println(" ")
+                        })
+                    })
+                }
+            }
+            
+            assert Op.class
+        '''
+    }
+
+    void testDuplicatedAnnotations2() {
+        assertScript '''
+            import groovy.transform.CompileStatic
+            import groovy.transform.TypeChecked
+            
+            @TypeChecked
+            @CompileStatic
+            class Data {
+                void getThing(Closure c){
+                    c("hello")
+                }
+            }
+            
+            @TypeChecked
+            @CompileStatic
+            class Op {
+                public Data d = new Data()
+            
+                void aFunc(Closure c){
+                    c()
+                }
+            
+                void broken() {
+                    aFunc({
+                        d.getThing({ String res ->
+                            println(" ")
+                        })
+                    })
+                }
+            }
+            
+            assert Op.class
+        '''
+    }
+
+    void testDuplicatedAnnotations3() {
+        assertScript '''
+            import groovy.transform.CompileStatic
+            import groovy.transform.TypeChecked
+            
+            @TypeChecked
+            @CompileStatic
+            class Data {
+                void getThing(Closure c){
+                    c("hello")
+                }
+            }
+            
+            @TypeChecked
+            @TypeChecked
+            class Op {
+                public Data d = new Data()
+            
+                void aFunc(Closure c){
+                    c()
+                }
+            
+                void broken() {
+                    aFunc({
+                        d.getThing({ String res ->
+                            println(" ")
+                        })
+                    })
+                }
+            }
+            
+            assert Op.class
+        '''
+    }
+
+    void testDuplicatedAnnotations4() {
+        assertScript '''
+            import groovy.transform.CompileDynamic
+            import groovy.transform.CompileStatic
+            import groovy.transform.TypeChecked
+            
+            class Person {
+                String name
+            }
+            
+            @CompileStatic  // ignored
+            @CompileDynamic // taken effect
+            @TypeChecked    // ignored
+            def x() {
+                Person.metaClass.introduce << {return "I'm $name"}
+                def person = new Person(name:"Daniel.Sun")
+                assert "I'm Daniel.Sun" == person.introduce()
+            }
+            
+            x()
+        '''
+    }
+}