You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/11/12 04:19:21 UTC
[groovy] 03/03: GROOVY-9270: add checks for invalid instanceof
reference types
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 91838d121b06196217753647e032633762479ea8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Oct 7 14:16:13 2019 -0500
GROOVY-9270: add checks for invalid instanceof reference types
http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.20.2
---
.../codehaus/groovy/control/CompilationUnit.java | 24 ++++--
.../groovy/control/InstanceOfVerifier.java | 58 +++++++++++++
.../java/org/codehaus/groovy/syntax/Types.java | 12 ++-
src/test/groovy/bugs/Groovy9270.groovy | 96 ++++++++++++++++++++++
4 files changed, 177 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
index 0aa7372..293403b 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
@@ -26,6 +26,7 @@ import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.CompileUnit;
+import org.codehaus.groovy.ast.GroovyClassVisitor;
import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.ModuleNode;
import org.codehaus.groovy.classgen.AsmClassGenerator;
@@ -810,8 +811,19 @@ public class CompilationUnit extends ProcessingUnit {
);
}
- LabelVerifier lv = new LabelVerifier(source);
- lv.visitClass(classNode);
+ GroovyClassVisitor visitor = new LabelVerifier(source);
+ visitor.visitClass(classNode);
+
+ visitor = new InstanceOfVerifier() {
+ @Override
+ protected SourceUnit getSourceUnit() {
+ return source;
+ }
+ };
+ visitor.visitClass(classNode);
+
+ visitor = new ClassCompletionVerifier(source);
+ visitor.visitClass(classNode);
ClassCompletionVerifier completionVerifier = new ClassCompletionVerifier(source);
completionVerifier.visitClass(classNode);
@@ -826,7 +838,7 @@ public class CompilationUnit extends ProcessingUnit {
//
// Prep the generator machinery
//
- ClassVisitor visitor = createClassVisitor();
+ ClassVisitor classVisitor = createClassVisitor();
String sourceName = (source == null ? classNode.getModule().getDescription() : source.getName());
// only show the file name and its extension like javac does in its stacktraces rather than the full path
@@ -835,21 +847,21 @@ public class CompilationUnit extends ProcessingUnit {
sourceName = sourceName.substring(Math.max(sourceName.lastIndexOf('\\'), sourceName.lastIndexOf('/')) + 1);
//TraceClassVisitor tracer = new TraceClassVisitor(visitor, new PrintWriter(System.err,true));
//AsmClassGenerator generator = new AsmClassGenerator(source, context, tracer, sourceName);
- AsmClassGenerator generator = new AsmClassGenerator(source, context, visitor, sourceName);
+ AsmClassGenerator generator = new AsmClassGenerator(source, context, classVisitor, sourceName);
//
// Run the generation and create the class (if required)
//
generator.visitClass(classNode);
- byte[] bytes = ((ClassWriter) visitor).toByteArray();
+ byte[] bytes = ((ClassWriter) classVisitor).toByteArray();
generatedClasses.add(new GroovyClass(classNode.getName(), bytes));
//
// Handle any callback that's been set
//
if (CompilationUnit.this.classgenCallback != null) {
- classgenCallback.call(visitor, classNode);
+ classgenCallback.call(classVisitor, classNode);
}
//
diff --git a/src/main/java/org/codehaus/groovy/control/InstanceOfVerifier.java b/src/main/java/org/codehaus/groovy/control/InstanceOfVerifier.java
new file mode 100644
index 0000000..8eacaf2
--- /dev/null
+++ b/src/main/java/org/codehaus/groovy/control/InstanceOfVerifier.java
@@ -0,0 +1,58 @@
+/*
+ * 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.control;
+
+import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.expr.BinaryExpression;
+import org.codehaus.groovy.ast.expr.ClassExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.syntax.Types;
+
+public abstract class InstanceOfVerifier extends ClassCodeVisitorSupport {
+
+ @Override
+ public void visitBinaryExpression(BinaryExpression expression) {
+ if (expression.getOperation().isA(Types.INSTANCEOF_OPERATOR) &&
+ expression.getRightExpression() instanceof ClassExpression) {
+ ClassNode referenceType = expression.getRightExpression().getType();
+
+ if (ClassHelper.isPrimitiveType(referenceType)) {
+ addTypeError(expression.getRightExpression(), "primitive type " + referenceType.getName());
+ } else {
+ while (referenceType.isArray()) {
+ referenceType = referenceType.getComponentType();
+ }
+
+ if (referenceType.isGenericsPlaceHolder()) {
+ addTypeError(expression.getRightExpression(), "type parameter " + referenceType.getUnresolvedName() +
+ ". Use its erasure " + referenceType.getNameWithoutPackage() + " instead since further generic type information will be erased at runtime");
+ } else if (referenceType.getGenericsTypes() != null) {
+ // TODO: Cannot perform instanceof check against parameterized type Class<Type>. Use the form Class<?> instead since further eneric type information will be erased at runtime
+ }
+ }
+ }
+ super.visitBinaryExpression(expression);
+ }
+
+ private void addTypeError(Expression referenceExpr, String referenceType) {
+ addError("Cannot perform instanceof check against " + referenceType, referenceExpr);
+ }
+}
diff --git a/src/main/java/org/codehaus/groovy/syntax/Types.java b/src/main/java/org/codehaus/groovy/syntax/Types.java
index 3b2aa70..463efe8 100644
--- a/src/main/java/org/codehaus/groovy/syntax/Types.java
+++ b/src/main/java/org/codehaus/groovy/syntax/Types.java
@@ -301,6 +301,7 @@ public class Types {
public static final int REGEX_COMPARISON_OPERATOR = 1105; // =~, etc.
public static final int DEREFERENCE_OPERATOR = 1106; // ., ->
public static final int BITWISE_OPERATOR = 1107; // |, &, <<, >>, >>>, ^, ~
+ public static final int INSTANCEOF_OPERATOR = 1108; // instanceof
public static final int PREFIX_OPERATOR = 1200; // ++, !, etc.
public static final int POSTFIX_OPERATOR = 1210; // ++, etc.
@@ -343,8 +344,6 @@ public class Types {
public static final int SIMPLE_EXPRESSION = 1910; // LITERAL, this, true, false, null
public static final int COMPLEX_EXPRESSION = 1911; // SIMPLE_EXPRESSION, and various molecules
-
-
//
// TYPE GROUPS (OPERATIONS SUPPORT)
@@ -361,14 +360,9 @@ public class Types {
public static final int PRECLUDES_CAST_OPERATOR = 2008; // anything that prevents (X) from being a cast
-
-
-
-
//---------------------------------------------------------------------------
// TYPE HIERARCHIES
-
/**
* Given two types, returns true if the second describes the first.
*/
@@ -418,6 +412,9 @@ public class Types {
case COMPARISON_OPERATOR:
return specific >= COMPARE_NOT_EQUAL && specific <= COMPARE_TO;
+ case INSTANCEOF_OPERATOR:
+ return specific == KEYWORD_INSTANCEOF;
+
case MATH_OPERATOR:
return (specific >= PLUS && specific <= RIGHT_SHIFT_UNSIGNED) || (specific >= NOT && specific <= LOGICAL_AND)
|| (specific >= BITWISE_OR && specific <= BITWISE_XOR);
@@ -1405,6 +1402,7 @@ public class Types {
addDescription( PREFIX_OPERATOR , "<prefix operator>" );
addDescription( POSTFIX_OPERATOR , "<postfix operator>" );
addDescription( INFIX_OPERATOR , "<infix operator>" );
+ addDescription( INSTANCEOF_OPERATOR , "<instanceof operator>" );
addDescription( KEYWORD , "<keyword>" );
addDescription( LITERAL , "<literal>" );
addDescription( NUMBER , "<number>" );
diff --git a/src/test/groovy/bugs/Groovy9270.groovy b/src/test/groovy/bugs/Groovy9270.groovy
new file mode 100644
index 0000000..c4c794a
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9270.groovy
@@ -0,0 +1,96 @@
+/*
+ * 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.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+@CompileStatic
+final class Groovy9270 {
+
+ @Test
+ void testInstanceOfPrimitive1() {
+ def err = shouldFail '''
+ void meth(obj) {
+ if (obj instanceof int) {
+ // ...
+ }
+ }
+ '''
+
+ assert err =~ / Cannot perform instanceof check against primitive type int/
+ }
+
+ @Test
+ void testInstanceOfPrimitive2() {
+ def err = shouldFail '''
+ void meth(obj) {
+ if (obj !instanceof int) {
+ // ...
+ }
+ }
+ '''
+
+ assert err =~ / Cannot perform instanceof check against primitive type int/
+ }
+
+ @Test
+ void testInstanceOfPrimitiveArray() {
+ assertScript '''
+ void meth(obj) {
+ if (obj instanceof double[]) {
+ // ...
+ }
+ }
+ '''
+ }
+
+ @Test
+ void testInstanceOfTypeParameter1() {
+ def err = shouldFail '''
+ class C<T extends Number> {
+ void meth(obj) {
+ if (obj instanceof T) {
+ // ...
+ }
+ }
+ }
+ '''
+
+ assert err =~ / Cannot perform instanceof check against type parameter T/
+ }
+
+ @Test
+ void testInstanceOfTypeParameter2() {
+ def err = shouldFail '''
+ class C<T> {
+ void meth(obj) {
+ if (obj instanceof T[]) {
+ // ...
+ }
+ }
+ }
+ '''
+
+ assert err =~ / Cannot perform instanceof check against type parameter T/
+ }
+}