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

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

commit e183dc8e04a8ca8762e85e21ac3ee0c03138b6e1
Author: Eric Milles <[email protected]>
AuthorDate: Sat Apr 30 14:41:47 2022 -0500

    minor refactor
---
 .../antlr4/TryWithResourcesASTTransformation.java  | 453 +++++++++------------
 1 file changed, 190 insertions(+), 263 deletions(-)

diff --git 
a/src/main/java/org/apache/groovy/parser/antlr4/TryWithResourcesASTTransformation.java
 
b/src/main/java/org/apache/groovy/parser/antlr4/TryWithResourcesASTTransformation.java
index 11c2f08fc6..624666c8d8 100644
--- 
a/src/main/java/org/apache/groovy/parser/antlr4/TryWithResourcesASTTransformation.java
+++ 
b/src/main/java/org/apache/groovy/parser/antlr4/TryWithResourcesASTTransformation.java
@@ -20,10 +20,7 @@ package org.apache.groovy.parser.antlr4;
 
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.Parameter;
-import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.BinaryExpression;
-import org.codehaus.groovy.ast.expr.BooleanExpression;
-import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.DeclarationExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
@@ -32,61 +29,81 @@ import org.codehaus.groovy.ast.stmt.BlockStatement;
 import org.codehaus.groovy.ast.stmt.CatchStatement;
 import org.codehaus.groovy.ast.stmt.EmptyStatement;
 import org.codehaus.groovy.ast.stmt.ExpressionStatement;
-import org.codehaus.groovy.ast.stmt.IfStatement;
 import org.codehaus.groovy.ast.stmt.Statement;
-import org.codehaus.groovy.ast.stmt.ThrowStatement;
 import org.codehaus.groovy.ast.stmt.TryCatchStatement;
-import org.codehaus.groovy.syntax.Types;
 import org.objectweb.asm.Opcodes;
 
-import java.util.Collections;
-import java.util.List;
-
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ASSIGN;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.assignS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.catchS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.declS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ifElseS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.notNullX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.tryCatchS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
-import static org.codehaus.groovy.syntax.Token.newSymbol;
 
 /**
  * Transform try-with-resources to try-catch-finally
  * Reference JLS "14.20.3. 
try-with-resources"(https://docs.oracle.com/javase/specs/jls/se7/html/jls-14.html)
  */
 public class TryWithResourcesASTTransformation {
-    private AstBuilder astBuilder;
 
-    public TryWithResourcesASTTransformation(AstBuilder astBuilder) {
+    public TryWithResourcesASTTransformation(final AstBuilder astBuilder) {
         this.astBuilder = astBuilder;
     }
+    private AstBuilder astBuilder;
+
+    private int resourceCount;
+    private String nextResourceName() {
+        return "__$$resource" + resourceCount++;
+    }
+
+    private int throwableCount;
+    private String nextThrowableName() {
+        return "__$$t" + throwableCount++;
+    }
+
+    private int primaryExCount;
+    private String nextPrimaryExName() {
+        return "__$$primaryExc" + primaryExCount++;
+    }
+
+    private int suppressedExCount;
+    private String nextSuppressedExName() {
+        return "__$$suppressedExc" + suppressedExCount++;
+    }
+
+    
//--------------------------------------------------------------------------
 
     /**
-     * Reference JLS "14.20.3. 
try-with-resources"(https://docs.oracle.com/javase/specs/jls/se7/html/jls-14.html)
-     *
      * @param tryCatchStatement the try-with-resources statement to transform
-     * @return try-catch-finally statement, which contains no resources clause
+     * @return try-catch-finally statement which contains no resources clause
      */
-    public Statement transform(TryCatchStatement tryCatchStatement) {
+    public Statement transform(final TryCatchStatement tryCatchStatement) {
         if (!asBoolean(tryCatchStatement.getResourceStatements())) {
             return tryCatchStatement;
         }
-
-        if (this.isBasicTryWithResourcesStatement(tryCatchStatement)) {
-            return 
this.transformBasicTryWithResourcesStatement(tryCatchStatement);
+        if (isBasicTryWithResourcesStatement(tryCatchStatement)) {
+            return transformBasicTryWithResourcesStatement(tryCatchStatement);
         } else {
-            return 
this.transformExtendedTryWithResourcesStatement(tryCatchStatement);
+            return 
transformExtendedTryWithResourcesStatement(tryCatchStatement);
         }
     }
 
-    private boolean isBasicTryWithResourcesStatement(TryCatchStatement 
tryCatchStatement) {
-        if (tryCatchStatement.getFinallyStatement() instanceof EmptyStatement 
&&
-                !asBoolean(tryCatchStatement.getCatchStatements())) {
-            return true;
-        }
-
-        return false;
+    private boolean isBasicTryWithResourcesStatement(final TryCatchStatement 
tryCatchStatement) {
+        return (!asBoolean(tryCatchStatement.getCatchStatements())
+            && tryCatchStatement.getFinallyStatement() instanceof 
EmptyStatement);
     }
 
-    private ExpressionStatement 
makeVariableDeclarationFinal(ExpressionStatement variableDeclaration) {
+    private Statement makeVariableDeclarationFinal(final ExpressionStatement 
variableDeclaration) {
         if (!asBoolean(variableDeclaration)) {
             return variableDeclaration;
         }
@@ -95,268 +112,188 @@ public class TryWithResourcesASTTransformation {
             throw new IllegalArgumentException("variableDeclaration is not a 
declaration statement");
         }
 
-        DeclarationExpression declarationExpression = (DeclarationExpression) 
variableDeclaration.getExpression();
-        if (!(declarationExpression.getLeftExpression() instanceof 
VariableExpression)) {
+        Expression targetExpression = ((DeclarationExpression) 
variableDeclaration.getExpression()).getLeftExpression();
+        if (!(targetExpression instanceof VariableExpression)) {
             throw astBuilder.createParsingFailedException("The expression 
statement is not a variable delcaration statement", variableDeclaration);
         }
 
-        VariableExpression variableExpression = (VariableExpression) 
declarationExpression.getLeftExpression();
+        VariableExpression variableExpression = (VariableExpression) 
targetExpression;
         variableExpression.setModifiers(variableExpression.getModifiers() | 
Opcodes.ACC_FINAL);
 
         return variableDeclaration;
     }
 
-    private int primaryExcCnt = 0;
-    private String genPrimaryExcName() {
-        return "__$$primaryExc" + primaryExcCnt++;
-    }
-
-    /*
-     *   try ResourceSpecification
-     *       Block
-     *   Catchesopt
-     *   Finallyopt
-     *
-     *   **The above AST should be transformed to the following AST**
-     *
-     *   try {
-     *       try ResourceSpecification
-     *           Block
-     *   }
-     *   Catchesopt
-     *   Finallyopt
+    /**
+     * Transforms:
+     * <pre>
+     * try ResourceSpecification
+     *     Block
+     * Catchesopt
+     * Finallyopt
+     * </pre>
+     * into:
+     * <pre>
+     * try {
+     *     try ResourceSpecification
+     *         Block
+     * }
+     * Catchesopt
+     * Finallyopt
+     * </pre>
      */
-    private Statement 
transformExtendedTryWithResourcesStatement(TryCatchStatement tryCatchStatement) 
{
+    private Statement transformExtendedTryWithResourcesStatement(final 
TryCatchStatement tryCatchFinally) {
         /*
-         *  try ResourceSpecification
-         *      Block
+         * try ResourceSpecification
+         *     Block
          */
-        TryCatchStatement newTryWithResourcesStatement = 
tryCatchS(tryCatchStatement.getTryStatement());
-        
tryCatchStatement.getResourceStatements().forEach(newTryWithResourcesStatement::addResource);
+        TryCatchStatement newTryWithResources = 
tryCatchS(tryCatchFinally.getTryStatement());
+        
tryCatchFinally.getResourceStatements().forEach(newTryWithResources::addResource);
 
         /*
-         *   try {
-         *       << the following try-with-resources has been transformed >>
-         *       try ResourceSpecification
-         *           Block
-         *   }
-         *   Catchesopt
-         *   Finallyopt
+         * try {
+         *     << the following try-with-resources has been transformed >>
+         *     try ResourceSpecification
+         *         Block
+         * }
+         * Catchesopt
+         * Finallyopt
          */
-        TryCatchStatement newTryCatchStatement = tryCatchS(
-                
astBuilder.createBlockStatement(this.transform(newTryWithResourcesStatement)),
-                tryCatchStatement.getFinallyStatement());
-        
tryCatchStatement.getCatchStatements().forEach(newTryCatchStatement::addCatch);
-
-        return newTryCatchStatement;
+        TryCatchStatement newTryCatchFinally = tryCatchS(
+                block(transform(newTryWithResources)),
+                tryCatchFinally.getFinallyStatement()
+        );
+        
tryCatchFinally.getCatchStatements().forEach(newTryCatchFinally::addCatch);
+        return newTryCatchFinally;
     }
 
-    /*
-     *   try (VariableModifiersopt R Identifier = Expression ...)
-     *      Block
-     *
-     *  **The above AST should be transformed to the following AST**
-     *
-     *   {
-     *       final VariableModifiers_minus_final R Identifier = Expression;
-     *       Throwable #primaryExc = null;
-     *
-     *       try ResourceSpecification_tail
-     *           Block
-     *       catch (Throwable #t) {
-     *           #primaryExc = #t;
-     *           throw #t;
-     *       } finally {
-     *           if (Identifier != null) {
-     *               if (#primaryExc != null) {
-     *                   try {
-     *                       Identifier.close();
-     *                   } catch (Throwable #suppressedExc) {
-     *                       #primaryExc.addSuppressed(#suppressedExc);
-     *                   }
-     *               } else {
-     *                   Identifier.close();
-     *               }
-     *           }
-     *       }
-     *   }
-     *
+    /**
+     * Transforms:
+     * <pre>
+     * try (VariableModifiersopt R Identifier = Expression ...)
+     *    Block
+     * </pre>
+     * into:
+     * <pre>
+     * {
+     *     final VariableModifiers_minus_final R resourceIdentifier = 
Expression;
+     *     Throwable #primaryException = null;
+     *     try ResourceSpecification_tail
+     *         Block
+     *     catch (Throwable #t) {
+     *         #primaryException = #t;
+     *         throw #t;
+     *     } finally {
+     *         if (resourceIdentifier != null) {
+     *             if (#primaryException != null) {
+     *                 try {
+     *                     resourceIdentifier?.close();
+     *                 } catch (Throwable #suppressedException) {
+     *                     
#primaryException.addSuppressed(#suppressedException);
+     *                 }
+     *             } else {
+     *                 resourceIdentifier?.close();
+     *             }
+     *         }
+     *     }
+     * }
+     * </pre>
      */
-    private Statement 
transformBasicTryWithResourcesStatement(TryCatchStatement tryCatchStatement) {
-        // { ... }
+    private Statement transformBasicTryWithResourcesStatement(final 
TryCatchStatement tryWithResources) {
         BlockStatement blockStatement = new BlockStatement();
 
         // final VariableModifiers_minus_final R Identifier = Expression;
-        ExpressionStatement firstResourceStatement =
-                this.makeVariableDeclarationFinal(
-                        tryCatchStatement.getResourceStatement(0));
-        astBuilder.appendStatementsToBlockStatement(blockStatement, 
firstResourceStatement);
-
-        // Throwable #primaryExc = null;
-        String primaryExcName = this.genPrimaryExcName();
-        VariableExpression primaryExcX = localVarX(primaryExcName, 
ClassHelper.THROWABLE_TYPE.getPlainNodeReference());
-        ExpressionStatement primaryExcDeclarationStatement =
-                new ExpressionStatement(
-                        new DeclarationExpression(
-                                primaryExcX,
-                                newSymbol(Types.ASSIGN, -1, -1),
-                                nullX()
-                        )
-                );
-        astBuilder.appendStatementsToBlockStatement(blockStatement, 
primaryExcDeclarationStatement);
-
-        // The generated try-catch-finally statement
-        String firstResourceIdentifierName =
-                ((DeclarationExpression) 
tryCatchStatement.getResourceStatement(0).getExpression()).getLeftExpression().getText();
-
-        TryCatchStatement newTryCatchStatement = tryCatchS(
-                tryCatchStatement.getTryStatement(),
-                this.createFinallyBlockForNewTryCatchStatement(primaryExcName, 
firstResourceIdentifierName));
-
-        List<ExpressionStatement> resourceStatements = 
tryCatchStatement.getResourceStatements();
-        // 2nd, 3rd, ..., n'th resources declared in resources
-        List<ExpressionStatement> tailResourceStatements = 
resourceStatements.subList(1, resourceStatements.size());
-        
tailResourceStatements.stream().forEach(newTryCatchStatement::addResource);
+        ExpressionStatement firstResourceDeclaration = 
tryWithResources.getResourceStatement(0);
+        
blockStatement.addStatement(makeVariableDeclarationFinal(firstResourceDeclaration));
 
-        
newTryCatchStatement.addCatch(this.createCatchBlockForOuterNewTryCatchStatement(primaryExcName));
-        astBuilder.appendStatementsToBlockStatement(blockStatement, 
this.transform(newTryCatchStatement));
+        // Throwable #primaryException = null;
+        String primaryExceptionName = nextPrimaryExName();
+        blockStatement.addStatement(declS(
+                localVarX(primaryExceptionName, 
ClassHelper.THROWABLE_TYPE.getPlainNodeReference()),
+                nullX()
+        ));
 
-        return blockStatement;
-    }
+        String firstResourceIdentifier = ((DeclarationExpression) 
firstResourceDeclaration.getExpression()).getLeftExpression().getText();
 
-    /*
-     *   catch (Throwable #t) {
-     *       #primaryExc = #t;
-     *       throw #t;
-     *   }
-     *
-     */
-    private CatchStatement createCatchBlockForOuterNewTryCatchStatement(String 
primaryExcName) {
-        // { ... }
-        BlockStatement blockStatement = new BlockStatement();
-        String tExcName = this.genTExcName();
-
-        // #primaryExc = #t;
-        ExpressionStatement primaryExcAssignStatement =
-                new ExpressionStatement(
-                        new BinaryExpression(
-                                new VariableExpression(primaryExcName),
-                                newSymbol(Types.ASSIGN, -1, -1),
-                                new VariableExpression(tExcName)));
-        astBuilder.appendStatementsToBlockStatement(blockStatement, 
primaryExcAssignStatement);
-
-        // throw #t;
-        ThrowStatement throwTExcStatement = new ThrowStatement(new 
VariableExpression(tExcName));
-        astBuilder.appendStatementsToBlockStatement(blockStatement, 
throwTExcStatement);
+        TryCatchStatement newTryCatchFinally = tryCatchS(
+                tryWithResources.getTryStatement(), // Block
+                
createFinallyBlockForNewTryCatchStatement(primaryExceptionName, 
firstResourceIdentifier) // close resource and propagate throwable(s)
+        );
+        // 2nd, 3rd, ..., n'th resources declared in resources
+        
tryWithResources.getResourceStatements().stream().skip(1).forEach(newTryCatchFinally::addResource);
+        
newTryCatchFinally.addCatch(createCatchBlockForOuterNewTryCatchStatement(primaryExceptionName));
 
-        // Throwable #t
-        Parameter tExcParameter = new 
Parameter(ClassHelper.THROWABLE_TYPE.getPlainNodeReference(), tExcName);
+        blockStatement.addStatement(transform(newTryCatchFinally)); // 
transform remaining resources
 
-        return new CatchStatement(tExcParameter, blockStatement);
+        return blockStatement;
     }
 
-    private int tExcCnt = 0;
-    private String genTExcName() {
-        return "__$$t" + tExcCnt++;
+    private CatchStatement createCatchBlockForOuterNewTryCatchStatement(final 
String primaryExceptionName) {
+        String throwableName = nextThrowableName();
+        Parameter catchParameter = new 
Parameter(ClassHelper.THROWABLE_TYPE.getPlainNodeReference(), throwableName);
+
+        return catchS(catchParameter, block( // catch (Throwable #t) {
+                assignS(varX(primaryExceptionName), varX(throwableName)), // 
#primaryException = #t;
+                throwS(varX(throwableName)) // throw #t;
+        )); // }
     }
 
-    /*
-     *   finally {
-     *       if (Identifier != null) {
-     *           if (#primaryExc != null) {
-     *              try {
-     *                  Identifier.close();
-     *              } catch (Throwable #suppressedExc) {
-     *                  #primaryExc.addSuppressed(#suppressedExc);
-     *              }
-     *           } else {
-     *               Identifier.close();
-     *           }
-     *       }
-     *   }
-     *
-     * We can simplify the above code to a Groovy version as follows:
-     *
-     *   finally {
-     *      if (#primaryExc != null)
-     *         try {
-     *             Identifier?.close();
-     *         } catch (Throwable #suppressedExc) {
-     *             #primaryExc.addSuppressed(#suppressedExc);
+    /**
+     * Transforms:
+     * <pre>
+     * finally {
+     *     if (#resourceIdentifier != null) {
+     *         if (#primaryException != null) {
+     *            try {
+     *                #resourceIdentifier?.close();
+     *            } catch (Throwable #suppressedException) {
+     *                #primaryException.addSuppressed(#suppressedException);
+     *            }
+     *         } else {
+     *             #resourceIdentifier?.close();
      *         }
-     *      else
-     *          Identifier?.close();
-     *
-     *   }
-     *
+     *     }
+     * }
+     * </pre>
+     * into:
+     * <pre>
+     * finally {
+     *    if (#primaryException != null)
+     *       try {
+     *           #resourceIdentifier?.close();
+     *       } catch (Throwable #suppressedException) {
+     *           #primaryException.addSuppressed(#suppressedException);
+     *       }
+     *    else
+     *        #resourceIdentifier?.close();
+     * }
+     * </pre>
      */
-    private BlockStatement createFinallyBlockForNewTryCatchStatement(String 
primaryExcName, String firstResourceIdentifierName) {
-        BlockStatement finallyBlock = new BlockStatement();
-
-        // primaryExc != null
-        BooleanExpression conditionExpression =
-                new BooleanExpression(
-                        new BinaryExpression(
-                                new VariableExpression(primaryExcName),
-                                newSymbol(Types.COMPARE_NOT_EQUAL, -1, -1),
-                                new ConstantExpression(null)));
-
-        // try-catch statement
-        TryCatchStatement newTryCatchStatement = tryCatchS(
-                
astBuilder.createBlockStatement(this.createCloseResourceStatement(firstResourceIdentifierName))
 /* { Identifier?.close(); }*/);
-
-        String suppressedExcName = this.genSuppressedExcName();
-        newTryCatchStatement.addCatch(
-                // catch (Throwable #suppressedExc) { .. }
-                new CatchStatement(
-                        new 
Parameter(ClassHelper.THROWABLE_TYPE.getPlainNodeReference(), 
suppressedExcName),
-                        
astBuilder.createBlockStatement(this.createAddSuppressedStatement(primaryExcName,
 suppressedExcName)) // #primaryExc.addSuppressed(#suppressedExc);
-                )
-        );
-
-        // if (#primaryExc != null) { ... }
-        IfStatement ifStatement =
-                new IfStatement(
-                        conditionExpression,
-                        newTryCatchStatement,
-                        
this.createCloseResourceStatement(firstResourceIdentifierName) // 
Identifier?.close();
-                );
-        astBuilder.appendStatementsToBlockStatement(finallyBlock, ifStatement);
-
-        return astBuilder.createBlockStatement(finallyBlock);
-    }
-
-    private int suppressedExcCnt = 0;
-    private String genSuppressedExcName() {
-        return "__$$suppressedExc" + suppressedExcCnt++;
+    private Statement createFinallyBlockForNewTryCatchStatement(final String 
primaryExceptionName, final String firstResourceIdentifierName) {
+        String suppressedExceptionName = nextSuppressedExName();
+        TryCatchStatement newTryCatch = 
tryCatchS(block(createCloseResourceStatement(firstResourceIdentifierName)));
+        newTryCatch.addCatch(catchS(
+                new 
Parameter(ClassHelper.THROWABLE_TYPE.getPlainNodeReference(), 
suppressedExceptionName),
+                block(createAddSuppressedStatement(primaryExceptionName, 
suppressedExceptionName))
+        ));
+
+        return block(ifElseS(notNullX(varX(primaryExceptionName)), // if 
(#primaryException != null)
+                newTryCatch, // try { #resource?.close() } catch (Throwable 
#suppressed) { #primary.addSuppressed(#suppressed) }
+                createCloseResourceStatement(firstResourceIdentifierName) // 
else #resource?.close()
+        ));
     }
 
-    /*
-     *  Identifier?.close();
-     */
-    private ExpressionStatement createCloseResourceStatement(String 
firstResourceIdentifierName) {
-        MethodCallExpression closeMethodCallExpression =
-                new MethodCallExpression(new 
VariableExpression(firstResourceIdentifierName), "close", new 
ArgumentListExpression());
-
+    private static Statement createCloseResourceStatement(final String 
resourceIdentifierName) {
+        MethodCallExpression closeMethodCallExpression = 
callX(varX(resourceIdentifierName), "close");
         closeMethodCallExpression.setImplicitThis(false);
         closeMethodCallExpression.setSafe(true);
-
-        return new ExpressionStatement(closeMethodCallExpression);
+        return stmt(closeMethodCallExpression);
     }
 
-    /*
-     *  #primaryExc.addSuppressed(#suppressedExc);
-     */
-    private ExpressionStatement createAddSuppressedStatement(String 
primaryExcName, String suppressedExcName) {
-        MethodCallExpression addSuppressedMethodCallExpression =
-                new MethodCallExpression(
-                        new VariableExpression(primaryExcName),
-                        "addSuppressed",
-                        new 
ArgumentListExpression(Collections.singletonList(new 
VariableExpression(suppressedExcName))));
+    private static Statement createAddSuppressedStatement(final String 
primaryException, final String suppressedException) {
+        MethodCallExpression addSuppressedMethodCallExpression = 
callX(varX(primaryException), "addSuppressed", varX(suppressedException));
         addSuppressedMethodCallExpression.setImplicitThis(false);
         addSuppressedMethodCallExpression.setSafe(true);
-
-        return new ExpressionStatement(addSuppressedMethodCallExpression);
+        return stmt(addSuppressedMethodCallExpression);
     }
 
     /**
@@ -372,17 +309,7 @@ public class TryWithResourcesASTTransformation {
      *      Block
      * }
      */
-    public BinaryExpression transformResourceAccess(Expression 
variableAccessExpression) {
-        return new BinaryExpression(
-                new VariableExpression(genResourceName()),
-                newSymbol(Types.ASSIGN, -1, -1),
-                variableAccessExpression
-        );
-    }
-
-    private int resourceCnt = 0;
-    private String genResourceName() {
-        return "__$$resource" + resourceCnt++;
+    public BinaryExpression transformResourceAccess(final Expression 
variableExpression) {
+        return binX(varX(nextResourceName()), ASSIGN, variableExpression);
     }
-
 }

Reply via email to