[groovy] branch GROOVY_3_0_X updated (efbf991 -> 4fb0893)

classic Classic list List threaded Threaded
21 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[groovy] branch GROOVY_3_0_X updated (efbf991 -> 4fb0893)

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

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


    from efbf991  GROOVY-9326: Bump CodeNarc to 1.5 (#1113)
     new 90f44af  organize members and other minor edits
     new 91a65f0  remove dead code
     new 58b35d4  reset flags after constructor (like after method) and other minor edits
     new 5515c09  GROOVY-9317: record field expression value for assert
     new 45610c5  GROOVY-9318: add support for ** syntax in star import white/black lists
     new 4d4be63  refactor to remove repeated creation of Class<Type>
     new 285e82e  GROOVY-8648: fix ASM error for this or super attribute expression on LHS
     new b8f8728  GROOVY-8762: add test case
     new 496524c  minor edits
     new d62a3f0  GROOVY-8762: make WriterController#isInClosure() more selective
     new a95cdb3  refactor to move property-only field checking to visitPropertyExpression
     new b977dc4  add test case for super attribte expression
     new 4dfdb1a  make parameters final and remove dead path in visitAttributeExpression
     new 1c9d4e5  refactor InvocationWriter to reduce duplication around makeCall
     new 4259a53  GROOVY-8468: add test case
     new 2731e71  GROOVY-8361: add test case
     new 8815125  refactor and reorganize
     new 9dd7eda  refactor dynamic variable handling
     new e15269a  track "in closure" and other minor edits
     new 4fb0893  fix for "in closure" tracking

The 20 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../groovy/classgen/AsmClassGenerator.java         | 534 ++++++++---------
 .../groovy/classgen/VariableScopeVisitor.java      | 578 +++++++++---------
 .../groovy/classgen/asm/DelegatingController.java  |  94 ++-
 .../groovy/classgen/asm/InvocationWriter.java      | 401 +++++--------
 .../classgen/asm/OptimizingStatementWriter.java    | 649 +++++++++++----------
 .../groovy/classgen/asm/WriterController.java      |  85 ++-
 .../classgen/asm/sc/StaticInvocationWriter.java    |  13 +-
 .../control/customizers/SecureASTCustomizer.java   |  84 +--
 .../transform/stc/StaticTypeCheckingVisitor.java   |  39 +-
 src/spec/test/semantics/PowerAssertTest.groovy     |  24 +-
 src/test/groovy/bugs/Groovy8361.groovy             | 115 ++++
 .../bugs/{Groovy9265.groovy => Groovy8468.groovy}  |  21 +-
 .../bugs/{Groovy9265.groovy => Groovy8648.groovy}  |  26 +-
 .../bugs/{Groovy6996.groovy => Groovy8762.groovy}  |  49 +-
 .../test/groovy/bugs/groovy8468/Face.java          |  46 +-
 .../groovy/bugs/groovy8468/FaceImpl.java}          |  46 +-
 .../groovy/bugs/groovy8468/Factory.java}           |  47 +-
 .../groovy/bugs/groovy8468/FactoryImpl.java}       |  58 +-
 .../groovy/classgen/asm/sc/bugs/Groovy7300.groovy  |  22 +-
 .../customizers/SecureASTCustomizerTest.groovy     | 449 ++++++++------
 .../powerassert/AssertionRenderingTest.groovy      | 290 +++++----
 .../runtime/powerassert/ValueRenderingTest.groovy  | 121 ++--
 22 files changed, 1973 insertions(+), 1818 deletions(-)
 create mode 100644 src/test/groovy/bugs/Groovy8361.groovy
 copy src/test/groovy/bugs/{Groovy9265.groovy => Groovy8468.groovy} (72%)
 copy src/test/groovy/bugs/{Groovy9265.groovy => Groovy8648.groovy} (68%)
 copy src/test/groovy/bugs/{Groovy6996.groovy => Groovy8762.groovy} (54%)
 copy subprojects/parser-antlr4/src/test/resources/fail/FieldDeclaration_01x.groovy => src/test/groovy/bugs/groovy8468/Face.java (91%)
 copy src/{main/java/groovy/lang/Buildable.java => test/groovy/bugs/groovy8468/FaceImpl.java} (89%)
 copy src/{main/java/groovy/lang/Buildable.java => test/groovy/bugs/groovy8468/Factory.java} (85%)
 copy src/{main/java/org/codehaus/groovy/runtime/wrappers/ShortWrapper.java => test/groovy/bugs/groovy8468/FactoryImpl.java} (64%)

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 01/20: organize members and other minor edits

Daniel.Sun
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 90f44afb90ac5f5b03584bd5c9d738b87cfe416b
Author: Eric Milles <[hidden email]>
AuthorDate: Sat Nov 23 11:55:04 2019 -0600

    organize members and other minor edits
   
    (cherry picked from commit b51fe06dd9aa5d41489106d7a3047dbda76bb68e)
---
 .../classgen/asm/OptimizingStatementWriter.java    | 527 +++++++++++----------
 1 file changed, 273 insertions(+), 254 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index fb07f94..5c4b50c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -60,8 +60,10 @@ import org.codehaus.groovy.syntax.Types;
 import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
 
+import java.util.Deque;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
 
 import static org.codehaus.groovy.ast.ClassHelper.BigDecimal_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.GROOVY_INTERCEPTABLE_TYPE;
@@ -85,40 +87,8 @@ import static org.objectweb.asm.Opcodes.IFEQ;
 import static org.objectweb.asm.Opcodes.IFNE;
 import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
 
-/**
- * A class to write out the optimized statements
- */
 public class OptimizingStatementWriter extends StatementWriter {
 
-    private static class FastPathData {
-        private Label pathStart = new Label();
-        private Label afterPath = new Label();
-    }
-
-    public static class ClassNodeSkip{}
-
-    public static class StatementMeta {
-        private boolean optimize=false;
-        protected MethodNode target;
-        protected ClassNode type;
-        protected VariableExpression declaredVariableExpression;
-        protected boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
-        public void chainInvolvedTypes(OptimizeFlagsCollector opt) {
-            for (int i=0; i<typeMapKeyNames.length; i++) {
-                if (opt.current.involvedTypes[i]) {
-                    this.involvedTypes[i] = true;
-                }
-            }
-        }
-        public String toString() {
-            StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
-            for (int i=0; i<typeMapKeyNames.length; i++) {
-                if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
-            }
-            return ret.toString();
-        }
-    }
-
     private static final MethodCaller[] guards = {
         null,
         MethodCaller.newStatic(BytecodeInterface8.class, "isOrigInt"),
@@ -132,8 +102,8 @@ public class OptimizingStatementWriter extends StatementWriter {
     };
 
     private static final MethodCaller disabledStandardMetaClass = MethodCaller.newStatic(BytecodeInterface8.class, "disabledStandardMetaClass");
-    private boolean fastPathBlocked = false;
     private final WriterController controller;
+    private boolean fastPathBlocked;
 
     public OptimizingStatementWriter(WriterController controller) {
         super(controller);
@@ -475,27 +445,69 @@ public class OptimizingStatementWriter extends StatementWriter {
         return meta;
     }
 
+    //--------------------------------------------------------------------------
+
+    public static class ClassNodeSkip {
+    }
+
+    private static class FastPathData {
+        private Label pathStart = new Label();
+        private Label afterPath = new Label();
+    }
+
+    public static class StatementMeta {
+        private boolean optimize;
+        protected ClassNode type;
+        protected MethodNode target;
+        protected VariableExpression declaredVariableExpression;
+        protected boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
+
+        public void chainInvolvedTypes(final OptimizeFlagsCollector opt) {
+            for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+                if (opt.current.involvedTypes[i]) {
+                    this.involvedTypes[i] = true;
+                }
+            }
+        }
+
+        @Override
+        public String toString() {
+            StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
+            for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+                if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+            }
+            return ret.toString();
+        }
+    }
+
     private static class OptimizeFlagsCollector {
         private static class OptimizeFlagsEntry {
-            private boolean canOptimize = false;
-            private boolean shouldOptimize = false;
+            private boolean canOptimize;
+            private boolean shouldOptimize;
             private boolean[] involvedTypes = new boolean[typeMapKeyNames.length];
         }
+
         private OptimizeFlagsEntry current = new OptimizeFlagsEntry();
-        private final LinkedList<OptimizeFlagsEntry> olderEntries = new LinkedList<OptimizeFlagsEntry>();
+        private final Deque<OptimizeFlagsEntry> previous = new LinkedList<>();
+
         public void push() {
-            olderEntries.addLast(current);
+            previous.addLast(current);
             current = new OptimizeFlagsEntry();
         }
-        public void pop(boolean propagateFlags){
+
+        public void pop(final boolean propagateFlags) {
             OptimizeFlagsEntry old = current;
-            current = olderEntries.removeLast();
+            current = previous.removeLast();
             if (propagateFlags) {
                 chainCanOptimize(old.canOptimize);
                 chainShouldOptimize(old.shouldOptimize);
-                for (int i=0; i<typeMapKeyNames.length; i++) current.involvedTypes[i] |= old.involvedTypes[i];
+                for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
+                    current.involvedTypes[i] |= old.involvedTypes[i];
+                }
             }
         }
+
+        @Override
         public String toString() {
             StringBuilder ret;
             if (current.shouldOptimize) {
@@ -506,40 +518,46 @@ public class OptimizingStatementWriter extends StatementWriter {
                 ret = new StringBuilder("don't optimize");
             }
             ret.append(" involvedTypes =");
-            for (int i=0; i<typeMapKeyNames.length; i++) {
+            for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
                 if (current.involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
             }
             return ret.toString();
         }
+
         /**
          * @return true iff we should Optimize - this is almost seen as must
          */
         private boolean shouldOptimize() {
             return current.shouldOptimize;
         }
+
         /**
          * @return true iff we can optimize, but not have to
          */
         private boolean canOptimize() {
             return current.canOptimize || current.shouldOptimize;
         }
+
         /**
          * set "should" to true, if not already
          */
-        public void chainShouldOptimize(boolean opt) {
+        public void chainShouldOptimize(final boolean opt) {
             current.shouldOptimize = shouldOptimize() || opt;
         }
+
         /**
          * set "can" to true, if not already
          */
-        public void chainCanOptimize(boolean opt) {
+        public void chainCanOptimize(final boolean opt) {
             current.canOptimize = current.canOptimize || opt;
         }
-        public void chainInvolvedType(ClassNode type) {
+
+        public void chainInvolvedType(final ClassNode type) {
             Integer res = typeMap.get(type);
-            if (res==null) return;
+            if (res == null) return;
             current.involvedTypes[res] = true;
         }
+
         public void reset() {
             current.canOptimize = false;
             current.shouldOptimize = false;
@@ -548,125 +566,115 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     private static class OptVisitor extends ClassCodeVisitorSupport {
+        private static final VariableScope nonStaticScope = new VariableScope();
+        private final OptimizeFlagsCollector opt = new OptimizeFlagsCollector();
+        private boolean optimizeMethodCall = true;
         private final TypeChooser typeChooser;
+        private VariableScope scope;
+        private ClassNode node;
 
-        public OptVisitor(final TypeChooser chooser) {
+        OptVisitor(final TypeChooser chooser) {
             this.typeChooser = chooser;
         }
 
-        @Override protected SourceUnit getSourceUnit() {return null;}
-
-        private ClassNode node;
-        private OptimizeFlagsCollector opt = new OptimizeFlagsCollector();
-        private boolean optimizeMethodCall = true;
-        private VariableScope scope;
-        private static final VariableScope nonStaticScope = new VariableScope();
+        @Override
+        protected SourceUnit getSourceUnit() {
+            return null;
+        }
 
         @Override
-        public void visitClass(ClassNode node) {
+        public void visitClass(final ClassNode node) {
             this.optimizeMethodCall = !node.implementsInterface(GROOVY_INTERCEPTABLE_TYPE);
             this.node = node;
             this.scope = nonStaticScope;
             super.visitClass(node);
-            this.scope=null;
-            this.node=null;
-        }
-
-        @Override
-        public void visitMethod(MethodNode node) {
-            scope = node.getVariableScope();
-            super.visitMethod(node);
-            opt.reset();
+            this.scope = null;
+            this.node = null;
         }
 
         @Override
-        public void visitConstructor(ConstructorNode node) {
+        public void visitConstructor(final ConstructorNode node) {
             scope = node.getVariableScope();
             super.visitConstructor(node);
         }
 
         @Override
-        public void visitReturnStatement(ReturnStatement statement) {
-            opt.push();
-            super.visitReturnStatement(statement);
-            if (opt.shouldOptimize()) addMeta(statement,opt);
-            opt.pop(opt.shouldOptimize());
+        public void visitMethod(final MethodNode node) {
+            scope = node.getVariableScope();
+            super.visitMethod(node);
+            opt.reset();
         }
 
-        @Override
-        public void visitUnaryMinusExpression(UnaryMinusExpression expression) {
-            //TODO: implement int operations for this
-            super.visitUnaryMinusExpression(expression);
-            StatementMeta meta = addMeta(expression);
-            meta.type = OBJECT_TYPE;
-        }
+        // statements:
 
         @Override
-        public void visitUnaryPlusExpression(UnaryPlusExpression expression) {
-            //TODO: implement int operations for this
-            super.visitUnaryPlusExpression(expression);
-            StatementMeta meta = addMeta(expression);
-            meta.type = OBJECT_TYPE;
+        public void visitBlockStatement(final BlockStatement statement) {
+            opt.push();
+            boolean optAll = true;
+            for (Statement stmt : statement.getStatements()) {
+                opt.push();
+                stmt.visit(this);
+                optAll = optAll && opt.canOptimize();
+                opt.pop(true);
+            }
+            if (statement.isEmpty()) {
+                opt.chainCanOptimize(true);
+                opt.pop(true);
+            } else {
+                opt.chainShouldOptimize(optAll);
+                if (optAll) {
+                    addMeta(statement, opt);
+                }
+                opt.pop(optAll);
+            }
         }
 
         @Override
-        public void visitBitwiseNegationExpression(BitwiseNegationExpression expression) {
-            //TODO: implement int operations for this
-            super.visitBitwiseNegationExpression(expression);
-            StatementMeta meta = addMeta(expression);
-            meta.type = OBJECT_TYPE;
-        }
-
-        private void addTypeInformation(Expression expression, Expression orig) {
-            ClassNode type = typeChooser.resolveType(expression, node);
-            if (isPrimitiveType(type)) {
-                StatementMeta meta = addMeta(orig);
-                meta.type = type;
-                opt.chainShouldOptimize(true);
-                opt.chainInvolvedType(type);
+        public void visitExpressionStatement(final ExpressionStatement statement) {
+            if (statement.getNodeMetaData(StatementMeta.class) != null) return;
+            opt.push();
+            super.visitExpressionStatement(statement);
+            if (opt.shouldOptimize()) {
+                addMeta(statement, opt);
             }
+            opt.pop(opt.shouldOptimize());
         }
 
         @Override
-        public void visitPrefixExpression(PrefixExpression expression) {
-            super.visitPrefixExpression(expression);
-            addTypeInformation(expression.getExpression(),expression);
+        public void visitForLoop(final ForStatement statement) {
+            opt.push();
+            super.visitForLoop(statement);
+            if (opt.shouldOptimize()) {
+                addMeta(statement, opt);
+            }
+            opt.pop(opt.shouldOptimize());
         }
 
         @Override
-        public void visitPostfixExpression(PostfixExpression expression) {
-            super.visitPostfixExpression(expression);
-            addTypeInformation(expression.getExpression(),expression);
+        public void visitIfElse(final IfStatement statement) {
+            opt.push();
+            super.visitIfElse(statement);
+            if (opt.shouldOptimize()) {
+                addMeta(statement, opt);
+            }
+            opt.pop(opt.shouldOptimize());
         }
 
         @Override
-        public void visitDeclarationExpression(DeclarationExpression expression) {
-            Expression right = expression.getRightExpression();
-            right.visit(this);
-
-            ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
-            Expression rightExpression = expression.getRightExpression();
-            ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType);
-            if (rightType==null) rightType = typeChooser.resolveType(expression.getRightExpression(), node);
-            if (isPrimitiveType(leftType) && isPrimitiveType(rightType)) {
-                // if right is a constant, then we optimize only if it makes
-                // a block complete, so we set a maybe
-                if (right instanceof ConstantExpression) {
-                    opt.chainCanOptimize(true);
-                } else {
-                    opt.chainShouldOptimize(true);
-                }
-                StatementMeta meta = addMeta(expression);
-                ClassNode declarationType = typeChooser.resolveType(expression, node);
-                meta.type = declarationType!=null?declarationType:leftType;
-                opt.chainInvolvedType(leftType);
-                opt.chainInvolvedType(rightType);
+        public void visitReturnStatement(final ReturnStatement statement) {
+            opt.push();
+            super.visitReturnStatement(statement);
+            if (opt.shouldOptimize()) {
+                addMeta(statement,opt);
             }
+            opt.pop(opt.shouldOptimize());
         }
 
+        // expressions:
+
         @Override
-        public void visitBinaryExpression(BinaryExpression expression) {
-            if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
+        public void visitBinaryExpression(final BinaryExpression expression) {
+            if (expression.getNodeMetaData(StatementMeta.class) != null) return;
             super.visitBinaryExpression(expression);
 
             ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
@@ -674,7 +682,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             ClassNode resultType = null;
             int operation = expression.getOperation().getType();
 
-            if (operation==Types.LEFT_SQUARE_BRACKET && leftType.isArray()) {
+            if (operation == Types.LEFT_SQUARE_BRACKET && leftType.isArray()) {
                 opt.chainShouldOptimize(true);
                 resultType = leftType.getComponentType();
             } else {
@@ -696,8 +704,10 @@ public class OptimizingStatementWriter extends StatementWriter {
                         }
                         resultType = boolean_TYPE;
                         break;
-                    case Types.LOGICAL_AND: case Types.LOGICAL_AND_EQUAL:
-                    case Types.LOGICAL_OR: case Types.LOGICAL_OR_EQUAL:
+                    case Types.LOGICAL_AND:
+                    case Types.LOGICAL_AND_EQUAL:
+                    case Types.LOGICAL_OR:
+                    case Types.LOGICAL_OR_EQUAL:
                         if (boolean_TYPE.equals(leftType) && boolean_TYPE.equals(rightType)) {
                             opt.chainShouldOptimize(true);
                         } else {
@@ -706,7 +716,8 @@ public class OptimizingStatementWriter extends StatementWriter {
                         expression.setType(boolean_TYPE);
                         resultType = boolean_TYPE;
                         break;
-                    case Types.DIVIDE: case Types.DIVIDE_EQUAL:
+                    case Types.DIVIDE:
+                    case Types.DIVIDE_EQUAL:
                         if (isLongCategory(leftType) && isLongCategory(rightType)) {
                             resultType = BigDecimal_TYPE;
                             opt.chainShouldOptimize(true);
@@ -718,8 +729,9 @@ public class OptimizingStatementWriter extends StatementWriter {
                             opt.chainShouldOptimize(true);
                         }
                         break;
-                    case Types.POWER: case Types.POWER_EQUAL:
-                        //TODO: implement
+                    case Types.POWER:
+                    case Types.POWER_EQUAL:
+                        // TODO: implement
                         break;
                     case Types.ASSIGN:
                         resultType = optimizeDivWithIntOrLongTarget(expression.getRightExpression(), leftType);
@@ -742,166 +754,189 @@ public class OptimizingStatementWriter extends StatementWriter {
                 }
             }
 
-            if (resultType!=null) {
-                StatementMeta meta = addMeta(expression);
-                meta.type = resultType;
+            if (resultType != null) {
+                addMeta(expression).type = resultType;
                 opt.chainInvolvedType(resultType);
                 opt.chainInvolvedType(leftType);
                 opt.chainInvolvedType(rightType);
             }
         }
 
-        /**
-         * method to optimize Z = X/Y with Z being int or long style
-         * @returns null if the optimization cannot be applied, otherwise it
-         * will return the new target type
-         */
-        private ClassNode optimizeDivWithIntOrLongTarget(Expression rhs, ClassNode assignmentTartgetType) {
-            if (!(rhs instanceof BinaryExpression)) return null;
-            BinaryExpression binExp = (BinaryExpression) rhs;
-            int op = binExp.getOperation().getType();
-            if (op!=Types.DIVIDE && op!=Types.DIVIDE_EQUAL) return null;
-
-            ClassNode originalResultType = typeChooser.resolveType(binExp, node);
-            if (    !originalResultType.equals(BigDecimal_TYPE) ||
-                    !(isLongCategory(assignmentTartgetType) || isFloatingCategory(assignmentTartgetType))
-            ) {
-                return null;
-            }
+        @Override
+        public void visitBitwiseNegationExpression(final BitwiseNegationExpression expression) {
+            // TODO: implement int operations for this
+            super.visitBitwiseNegationExpression(expression);
+            addMeta(expression).type = OBJECT_TYPE;
+        }
 
-            ClassNode leftType = typeChooser.resolveType(binExp.getLeftExpression(), node);
-            if (!isLongCategory(leftType)) return null;
-            ClassNode rightType = typeChooser.resolveType(binExp.getRightExpression(), node);
-            if (!isLongCategory(rightType)) return null;
+        @Override
+        public void visitClosureExpression(final ClosureExpression expression) {
+        }
 
-            ClassNode target;
-            if (isIntCategory(leftType) && isIntCategory(rightType)) {
-                target = int_TYPE;
-            } else if (isLongCategory(leftType) && isLongCategory(rightType)) {
-                target = long_TYPE;
-            } else if (isDoubleCategory(leftType) && isDoubleCategory(rightType)) {
-                target = double_TYPE;
-            } else {
-                return null;
-            }
-            StatementMeta meta = addMeta(rhs);
-            meta.type = target;
-            opt.chainInvolvedType(target);
-            return target;
+        @Override
+        public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
+            if (expression.getNodeMetaData(StatementMeta.class) != null) return;
+            super.visitConstructorCallExpression(expression);
+            // we cannot set a target for the constructor call, since we cannot easily check the meta class of the other class
+            //setMethodTarget(call, "<init>", call.getArguments(), false);
         }
 
         @Override
-        public void visitExpressionStatement(ExpressionStatement statement) {
-            if (statement.getNodeMetaData(StatementMeta.class)!=null) return;
-            opt.push();
-            super.visitExpressionStatement(statement);
-            if (opt.shouldOptimize()) addMeta(statement,opt);
-            opt.pop(opt.shouldOptimize());
+        public void visitDeclarationExpression(final DeclarationExpression expression) {
+            Expression right = expression.getRightExpression();
+            right.visit(this);
+
+            ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
+            Expression rightExpression = expression.getRightExpression();
+            ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType);
+            if (rightType == null) rightType = typeChooser.resolveType(expression.getRightExpression(), node);
+            if (isPrimitiveType(leftType) && isPrimitiveType(rightType)) {
+                // if right is a constant, then we optimize only if it makes a block complete, so we set a maybe
+                if (right instanceof ConstantExpression) {
+                    opt.chainCanOptimize(true);
+                } else {
+                    opt.chainShouldOptimize(true);
+                }
+                addMeta(expression).type = Optional.ofNullable(typeChooser.resolveType(expression, node)).orElse(leftType);
+                opt.chainInvolvedType(leftType);
+                opt.chainInvolvedType(rightType);
+            }
         }
 
         @Override
-        public void visitBlockStatement(BlockStatement block) {
-            opt.push();
-            boolean optAll = true;
-            for (Statement statement : block.getStatements()) {
-                opt.push();
-                statement.visit(this);
-                optAll = optAll && opt.canOptimize();
-                opt.pop(true);
+        public void visitMethodCallExpression(final MethodCallExpression expression) {
+            if (expression.getNodeMetaData(StatementMeta.class) != null) return;
+            super.visitMethodCallExpression(expression);
+
+            Expression objectExpression = expression.getObjectExpression();
+            boolean setTarget = AsmClassGenerator.isThisExpression(objectExpression);
+            if (!setTarget) {
+                if (!(objectExpression instanceof ClassExpression)) return;
+                setTarget = objectExpression.equals(node); // XXX: comparing Expression and ClassNode
             }
-            if (block.isEmpty()) {
-                opt.chainCanOptimize(true);
-                opt.pop(true);
-            } else {
-                opt.chainShouldOptimize(optAll);
-                if (optAll) addMeta(block,opt);
-                opt.pop(optAll);
+            if (setTarget) {
+                setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
             }
         }
 
         @Override
-        public void visitIfElse(IfStatement statement) {
-            opt.push();
-            super.visitIfElse(statement);
-            if (opt.shouldOptimize()) addMeta(statement,opt);
-            opt.pop(opt.shouldOptimize());
+        public void visitPostfixExpression(final PostfixExpression expression) {
+            super.visitPostfixExpression(expression);
+            addTypeInformation(expression.getExpression(), expression);
         }
 
-        /*@Override
-        public void visitConstantExpression(ConstantExpression expression) {
-            super.visitConstantExpression(expression);
-            opt.chainShouldOptimize(true);
-        }*/
+        @Override
+        public void visitPrefixExpression(final PrefixExpression expression) {
+            super.visitPrefixExpression(expression);
+            addTypeInformation(expression.getExpression(), expression);
+        }
 
         @Override
-        public void visitStaticMethodCallExpression(StaticMethodCallExpression expression) {
-            if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
+        public void visitStaticMethodCallExpression(final StaticMethodCallExpression expression) {
+            if (expression.getNodeMetaData(StatementMeta.class) != null) return;
             super.visitStaticMethodCallExpression(expression);
+            setMethodTarget(expression, expression.getMethod(), expression.getArguments(), true);
+        }
 
-            setMethodTarget(expression,expression.getMethod(), expression.getArguments(), true);
+        @Override
+        public void visitUnaryMinusExpression(final UnaryMinusExpression expression) {
+            // TODO: implement int operations for this
+            super.visitUnaryMinusExpression(expression);
+            addMeta(expression).type = OBJECT_TYPE;
         }
 
         @Override
-        public void visitMethodCallExpression(MethodCallExpression expression) {
-            if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
-            super.visitMethodCallExpression(expression);
+        public void visitUnaryPlusExpression(final UnaryPlusExpression expression) {
+            // TODO: implement int operations for this
+            super.visitUnaryPlusExpression(expression);
+            addMeta(expression).type = OBJECT_TYPE;
+        }
 
-            Expression object = expression.getObjectExpression();
-            boolean setTarget = AsmClassGenerator.isThisExpression(object);
-            if (!setTarget) {
-                if (!(object instanceof ClassExpression)) return;
-                setTarget = object.equals(node);
-            }
+        //
 
-            if (!setTarget) return;
-            setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
+        private void addTypeInformation(final Expression expression, final Expression orig) {
+            ClassNode type = typeChooser.resolveType(expression, node);
+            if (isPrimitiveType(type)) {
+                addMeta(orig).type = type;
+                opt.chainShouldOptimize(true);
+                opt.chainInvolvedType(type);
+            }
         }
 
-        @Override
-        public void visitConstructorCallExpression(ConstructorCallExpression call) {
-            if (call.getNodeMetaData(StatementMeta.class)!=null) return;
-            super.visitConstructorCallExpression(call);
+        /**
+         * Optimizes "Z = X/Y" with Z being int or long style.
+         *
+         * @returns null if the optimization cannot be applied, otherwise it will return the new target type
+         */
+        private ClassNode optimizeDivWithIntOrLongTarget(final Expression rhs, final ClassNode assignmentTartgetType) {
+            if (!(rhs instanceof BinaryExpression)) return null;
+            BinaryExpression binExp = (BinaryExpression) rhs;
+            int op = binExp.getOperation().getType();
+            if (op != Types.DIVIDE && op != Types.DIVIDE_EQUAL) return null;
 
-            // we cannot a target for the constructor call, since we cannot easily
-            // check the meta class of the other class
-            // setMethodTarget(call, "<init>", call.getArguments(), false);
+            ClassNode originalResultType = typeChooser.resolveType(binExp, node);
+            if (!originalResultType.equals(BigDecimal_TYPE)
+                    || !(isLongCategory(assignmentTartgetType)
+                    || isFloatingCategory(assignmentTartgetType))) {
+                return null;
+            }
+
+            ClassNode leftType = typeChooser.resolveType(binExp.getLeftExpression(), node);
+            if (!isLongCategory(leftType)) return null;
+            ClassNode rightType = typeChooser.resolveType(binExp.getRightExpression(), node);
+            if (!isLongCategory(rightType)) return null;
+
+            ClassNode target;
+            if (isIntCategory(leftType) && isIntCategory(rightType)) {
+                target = int_TYPE;
+            } else if (isLongCategory(leftType) && isLongCategory(rightType)) {
+                target = long_TYPE;
+            } else if (isDoubleCategory(leftType) && isDoubleCategory(rightType)) {
+                target = double_TYPE;
+            } else {
+                return null;
+            }
+            addMeta(rhs).type = target;
+            opt.chainInvolvedType(target);
+            return target;
         }
 
-        private void setMethodTarget(Expression expression, String name, Expression callArgs, boolean isMethod) {
-            if (name==null) return;
+        private void setMethodTarget(final Expression expression, final String name, final Expression callArgs, final boolean isMethod) {
+            if (name == null) return;
             if (!optimizeMethodCall) return;
             if (AsmClassGenerator.containsSpreadExpression(callArgs)) return;
+
             // find method call target
             Parameter[] paraTypes = null;
             if (callArgs instanceof ArgumentListExpression) {
                 ArgumentListExpression args = (ArgumentListExpression) callArgs;
                 int size = args.getExpressions().size();
                 paraTypes = new Parameter[size];
-                int i=0;
-                for (Expression exp: args.getExpressions()) {
+                int i = 0;
+                for (Expression exp : args.getExpressions()) {
                     ClassNode type = typeChooser.resolveType(exp, node);
                     if (!validTypeForCall(type)) return;
-                    paraTypes[i] = new Parameter(type,"");
-                    i++;
+                    paraTypes[i] = new Parameter(type, "");
+                    i += 1;
                 }
             } else {
                 ClassNode type = typeChooser.resolveType(callArgs, node);
                 if (!validTypeForCall(type)) return;
-                paraTypes = new Parameter[]{new Parameter(type,"")};
+                paraTypes = new Parameter[]{new Parameter(type, "")};
             }
 
             MethodNode target;
             ClassNode type;
             if (isMethod) {
                 target = node.getMethod(name, paraTypes);
-                if (target==null) return;
+                if (target == null) return;
                 if (!target.getDeclaringClass().equals(node)) return;
                 if (scope.isInStaticContext() && !target.isStatic()) return;
                 type = target.getReturnType().redirect();
             } else {
                 type = expression.getType();
                 target = selectConstructor(type, paraTypes);
-                if (target==null) return;
+                if (target == null) return;
             }
 
             StatementMeta meta = addMeta(expression);
@@ -910,37 +945,21 @@ public class OptimizingStatementWriter extends StatementWriter {
             opt.chainShouldOptimize(true);
         }
 
-        private static MethodNode selectConstructor(ClassNode node, Parameter[] paraTypes) {
-            List<ConstructorNode> cl = node.getDeclaredConstructors();
-            MethodNode res = null;
-            for (ConstructorNode cn : cl) {
-                if (ParameterUtils.parametersEqual(cn.getParameters(), paraTypes)) {
-                    res = cn;
+        private static MethodNode selectConstructor(final ClassNode node, final Parameter[] parameters) {
+            List<ConstructorNode> ctors = node.getDeclaredConstructors();
+            MethodNode result = null;
+            for (ConstructorNode ctor : ctors) {
+                if (ParameterUtils.parametersEqual(ctor.getParameters(), parameters)) {
+                    result = ctor;
                     break;
                 }
             }
-            if (res !=null && res.isPublic()) return res;
-            return null;
+            return (result != null && result.isPublic() ? result : null);
         }
 
-        private static boolean validTypeForCall(ClassNode type) {
+        private static boolean validTypeForCall(final ClassNode type) {
             // do call only for final classes and primitive types
-            if (isPrimitiveType(type)) return true;
-            return (type.getModifiers() & ACC_FINAL) > 0;
-        }
-
-        @Override
-        public void visitClosureExpression(ClosureExpression expression) {
-        }
-
-        @Override
-        public void visitForLoop(ForStatement statement) {
-            opt.push();
-            super.visitForLoop(statement);
-            if (opt.shouldOptimize()) addMeta(statement,opt);
-            opt.pop(opt.shouldOptimize());
+            return isPrimitiveType(type) || (type.getModifiers() & ACC_FINAL) > 0;
         }
     }
-
-
 }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 02/20: remove dead code

Daniel.Sun
In reply to this post by Daniel.Sun
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 91a65f05f397812f0c57aa81c2b227cd27dab316
Author: Eric Milles <[hidden email]>
AuthorDate: Sat Nov 23 16:12:37 2019 -0600

    remove dead code
   
    - a ClassNode will never be equal to an Expression
   
    (cherry picked from commit 1ed200297104752138b59decdad9e4315bfe4ff6)
---
 .../classgen/asm/OptimizingStatementWriter.java       | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index 5c4b50c..94ae2ef 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -29,7 +29,6 @@ import org.codehaus.groovy.ast.VariableScope;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
 import org.codehaus.groovy.ast.expr.BitwiseNegationExpression;
-import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
@@ -170,7 +169,7 @@ public class OptimizingStatementWriter extends StatementWriter {
         if (fastPathData==null) {
             // normal mode with different paths
             // important is to not to have a fastpathblock here,
-            // otherwise the per expression statement improvement
+            // otherwise the per expression statement improvement
             // is impossible
             super.writeBlockStatement(statement);
         } else {
@@ -346,15 +345,15 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.writeExpressionStatement(statement);
         } else {
             StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
-            // we have to have handle DelcarationExpressions special, since their
+            // we have to have handle DelcarationExpressions special, since their
             // entry should be outside the optimization path, we have to do that of
-            // course only if we are actually going to do two different paths,
+            // course only if we are actually going to do two different paths,
             // otherwise it is not needed
             //
             // there are several cases to be considered now.
             // (1) no fast path possible, so just do super
-            // (2) fast path possible, and at path split point (meaning not in
-            //     fast path and not in slow path). Here we have to extract the
+            // (2) fast path possible, and at path split point (meaning not in
+            //     fast path and not in slow path). Here we have to extract the
             //     Declaration and replace by an assignment
             // (3) fast path possible and in slow or fastPath. Nothing to do here.
             //
@@ -808,13 +807,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             if (expression.getNodeMetaData(StatementMeta.class) != null) return;
             super.visitMethodCallExpression(expression);
 
-            Expression objectExpression = expression.getObjectExpression();
-            boolean setTarget = AsmClassGenerator.isThisExpression(objectExpression);
-            if (!setTarget) {
-                if (!(objectExpression instanceof ClassExpression)) return;
-                setTarget = objectExpression.equals(node); // XXX: comparing Expression and ClassNode
-            }
-            if (setTarget) {
+            if (AsmClassGenerator.isThisExpression(expression.getObjectExpression())) {
                 setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
             }
         }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 03/20: reset flags after constructor (like after method) and other minor edits

Daniel.Sun
In reply to this post by Daniel.Sun
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 58b35d466a07bcd41a7c6e4523944aa2bfb35dba
Author: Eric Milles <[hidden email]>
AuthorDate: Sat Nov 23 16:44:25 2019 -0600

    reset flags after constructor (like after method) and other minor edits
   
    (cherry picked from commit e03c5a6362de3d4239eedfeb0b5ef96c9c8aa597)
---
 .../classgen/asm/OptimizingStatementWriter.java    | 129 ++++++++++-----------
 1 file changed, 64 insertions(+), 65 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index 94ae2ef..75fbf73 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -50,7 +50,6 @@ import org.codehaus.groovy.ast.stmt.IfStatement;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
 import org.codehaus.groovy.ast.stmt.WhileStatement;
-import org.codehaus.groovy.ast.tools.ParameterUtils;
 import org.codehaus.groovy.classgen.AsmClassGenerator;
 import org.codehaus.groovy.classgen.Verifier;
 import org.codehaus.groovy.control.SourceUnit;
@@ -72,6 +71,7 @@ import static org.codehaus.groovy.ast.ClassHelper.double_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
 import static org.codehaus.groovy.ast.ClassHelper.long_TYPE;
+import static org.codehaus.groovy.ast.tools.ParameterUtils.parametersEqual;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isDoubleCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isFloatingCategory;
@@ -88,6 +88,7 @@ import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
 
 public class OptimizingStatementWriter extends StatementWriter {
 
+    // values correspond to BinaryExpressionMultiTypeDispatcher.typeMapKeyNames
     private static final MethodCaller[] guards = {
         null,
         MethodCaller.newStatic(BytecodeInterface8.class, "isOrigInt"),
@@ -104,24 +105,20 @@ public class OptimizingStatementWriter extends StatementWriter {
     private final WriterController controller;
     private boolean fastPathBlocked;
 
-    public OptimizingStatementWriter(WriterController controller) {
+    public OptimizingStatementWriter(final WriterController controller) {
         super(controller);
         this.controller = controller;
     }
 
-    private boolean notEnableFastPath(StatementMeta meta) {
-        // return false if cannot do fast path and if are already on the path
-        return fastPathBlocked || meta==null || !meta.optimize || controller.isFastPath();
-    }
+    private FastPathData writeGuards(final StatementMeta meta, final Statement statement) {
+        if (fastPathBlocked || controller.isFastPath() || meta == null || !meta.optimize) return null;
 
-    private FastPathData writeGuards(StatementMeta meta, Statement statement) {
-        if (notEnableFastPath(meta)) return null;
         controller.getAcg().onLineNumber(statement, null);
         MethodVisitor mv = controller.getMethodVisitor();
         FastPathData fastPathData = new FastPathData();
         Label slowPath = new Label();
 
-        for (int i=0; i<guards.length; i++) {
+        for (int i = 0, n = guards.length; i < n; i += 1) {
             if (meta.involvedTypes[i]) {
                 guards[i].call(mv);
                 mv.visitJumpInsn(IFEQ, slowPath);
@@ -131,42 +128,40 @@ public class OptimizingStatementWriter extends StatementWriter {
         // meta class check with boolean holder
         String owner = BytecodeHelper.getClassInternalName(controller.getClassNode());
         MethodNode mn = controller.getMethodNode();
-        if (mn!=null) {
+        if (mn != null) {
             mv.visitFieldInsn(GETSTATIC, owner, Verifier.STATIC_METACLASS_BOOL, "Z");
             mv.visitJumpInsn(IFNE, slowPath);
         }
 
-        //standard metaclass check
+        // standard metaclass check
         disabledStandardMetaClass.call(mv);
         mv.visitJumpInsn(IFNE, slowPath);
 
         // other guards here
-
         mv.visitJumpInsn(GOTO, fastPathData.pathStart);
         mv.visitLabel(slowPath);
 
         return fastPathData;
     }
 
-    private void writeFastPathPrelude(FastPathData meta) {
+    private void writeFastPathPrelude(final FastPathData meta) {
         MethodVisitor mv = controller.getMethodVisitor();
         mv.visitJumpInsn(GOTO, meta.afterPath);
         mv.visitLabel(meta.pathStart);
         controller.switchToFastPath();
     }
 
-    private void writeFastPathEpilogue(FastPathData meta) {
+    private void writeFastPathEpilogue(final FastPathData meta) {
         MethodVisitor mv = controller.getMethodVisitor();
         mv.visitLabel(meta.afterPath);
         controller.switchToSlowPath();
     }
 
     @Override
-    public void writeBlockStatement(BlockStatement statement) {
+    public void writeBlockStatement(final BlockStatement statement) {
         StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
         FastPathData fastPathData = writeGuards(meta, statement);
-
-        if (fastPathData==null) {
+        if (fastPathData == null) {
             // normal mode with different paths
             // important is to not to have a fastpathblock here,
             // otherwise the per expression statement improvement
@@ -186,7 +181,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    public void writeDoWhileLoop(DoWhileStatement statement) {
+    public void writeDoWhileLoop(final DoWhileStatement statement) {
         if (controller.isFastPath()) {
             super.writeDoWhileLoop(statement);
         } else {
@@ -198,7 +193,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.writeDoWhileLoop(statement);
             fastPathBlocked = oldFastPathBlock;
 
-            if (fastPathData==null) return;
+            if (fastPathData == null) return;
             writeFastPathPrelude(fastPathData);
             super.writeDoWhileLoop(statement);
             writeFastPathEpilogue(fastPathData);
@@ -206,7 +201,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    protected void writeIteratorHasNext(MethodVisitor mv) {
+    protected void writeIteratorHasNext(final MethodVisitor mv) {
         if (controller.isFastPath()) {
             mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Iterator", "hasNext", "()Z", true);
         } else {
@@ -215,7 +210,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    protected void writeIteratorNext(MethodVisitor mv) {
+    protected void writeIteratorNext(final MethodVisitor mv) {
         if (controller.isFastPath()) {
             mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Iterator", "next", "()Ljava/lang/Object;", true);
         } else {
@@ -224,7 +219,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    protected void writeForInLoop(ForStatement statement) {
+    protected void writeForInLoop(final ForStatement statement) {
         if (controller.isFastPath()) {
             super.writeForInLoop(statement);
         } else {
@@ -236,7 +231,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.writeForInLoop(statement);
             fastPathBlocked = oldFastPathBlock;
 
-            if (fastPathData==null) return;
+            if (fastPathData == null) return;
             writeFastPathPrelude(fastPathData);
             super.writeForInLoop(statement);
             writeFastPathEpilogue(fastPathData);
@@ -244,7 +239,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    protected void writeForLoopWithClosureList(ForStatement statement) {
+    protected void writeForLoopWithClosureList(final ForStatement statement) {
         if (controller.isFastPath()) {
             super.writeForLoopWithClosureList(statement);
         } else {
@@ -256,7 +251,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.writeForLoopWithClosureList(statement);
             fastPathBlocked = oldFastPathBlock;
 
-            if (fastPathData==null) return;
+            if (fastPathData == null) return;
             writeFastPathPrelude(fastPathData);
             super.writeForLoopWithClosureList(statement);
             writeFastPathEpilogue(fastPathData);
@@ -264,7 +259,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    public void writeWhileLoop(WhileStatement statement) {
+    public void writeWhileLoop(final WhileStatement statement) {
         if (controller.isFastPath()) {
             super.writeWhileLoop(statement);
         } else {
@@ -276,7 +271,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             super.writeWhileLoop(statement);
             fastPathBlocked = oldFastPathBlock;
 
-            if (fastPathData==null) return;
+            if (fastPathData == null) return;
             writeFastPathPrelude(fastPathData);
             super.writeWhileLoop(statement);
             writeFastPathEpilogue(fastPathData);
@@ -284,11 +279,10 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    public void writeIfElse(IfStatement statement) {
+    public void writeIfElse(final IfStatement statement) {
         StatementMeta meta = statement.getNodeMetaData(StatementMeta.class);
         FastPathData fastPathData = writeGuards(meta, statement);
-
-        if (fastPathData==null) {
+        if (fastPathData == null) {
             super.writeIfElse(statement);
         } else {
             boolean oldFastPathBlock = fastPathBlocked;
@@ -302,17 +296,8 @@ public class OptimizingStatementWriter extends StatementWriter {
         }
     }
 
-    private boolean isNewPathFork(StatementMeta meta) {
-        // meta.optimize -> can do fast path
-        if (meta==null || !meta.optimize) return false;
-        // fastPathBlocked -> slow path
-        if (fastPathBlocked) return false;
-        // controller.isFastPath() -> fastPath
-        return !controller.isFastPath();
-    }
-
     @Override
-    public void writeReturn(ReturnStatement statement) {
+    public void writeReturn(final ReturnStatement statement) {
         if (controller.isFastPath()) {
             super.writeReturn(statement);
         } else {
@@ -329,7 +314,7 @@ public class OptimizingStatementWriter extends StatementWriter {
                 super.writeReturn(statement);
                 fastPathBlocked = oldFastPathBlock;
 
-                if (fastPathData==null) return;
+                if (fastPathData == null) return;
                 writeFastPathPrelude(fastPathData);
                 super.writeReturn(statement);
                 writeFastPathEpilogue(fastPathData);
@@ -340,7 +325,7 @@ public class OptimizingStatementWriter extends StatementWriter {
     }
 
     @Override
-    public void writeExpressionStatement(ExpressionStatement statement) {
+    public void writeExpressionStatement(final ExpressionStatement statement) {
         if (controller.isFastPath()) {
             super.writeExpressionStatement(statement);
         } else {
@@ -358,7 +343,6 @@ public class OptimizingStatementWriter extends StatementWriter {
             // (3) fast path possible and in slow or fastPath. Nothing to do here.
             //
             // the only case we need to handle is then (2).
-
             if (isNewPathFork(meta) && writeDeclarationExtraction(statement)) {
                 if (meta.declaredVariableExpression != null) {
                     // declaration was replaced by assignment so we need to define the variable
@@ -371,7 +355,7 @@ public class OptimizingStatementWriter extends StatementWriter {
                 super.writeExpressionStatement(statement);
                 fastPathBlocked = oldFastPathBlock;
 
-                if (fastPathData==null) return;
+                if (fastPathData == null) return;
                 writeFastPathPrelude(fastPathData);
                 super.writeExpressionStatement(statement);
                 writeFastPathEpilogue(fastPathData);
@@ -381,7 +365,7 @@ public class OptimizingStatementWriter extends StatementWriter {
         }
     }
 
-    private boolean writeDeclarationExtraction(Statement statement) {
+    private boolean writeDeclarationExtraction(final Statement statement) {
         Expression ex = null;
         if (statement instanceof ReturnStatement) {
             ReturnStatement rs = (ReturnStatement) statement;
@@ -390,7 +374,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             ExpressionStatement es = (ExpressionStatement) statement;
             ex = es.getExpression();
         } else {
-            throw new GroovyBugError("unknown statement type :"+statement.getClass());
+            throw new GroovyBugError("unknown statement type :" + statement.getClass());
         }
         if (!(ex instanceof DeclarationExpression)) return true;
         DeclarationExpression declaration = (DeclarationExpression) ex;
@@ -419,26 +403,32 @@ public class OptimizingStatementWriter extends StatementWriter {
             ExpressionStatement es = (ExpressionStatement) statement;
             es.setExpression(assignment);
         } else {
-            throw new GroovyBugError("unknown statement type :"+statement.getClass());
+            throw new GroovyBugError("unknown statement type :" + statement.getClass());
         }
         return true;
     }
 
-    public static void setNodeMeta(TypeChooser chooser, ClassNode classNode) {
-        if (classNode.getNodeMetaData(ClassNodeSkip.class)!=null) return;
+    private boolean isNewPathFork(final StatementMeta meta) {
+        // meta.optimize -> can do fast path
+        if (meta == null || !meta.optimize) return false;
+        // fastPathBlocked -> slow path
+        if (fastPathBlocked) return false;
+        // controller.isFastPath() -> fastPath
+        return !controller.isFastPath();
+    }
+
+    public static void setNodeMeta(final TypeChooser chooser, final ClassNode classNode) {
+        if (classNode.getNodeMetaData(ClassNodeSkip.class) != null) return;
         new OptVisitor(chooser).visitClass(classNode);
     }
 
-    private static StatementMeta addMeta(ASTNode node) {
-        StatementMeta metaOld = node.getNodeMetaData(StatementMeta.class);
-        StatementMeta meta = metaOld;
-        if (meta==null) meta = new StatementMeta();
+    private static StatementMeta addMeta(final ASTNode node) {
+        StatementMeta meta = node.getNodeMetaData(StatementMeta.class, x -> new StatementMeta());
         meta.optimize = true;
-        if (metaOld==null) node.setNodeMetaData(StatementMeta.class, meta);
         return meta;
     }
 
-    private static StatementMeta addMeta(ASTNode node, OptimizeFlagsCollector opt) {
+    private static StatementMeta addMeta(final ASTNode node, final OptimizeFlagsCollector opt) {
         StatementMeta meta = addMeta(node);
         meta.chainInvolvedTypes(opt);
         return meta;
@@ -471,9 +461,15 @@ public class OptimizingStatementWriter extends StatementWriter {
 
         @Override
         public String toString() {
-            StringBuilder ret = new StringBuilder("optimize=" + optimize + " target=" + target + " type=" + type + " involvedTypes=");
+            StringBuilder ret = new StringBuilder();
+            ret.append("optimize=").append(optimize);
+            ret.append(" target=").append(target);
+            ret.append(" type=").append(type);
+            ret.append(" involvedTypes=");
             for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
-                if (involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+                if (involvedTypes[i]) {
+                    ret.append(' ').append(typeMapKeyNames[i]);
+                }
             }
             return ret.toString();
         }
@@ -490,13 +486,13 @@ public class OptimizingStatementWriter extends StatementWriter {
         private final Deque<OptimizeFlagsEntry> previous = new LinkedList<>();
 
         public void push() {
-            previous.addLast(current);
+            previous.push(current);
             current = new OptimizeFlagsEntry();
         }
 
         public void pop(final boolean propagateFlags) {
             OptimizeFlagsEntry old = current;
-            current = previous.removeLast();
+            current = previous.pop();
             if (propagateFlags) {
                 chainCanOptimize(old.canOptimize);
                 chainShouldOptimize(old.shouldOptimize);
@@ -508,17 +504,19 @@ public class OptimizingStatementWriter extends StatementWriter {
 
         @Override
         public String toString() {
-            StringBuilder ret;
+            StringBuilder ret = new StringBuilder();
             if (current.shouldOptimize) {
-                ret = new StringBuilder("should optimize, can = " + current.canOptimize);
+                ret.append("should optimize, can = " + current.canOptimize);
             } else if (current.canOptimize) {
-                ret = new StringBuilder("can optimize");
+                ret.append("can optimize");
             } else {
-                ret = new StringBuilder("don't optimize");
+                ret.append("don't optimize");
             }
             ret.append(" involvedTypes =");
             for (int i = 0, n = typeMapKeyNames.length; i < n; i += 1) {
-                if (current.involvedTypes[i]) ret.append(" ").append(typeMapKeyNames[i]);
+                if (current.involvedTypes[i]) {
+                    ret.append(' ').append(typeMapKeyNames[i]);
+                }
             }
             return ret.toString();
         }
@@ -595,6 +593,7 @@ public class OptimizingStatementWriter extends StatementWriter {
         public void visitConstructor(final ConstructorNode node) {
             scope = node.getVariableScope();
             super.visitConstructor(node);
+            opt.reset();
         }
 
         @Override
@@ -942,7 +941,7 @@ public class OptimizingStatementWriter extends StatementWriter {
             List<ConstructorNode> ctors = node.getDeclaredConstructors();
             MethodNode result = null;
             for (ConstructorNode ctor : ctors) {
-                if (ParameterUtils.parametersEqual(ctor.getParameters(), parameters)) {
+                if (parametersEqual(ctor.getParameters(), parameters)) {
                     result = ctor;
                     break;
                 }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 04/20: GROOVY-9317: record field expression value for assert

Daniel.Sun
In reply to this post by Daniel.Sun
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 5515c094dfa13171e382156f382bfa7becbde2ca
Author: Eric Milles <[hidden email]>
AuthorDate: Sat Nov 23 19:50:08 2019 -0600

    GROOVY-9317: record field expression value for assert
   
    (cherry picked from commit 9d75b171be28d3f33033ac3fa13b1296d5028cb0)
---
 .../groovy/classgen/AsmClassGenerator.java         |  33 +--
 src/spec/test/semantics/PowerAssertTest.groovy     |  24 +-
 .../powerassert/AssertionRenderingTest.groovy      | 290 +++++++++++++--------
 .../runtime/powerassert/ValueRenderingTest.groovy  | 121 +++++----
 4 files changed, 291 insertions(+), 177 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index bcc140f..2c688f8 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -962,18 +962,17 @@ public class AsmClassGenerator extends ClassGenerator {
                             && controller.isStaticContext()) {
                         // GROOVY-6183
                         ClassNode current = classNode.getSuperClass();
-                        while (field==null && current!=null) {
+                        while (field == null && current != null) {
                             field = current.getDeclaredField(name);
                             current = current.getSuperClass();
                         }
-                        if (field!=null && (field.isProtected() || field.isPublic())) {
+                        if (field != null && (field.isProtected() || field.isPublic())) {
                             visitFieldExpression(new FieldExpression(field));
                             return;
                         }
                     }
                 }
-                if (field != null && !privateSuperField) {
-                    // GROOVY-4497: don't visit super field if it is private
+                if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
                     visitFieldExpression(new FieldExpression(field));
                     return;
                 }
@@ -1157,9 +1156,9 @@ public class AsmClassGenerator extends ClassGenerator {
             if (name != null) {
                 FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
                 if (field != null) {
-                    FieldExpression exp = new FieldExpression(field);
-                    exp.setSourcePosition(expression);
-                    visitFieldExpression(exp);
+                    FieldExpression fldExp = new FieldExpression(field);
+                    fldExp.setSourcePosition(expression.getProperty());
+                    visitFieldExpression(fldExp);
                     return;
                 }
             }
@@ -1178,17 +1177,17 @@ public class AsmClassGenerator extends ClassGenerator {
             if (usesSuper(expression)) adapter = getFieldOnSuper;
         }
         visitAttributeOrProperty(expression, adapter);
-        if (!controller.getCompileStack().isLHS()) {
-            controller.getAssertionWriter().record(expression.getProperty());
-        } else {
+        if (controller.getCompileStack().isLHS()) {
             operandStack.remove(operandStack.getStackLength() - mark);
+        } else {
+            controller.getAssertionWriter().record(expression.getProperty());
         }
     }
 
     private static boolean usesSuper(PropertyExpression pe) {
-        Expression expression = pe.getObjectExpression();
-        if (expression instanceof VariableExpression) {
-            VariableExpression varExp = (VariableExpression) expression;
+        Expression objExp = pe.getObjectExpression();
+        if (objExp instanceof VariableExpression) {
+            VariableExpression varExp = (VariableExpression) objExp;
             String variable = varExp.getName();
             return variable.equals("super");
         }
@@ -1211,7 +1210,9 @@ public class AsmClassGenerator extends ClassGenerator {
                 loadInstanceField(expression);
             }
         }
-        if (controller.getCompileStack().isLHS()) controller.getAssertionWriter().record(expression);
+        if (!controller.getCompileStack().isLHS()) {
+            controller.getAssertionWriter().record(expression);
+        }
     }
 
     public void loadStaticField(FieldExpression fldExp) {
@@ -1348,7 +1349,9 @@ public class AsmClassGenerator extends ClassGenerator {
         } else {
             controller.getOperandStack().loadOrStoreVariable(variable, expression.isUseReferenceDirectly());
         }
-        if (!controller.getCompileStack().isLHS()) controller.getAssertionWriter().record(expression);
+        if (!controller.getCompileStack().isLHS()) {
+            controller.getAssertionWriter().record(expression);
+        }
     }
 
     private void loadThis(VariableExpression thisExpression) {
diff --git a/src/spec/test/semantics/PowerAssertTest.groovy b/src/spec/test/semantics/PowerAssertTest.groovy
index 5e42027..85d5b6c 100644
--- a/src/spec/test/semantics/PowerAssertTest.groovy
+++ b/src/spec/test/semantics/PowerAssertTest.groovy
@@ -18,17 +18,21 @@
  */
 package semantics
 
-import asciidoctor.Utils
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-class PowerAssertTest extends GroovyTestCase {
+import static asciidoctor.Utils.stripAsciidocMarkup
+import static groovy.test.GroovyAssert.shouldFail
+
+final class PowerAssertTest {
+
+    @Test
     void testPowerAssert() {
-        def msg = shouldFail {
+        def err = shouldFail {
             //tag::assert_code_1[]
             assert 1+1 == 3
             //end::assert_code_1[]
         }
-        assert msg == Utils.stripAsciidocMarkup('''
+        assert err.message == stripAsciidocMarkup('''
 //tag::assert_error_1[]
 assert 1+1 == 3
         |  |
@@ -37,8 +41,9 @@ assert 1+1 == 3
 ''')
     }
 
+    @Test
     void testPowerAssert2() {
-        def msg = shouldFail {
+        def err = shouldFail {
             //tag::assert_code_2[]
             def x = 2
             def y = 7
@@ -47,7 +52,7 @@ assert 1+1 == 3
             assert calc(x,y) == [x,z].sum()
             //end::assert_code_2[]
         }
-        assert msg == Utils.stripAsciidocMarkup('''
+        assert err.message == stripAsciidocMarkup('''
 //tag::assert_error_2[]
 assert calc(x,y) == [x,z].sum()
        |    | |  |   | |  |
@@ -57,8 +62,9 @@ assert calc(x,y) == [x,z].sum()
 ''')
     }
 
+    @Test
     void testCustomAssertMessage() {
-        def msg = shouldFail {
+        def err = shouldFail {
             //tag::assert_code_3[]
             def x = 2
             def y = 7
@@ -67,7 +73,7 @@ assert calc(x,y) == [x,z].sum()
             assert calc(x,y) == z*z : 'Incorrect computation result'
             //end::assert_code_3[]
         }
-        assert msg == Utils.stripAsciidocMarkup('''
+        assert err.message == stripAsciidocMarkup('''
 //tag::assert_error_3[]
 Incorrect computation result. Expression: (calc.call(x, y) == (z * z)). Values: z = 5, z = 5
 //end::assert_error_3[]
diff --git a/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy b/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
index 9ce3a7c..f34d762 100644
--- a/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/powerassert/AssertionRenderingTest.groovy
@@ -18,34 +18,42 @@
  */
 package org.codehaus.groovy.runtime.powerassert
 
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-import static AssertionTestUtil.*
 import static java.lang.Math.min
+import static org.codehaus.groovy.runtime.powerassert.AssertionTestUtil.isRendered
 
 /**
  * Tests rendering of whole assertions.
  */
+final class AssertionRenderingTest {
 
-class AssertionRenderingTest extends GroovyTestCase {
+    private one(x) { 0 }
+
+    private two(a, b) { 0 }
+
+    private three(a, b, c) { 0 }
+
+    @Test
     void testSimpleAssertion() {
-        isRendered """
+        isRendered '''
 assert x == 1
        | |
        2 false
-        """, {
+        ''', { ->
             def x = 2
             assert x == 1
         }
     }
 
+    @Test
     void testMultiLineAssertion() {
-        isRendered """
+        isRendered '''
 assert 1 + 2 == 4 - 2
          |   |    |
          3   |    2
              false
-        """, {
+        ''', { ->
             assert 1 +
                 2 ==
 
@@ -58,118 +66,125 @@ assert 1 + 2 == 4 - 2
         }
     }
 
-    private one(x) { 0 }
-
+    @Test
     void testMethodCallExpressionWithImplicitTarget() {
-        isRendered """
+        isRendered '''
 assert one(a)
        |   |
        0   1
-        """, {
+        ''', { ->
             def a = 1
             assert one(a)
         }
     }
 
+    @Test
     void testMethodCallExpressionWithExplicitTarget() {
-        isRendered """
+        isRendered '''
 assert a.get(b) == null
        | |   |  |
        | 1   0  false
        [1]
-        """, {
+        ''', { ->
             def a = [1]
             def b = 0
             assert a.get(b) == null
         }
     }
 
+    @Test
     void testMethodCallExpressionWithGStringMethod() {
-        isRendered """
+        isRendered '''
 assert [1]."\$x"(0) == null
            | |     |
            1 'get' false
-        """, {
+        ''', { ->
             def x = "get"
             assert [1]."$x"(0) == null
         }
     }
 
+    @Test
     void testMethodCallExpressionCallingStaticMethod() {
-        isRendered """
+        isRendered '''
 assert Math.max(a,b) == null
             |   | |  |
             2   1 2  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert Math.max(a,b) == null
         }
     }
 
+    @Test
     void testMethodCallExpressionSpreadDot() {
-        isRendered """
+        isRendered '''
 assert ["1", "22"]*.size() == null
                     |      |
                     [1, 2] false
-        """, {
+        ''', { ->
             assert ["1", "22"]*.size() == null
         }
     }
 
+    @Test
     void testMethodCallExpressionSafe() {
-        isRendered """
+        isRendered '''
 assert a?.foo()
        |  |
        |  null
        null
-        """, {
+        ''', { ->
             def a = null
             assert a?.foo()
         }
     }
 
+    @Test
     void testStaticMethodCallExpression() {
-        isRendered """
+        isRendered '''
 assert min(a,b) == null
        |   | |  |
        1   1 2  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert min(a,b) == null
         }
     }
 
+    @Test
     void testConstructorCallExpression() {
-        isRendered """
+        isRendered '''
 assert new ArrayList(a) == null
        |             |  |
        []            1  false
-        """, {
+        ''', { ->
             def a = 1
             assert new ArrayList(a) == null
         }
 
     }
 
+    @Test
     void testTernaryExpression() {
-        isRendered """
+        isRendered '''
 assert a ? b : c
        |   |
        1   0
-        """, {
+        ''', { ->
             def a = 1
             def b = 0
             def c = 1
             assert a ? b : c
         }
 
-        isRendered """
+        isRendered '''
 assert a ? b : c
        |       |
        0       0
-        """, {
+        ''', { ->
             def a = 0
             def b = 1
             def c = 0
@@ -177,126 +192,134 @@ assert a ? b : c
         }
     }
 
+    @Test
     void testShortTernaryExpression() {
-        isRendered """
+        isRendered '''
 assert (a ?: b) == null
         |       |
         1       false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert (a ?: b) == null
         }
 
-        isRendered """
+        isRendered '''
 assert a ?: b
        |    |
        0    0
-        """, {
+        ''', { ->
             def a = 0
             def b = 0
             assert a ?: b
         }
     }
 
+    @Test
     void testBinaryExpression() {
-        isRendered """
+        isRendered '''
 assert a * b
        | | |
        0 0 1
-        """, {
+        ''', { ->
             def a = 0
             def b = 1
             assert a * b
         }
 
-        isRendered """
+        isRendered '''
 assert a[b]
        |||
        ||0
        |false
        [false]
-        """, {
+        ''', { ->
             def a = [false]
             def b = 0
             assert a[b]
         }
     }
 
+    @Test
     void testPrefixExpression() {
-        isRendered """
+        isRendered '''
 assert ++x == null
        | | |
        1 0 false
-        """, {
+        ''', { ->
             def x = 0
             assert ++x == null
         }
     }
 
+    @Test
     void testPostfixExpression() {
-        isRendered """
+        isRendered '''
 assert x++ == null
        ||  |
        |0  false
        0
-        """, {
+        ''', { ->
             def x = 0
             assert x++ == null
         }
     }
 
+    @Test
     void testBooleanExpression() {
-        isRendered """
+        isRendered '''
 assert a
        |
        null
-        """, {
+        ''', { ->
             def a = null
             assert a
         }
     }
 
+    @Test
     void testClosureExpression() {
-        isRendered """
+        isRendered '''
 assert { 1 + 2 } == null
                  |
                  false
-        """, {
+        ''', { ->
             assert { 1 + 2 } == null
         }
     }
 
+    @Test
     void testTupleExpression() {
         // TupleExpression is only used on LHS of (multi-)assignment,
         // but LHS of assignment is not rewritten
-        isRendered """
+        isRendered '''
 assert ((a,b) = [1,2]) && false
               |        |
               [1, 2]   false
-        """, {
+        ''', { ->
           def a
           def b
           assert ((a,b) = [1,2]) && false
         }
     }
 
+    @Test
     void testMapExpression() {
-        isRendered """
+        isRendered '''
 assert [a:b, c:d] == null
           |    |  |
           2    4  false
-        """, {
+        ''', { ->
             def b = 2
             def d = 4
             assert [a:b, c:d] == null
         }
 
-        isRendered """
+        isRendered '''
 assert [(a):b, (c):d] == null
         |   |  |   |  |
         1   2  3   4  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             def c = 3
@@ -305,12 +328,13 @@ assert [(a):b, (c):d] == null
         }
     }
 
+    @Test
     void testListExpression() {
-        isRendered """
+        isRendered '''
 assert [a,b,c] == null
         | | |  |
         1 2 3  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             def c = 3
@@ -318,111 +342,165 @@ assert [a,b,c] == null
         }
     }
 
+    @Test
     void testRangeExpression() {
-        isRendered """
+        isRendered '''
 assert (a..b) == null
         |  |  |
         1  2  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert (a..b) == null
         }
 
-        isRendered """
+        isRendered '''
 assert (a..<b) == null
         |   |  |
         1   2  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert (a..<b) == null
         }
     }
 
+    @Test
     void testPropertyExpression() {
-        isRendered """
+        isRendered '''
 assert a.bytes == null
        | |     |
        | [65]  false
        'A'
-
-        """, {
+        ''', { ->
             def a = 'A'
             assert a.bytes == null
         }
 
-        isRendered """
+        isRendered '''
 assert Integer.MIN_VALUE == null
                |         |
                |         false
                -2147483648
-        """, {
+        ''', { ->
             assert Integer.MIN_VALUE == null
         }
     }
 
+    @Test
     void testAttributeExpression() {
-        isRendered """
+        isRendered '''
 assert holder.@x
        |       |
        h       0
-        """, {
+        ''', { ->
             def holder = new Holder()
             assert holder.@x
         }
+
+        isRendered '''
+assert holder.@x != 0
+       |       | |
+       h       0 false
+        ''', { ->
+            def holder = new Holder()
+            assert holder.@x != 0
+        }
+
+        isRendered '''
+assert 0 != holder.@x
+         |  |       |
+         |  h       0
+         false
+        ''', { ->
+            def holder = new Holder()
+            assert 0 != holder.@x
+        }
+
+        isRendered '''
+assert this.@field == 0
+             |     |
+             1     false
+        ''', { ->
+            new Runnable() {
+                private int field = 1
+                @Override
+                public void run() {
+                    assert this.@field == 0
+                }
+            }.run()
+        }
+
+        isRendered '''
+assert 0 == this.@field
+         |        |
+         false    1
+        ''', { ->
+            new Runnable() {
+                private int field = 1
+                @Override
+                public void run() {
+                    assert 0 == this.@field
+                }
+            }.run()
+        }
     }
 
+    @Test
     void testMethodPointerExpression() {
-        isRendered """
+        isRendered '''
 assert a.&"\$b" == null
        |    |  |
        []   |  false
             'get'
-        """, {
+        ''', { ->
             def a = []
             def b = "get"
             assert a.&"$b" == null
         }
     }
 
+    @Test
     void testConstantExpression() {
-        isRendered """
+        isRendered '''
 assert 1 == "abc"
          |
          false
-        """, {
+        ''', { ->
             assert 1 == "abc"
         }
     }
 
+    @Test
     void testClassExpression() {
-        isRendered """
+        isRendered '''
 assert List == String
             |
             false
-        """, {
+        ''', { ->
             assert List == String
         }
     }
 
+    @Test
     void testVariableExpression() {
-        isRendered """
+        isRendered '''
 assert x
        |
        0
-        """, {
+        ''', { ->
             def x = 0
             assert x
         }
     }
 
+    @Test
     void testGStringExpression() {
         isRendered '''
 assert "$a and ${b + c}" == null
          |       | | |   |
          1       2 5 3   false
-        ''', {
+        ''', { ->
             def a = 1
             def b = 2
             def c = 3
@@ -430,138 +508,143 @@ assert "$a and ${b + c}" == null
         }
     }
 
+    @Test
     void testArrayExpression() {
-        isRendered """
+        isRendered '''
 assert new int[a][b] == null
                |  |  |
                1  2  false
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             assert new int[a][b] == null
         }
     }
 
-    private two(a, b) { 0 }
-
+    @Test
     void testSpreadExpression() {
-        isRendered """
+        isRendered '''
 assert two(*a)
        |    |
        0    [1, 2]
-        """, {
+        ''', { ->
             def a = [1, 2]
             assert two(*a)
         }
 
-        isRendered """
+        isRendered '''
 assert [1, *a] == null
             |  |
             |  false
             [2, 3]
-        """, {
+        ''', { ->
             def a = [2, 3]
             assert [1, *a] == null
         }
     }
 
+    @Test
     void testSpreadMapExpression() {
-        isRendered """
+        isRendered '''
 assert one(*:m)
        |     |
        0     ['a':1, 'b':2]
-        """, {
+        ''', { ->
             def m = [a:1, b:2]
             assert one(*:m)
         }
 
-        isRendered """
+        isRendered '''
 assert [a:1, *:m] == null
                |  |
                |  false
                ['b':2, 'c':3]
-        """, {
+        ''', { ->
             def m = [b:2, c:3]
             assert [a:1, *:m] == null
         }
     }
 
+    @Test
     void testNotExpression() {
-        isRendered """
+        isRendered '''
 assert !a
        ||
        |true
        false
-        """, {
+        ''', { ->
             def a = true
             assert !a
         }
     }
 
+    @Test
     void testUnaryMinusExpression() {
-        isRendered """
+        isRendered '''
 assert -a == null
        || |
        |1 false
        -1
-        """, {
+        ''', { ->
             def a = 1
             assert -a == null
         }
     }
 
+    @Test
     void testUnaryPlusExpression() {
-        isRendered """
+        isRendered '''
 assert +a == null
        || |
        |1 false
        1
-        """, {
+        ''', { ->
             def a = 1
             assert +a == null
         }
     }
 
+    @Test
     void testBitwiseNegationExpression() {
-        isRendered """
+        isRendered '''
 assert ~a == null
        || |
        |1 false
        -2
-        """, {
+        ''', { ->
             def a = 1
             assert ~a == null
         }
     }
 
+    @Test
     void testCastExpression() {
-        isRendered """
+        isRendered '''
 assert (List)a
              |
              null
-        """, {
+        ''', { ->
             def a = null
             assert (List)a
         }
 
-        isRendered """
+        isRendered '''
 assert a as int[]
        |
        null
-        """, {
+        ''', { ->
             def a = null
             assert a as int[]
         }
     }
 
-    private three(a,b,c) { 0 }
-
+    @Test
     void testArgumentListExpression() {
-        isRendered """
+        isRendered '''
 assert three(a, b,c)
        |     |  | |
        0     1  2 3
-        """, {
+        ''', { ->
             def a = 1
             def b = 2
             def c = 3
@@ -569,6 +652,7 @@ assert three(a, b,c)
         }
     }
 
+    @Test
     void testExplicitClosureCall() {
         def func = { it }
 
@@ -577,7 +661,7 @@ assert func.call(42) == null
        |    |        |
        |    42       false
        ${func.toString()}
-        """, {
+        """, { ->
             assert func.call(42) == null
         }
     }
@@ -612,5 +696,5 @@ assert func.call(42) == null
 @groovy.transform.PackageScope class Holder {
     public x = 0
     def getX() { 9 }
-    String toString() { "h" }
+    String toString() { 'h' }
 }
diff --git a/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy b/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
index 3c69d96..7f08416 100644
--- a/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/powerassert/ValueRenderingTest.groovy
@@ -18,85 +18,92 @@
  */
 package org.codehaus.groovy.runtime.powerassert
 
-import groovy.test.GroovyTestCase
+import org.junit.Test
 
-import static AssertionTestUtil.*
+import static org.codehaus.groovy.runtime.powerassert.AssertionTestUtil.isRendered
 
 /**
  * Tests rendering of individual values.
  */
+final class ValueRenderingTest {
 
-class ValueRenderingTest extends GroovyTestCase {
+    @Test
     void testNullValue() {
-        isRendered """
+        isRendered '''
 assert x
        |
        null
-        """, {
+        ''', { ->
             def x = null
             assert x
         }
     }
 
+    @Test
     void testCharValue() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        c false
-        """, {
+        ''', { ->
             def x = "c" as char
             assert x == null
         }
     }
 
+    @Test
     void testStringValue() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        | false
        'foo'
-        """, {
+        ''', { ->
             def x = "foo"
             assert x == null
         }
     }
 
+    @Test
     void testMultiLineStringValue() {
-        isRendered """
+        isRendered '''
 assert null == x
             |  |
             |  'one\\ntwo\\rthree\\r\\nfour'
             false
-        """, {
+        ''', { ->
             def x = "one\ntwo\rthree\r\nfour"
             assert null == x
         }
     }
 
+    @Test
     void testPrimitiveArrayValue() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        | false
        [1, 2]
-        """, {
+        ''', { ->
             def x = [1, 2] as int[]
             assert x == null
         }
     }
 
+    @Test
     void testObjectArrayValue() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        | false
        ['one', 'two']
-        """, {
+        ''', { ->
             def x = ["one", "two"] as String[]
             assert x == null
         }
     }
 
+    @Test
     void testEmptyStringValue() {
         def x = new String()
 
@@ -105,11 +112,12 @@ assert x == null
        | |
        | false
        ''
-        ''', {
+        ''', { ->
             assert x == null
         }
     }
 
+    @Test
     void testEmptyStringBuilderValue() {
         def x = new StringBuilder()
 
@@ -118,11 +126,12 @@ assert x == null
        | |
        | false
        ""
-        ''', {
+        ''', { ->
             assert x == null
         }
     }
 
+    @Test
     void testEmptyStringBufferValue() {
         def x = new StringBuffer()
 
@@ -131,37 +140,40 @@ assert x == null
        | |
        | false
        ""
-        ''', {
+        ''', { ->
             assert x == null
         }
     }
 
+    @Test
     void testSingleLineToString() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        | false
        single line
-        """, {
+        ''', { ->
             def x = new SingleLineToString()
             assert x == null
         }
     }
 
+    @Test
     void testMultiLineToString() {
-        isRendered """
+        isRendered '''
 assert x == null
        | |
        | false
        mul
        tiple
           lines
-        """, {
+        ''', { ->
             def x = new MultiLineToString()
             assert x == null
         }
     }
 
+    @Test
     void testNullToString() {
         def x = new NullToString()
 
@@ -170,11 +182,12 @@ assert x == null
        | |
        | false
        ${x.objectToString()} (toString() == null)
-        """, {
+        """, { ->
             assert x == null
         }
     }
 
+    @Test
     void testEmptyToString() {
         def x = new EmptyToString()
 
@@ -183,11 +196,12 @@ assert x == null
        | |
        | false
        ${x.objectToString()} (toString() == \"\")
-        """, {
+        """, { ->
             assert x == null
         }
     }
 
+    @Test
     void testThrowingToString() {
         def x = new ThrowingToString()
 
@@ -196,46 +210,53 @@ assert x == null
        | |
        | false
        ${x.objectToString()} (toString() threw java.lang.UnsupportedOperationException)
-        """, {
+        """, { ->
             assert x == null
         }
     }
-}
 
-@groovy.transform.PackageScope class SingleLineToString {
-    String toString() {
-        "single line"
-    }
-}
+    //--------------------------------------------------------------------------
 
-@groovy.transform.PackageScope class MultiLineToString {
-    String toString() {
-        "mul\ntiple\n   lines"
+    private static class SingleLineToString {
+        @Override
+        String toString() {
+            'single line'
+        }
     }
-}
 
-@groovy.transform.PackageScope class NullToString {
-    String objectToString() {
-        super.toString()
+    private static class MultiLineToString {
+        @Override
+        String toString() {
+            'mul\ntiple\n   lines'
+        }
     }
 
-    String toString() { null }
-}
+    private static class NullToString {
+        String objectToString() {
+            super.toString()
+        }
 
-@groovy.transform.PackageScope class EmptyToString {
-    String objectToString() {
-        super.toString()
+        @Override
+        String toString() { null }
     }
 
-    String toString() { "" }
-}
+    private static class EmptyToString {
+        String objectToString() {
+            super.toString()
+        }
 
-@groovy.transform.PackageScope class ThrowingToString {
-    String objectToString() {
-        super.toString()
+        @Override
+        String toString() { '' }
     }
 
-    String toString() {
-        throw new UnsupportedOperationException()
+    private static class ThrowingToString {
+        String objectToString() {
+            super.toString()
+        }
+
+        @Override
+        String toString() {
+            throw new UnsupportedOperationException()
+        }
     }
 }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 05/20: GROOVY-9318: add support for ** syntax in star import white/black lists

Daniel.Sun
In reply to this post by Daniel.Sun
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 45610c5539442da3036483ed78dcc955a191d1f1
Author: Eric Milles <[hidden email]>
AuthorDate: Sun Nov 24 17:25:34 2019 -0600

    GROOVY-9318: add support for ** syntax in star import white/black lists
   
    This closes #1100
    This closes #1101
   
    (cherry picked from commit 4483012e08ed4115bf25280260d7b2e3c95efeed)
---
 .../control/customizers/SecureASTCustomizer.java   |  84 ++--
 .../customizers/SecureASTCustomizerTest.groovy     | 449 +++++++++++++--------
 2 files changed, 315 insertions(+), 218 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
index 5f2c2c4..572d596 100644
--- a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
+++ b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
@@ -18,7 +18,6 @@
  */
 package org.codehaus.groovy.control.customizers;
 
-import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.CodeVisitorSupport;
 import org.codehaus.groovy.ast.GroovyCodeVisitor;
@@ -176,14 +175,14 @@ import java.util.Map;
  *             config.addCompilationCustomizers(imports, secure)
  *             GroovyClassLoader loader = new GroovyClassLoader(this.class.classLoader, config)
  *  </pre>
- *  
+ *
  * @since 1.8.0
  */
 public class SecureASTCustomizer extends CompilationCustomizer {
 
     private boolean isPackageAllowed = true;
-    private boolean isMethodDefinitionAllowed = true;
     private boolean isClosuresAllowed = true;
+    private boolean isMethodDefinitionAllowed = true;
 
     // imports
     private List<String> importsWhitelist;
@@ -201,7 +200,6 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     private List<String> staticStarImportsWhitelist;
     private List<String> staticStarImportsBlacklist;
 
-
     // indirect import checks
     // if set to true, then security rules on imports will also be applied on classnodes.
     // Direct instantiation of classes without imports will therefore also fail if this option is enabled
@@ -210,12 +208,12 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     // statements
     private List<Class<? extends Statement>> statementsWhitelist;
     private List<Class<? extends Statement>> statementsBlacklist;
-    private final List<StatementChecker> statementCheckers = new LinkedList<StatementChecker>();
+    private final List<StatementChecker> statementCheckers = new LinkedList<>();
 
     // expressions
     private List<Class<? extends Expression>> expressionsWhitelist;
     private List<Class<? extends Expression>> expressionsBlacklist;
-    private final List<ExpressionChecker> expressionCheckers = new LinkedList<ExpressionChecker>();
+    private final List<ExpressionChecker> expressionCheckers = new LinkedList<>();
 
     // tokens from Types
     private List<Integer> tokensWhitelist;
@@ -303,14 +301,13 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         if (this.importsWhitelist == null) importsWhitelist = Collections.emptyList();
     }
 
-    /**
-     * Ensures that every star import ends with .* as this is the expected syntax in import checks.
-     */
     private static List<String> normalizeStarImports(List<String> starImports) {
-        List<String> result = new ArrayList<String>(starImports.size());
+        List<String> result = new ArrayList<>(starImports.size());
         for (String starImport : starImports) {
             if (starImport.endsWith(".*")) {
                 result.add(starImport);
+            } else if (starImport.endsWith("**")) {
+                result.add(starImport.replaceFirst("\\*+$", ""));
             } else if (starImport.endsWith(".")) {
                 result.add(starImport + "*");
             } else {
@@ -493,7 +490,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param constantTypesWhiteList a list of classes.
      */
     public void setConstantTypesClassesWhiteList(final List<Class> constantTypesWhiteList) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : constantTypesWhiteList) {
             values.add(aClass.getName());
         }
@@ -506,7 +503,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param constantTypesBlackList a list of classes.
      */
     public void setConstantTypesClassesBlackList(final List<Class> constantTypesBlackList) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : constantTypesBlackList) {
             values.add(aClass.getName());
         }
@@ -519,8 +516,8 @@ public class SecureASTCustomizer extends CompilationCustomizer {
 
     /**
      * Sets the list of classes which deny method calls.
-     *
-     * Please note that since Groovy is a dynamic language, and
+     *
+     * Please note that since Groovy is a dynamic language, and
      * this class performs a static type check, it will be reletively
      * simple to bypass any blacklist unless the receivers blacklist contains, at
      * a minimum, Object, Script, GroovyShell, and Eval. Additionally,
@@ -543,7 +540,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param receiversBlacklist a list of classes.
      */
     public void setReceiversClassesBlackList(final List<Class> receiversBlacklist) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : receiversBlacklist) {
             values.add(aClass.getName());
         }
@@ -572,7 +569,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param receiversWhitelist a list of classes.
      */
     public void setReceiversClassesWhiteList(final List<Class> receiversWhitelist) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : receiversWhitelist) {
             values.add(aClass.getName());
         }
@@ -581,7 +578,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
 
     @Override
     public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException {
-        final ModuleNode ast = source.getAST();
+        ModuleNode ast = source.getAST();
         if (!isPackageAllowed && ast.getPackage() != null) {
             throw new SecurityException("Package definitions are not allowed");
         }
@@ -590,12 +587,10 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         // verify imports
         if (importsBlacklist != null || importsWhitelist != null || starImportsBlacklist != null || starImportsWhitelist != null) {
             for (ImportNode importNode : ast.getImports()) {
-                final String className = importNode.getClassName();
-                assertImportIsAllowed(className);
+                assertImportIsAllowed(importNode.getClassName());
             }
             for (ImportNode importNode : ast.getStarImports()) {
-                final String className = importNode.getPackageName();
-                assertStarImportIsAllowed(className + "*");
+                assertStarImportIsAllowed(importNode.getPackageName() + "*");
             }
         }
 
@@ -611,7 +606,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
             }
         }
 
-        final GroovyCodeVisitor visitor = createGroovyCodeVisitor();
+        GroovyCodeVisitor visitor = createGroovyCodeVisitor();
         ast.getStatementBlock().visit(visitor);
         for (ClassNode clNode : ast.getClasses()) {
             if (clNode!=classNode) {
@@ -627,7 +622,9 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         List<MethodNode> methods = filterMethods(classNode);
         if (isMethodDefinitionAllowed) {
             for (MethodNode method : methods) {
-                if (method.getDeclaringClass()==classNode && method.getCode() != null) method.getCode().visit(visitor);
+                if (method.getDeclaringClass() == classNode && method.getCode() != null) {
+                    method.getCode().visit(visitor);
+                }
             }
         }
     }
@@ -643,7 +640,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     }
 
     protected static List<MethodNode> filterMethods(ClassNode owner) {
-        List<MethodNode> result = new LinkedList<MethodNode>();
+        List<MethodNode> result = new LinkedList<>();
         List<MethodNode> methods = owner.getMethods();
         for (MethodNode method : methods) {
             if (method.getDeclaringClass() == owner && !method.isSynthetic()) {
@@ -655,37 +652,40 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     }
 
     protected void assertStarImportIsAllowed(final String packageName) {
-        if (starImportsWhitelist != null && !starImportsWhitelist.contains(packageName)) {
+        if (starImportsWhitelist != null && !(starImportsWhitelist.contains(packageName)
+                || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
             throw new SecurityException("Importing [" + packageName + "] is not allowed");
         }
-        if (starImportsBlacklist != null && starImportsBlacklist.contains(packageName)) {
+        if (starImportsBlacklist != null && (starImportsBlacklist.contains(packageName)
+                || starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
             throw new SecurityException("Importing [" + packageName + "] is not allowed");
         }
     }
 
     protected void assertImportIsAllowed(final String className) {
-        if (importsWhitelist != null && !importsWhitelist.contains(className)) {
+        if (importsWhitelist != null || starImportsWhitelist != null) {
+            if (importsWhitelist != null && importsWhitelist.contains(className)) {
+                return;
+            }
             if (starImportsWhitelist != null) {
-                // we should now check if the import is in the star imports
-                ClassNode node = ClassHelper.make(className);
-                final String packageName = node.getPackageName();
-                if (!starImportsWhitelist.contains(packageName + ".*")) {
-                    throw new SecurityException("Importing [" + className + "] is not allowed");
+                String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+                if (starImportsWhitelist.contains(packageName)
+                        || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+                    return;
                 }
-            } else {
-                throw new SecurityException("Importing [" + className + "] is not allowed");
             }
-        }
-        if (importsBlacklist != null && importsBlacklist.contains(className)) {
             throw new SecurityException("Importing [" + className + "] is not allowed");
-        }
-        // check that there's no star import blacklist
-        if (starImportsBlacklist != null) {
-            ClassNode node = ClassHelper.make(className);
-            final String packageName = node.getPackageName();
-            if (starImportsBlacklist.contains(packageName + ".*")) {
+        } else {
+            if (importsBlacklist != null && importsBlacklist.contains(className)) {
                 throw new SecurityException("Importing [" + className + "] is not allowed");
             }
+            if (starImportsBlacklist != null) {
+                String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+                if (starImportsBlacklist.contains(packageName) ||
+                        starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+                    throw new SecurityException("Importing [" + className + "] is not allowed");
+                }
+            }
         }
     }
 
diff --git a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
index d4ea67f..2218f9b 100644
--- a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
+++ b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
@@ -18,48 +18,49 @@
  */
 package org.codehaus.groovy.control.customizers
 
-import groovy.test.GroovyTestCase
 import org.codehaus.groovy.ast.expr.BinaryExpression
 import org.codehaus.groovy.ast.expr.ConstantExpression
 import org.codehaus.groovy.ast.expr.MethodCallExpression
 import org.codehaus.groovy.control.CompilerConfiguration
 import org.codehaus.groovy.control.MultipleCompilationErrorsException
 import org.codehaus.groovy.syntax.Types
+import org.junit.Before
+import org.junit.Test
 
 /**
- * Tests for the {@link SecureASTCustomizer} class.
+ * Tests for {@link SecureASTCustomizer}.
  */
-class SecureASTCustomizerTest extends GroovyTestCase {
-    CompilerConfiguration configuration
-    SecureASTCustomizer customizer
+final class SecureASTCustomizerTest {
 
+    private final CompilerConfiguration configuration = new CompilerConfiguration()
+    private final SecureASTCustomizer customizer = new SecureASTCustomizer()
+
+    @Before
     void setUp() {
-        configuration = new CompilerConfiguration()
-        customizer = new SecureASTCustomizer()
         configuration.addCompilationCustomizers(customizer)
     }
 
-    private boolean hasSecurityException(Closure closure) {
-        boolean result = false;
+    private static boolean hasSecurityException(Closure closure) {
+        boolean result = false
         try {
             closure()
         } catch (SecurityException e) {
             result = true
         } catch (MultipleCompilationErrorsException e) {
-            result = e.errorCollector.errors.any {it.cause?.class == SecurityException }
+            result = e.errorCollector.errors.any { it.cause?.class == SecurityException }
         }
-
-        result
+        return result
     }
 
+    @Test
     void testPackageDefinition() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             package dummy
             class A {
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.packageAllowed = false
@@ -68,14 +69,15 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testMethodDefinition() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             def method() {
                 true
             }
             method()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
@@ -84,16 +86,17 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testMethodDefinitionInClass() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             class A {
                 def method() {
                     true
                 }
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
@@ -102,328 +105,433 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testClassExtendingClassWithMethods() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             class A extends LinkedList {
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
         shell.evaluate(script)
     }
 
+    @Test
     void testExpressionWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.expressionsWhitelist = [BinaryExpression, ConstantExpression]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 class A {}
                 new A()
-            """)
+            ''')
         }
     }
 
+    @Test
     void testExpressionBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.expressionsBlacklist = [MethodCallExpression]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 1+1
                 if (1+1==2) {
                     "test".length()
                 }
-            """)
+            ''')
         }
     }
 
+    @Test
     void testTokenWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.tokensWhitelist = [Types.PLUS, Types.MINUS]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1;1-1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 if (i==2) println 'ok'
-            """)
+            ''')
         }
     }
 
+    @Test
     void testTokenBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.tokensBlacklist = [Types.PLUS_PLUS]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1;1-1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 i++
-            """)
+            ''')
         }
     }
 
+    @Test
     void testImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsWhitelist = ['java.util.ArrayList']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.LinkedList
-            new LinkedList()
-        """)
+            shell.evaluate('''
+                import java.util.LinkedList
+                new LinkedList()
+            ''')
         }
     }
 
-    void testStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
+    @Test
+    void testStarImportWhiteList1() {
         customizer.starImportsWhitelist = ['java.util.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.concurrent.atomic.AtomicInteger
-            new AtomicInteger(0)
-        """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicInteger
+                new AtomicInteger(0)
+            ''')
         }
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.*
-            import java.util.concurrent.atomic.*
-            new ArrayList()
-            new AtomicInteger(0)
-        """)
+            shell.evaluate('''
+                import java.util.*
+                import java.util.concurrent.atomic.*
+                new ArrayList()
+                new AtomicInteger(0)
+            ''')
         }
     }
 
-    void testStarImportWhiteListWithImportWhiteList() {
+    @Test
+    void testStarImportWhiteList2() {
+        customizer.starImportsWhitelist = ['java.**']
         def shell = new GroovyShell(configuration)
+        shell.evaluate('''
+            import java.lang.Object
+            Object obj
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import javax.swing.Action
+                Action act
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.util.*
+                import javax.swing.*
+                Object obj
+                Action act
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportWhiteListWithImportWhiteList() {
         customizer.importsWhitelist = ['java.util.concurrent.atomic.AtomicInteger']
         customizer.starImportsWhitelist = ['java.util.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
-        shell.evaluate("""
+        ''')
+        shell.evaluate('''
             import java.util.concurrent.atomic.AtomicInteger
             new AtomicInteger(0)
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.concurrent.atomic.AtomicBoolean
-            new AtomicBoolean(false)
-        """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicBoolean
+                new AtomicBoolean(false)
+            ''')
         }
     }
 
+    @Test
     void testImportBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsBlacklist = ['java.util.LinkedList']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
+                import java.util.LinkedList
+                new LinkedList()
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportBlackList1() {
+        customizer.starImportsBlacklist = ['java.lang.*']
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.LinkedList
-            new LinkedList()
-        """)
+            import javax.swing.Action
+            LinkedList list
+            Action act
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.lang.Object
+                Object obj
+            ''')
         }
     }
 
-    void testStarImportBlackListWithImportBlackList() {
+    @Test
+    void testStarImportBlackList2() {
+        customizer.starImportsBlacklist = ['java.**']
         def shell = new GroovyShell(configuration)
+        shell.evaluate('''
+            import javax.swing.Action
+            Action act
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.lang.Object
+                Object obj
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.util.Deque
+                Deque deck
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportBlackListWithImportBlackList() {
         customizer.importsBlacklist = ['java.util.concurrent.atomic.AtomicBoolean']
         customizer.starImportsBlacklist = ['java.util.*']
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
-            shell.evaluate("""
-                 import java.util.ArrayList
-                 new ArrayList()
-             """)
+            shell.evaluate('''
+                import java.util.ArrayList
+                new ArrayList()
+            ''')
         }
-        shell.evaluate("""
-             import java.util.concurrent.atomic.AtomicInteger
-             new AtomicInteger(0)
-         """)
+        shell.evaluate('''
+            import java.util.concurrent.atomic.AtomicInteger
+            new AtomicInteger(0)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-             import java.util.concurrent.atomic.AtomicBoolean
-             new AtomicBoolean(false)
-         """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicBoolean
+                new AtomicBoolean(false)
+            ''')
         }
     }
 
-
+    @Test
     void testIndirectImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsWhitelist = ['java.util.ArrayList']
         customizer.indirectImportCheckEnabled = true
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""            
-            new java.util.LinkedList()
-        """)
-
-            assert hasSecurityException {
-                shell.evaluate("""
-            return java.util.LinkedList.&size
-        """)
-            }
+            shell.evaluate('''
+                new java.util.LinkedList()
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                return java.util.LinkedList.&size
+            ''')
         }
     }
 
+    @Test
     void testIndirectStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.starImportsWhitelist = ['java.util.*']
         customizer.indirectImportCheckEnabled = true
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
-        shell.evaluate("""
+        ''')
+        shell.evaluate('''
             new java.util.ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            new java.util.concurrent.atomic.AtomicBoolean(false)
-        """)
-
-            assert hasSecurityException {
-                shell.evaluate("""
-            return java.util.concurrent.atomic.AtomicBoolean.&get
-        """)
-            }
+            shell.evaluate('''
+                new java.util.concurrent.atomic.AtomicBoolean(false)
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                return java.util.concurrent.atomic.AtomicBoolean.&get
+            ''')
         }
     }
 
+    @Test
     void testStaticImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.staticImportsWhitelist = ['java.lang.Math.PI']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import static java.lang.Math.PI
             PI
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import static java.lang.Math.PI
-            import static java.lang.Math.cos
-            cos(PI)
-        """)
+            shell.evaluate('''
+                import static java.lang.Math.PI
+                import static java.lang.Math.cos
+                cos(PI)
+            ''')
         }
     }
 
+    @Test
     void testStaticStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import static java.lang.Math.PI
             import static java.lang.Math.cos
             cos(PI)
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import static java.util.Collections.*
-            sort([5,4,2])
-        """)
+            shell.evaluate('''
+                import static java.util.Collections.*
+                sort([5,4,2])
+            ''')
         }
     }
 
+    @Test
     void testIndirectStaticImport() {
-        def shell = new GroovyShell(configuration)
         customizer.staticImportsWhitelist = ['java.lang.Math.PI']
         customizer.indirectImportCheckEnabled = true
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(1)')}
+        def shell = new GroovyShell(configuration)
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(1)')
+        }
     }
 
+    @Test
     void testIndirectStaticStarImport() {
-        def shell = new GroovyShell(configuration)
         customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
         customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
         shell.evaluate('java.lang.Math.cos(1)')
-        assert hasSecurityException {shell.evaluate('java.util.Collections.unmodifiableList([1])')}
+        assert hasSecurityException {
+            shell.evaluate('java.util.Collections.unmodifiableList([1])')
+        }
     }
 
+    @Test
     void testConstantTypeWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.constantTypesClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1')
-        assert hasSecurityException {shell.evaluate('"string"')}
-        assert hasSecurityException {shell.evaluate('2d')}
+        assert hasSecurityException {
+            shell.evaluate('"string"')
+        }
+        assert hasSecurityException {
+            shell.evaluate('2d')
+        }
     }
 
+    @Test
     void testConstantTypeBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.constantTypesClassesBlackList = [String]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1')
         shell.evaluate('2d')
-        assert hasSecurityException {shell.evaluate('"string"')}
+        assert hasSecurityException {
+            shell.evaluate('"string"')
+        }
     }
 
+    @Test
     void testReceiverWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
-        assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
-        assert hasSecurityException {shell.evaluate('2.0.multiply(4)')}
+        assert hasSecurityException {
+            shell.evaluate('"string".toUpperCase()')
+        }
+        assert hasSecurityException {
+            shell.evaluate('2.0.multiply(4)')
+        }
     }
 
+    @Test
     void testReceiverBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesBlackList = [String]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
         shell.evaluate('2.0.multiply(4)')
-        assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
+        assert hasSecurityException {
+            shell.evaluate('"string".toUpperCase()')
+        }
     }
 
+    @Test
     void testReceiverWhiteListWithStaticMethod() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(2)')
+        }
     }
 
+    @Test
     void testReceiverBlackListWithStaticMethod() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesBlackList = [Math]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
         shell.evaluate('Collections.sort([])')
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(2)')
+        }
     }
 
-    // Testcase for GROOVY-4978
+    @Test // GROOVY-4978
     void testVisitMethodBody() {
-        def shell = new GroovyShell(configuration)
         customizer.importsBlacklist = [
                 "java.lang.System",
                 "groovy.lang.GroovyShell",
                 "groovy.lang.GroovyClassLoader"]
         customizer.indirectImportCheckEnabled = true
-
-        assert hasSecurityException {shell.evaluate('System.println(1)')}
-        assert hasSecurityException {shell.evaluate('def x() { System.println(1) }')}
+        def shell = new GroovyShell(configuration)
+        assert hasSecurityException {
+            shell.evaluate('System.println(1)')
+        }
+        assert hasSecurityException {
+            shell.evaluate('def x() { System.println(1) }')
+        }
     }
 
-    // GROOVY-7424
+    @Test // GROOVY-7424
     void testClassWithInterfaceVisitable() {
         def shell = new GroovyShell(configuration)
         shell.evaluate '''
             interface Foo { def baz() }
-            class Bar implements Foo { def baz() { 42 }}
+            class Bar implements Foo { def baz() { 42 } }
             assert new Bar().baz() == 42
         '''
     }
 
-    // GROOVY-6153
+    @Test // GROOVY-6153
     void testDeterministicWhitelistBehaviour() {
-        def shell = new GroovyShell(configuration)
         def classWhiteList = ["java.lang.Object", "test"]
         customizer.with {
             setIndirectImportCheckEnabled(true);
@@ -433,7 +541,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
             setClosuresAllowed(true);
             setMethodDefinitionAllowed(true);
         }
-
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
             shell.evaluate '''
                 java.lang.System.out.println("run ")
@@ -441,9 +549,8 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
-    // GROOVY-6153
+    @Test // GROOVY-6153
     void testDeterministicWhitelistBehaviour2() {
-        def shell = new GroovyShell(configuration)
         def classWhiteList = ["java.lang.Object", "test"]
         customizer.with {
             setIndirectImportCheckEnabled(true);
@@ -453,7 +560,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
             setClosuresAllowed(true);
             setMethodDefinitionAllowed(true);
         }
-
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
             shell.evaluate '''
                 java.lang.Long x = 666L
@@ -461,24 +568,14 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
-    // GROOVY-8135
+    @Test // GROOVY-8135
     void testStarImportsWhiteListWithIndirectImportCheckEnabled() {
-        SecureASTCustomizer customizer = new SecureASTCustomizer()
-        customizer.setIndirectImportCheckEnabled(true)
-
-        List<String> starImportsWhitelist = new ArrayList<String>()
-        starImportsWhitelist.add("java.lang")
-        customizer.setStarImportsWhitelist(starImportsWhitelist)
-
-        CompilerConfiguration cc = new CompilerConfiguration()
-        cc.addCompilationCustomizers(customizer)
-
-        ClassLoader parent = getClass().getClassLoader()
-        GroovyClassLoader loader = new GroovyClassLoader(parent, cc)
-        loader.parseClass("Object object = new Object()")
-        loader.parseClass("Object object = new Object(); object.hashCode()")
-        loader.parseClass("Object[] array = new Object[0]; array.size()")
-        loader.parseClass("Object[][] array = new Object[0][0]; array.size()")
+        customizer.indirectImportCheckEnabled = true
+        customizer.starImportsWhitelist = ['java.lang']
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('Object object = new Object()')
+        shell.evaluate('Object object = new Object(); object.hashCode()')
+        shell.evaluate('Object[] array = new Object[0]; array.size()')
+        shell.evaluate('Object[][] array = new Object[0][0]; array.size()')
     }
-
 }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 06/20: refactor to remove repeated creation of Class<Type>

Daniel.Sun
In reply to this post by Daniel.Sun
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 4d4be630e784a02b01852ce410f34de5aea202ea
Author: Eric Milles <[hidden email]>
AuthorDate: Mon Nov 25 10:52:53 2019 -0600

    refactor to remove repeated creation of Class<Type>
   
    (cherry picked from commit 7127bb5b3f120ddaffa6630adb436ce68117ed04)
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 39 +++++++---------------
 1 file changed, 12 insertions(+), 27 deletions(-)

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 bc36c28..cb5dd85 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4913,42 +4913,27 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
     private ClassNode getGenericsResolvedTypeOfFieldOrProperty(final AnnotatedNode an, final ClassNode type) {
         if (!type.isUsingGenerics()) return type;
         Map<GenericsTypeName, GenericsType> connections = new HashMap<>();
-        //TODO: inner classes mean a different this-type. This is ignored here!
+        // TODO: inner classes mean a different this-type. This is ignored here!
         extractGenericsConnections(connections, typeCheckingContext.getEnclosingClassNode(), an.getDeclaringClass());
         return applyGenericsContext(connections, type);
     }
 
-    private ClassNode makeSuper() {
-        ClassNode ret = typeCheckingContext.getEnclosingClassNode().getSuperClass();
-        if (typeCheckingContext.isInStaticContext) {
-            ClassNode staticRet = CLASS_Type.getPlainNodeReference();
-            GenericsType gt = new GenericsType(ret);
-            staticRet.setGenericsTypes(new GenericsType[]{gt});
-            ret = staticRet;
+    private static ClassNode makeSelf(final ClassNode trait) {
+        ClassNode selfType = trait;
+        Set<ClassNode> selfTypes = Traits.collectSelfTypes(selfType, new LinkedHashSet<>());
+        if (!selfTypes.isEmpty()) {
+            selfTypes.add(selfType);
+            selfType = new UnionTypeClassNode(selfTypes.toArray(ClassNode.EMPTY_ARRAY));
         }
-        return ret;
+        return selfType;
     }
 
-    private ClassNode makeThis() {
-        ClassNode ret = typeCheckingContext.getEnclosingClassNode();
-        if (typeCheckingContext.isInStaticContext) {
-            ClassNode staticRet = CLASS_Type.getPlainNodeReference();
-            GenericsType gt = new GenericsType(ret);
-            staticRet.setGenericsTypes(new GenericsType[]{gt});
-            ret = staticRet;
-        }
-        return ret;
+    private ClassNode makeSuper() {
+        return makeType(typeCheckingContext.getEnclosingClassNode().getSuperClass(), typeCheckingContext.isInStaticContext);
     }
 
-    private static ClassNode makeSelf(final ClassNode trait) {
-        ClassNode ret = trait;
-        LinkedHashSet<ClassNode> selfTypes = new LinkedHashSet<>();
-        Traits.collectSelfTypes(ret, selfTypes);
-        if (!selfTypes.isEmpty()) {
-            selfTypes.add(ret);
-            ret = new UnionTypeClassNode(selfTypes.toArray(ClassNode.EMPTY_ARRAY));
-        }
-        return ret;
+    private ClassNode makeThis() {
+        return makeType(typeCheckingContext.getEnclosingClassNode(), typeCheckingContext.isInStaticContext);
     }
 
     /**

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 07/20: GROOVY-8648: fix ASM error for this or super attribute expression on LHS

Daniel.Sun
In reply to this post by Daniel.Sun
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 285e82e948e1baabed273a0432b5a0540913bad9
Author: Eric Milles <[hidden email]>
AuthorDate: Mon Nov 25 11:55:01 2019 -0600

    GROOVY-8648: fix ASM error for this or super attribute expression on LHS
   
    java.lang.NegativeArraySizeException
      at groovyjarjarasm.asm.Frame.merge(Frame.java:1222)
      at groovyjarjarasm.asm.MethodWriter.computeAllFrames(MethodWriter.java:1610)
      at groovyjarjarasm.asm.MethodWriter.visitMaxs(MethodWriter.java:1546)
      at org.codehaus.groovy.classgen.AsmClassGenerator.visitConstructorOrMethod(AsmClassGenerator.java:410)
   
    (cherry picked from commit 06da9561f0a274ce71c7a1b669841bc70c61766d)
---
 .../groovy/classgen/AsmClassGenerator.java         | 36 ++++++++--------
 src/test/groovy/bugs/Groovy8648.groovy             | 49 ++++++++++++++++++++++
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 2c688f8..3e50687 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -1147,36 +1147,41 @@ public class AsmClassGenerator extends ClassGenerator {
     @Override
     public void visitAttributeExpression(AttributeExpression expression) {
         Expression objectExpression = expression.getObjectExpression();
-        ClassNode classNode = controller.getClassNode();
+        OperandStack operandStack = controller.getOperandStack();
+        int mark = operandStack.getStackLength() - 1;
+        boolean visited = false;
+
         // TODO: checking for isThisOrSuper is enough for AttributeExpression, but if this is moved into
         // visitAttributeOrProperty to handle attributes and properties equally, then the extended check should be done
         if (isThisOrSuper(objectExpression)) {
             // let's use the field expression if it's available
             String name = expression.getPropertyAsString();
             if (name != null) {
+                ClassNode classNode = controller.getClassNode();
                 FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
                 if (field != null) {
                     FieldExpression fldExp = new FieldExpression(field);
                     fldExp.setSourcePosition(expression.getProperty());
                     visitFieldExpression(fldExp);
-                    return;
+                    visited = true;
                 }
             }
         }
 
-        MethodCallerMultiAdapter adapter;
-        OperandStack operandStack = controller.getOperandStack();
-        int mark = operandStack.getStackLength() - 1;
-        if (controller.getCompileStack().isLHS()) {
-            adapter = setField;
-            if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
-            if (usesSuper(expression)) adapter = setFieldOnSuper;
-        } else {
-            adapter = getField;
-            if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
-            if (usesSuper(expression)) adapter = getFieldOnSuper;
+        if (!visited) {
+            MethodCallerMultiAdapter adapter;
+            if (controller.getCompileStack().isLHS()) {
+                adapter = setField;
+                if (isGroovyObject(objectExpression)) adapter = setGroovyObjectField;
+                if (usesSuper(expression)) adapter = setFieldOnSuper;
+            } else {
+                adapter = getField;
+                if (isGroovyObject(objectExpression)) adapter = getGroovyObjectField;
+                if (usesSuper(expression)) adapter = getFieldOnSuper;
+            }
+            visitAttributeOrProperty(expression, adapter);
         }
-        visitAttributeOrProperty(expression, adapter);
+
         if (controller.getCompileStack().isLHS()) {
             operandStack.remove(operandStack.getStackLength() - mark);
         } else {
@@ -1210,9 +1215,6 @@ public class AsmClassGenerator extends ClassGenerator {
                 loadInstanceField(expression);
             }
         }
-        if (!controller.getCompileStack().isLHS()) {
-            controller.getAssertionWriter().record(expression);
-        }
     }
 
     public void loadStaticField(FieldExpression fldExp) {
diff --git a/src/test/groovy/bugs/Groovy8648.groovy b/src/test/groovy/bugs/Groovy8648.groovy
new file mode 100644
index 0000000..2687c2a
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8648.groovy
@@ -0,0 +1,49 @@
+/*
+ *  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
+
+@CompileStatic
+final class Groovy8648 {
+
+    @Test
+    void testDirectFieldAccessOnLHS() {
+        assertScript '''
+            class Account {
+                private int balance = 0
+                int getBalance() {
+                    return balance
+                }
+                void deposit(int amount) {
+                    assert amount > 0
+                    this.@balance += amount // ASM error for LHS attribute expression
+                }
+            }
+
+            new Account().with {
+                deposit(42)
+                assert balance == 42
+            }
+        '''
+    }
+}

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 08/20: GROOVY-8762: add test case

Daniel.Sun
In reply to this post by Daniel.Sun
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 b8f8728af603f6d45e69ab0519f369adffd37646
Author: Eric Milles <[hidden email]>
AuthorDate: Mon Nov 25 14:51:01 2019 -0600

    GROOVY-8762: add test case
   
    (cherry picked from commit 683ff9ee6fee665cd4a6407b84188e5b9cc83799)
---
 src/test/groovy/bugs/Groovy8762.groovy | 72 ++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/src/test/groovy/bugs/Groovy8762.groovy b/src/test/groovy/bugs/Groovy8762.groovy
new file mode 100644
index 0000000..cb8c390
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8762.groovy
@@ -0,0 +1,72 @@
+/*
+ *  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.NotYetImplemented
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy8762 {
+
+    @Test @NotYetImplemented
+    void testExplicitThisObjectExpressionInInnerClassConstructor() {
+        assertScript '''
+            class Outer {
+                static class Inner extends Closure {
+                    private int x, y
+
+                    Inner(int i) {
+                        super(null, null)
+                        x = i
+                        this.y = i // NPE at groovy.bugs.Outer$Inner.<init>
+                    }
+
+                    int getMaximumNumberOfParameters() {
+                        throw new UnsupportedOperationException()
+                    }
+
+                    Class<?>[] getParameterTypes() {
+                        throw new UnsupportedOperationException()
+                    }
+
+                    Object call(Object... args) {
+                        throw new UnsupportedOperationException()
+                    }
+
+                    Object call(Object arguments) {
+                        throw new UnsupportedOperationException()
+                    }
+
+                    Object call() {
+                        throw new UnsupportedOperationException()
+                    }
+                }
+
+                def makeCallable() {
+                    new Inner(1)
+                }
+            }
+
+            assert new Outer().makeCallable()
+        '''
+    }
+}

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
Reply | Threaded
Open this post in threaded view
|

[groovy] 09/20: minor edits

Daniel.Sun
In reply to this post by Daniel.Sun
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 496524cd4e74d53a715705cd90f78054e6b21b4a
Author: Eric Milles <[hidden email]>
AuthorDate: Mon Nov 25 17:21:08 2019 -0600

    minor edits
   
    (cherry picked from commit ceb9f95d8a7ae6a1da48160aee6e56a99f71df74)
---
 .../groovy/classgen/AsmClassGenerator.java         |  2 +-
 .../groovy/classgen/asm/DelegatingController.java  | 94 +++++++++++-----------
 .../groovy/classgen/asm/WriterController.java      | 78 ++++++++----------
 3 files changed, 79 insertions(+), 95 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 3e50687..da3d97c 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -934,7 +934,7 @@ public class AsmClassGenerator extends ClassGenerator {
                     if (field != null && field.isPrivate()) {
                         privateSuperField = true;
                     }
-                } else if (controller.isNotExplicitThisInClosure(expression.isImplicitThis())) {
+                } else if (expression.isImplicitThis() || !controller.isInClosure()) {
                     field = classNode.getDeclaredField(name);
                     ClassNode outer = classNode.getOuterClass();
                     if (field == null && outer != null) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
index aa61413..de5cedf 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/DelegatingController.java
@@ -29,15 +29,16 @@ import org.objectweb.asm.ClassVisitor;
 import org.objectweb.asm.MethodVisitor;
 
 /**
- * This class will delegate all calls to a WriterController given in the constructor.
+ * This class will delegate all calls to a WriterController given in the constructor.
  */
 public class DelegatingController extends WriterController {
+
     private final WriterController delegationController;
-    
+
     public DelegatingController(WriterController normalController) {
         this.delegationController = normalController;
     }
-    
+
     @Override
     public void init(final AsmClassGenerator asmClassGenerator, final GeneratorContext gcon, final ClassVisitor cv, final ClassNode cn) {
         delegationController.init(asmClassGenerator, gcon, cv, cn);
@@ -52,37 +53,37 @@ public class DelegatingController extends WriterController {
     public void setConstructorNode(final ConstructorNode cn) {
         delegationController.setConstructorNode(cn);
     }
-    
+
     @Override
     public boolean isFastPath() {
         return delegationController.isFastPath();
     }
-    
+
     @Override
     public CallSiteWriter getCallSiteWriter() {
         return delegationController.getCallSiteWriter();
     }
-        
+
     @Override
     public StatementWriter getStatementWriter() {
-        return delegationController.getStatementWriter();            
+        return delegationController.getStatementWriter();
     }
-    
+
     @Override
     public TypeChooser getTypeChooser() {
         return delegationController.getTypeChooser();
     }
-    
+
     @Override
     public AsmClassGenerator getAcg() {
         return delegationController.getAcg();
     }
-    
+
     @Override
     public AssertionWriter getAssertionWriter() {
         return delegationController.getAssertionWriter();
     }
-    
+
     @Override
     public BinaryExpressionHelper getBinaryExpressionHelper() {
         return delegationController.getBinaryExpressionHelper();
@@ -97,17 +98,17 @@ public class DelegatingController extends WriterController {
     public String getClassName() {
         return delegationController.getClassName();
     }
-    
+
     @Override
     public ClassNode getClassNode() {
         return delegationController.getClassNode();
     }
-    
+
     @Override
     public ClassVisitor getClassVisitor() {
         return delegationController.getClassVisitor();
     }
-    
+
     @Override
     public ClosureWriter getClosureWriter() {
         return delegationController.getClosureWriter();
@@ -127,72 +128,72 @@ public class DelegatingController extends WriterController {
     public MethodReferenceExpressionWriter getMethodReferenceExpressionWriter() {
         return delegationController.getMethodReferenceExpressionWriter();
     }
-    
+
     @Override
     public CompileStack getCompileStack() {
         return delegationController.getCompileStack();
     }
-    
+
     @Override
     public ConstructorNode getConstructorNode() {
         return delegationController.getConstructorNode();
     }
-    
+
     @Override
     public GeneratorContext getContext() {
         return delegationController.getContext();
     }
-    
+
     @Override
     public ClassVisitor getCv() {
         return delegationController.getCv();
     }
-    
+
     @Override
     public InterfaceHelperClassNode getInterfaceClassLoadingClass() {
         return delegationController.getInterfaceClassLoadingClass();
     }
-    
+
     @Override
     public String getInternalBaseClassName() {
         return delegationController.getInternalBaseClassName();
     }
-    
+
     @Override
     public String getInternalClassName() {
         return delegationController.getInternalClassName();
     }
-    
+
     @Override
     public InvocationWriter getInvocationWriter() {
         return delegationController.getInvocationWriter();
     }
-    
+
     @Override
     public MethodNode getMethodNode() {
         return delegationController.getMethodNode();
     }
-    
+
     @Override
     public MethodVisitor getMethodVisitor() {
         return delegationController.getMethodVisitor();
     }
-    
+
     @Override
     public OperandStack getOperandStack() {
         return delegationController.getOperandStack();
     }
-    
+
     @Override
     public ClassNode getOutermostClass() {
         return delegationController.getOutermostClass();
     }
-    
+
     @Override
     public ClassNode getReturnType() {
         return delegationController.getReturnType();
     }
-    
+
     @Override
     public SourceUnit getSourceUnit() {
         return delegationController.getSourceUnit();
@@ -202,87 +203,82 @@ public class DelegatingController extends WriterController {
     public boolean isConstructor() {
         return delegationController.isConstructor();
     }
-    
+
     @Override
     public boolean isInClosure() {
         return delegationController.isInClosure();
     }
-    
+
     @Override
     public boolean isInClosureConstructor() {
         return delegationController.isInClosureConstructor();
     }
-    
+
     @Override
     public boolean isNotClinit() {
         return delegationController.isNotClinit();
     }
-    
+
     @Override
     public boolean isInScriptBody() {
         return delegationController.isInScriptBody();
     }
-    
-    @Override
-    public boolean isNotExplicitThisInClosure(boolean implicitThis) {
-        return delegationController.isNotExplicitThisInClosure(implicitThis);
-    }
-    
+
     @Override
     public boolean isStaticConstructor() {
         return delegationController.isStaticConstructor();
     }
-    
+
     @Override
     public boolean isStaticContext() {
         return delegationController.isStaticContext();
     }
-    
+
     @Override
     public boolean isStaticMethod() {
         return delegationController.isStaticMethod();
     }
-    
+
     @Override
     public void setInterfaceClassLoadingClass(InterfaceHelperClassNode ihc) {
         delegationController.setInterfaceClassLoadingClass(ihc);
     }
-    
+
     @Override
     public void setMethodVisitor(MethodVisitor methodVisitor) {
         delegationController.setMethodVisitor(methodVisitor);
     }
-    
+
     @Override
     public boolean shouldOptimizeForInt() {
         return delegationController.shouldOptimizeForInt();
     }
-    
+
     @Override
     public void switchToFastPath() {
         delegationController.switchToFastPath();
     }
-    
+
     @Override
     public void switchToSlowPath() {
         delegationController.switchToSlowPath();
     }
-    
+
     @Override
     public int getBytecodeVersion() {
         return delegationController.getBytecodeVersion();
     }
-    
+
     @Override
     public void setLineNumber(int n) {
         delegationController.setLineNumber(n);
     }
-    
+
     @Override
     public int getLineNumber() {
         return delegationController.getLineNumber();
     }
-    
+
     @Override
     public void resetLineNumber() {
         delegationController.resetLineNumber();
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
index 954ae2f..a3cb6df 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java
@@ -22,7 +22,6 @@ import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.ConstructorNode;
-import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.InterfaceHelperClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.classgen.AsmClassGenerator;
@@ -44,8 +43,9 @@ import java.util.Map;
 import static org.apache.groovy.util.SystemUtil.getBooleanSafe;
 
 public class WriterController {
-    private static final String GROOVY_LOG_CLASSGEN = "groovy.log.classgen";
-    private final boolean LOG_CLASSGEN = getBooleanSafe(GROOVY_LOG_CLASSGEN);
+
+    private final boolean LOG_CLASSGEN = getBooleanSafe("groovy.log.classgen");
+
     private AsmClassGenerator acg;
     private MethodVisitor methodVisitor;
     private CompileStack compileStack;
@@ -74,23 +74,23 @@ public class WriterController {
     private int bytecodeVersion = Opcodes.V1_8;
     private int lineNumber = -1;
     private int helperMethodIndex = 0;
-    private List<String> superMethodNames = new ArrayList<String>();
+    private List<String> superMethodNames = new ArrayList<>();
     private MethodPointerExpressionWriter methodPointerExpressionWriter;
     private MethodReferenceExpressionWriter methodReferenceExpressionWriter;
 
-    public void init(AsmClassGenerator asmClassGenerator, GeneratorContext gcon, ClassVisitor cv, ClassNode cn) {
+    public void init(final AsmClassGenerator asmClassGenerator, final GeneratorContext gcon, final ClassVisitor cv, final ClassNode cn) {
         CompilerConfiguration config = cn.getCompileUnit().getConfig();
         Map<String,Boolean> optOptions = config.getOptimizationOptions();
-        boolean invokedynamic=false;
+        boolean invokedynamic = false;
         if (optOptions.isEmpty()) {
             // IGNORE
         } else if (Boolean.FALSE.equals(optOptions.get("all"))) {
-            optimizeForInt=false;
+            optimizeForInt = false;
             // set other optimizations options to false here
         } else {
-            if (Boolean.TRUE.equals(optOptions.get(CompilerConfiguration.INVOKEDYNAMIC))) invokedynamic=true;
-            if (Boolean.FALSE.equals(optOptions.get("int"))) optimizeForInt=false;
-            if (invokedynamic) optimizeForInt=false;
+            if (Boolean.TRUE.equals(optOptions.get(CompilerConfiguration.INVOKEDYNAMIC))) invokedynamic = true;
+            if (Boolean.FALSE.equals(optOptions.get("int"))) optimizeForInt = false;
+            if (invokedynamic) optimizeForInt = false;
             // set other optimizations options to false here
         }
         this.classNode = cn;
@@ -139,7 +139,7 @@ public class WriterController {
         this.typeChooser = new StatementMetaTypeChooser();
     }
 
-    private ClassVisitor createClassVisitor(ClassVisitor cv) {
+    private ClassVisitor createClassVisitor(final ClassVisitor cv) {
         if (!LOG_CLASSGEN) {
             return cv;
         }
@@ -167,7 +167,7 @@ public class WriterController {
         return acg;
     }
 
-    public void setMethodVisitor(MethodVisitor methodVisitor) {
+    public void setMethodVisitor(final MethodVisitor methodVisitor) {
         this.methodVisitor = methodVisitor;
     }
 
@@ -255,22 +255,18 @@ public class WriterController {
         return methodNode;
     }
 
-    public void setMethodNode(MethodNode mn) {
-        methodNode = mn;
-        constructorNode = null;
+    public void setMethodNode(final MethodNode methodNode) {
+        this.methodNode = methodNode;
+        this.constructorNode = null;
     }
 
     public ConstructorNode getConstructorNode(){
         return constructorNode;
     }
 
-    public void setConstructorNode(ConstructorNode cn) {
-        constructorNode = cn;
-        methodNode = null;
-    }
-
-    public boolean isNotClinit() {
-        return methodNode == null || !methodNode.getName().equals("<clinit>");
+    public void setConstructorNode(final ConstructorNode constructorNode) {
+        this.constructorNode = constructorNode;
+        this.methodNode = null;
     }
 
     public SourceUnit getSourceUnit() {
@@ -278,12 +274,12 @@ public class WriterController {
     }
 
     public boolean isStaticContext() {
-        if (compileStack!=null && compileStack.getScope()!=null) {
+        if (compileStack != null && compileStack.getScope() != null) {
             return compileStack.getScope().isInStaticContext();
         }
         if (!isInClosure()) return false;
-        if (constructorNode != null) return false;
-        return classNode.isStaticClass() || methodNode.isStatic();
+        if (isConstructor()) return false;
+        return classNode.isStaticClass() || isStaticMethod();
     }
 
     public boolean isInClosure() {
@@ -297,11 +293,6 @@ public class WriterController {
                 && classNode.getSuperClass() == ClassHelper.CLOSURE_TYPE;
     }
 
-    public boolean isNotExplicitThisInClosure(boolean implicitThis) {
-        return implicitThis || !isInClosure();
-    }
-
-
     public boolean isStaticMethod() {
         return methodNode != null && methodNode.isStatic();
     }
@@ -316,12 +307,16 @@ public class WriterController {
         }
     }
 
+    public boolean isNotClinit() {
+        return methodNode == null || !methodNode.getName().equals("<clinit>");
+    }
+
     public boolean isStaticConstructor() {
         return methodNode != null && methodNode.getName().equals("<clinit>");
     }
 
     public boolean isConstructor() {
-        return constructorNode!=null;
+        return constructorNode != null;
     }
 
     /**
@@ -329,11 +324,7 @@ public class WriterController {
      *         local variables but are properties
      */
     public boolean isInScriptBody() {
-        if (classNode.isScriptBody()) {
-            return true;
-        } else {
-            return classNode.isScript() && methodNode != null && methodNode.getName().equals("run");
-        }
+        return classNode.isScriptBody() || (classNode.isScript() && methodNode != null && methodNode.getName().equals("run"));
     }
 
     public String getClassName() {
@@ -348,10 +339,8 @@ public class WriterController {
 
     public ClassNode getOutermostClass() {
         if (outermostClass == null) {
-            outermostClass = classNode;
-            while (outermostClass instanceof InnerClassNode) {
-                outermostClass = outermostClass.getOuterClass();
-            }
+            List<ClassNode> outers = classNode.getOuterClasses();
+            outermostClass = !outers.isEmpty() ? outers.get(outers.size() - 1) : classNode;
         }
         return outermostClass;
     }
@@ -360,7 +349,7 @@ public class WriterController {
         return context;
     }
 
-    public void setInterfaceClassLoadingClass(InterfaceHelperClassNode ihc) {
+    public void setInterfaceClassLoadingClass(final InterfaceHelperClassNode ihc) {
         interfaceClassLoadingClass = ihc;
     }
 
@@ -398,8 +387,8 @@ public class WriterController {
         return lineNumber;
     }
 
-    public void setLineNumber(int n) {
-        lineNumber = n;
+    public void setLineNumber(final int lineNumber) {
+        this.lineNumber = lineNumber;
     }
 
     public void resetLineNumber() {
@@ -407,11 +396,10 @@ public class WriterController {
     }
 
     public int getNextHelperMethodIndex() {
-        return helperMethodIndex++;
+        return helperMethodIndex += 1;
     }
 
     public List<String> getSuperMethodNames() {
         return superMethodNames;
     }
-
 }

Apache Groovy committer & PMC member

Blog: http://blog.sunlan.me
Twitter: @daniel_sun
123