From 6630009aaa5d8c8019e7a6bc79d3a3d7c55b20ce Mon Sep 17 00:00:00 2001 From: Daniel Sun Date: Tue, 1 Oct 2019 17:14:04 +0800 Subject: [PATCH] GROOVY-9261: Refine type checking for ARM --- build.gradle | 5 +- .../org/codehaus/groovy/ast/ClassHelper.java | 1 + .../groovy/ast/stmt/TryCatchStatement.java | 20 +++-- .../stc/StaticTypeCheckingVisitor.java | 15 +++- src/test/groovy/bugs/Groovy9261.groovy | 80 +++++++++++++++++++ 5 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 src/test/groovy/bugs/Groovy9261.groovy diff --git a/build.gradle b/build.gradle index 6cf78e3f065..350c81a9ab5 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 0bdf5855607..d1d8790e012 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 4bb8a4dc9d9..e21b3f91a79 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 @@ * 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 resourceStatements = new ArrayList(); - private List catchStatements = new ArrayList(); + private List resourceStatements = new ArrayList<>(); + private List catchStatements = new ArrayList<>(); private Statement finallyStatement; @@ -62,13 +64,21 @@ public Statement getTryStatement() { } 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 37a3c585b6f..878cbd060c6 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 void visitRangeExpression(final RangeExpression expression) { } @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 void visitBinaryExpression(BinaryExpression expression) { } if (null == lType) lType = getType(leftExpression); + ClassNode rType = getType(rightExpression); if (isNullConstant(rightExpression)) { if (!isPrimitiveType(lType)) @@ -995,11 +996,23 @@ && isAssignment(enclosingBinaryExpression.getOperation().getType()) 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 00000000000..347d5c72af6 --- /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.') + } +}