groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject [groovy] 05/05: GROOVY-9261: Refine type checking for ARM
Date Mon, 07 Oct 2019 03:21:30 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit 901eec6f1583c2e908753326502fa4c4b898e3a2
Author: Daniel Sun <sunlan@apache.org>
AuthorDate: Tue Oct 1 17:14:04 2019 +0800

    GROOVY-9261: Refine type checking for ARM
    
    (cherry picked from commit 68fe49af7e53ce73a2e64a0899536563397c2609)
---
 build.gradle                                       |  5 +-
 .../java/org/codehaus/groovy/ast/ClassHelper.java  |  1 +
 .../groovy/ast/stmt/TryCatchStatement.java         | 20 ++++--
 .../transform/stc/StaticTypeCheckingVisitor.java   | 15 +++-
 src/test/groovy/bugs/Groovy9261.groovy             | 80 ++++++++++++++++++++++
 5 files changed, 111 insertions(+), 10 deletions(-)

diff --git a/build.gradle b/build.gradle
index ce010fb..03a9c98 100644
--- a/build.gradle
+++ b/build.gradle
@@ -116,10 +116,6 @@ allprojects {
     tasks.withType(org.asciidoctor.gradle.AsciidoctorTask) {
         outputs.cacheIf { true }
     }
-
-    tasks.withType(GroovyCompile) {
-        groovyOptions.forkOptions.jvmArgs += ["-Dgroovy.antlr4.cache.threshold=100"]
-    }
 }
 
 task(copyTestResources, type: Copy)
@@ -403,6 +399,7 @@ allprojects {
     }
 
     tasks.withType(GroovyCompile) {
+        groovyOptions.forkOptions.jvmArgs += ["-Dgroovy.antlr4.cache.threshold=100"]
         groovyOptions.fork(memoryMaximumSize: groovycMain_mx)
         groovyOptions.encoding = 'UTF-8'
         //options.compilerArgs << "-Xlint:unchecked" << "-Xlint:deprecation"
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
index 0bdf585..d1d8790 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassHelper.java
@@ -123,6 +123,7 @@ public class ClassHelper {
 
     void_WRAPPER_TYPE = makeCached(Void.class), METACLASS_TYPE = makeCached(MetaClass.class),
             Iterator_TYPE = makeCached(Iterator.class),
+            AUTOCLOSEABLE_TYPE = makeCached(AutoCloseable.class),
 
     Enum_Type = makeWithoutCaching(Enum.class),
             Annotation_TYPE = makeCached(Annotation.class),
diff --git a/src/main/java/org/codehaus/groovy/ast/stmt/TryCatchStatement.java b/src/main/java/org/codehaus/groovy/ast/stmt/TryCatchStatement.java
index 4bb8a4d..e21b3f9 100644
--- a/src/main/java/org/codehaus/groovy/ast/stmt/TryCatchStatement.java
+++ b/src/main/java/org/codehaus/groovy/ast/stmt/TryCatchStatement.java
@@ -18,8 +18,10 @@
  */
 package org.codehaus.groovy.ast.stmt;
 
+import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.GroovyCodeVisitor;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
+import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.VariableExpression;
 
 import java.util.ArrayList;
@@ -29,10 +31,10 @@ import java.util.List;
  * Represents a try { ... } catch () finally {} statement in Groovy
  */
 public class TryCatchStatement extends Statement {
-
+    private static final String IS_RESOURCE = "_IS_RESOURCE";
     private Statement tryStatement;
-    private List<ExpressionStatement> resourceStatements = new ArrayList<ExpressionStatement>();
-    private List<CatchStatement> catchStatements = new ArrayList<CatchStatement>();
+    private List<ExpressionStatement> resourceStatements = new ArrayList<>();
+    private List<CatchStatement> catchStatements = new ArrayList<>();
     private Statement finallyStatement;
     
 
@@ -62,13 +64,21 @@ public class TryCatchStatement extends Statement {
     }
 
     public void addResource(ExpressionStatement resourceStatement) {
-        if (!(resourceStatement.getExpression() instanceof DeclarationExpression || resourceStatement.getExpression()
instanceof VariableExpression)) {
-            throw new IllegalArgumentException("resourceStatement should be a variable declaration
statement or a variable");
+        Expression resourceExpression = resourceStatement.getExpression();
+        if (!(resourceExpression instanceof DeclarationExpression || resourceExpression instanceof
VariableExpression)) {
+            throw new GroovyBugError("resourceStatement should be a variable declaration
statement or a variable");
         }
 
+        resourceExpression.putNodeMetaData(IS_RESOURCE, Boolean.TRUE);
+
         resourceStatements.add(resourceStatement);
     }
 
+    public static boolean isResource(Expression expression) {
+        Boolean r = expression.getNodeMetaData(IS_RESOURCE);
+        return null != r && r;
+    }
+
     public void addCatch(CatchStatement catchStatement) {
         catchStatements.add(catchStatement);
     }
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 37a3c58..878cbd0 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -814,7 +814,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
     }
 
     @Override
-    public void visitBinaryExpression(BinaryExpression expression) {
+    public void visitBinaryExpression(final BinaryExpression expression) {
         BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
         typeCheckingContext.pushEnclosingBinaryExpression(expression);
         try {
@@ -856,6 +856,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
             }
 
             if (null == lType) lType = getType(leftExpression);
+
             ClassNode rType = getType(rightExpression);
             if (isNullConstant(rightExpression)) {
                 if (!isPrimitiveType(lType))
@@ -995,11 +996,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport
{
             if (!isEmptyDeclaration) {
                 storeType(expression, resultType);
             }
+
+            validateResourceInARM(expression, resultType);
         } finally {
             typeCheckingContext.popEnclosingBinaryExpression();
         }
     }
 
+    private void validateResourceInARM(BinaryExpression expression, ClassNode lType) {
+        if (expression instanceof DeclarationExpression) {
+            if (TryCatchStatement.isResource(expression)) {
+                if (!lType.implementsInterface(ClassHelper.AUTOCLOSEABLE_TYPE)) {
+                    addError("Resource[" + lType.getName() + "] in ARM should be of type
AutoCloseable", expression);
+                }
+            }
+        }
+    }
+
     private void inferParameterAndReturnTypesOfClosureOnRHS(ClassNode lType, Expression rightExpression,
int op) {
         if (ASSIGN == op) {
             if (rightExpression instanceof ClosureExpression && ClassHelper.isFunctionalInterface(lType))
{
diff --git a/src/test/groovy/bugs/Groovy9261.groovy b/src/test/groovy/bugs/Groovy9261.groovy
new file mode 100644
index 0000000..347d5c7
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9261.groovy
@@ -0,0 +1,80 @@
+/*
+ *  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 groovy.test.GroovyTestCase
+
+class Groovy9261 extends GroovyTestCase {
+    void testInvalidResourceInARM() {
+        String errMsg = shouldFail '''\
+            @groovy.transform.CompileStatic
+            void test() {
+                try (String str = '123') {
+                }
+            }
+            test()
+        '''
+
+        assert errMsg.contains('Resource[java.lang.String] in ARM should be of type AutoCloseable')
+        assert errMsg.contains('@ line 3, column 22.')
+    }
+
+    void testInvalidResourceInARM2() {
+        String errMsg = shouldFail '''\
+            @groovy.transform.CompileStatic
+            void test() {
+                try (str = '123') {
+                }
+            }
+            test()
+        '''
+
+        assert errMsg.contains('Resource[java.lang.String] in ARM should be of type AutoCloseable')
+        assert errMsg.contains('@ line 3, column 22.')
+    }
+
+    void testInvalidResourceInARM3() {
+        String errMsg = shouldFail '''\
+            @groovy.transform.CompileStatic
+            void test() {
+                try (def sr = new StringReader(''); str = '123') {
+                }
+            }
+            test()
+        '''
+
+        assert errMsg.contains('Resource[java.lang.String] in ARM should be of type AutoCloseable')
+        assert errMsg.contains('@ line 3, column 53.')
+    }
+
+    void testInvalidResourceInEnhancedARM() {
+        String errMsg = shouldFail '''\
+            @groovy.transform.CompileStatic
+            void test() {
+                String str = '123'
+                try (str) {
+                }
+            }
+            test()
+        '''
+
+        assert errMsg.contains('Resource[java.lang.String] in ARM should be of type AutoCloseable')
+        assert errMsg.contains('@ line 4, column 22.')
+    }
+}


Mime
View raw message