You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2019/11/01 00:23:21 UTC

[groovy] branch master updated: refactor @NotYetImplemented AST transform

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


The following commit(s) were added to refs/heads/master by this push:
     new 9830718  refactor @NotYetImplemented AST transform
9830718 is described below

commit 983071852d8a0a3080fad7f31706e27c00c74b49
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Oct 31 18:45:07 2019 -0500

    refactor @NotYetImplemented AST transform
---
 .../codehaus/groovy/ast/tools/GeneralUtils.java    |   9 ++
 .../NotYetImplementedASTTransformation.java        |  63 ++------
 .../NotYetImplementedTransformTest.groovy          | 173 +++++++++++----------
 3 files changed, 112 insertions(+), 133 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
index b970f3d..b7360ba 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -59,6 +59,7 @@ 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.ThrowStatement;
+import org.codehaus.groovy.ast.stmt.TryCatchStatement;
 import org.codehaus.groovy.classgen.Verifier;
 import org.codehaus.groovy.control.io.ReaderSource;
 import org.codehaus.groovy.runtime.GeneratedClosure;
@@ -783,6 +784,14 @@ public class GeneralUtils {
         return new CatchStatement(variable, code);
     }
 
+    public static TryCatchStatement tryCatchS(Statement tryStatement) {
+        return tryCatchS(tryStatement, EmptyStatement.INSTANCE);
+    }
+
+    public static TryCatchStatement tryCatchS(Statement tryStatement, Statement finallyStatement) {
+        return new TryCatchStatement(tryStatement, finallyStatement);
+    }
+
     /**
      * This method is similar to {@link #propX(Expression, Expression)} but will make sure that if the property
      * being accessed is defined inside the classnode provided as a parameter, then a getter call is generated
diff --git a/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java b/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
index 9ff1fb3..2b8cba3 100644
--- a/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
+++ b/subprojects/groovy-test/src/main/java/org/apache/groovy/test/transform/NotYetImplementedASTTransformation.java
@@ -19,22 +19,19 @@
 package org.apache.groovy.test.transform;
 
 import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.stmt.BlockStatement;
-import org.codehaus.groovy.ast.stmt.EmptyStatement;
 import org.codehaus.groovy.ast.stmt.ReturnStatement;
-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.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.transform.AbstractASTTransformation;
 import org.codehaus.groovy.transform.GroovyASTTransformation;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
@@ -44,63 +41,35 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.tryCatchS;
 
 /**
- * Handles generation of code for the {@code @NotYetImplemented} annotation.
- * 
+ * Generates code for the {@code @NotYetImplemented} annotation.
+ *
  * @see groovy.test.NotYetImplemented
  */
 @GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
 public class NotYetImplementedASTTransformation extends AbstractASTTransformation {
 
-    private static final ClassNode CATCHED_THROWABLE_TYPE = ClassHelper.make(Throwable.class);
-    private static final ClassNode ASSERTION_FAILED_ERROR_TYPE = ClassHelper.make("junit.framework.AssertionFailedError");
+    private static final ClassNode CATCH_TYPE = ClassHelper.make(Throwable.class);
+    private static final ClassNode THROW_TYPE = ClassHelper.make("junit.framework.AssertionFailedError"); // TODO: java.lang.AssertionError
 
     public void visit(ASTNode[] nodes, SourceUnit source) {
-        if (nodes.length != 2 || !(nodes[0] instanceof AnnotationNode) || !(nodes[1] instanceof AnnotatedNode)) {
-            throw new RuntimeException("Internal error: expecting [AnnotationNode, AnnotatedNode] but got: " + Arrays.asList(nodes));
-        }
-
-        AnnotationNode annotationNode = (AnnotationNode) nodes[0];
-        ASTNode node = nodes[1];
-
-        if (!(node instanceof MethodNode))  {
-            addError("@NotYetImplemented must only be applied on test methods!",node);
-            return;
-        }
-
-        MethodNode methodNode = (MethodNode) node;
-
-        ArrayList<Statement> statements = new ArrayList<Statement>();
-        Statement statement = methodNode.getCode();
-        if (statement instanceof BlockStatement)  {
-            statements.addAll(((BlockStatement) statement).getStatements());
+        if (!(nodes.length == 2 && nodes[0] instanceof AnnotationNode && nodes[1] instanceof MethodNode)) {
+            throw new RuntimeException("Internal error: expecting [AnnotationNode, MethodNode] but got: " + Arrays.toString(nodes));
         }
 
-        if (statements.isEmpty()) return;
-
-        BlockStatement rewrittenMethodCode = new BlockStatement();
-
-        rewrittenMethodCode.addStatement(tryCatchAssertionFailedError(annotationNode, methodNode, statements));
-        rewrittenMethodCode.addStatement(throwAssertionFailedError(annotationNode));
+        MethodNode methodNode = (MethodNode) nodes[1];
 
-        methodNode.setCode(rewrittenMethodCode);
-    }
+        if (methodNode.getCode() instanceof BlockStatement && !((BlockStatement) methodNode.getCode()).isEmpty()) {
+            // wrap code in try/catch with return for failure path followed by throws for success path
 
-    private TryCatchStatement tryCatchAssertionFailedError(AnnotationNode annotationNode, MethodNode methodNode, ArrayList<Statement> statements) {
-        TryCatchStatement tryCatchStatement = new TryCatchStatement(
-                block(methodNode.getVariableScope(), statements),
-                EmptyStatement.INSTANCE);
-        tryCatchStatement.addCatch(catchS(param(CATCHED_THROWABLE_TYPE, "ex"), ReturnStatement.RETURN_NULL_OR_VOID));
-        return tryCatchStatement;
-    }
+            TryCatchStatement tryCatchStatement = tryCatchS(methodNode.getCode());
+            tryCatchStatement.addCatch(catchS(param(CATCH_TYPE, "ignore"), ReturnStatement.RETURN_NULL_OR_VOID));
 
-    private Statement throwAssertionFailedError(AnnotationNode annotationNode) {
-        Statement throwStatement = throwS(
-                ctorX(ASSERTION_FAILED_ERROR_TYPE,
-                        args(constX("Method is marked with @NotYetImplemented but passes unexpectedly"))));
-        throwStatement.setSourcePosition(annotationNode);
+            ThrowStatement throwStatement = throwS(ctorX(THROW_TYPE, args(constX("Method is marked with @NotYetImplemented but passes unexpectedly"))));
 
-        return throwStatement;
+            methodNode.setCode(block(tryCatchStatement, throwStatement));
+        }
     }
 }
diff --git a/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy b/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
index b4ab0da..99314d9 100644
--- a/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
+++ b/subprojects/groovy-test/src/test/groovy/org/apache/groovy/test/transform/NotYetImplementedTransformTest.groovy
@@ -18,122 +18,123 @@
  */
 package org.apache.groovy.test.transform
 
-import groovy.test.GroovyShellTestCase
 import junit.framework.AssertionFailedError
+import org.junit.Test
 
-class NotYetImplementedTransformTest extends GroovyShellTestCase {
+final class NotYetImplementedTransformTest {
 
-    void testNotYetImplemented() {
-        def output = evaluate("""
-              import groovy.test.GroovyTestCase
-              import groovy.test.NotYetImplemented
+    private final GroovyShell shell = new GroovyShell()
 
-              class MyTests extends GroovyTestCase {
-                @NotYetImplemented void testShouldNotFail()  {
-                    assertFalse true
+    @Test
+    void testNotYetImplementedJUnit3Failure() {
+        def output = shell.evaluate('''
+            import groovy.test.GroovyTestCase
+            import groovy.test.NotYetImplemented
+
+            class MyTests extends GroovyTestCase {
+                @NotYetImplemented void testThatFails()  {
+                    assertTrue(false)
                 }
-              }
+            }
 
-              junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
-        """)
+            junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
+        ''')
 
-        assertNotNull output
-        assertTrue "test method marked with @NotYetImplemented must NOT throw an AsssertionFailedError", output.wasSuccessful()
+        assert output.wasSuccessful() : 'test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
     }
 
-    void testNotYetImplementedWithException() {
-            def output = evaluate("""
-                  import groovy.test.GroovyTestCase
-                  import groovy.test.NotYetImplemented
+    @Test
+    void testNotYetImplementedJUnit3Exception() {
+        def output = shell.evaluate('''
+            import groovy.test.GroovyTestCase
+            import groovy.test.NotYetImplemented
 
-                  class MyTests extends GroovyTestCase {
-                    @NotYetImplemented void testShouldNotFail()  {
-                        'test'.yetUnkownMethod()
-                    }
-                  }
+            class MyTests extends GroovyTestCase {
+                @NotYetImplemented void testThatFails()  {
+                    'test'.missingMethod()
+                }
+            }
 
-                  junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
-            """)
+            junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
+        ''')
 
-            assertNotNull output
-            assertTrue "test method marked with @NotYetImplemented must NOT throw an AsssertionFailedError", output.wasSuccessful()
-        }
+        assert output.wasSuccessful() : 'test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
+    }
 
-    void testNotYetImplementedPassThrough() {
-        def output = evaluate("""
-              import groovy.test.GroovyTestCase
-              import groovy.test.NotYetImplemented
+    @Test
+    void testNotYetImplementedJunit3PassThrough() {
+        def output = shell.evaluate('''
+            import groovy.test.GroovyTestCase
+            import groovy.test.NotYetImplemented
 
-              class MyTests extends GroovyTestCase {
-                @NotYetImplemented void testShouldFail()  {
-                    assertTrue true
+            class MyTests extends GroovyTestCase {
+                @NotYetImplemented void testThatPasses()  {
+                    assertTrue(true)
                 }
-              }
-
-              junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
-        """)
+            }
 
-        assertNotNull output
+            junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
+        ''')
 
-        assertEquals "test method marked with @NotYetImplemented must throw an AssertionFailedError", 1, output.failureCount()
-        assertEquals "test method marked with @NotYetImplemented must throw an AssertionFailedError", AssertionFailedError, output.failures().nextElement().thrownException().class
+        assert output.failureCount() == 1 : 'test method marked with @NotYetImplemented must throw an AssertionFailedError'
+        assert output.failures().nextElement().thrownException() instanceof AssertionFailedError : 'test method marked with @NotYetImplemented must throw an AssertionFailedError'
     }
 
-    void testEmptyTestMethod() {
-        def output = evaluate("""
-              import groovy.test.GroovyTestCase
-              import groovy.test.NotYetImplemented
+    @Test
+    void testNotYetImplementedJUnit3EmptyMethod() {
+        def output = shell.evaluate('''
+            import groovy.test.GroovyTestCase
+            import groovy.test.NotYetImplemented
 
-              class MyTests extends GroovyTestCase {
-                @NotYetImplemented void testShouldNotFail()  {}
-              }
-
-              junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
-        """)
+            class MyTests extends GroovyTestCase {
+                @NotYetImplemented void testShouldNotFail() {
+                }
+            }
 
-        assertNotNull output
+            junit.textui.TestRunner.run(new junit.framework.TestSuite(MyTests))
+        ''')
 
-        assertTrue "empty test method must not throw an AssertionFailedError", output.wasSuccessful()
+        assert output.wasSuccessful() : 'empty test method must not throw an AssertionFailedError'
     }
 
-    void testNotYetImplementedJUnit4()  {
-
-        def output = evaluate("""
-        import groovy.test.NotYetImplemented
-        import org.junit.Test
-        import org.junit.runner.JUnitCore
-
-        class MyTests {
-            @NotYetImplemented @Test void testShouldNotFail()  {
-                junit.framework.Assert.assertFalse true
+    @Test
+    void testNotYetImplementedJUnit4Failure()  {
+        def output = shell.evaluate('''
+            import groovy.test.NotYetImplemented
+            import org.junit.Assert
+            import org.junit.Test
+            import org.junit.runner.JUnitCore
+
+            class MyTests {
+                @NotYetImplemented @Test void testThatFails()  {
+                    Assert.assertTrue(false)
+                }
             }
-        }
 
-        new JUnitCore().run(MyTests)
-        """)
-
-        assertTrue output.wasSuccessful()
+            new JUnitCore().run(MyTests)
+        ''')
 
+        assert output.wasSuccessful() : '@Test method marked with @NotYetImplemented must NOT throw an AssertionFailedError'
     }
 
-    void testNotYetImplementedPassThroughJUnit4() {
-        def output = evaluate("""
-              import groovy.test.NotYetImplemented
-              import org.junit.Test
-              import org.junit.runner.JUnitCore
-
-              class MyTests {
-                @NotYetImplemented @Test void shouldFail()  {
-                    junit.framework.Assert.assertTrue true
+    @Test
+    void testNotYetImplementedJUnit4Success() {
+        def output = shell.evaluate('''
+            import groovy.test.NotYetImplemented
+            import org.junit.Assert
+            import org.junit.Test
+            import org.junit.runner.JUnitCore
+
+            class MyTests {
+                @NotYetImplemented @Test void testThatPasses()  {
+                    Assert.assertTrue(true)
                 }
-              }
-
-              new JUnitCore().run(MyTests)
-        """)
+            }
 
-        assertNotNull output
+            new JUnitCore().run(MyTests)
+        ''')
 
-        assertEquals "test method marked with @NotYetImplemented must throw an AssertionFailedError", 1, output.failureCount
-        assertEquals "test method marked with @NotYetImplemented must throw an AssertionFailedError", AssertionFailedError, output.failures.first().exception.class
+        assert output.failureCount == 1 : '@Test method marked with @NotYetImplemented must throw an AssertionFailedError'
+        assert output.failures.first().exception instanceof AssertionFailedError : '@Test method marked with @NotYetImplemented must throw an AssertionFailedError'
     }
-}
\ No newline at end of file
+}