groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [groovy] 03/03: GROOVY-9270: add checks for invalid instanceof reference types
Date Tue, 12 Nov 2019 04:19:21 GMT
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 <eric.milles@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/
+    }
+}


Mime
View raw message