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 2020/11/16 16:04:51 UTC
[groovy] branch master updated: AutoImplement: remove extra block
from body
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 eed0d63 AutoImplement: remove extra block from body
eed0d63 is described below
commit eed0d633882d815c2670bf97fbfd66dfcef8989c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Nov 16 09:46:59 2020 -0600
AutoImplement: remove extra block from body
- refactor shared logic
---
.../codehaus/groovy/ast/tools/GeneralUtils.java | 15 +++++-
.../transform/AutoImplementASTTransformation.java | 59 +++++++++++-----------
.../transform/NamedVariantASTTransformation.java | 9 +---
.../transform/AutoImplementTransformTest.groovy | 17 ++++++-
4 files changed, 60 insertions(+), 40 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 e6cfe36..32b1477 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java
@@ -78,9 +78,12 @@ import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
+import static org.codehaus.groovy.antlr.PrimitiveHelper.getDefaultValueForPrimitive;
+
/**
* Handy methods when working with the Groovy AST
*/
@@ -340,6 +343,16 @@ public class GeneralUtils {
return new DeclarationExpression(target, ASSIGN, init);
}
+ /**
+ * Returns a constant expression with the default value for the given type
+ * (i.e., {@code false} for boolean, {@code 0} for numbers or {@code null}.
+ *
+ * @since 4.0.0
+ */
+ public static ConstantExpression defaultValueX(final ClassNode type) {
+ return Optional.ofNullable((ConstantExpression) getDefaultValueForPrimitive(type)).orElse(nullX());
+ }
+
public static MapEntryExpression entryX(final Expression key, final Expression value) {
return new MapEntryExpression(key, value);
}
@@ -758,7 +771,7 @@ public class GeneralUtils {
}
public static Statement returnS(final Expression expr) {
- return new ReturnStatement(new ExpressionStatement(expr));
+ return new ReturnStatement(expr);
}
public static Statement safeExpression(final Expression fieldExpr, final Expression expression) {
diff --git a/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
index d618a17..040d084 100644
--- a/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/AutoImplementASTTransformation.java
@@ -30,7 +30,6 @@ import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.expr.ClosureExpression;
import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.EmptyStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.control.CompilePhase;
@@ -46,11 +45,10 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
import static org.apache.groovy.util.BeanUtils.capitalize;
-import static org.codehaus.groovy.antlr.PrimitiveHelper.getDefaultValueForPrimitive;
-import static org.codehaus.groovy.ast.expr.ArgumentListExpression.EMPTY_ARGUMENTS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
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.defaultValueX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getGetterName;
import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS;
@@ -102,29 +100,45 @@ public class AutoImplementASTTransformation extends AbstractASTTransformation {
private void createMethods(final ClassNode cNode, final ClassNode exception, final String message, final ClosureExpression code) {
for (MethodNode candidate : getAllCorrectedMethodsMap(cNode).values()) {
if (candidate.isAbstract()) {
- Statement statement = buildMethodBody(exception, message, code, candidate.getReturnType());
- String propertyName = getPropertyName(candidate);
- if (propertyName != null && candidate.getParameters().length == 0) {
- String accessorName = candidate.getName().startsWith("is")
- ? getGetterName(propertyName) : "is" + capitalize(propertyName);
- if (cNode.hasMethod(accessorName, Parameter.EMPTY_ARRAY)) {
- // delegate to existing accessor to reduce the surprise
- statement = returnS(callX(varX("this"), accessorName));
- }
- }
-
addGeneratedMethod(cNode,
candidate.getName(),
candidate.getModifiers() & 0x7, // visibility only
candidate.getReturnType(),
candidate.getParameters(),
candidate.getExceptions(),
- statement
+ createMethodBody(cNode, candidate, exception, message, code)
);
}
}
}
+ private static Statement createMethodBody(final ClassNode cNode, final MethodNode mNode, final ClassNode exception, final String message, final ClosureExpression code) {
+ if (mNode.getParameters().length == 0) {
+ String propertyName = getPropertyName(mNode);
+ if (propertyName != null) {
+ String accessorName = mNode.getName().startsWith("is") ? getGetterName(propertyName) : "is" + capitalize(propertyName);
+ if (cNode.hasMethod(accessorName, Parameter.EMPTY_ARRAY)) {
+ // delegate to existing accessor to reduce the surprise
+ return returnS(callX(varX("this"), accessorName));
+ }
+ }
+ }
+
+ if (code != null) {
+ return code.getCode();
+ }
+
+ if (exception != null) {
+ if (message == null) {
+ return throwS(ctorX(exception));
+ } else {
+ return throwS(ctorX(exception, constX(message)));
+ }
+ }
+
+ return returnS(defaultValueX(mNode.getReturnType()));
+ }
+
/**
* Returns all methods including abstract super/interface methods but only
* if not overridden by a concrete declared/inherited method.
@@ -217,19 +231,4 @@ public class AutoImplementASTTransformation extends AbstractASTTransformation {
}
return null;
}
-
- private BlockStatement buildMethodBody(final ClassNode exception, final String message, final ClosureExpression code, final ClassNode returnType) {
- BlockStatement body = new BlockStatement();
- if (code != null) {
- body.addStatement(code.getCode());
- } else if (exception != null) {
- body.addStatement(throwS(ctorX(exception, message == null ? EMPTY_ARGUMENTS : constX(message))));
- } else {
- Expression result = getDefaultValueForPrimitive(returnType);
- if (result != null) {
- body.addStatement(returnS(result));
- }
- }
- return body;
- }
}
diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
index 6ce13c5..87f8836 100644
--- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java
@@ -49,7 +49,6 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.ast.tools.ClassNodeUtils.isInnerClass;
import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility;
-import static org.codehaus.groovy.antlr.PrimitiveHelper.getDefaultValueForPrimitive;
import static org.codehaus.groovy.ast.ClassHelper.MAP_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.make;
@@ -63,11 +62,11 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
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.defaultValueX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.entryX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties;
import static org.codehaus.groovy.ast.tools.GeneralUtils.list2args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.mapX;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.param;
import static org.codehaus.groovy.ast.tools.GeneralUtils.plusX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
@@ -191,7 +190,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
for (PropertyNode pNode : props) {
String name = pNode.getName();
// create entry [name: __namedArgs.getOrDefault('name', initialValue)]
- Expression defaultValue = Optional.ofNullable(pNode.getInitialExpression()).orElseGet(() -> getDefaultExpression(pNode.getType()));
+ Expression defaultValue = Optional.ofNullable(pNode.getInitialExpression()).orElseGet(() -> defaultValueX(pNode.getType()));
entries.add(entryX(constX(name), asType(callX(varX(mapParam), "getOrDefault", args(constX(name), defaultValue)), pNode.getType(), coerce)));
// create annotation @NamedParam(value='name', type=DelegateType)
AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE);
@@ -204,10 +203,6 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation {
return true;
}
- private Expression getDefaultExpression(ClassNode pType) {
- return Optional.ofNullable(getDefaultValueForPrimitive(pType)).orElse(nullX());
- }
-
private boolean hasDuplicates(final MethodNode mNode, final List<String> propNames, final String next) {
if (propNames.contains(next)) {
addError("Error during " + NAMED_VARIANT + " processing. Duplicate property '" + next + "' found.", mNode);
diff --git a/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy b/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
index f6468bd..ba86e77 100644
--- a/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/AutoImplementTransformTest.groovy
@@ -18,7 +18,6 @@
*/
package org.codehaus.groovy.transform
-import groovy.test.NotYetImplemented
import org.junit.Test
import static groovy.test.GroovyAssert.assertScript
@@ -172,6 +171,20 @@ final class AutoImplementTransformTest {
}
@Test
+ void testVoidReturnType() {
+ assertScript '''
+ interface Bar {
+ void baz()
+ }
+
+ @groovy.transform.AutoImplement
+ class Foo implements Bar { }
+
+ new Foo().baz() // no value to assert
+ '''
+ }
+
+ @Test
void testGenericReturnTypes() {
assertScript '''
interface HasXs<T> {
@@ -193,7 +206,7 @@ final class AutoImplementTransformTest {
}
@Test // GROOVY-8270
- void testGenericsParameterTypes() {
+ void testGenericParameterTypes() {
assertScript '''
@groovy.transform.AutoImplement
class Foo implements Comparator<String> { }