You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/02/25 02:11:35 UTC
[groovy] branch master updated: GROOVY-9417: Make @NullCheck play
nicer with @Immutable (closes #1172)
This is an automated email from the ASF dual-hosted git repository.
paulk 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 8adac59 GROOVY-9417: Make @NullCheck play nicer with @Immutable (closes #1172)
8adac59 is described below
commit 8adac593c1df45192d4e4ee85d6127a582d0e919
Author: Paul King <pa...@asert.com.au>
AuthorDate: Sun Feb 23 08:30:14 2020 +1000
GROOVY-9417: Make @NullCheck play nicer with @Immutable (closes #1172)
---
src/main/java/groovy/transform/NullCheck.java | 2 +-
.../options/ImmutablePropertyHandler.java | 71 ++++++---
.../EqualsAndHashCodeASTTransformation.java | 9 +-
.../transform/NullCheckASTTransformation.java | 59 ++++++--
.../groovy/transform/NullCheckTransformTest.groovy | 164 ++++++++++++++++++++-
5 files changed, 266 insertions(+), 39 deletions(-)
diff --git a/src/main/java/groovy/transform/NullCheck.java b/src/main/java/groovy/transform/NullCheck.java
index a27afbb..fca525a 100644
--- a/src/main/java/groovy/transform/NullCheck.java
+++ b/src/main/java/groovy/transform/NullCheck.java
@@ -78,7 +78,7 @@ import java.lang.annotation.Target;
*/
@java.lang.annotation.Documented
@Retention(RetentionPolicy.SOURCE)
-@Target({ElementType.TYPE})
+@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR})
@GroovyASTTransformationClass("org.codehaus.groovy.transform.NullCheckASTTransformation")
public @interface NullCheck {
/**
diff --git a/src/main/java/groovy/transform/options/ImmutablePropertyHandler.java b/src/main/java/groovy/transform/options/ImmutablePropertyHandler.java
index 32d56a6..50dac9a 100644
--- a/src/main/java/groovy/transform/options/ImmutablePropertyHandler.java
+++ b/src/main/java/groovy/transform/options/ImmutablePropertyHandler.java
@@ -38,6 +38,7 @@ import org.codehaus.groovy.runtime.DefaultGroovyMethods;
import org.codehaus.groovy.transform.AbstractASTTransformation;
import org.codehaus.groovy.transform.ImmutableASTTransformation;
import org.codehaus.groovy.transform.MapConstructorASTTransformation;
+import org.codehaus.groovy.transform.NullCheckASTTransformation;
import java.util.Collection;
import java.util.List;
@@ -203,23 +204,25 @@ public class ImmutablePropertyHandler extends PropertyHandler {
FieldNode fNode = pNode.getField();
final ClassNode fType = fNode.getType();
Statement statement;
+ boolean shouldNullCheck = NullCheckASTTransformation.hasIncludeGenerated(cNode);
if (ImmutablePropertyUtils.isKnownImmutableType(fType, knownImmutableClasses) || isKnownImmutable(pNode.getName(), knownImmutables)) {
- statement = createConstructorStatementDefault(fNode, namedArgsMap);
+ statement = createConstructorStatementDefault(fNode, namedArgsMap, shouldNullCheck);
} else if (fType.isArray() || implementsCloneable(fType)) {
- statement = createConstructorStatementArrayOrCloneable(fNode, namedArgsMap);
+ statement = createConstructorStatementArrayOrCloneable(fNode, namedArgsMap, shouldNullCheck);
} else if (derivesFromDate(fType)) {
- statement = createConstructorStatementDate(fNode, namedArgsMap);
+ statement = createConstructorStatementDate(fNode, namedArgsMap, shouldNullCheck);
} else if (isOrImplements(fType, COLLECTION_TYPE) || fType.isDerivedFrom(COLLECTION_TYPE) || isOrImplements(fType, MAP_TYPE) || fType.isDerivedFrom(MAP_TYPE)) {
- statement = createConstructorStatementCollection(fNode, namedArgsMap);
+ statement = createConstructorStatementCollection(fNode, namedArgsMap, shouldNullCheck);
} else if (fType.isResolved()) {
xform.addError(ImmutablePropertyUtils.createErrorMessage(cNode.getName(), fNode.getName(), fType.getName(), "compiling"), fNode);
statement = EmptyStatement.INSTANCE;
} else {
- statement = createConstructorStatementGuarded(fNode, namedArgsMap, knownImmutables, knownImmutableClasses);
+ statement = createConstructorStatementGuarded(fNode, namedArgsMap, knownImmutables, knownImmutableClasses, shouldNullCheck);
}
return statement;
}
+ // only used by deprecated method
private static Statement createConstructorStatementDefault(FieldNode fNode, boolean namedArgs) {
final ClassNode fType = fNode.getType();
final Expression fieldExpr = propX(varX("this"), fNode.getName());
@@ -240,18 +243,21 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return assignWithDefault(namedArgs, assignInit, param, assignStmt);
}
- private static Statement createConstructorStatementDefault(FieldNode fNode, Parameter namedArgsMap) {
+ private static Statement createConstructorStatementDefault(FieldNode fNode, Parameter namedArgsMap, boolean shouldNullCheck) {
final ClassNode fType = fNode.getType();
final Expression fieldExpr = propX(varX("this"), fNode.getName());
Expression param = getParam(fNode, namedArgsMap != null);
Statement assignStmt = assignS(fieldExpr, castX(fType, param));
+ if (shouldNullCheck) {
+ assignStmt = ifElseS(equalsNullX(param), NullCheckASTTransformation.makeThrowStmt(fNode.getName()), assignStmt);
+ }
Expression initExpr = fNode.getInitialValueExpression();
Statement assignInit;
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression)initExpr).isNullExpression())) {
if (ClassHelper.isPrimitiveType(fType)) {
assignInit = EmptyStatement.INSTANCE;
} else {
- assignInit = assignNullS(fieldExpr);
+ assignInit = shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr);
}
} else {
assignInit = assignS(fieldExpr, initExpr);
@@ -279,6 +285,7 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return ifElseS(equalsNullX(param), assignInit, assignStmt);
}
+ // only used by deprecated method
private static Statement createConstructorStatementGuarded(ClassNode cNode, FieldNode fNode, boolean namedArgs, List<String> knownImmutables, List<String> knownImmutableClasses) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
Expression initExpr = fNode.getInitialValueExpression();
@@ -286,33 +293,38 @@ public class ImmutablePropertyHandler extends PropertyHandler {
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) {
assignInit = assignNullS(fieldExpr);
} else {
- assignInit = assignS(fieldExpr, checkUnresolved(fNode, initExpr, knownImmutables, knownImmutableClasses));
+ assignInit = assignS(fieldExpr, createCheckImmutable(fNode, initExpr, knownImmutables, knownImmutableClasses));
}
Expression param = getParam(fNode, namedArgs);
- Statement assignStmt = assignS(fieldExpr, checkUnresolved(fNode, param, knownImmutables, knownImmutableClasses));
+ Statement assignStmt = assignS(fieldExpr, createCheckImmutable(fNode, param, knownImmutables, knownImmutableClasses));
return assignWithDefault(namedArgs, assignInit, param, assignStmt);
}
- private static Statement createConstructorStatementGuarded(FieldNode fNode, Parameter namedArgsMap, List<String> knownImmutables, List<String> knownImmutableClasses) {
+ private static Statement createConstructorStatementGuarded(FieldNode fNode, Parameter namedArgsMap, List<String> knownImmutables, List<String> knownImmutableClasses, boolean shouldNullCheck) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
Expression param = getParam(fNode, namedArgsMap != null);
- Statement assignStmt = assignS(fieldExpr, checkUnresolved(fNode, param, knownImmutables, knownImmutableClasses));
- assignStmt = ifElseS(equalsNullX(param), assignNullS(fieldExpr), assignStmt);
+ Statement assignStmt = assignS(fieldExpr, createCheckImmutable(fNode, param, knownImmutables, knownImmutableClasses));
+ assignStmt = ifElseS(
+ equalsNullX(param),
+ shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr),
+ assignStmt);
Expression initExpr = fNode.getInitialValueExpression();
final Statement assignInit;
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) {
- assignInit = assignNullS(fieldExpr);
+ assignInit = shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr);
} else {
- assignInit = assignS(fieldExpr, checkUnresolved(fNode, initExpr, knownImmutables, knownImmutableClasses));
+ assignInit = assignS(fieldExpr, createCheckImmutable(fNode, initExpr, knownImmutables, knownImmutableClasses));
}
return assignFieldWithDefault(namedArgsMap, fNode, assignStmt, assignInit);
}
- private static Expression checkUnresolved(FieldNode fNode, Expression value, List<String> knownImmutables, List<String> knownImmutableClasses) {
+ // check at runtime since classes might not be resolved
+ private static Expression createCheckImmutable(FieldNode fNode, Expression value, List<String> knownImmutables, List<String> knownImmutableClasses) {
Expression args = args(callThisX("getClass"), constX(fNode.getName()), value, list2args(knownImmutables), classList2args(knownImmutableClasses));
return callX(SELF_TYPE, "checkImmutable", args);
}
+ // only used by deprecated method
private Statement createConstructorStatementCollection(FieldNode fNode, boolean namedArgs) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
ClassNode fieldType = fieldExpr.getType();
@@ -331,7 +343,7 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return assignWithDefault(namedArgs, assignInit, param, assignStmt);
}
- private Statement createConstructorStatementCollection(FieldNode fNode, Parameter namedArgsMap) {
+ private Statement createConstructorStatementCollection(FieldNode fNode, Parameter namedArgsMap, boolean shouldNullCheck) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
ClassNode fieldType = fieldExpr.getType();
Expression param = getParam(fNode, namedArgsMap != null);
@@ -339,17 +351,21 @@ public class ImmutablePropertyHandler extends PropertyHandler {
isInstanceOfX(param, CLONEABLE_TYPE),
assignS(fieldExpr, cloneCollectionExpr(cloneArrayOrCloneableExpr(param, fieldType), fieldType)),
assignS(fieldExpr, cloneCollectionExpr(param, fieldType)));
- assignStmt = ifElseS(equalsNullX(param), assignNullS(fieldExpr), assignStmt);
+ assignStmt = ifElseS(
+ equalsNullX(param),
+ shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr),
+ assignStmt);
Expression initExpr = fNode.getInitialValueExpression();
final Statement assignInit;
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) {
- assignInit = assignNullS(fieldExpr);
+ assignInit = shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr);
} else {
assignInit = assignS(fieldExpr, cloneCollectionExpr(initExpr, fieldType));
}
return assignFieldWithDefault(namedArgsMap, fNode, assignStmt, assignInit);
}
+ // only used by deprecated method
private static Statement createConstructorStatementArrayOrCloneable(FieldNode fNode, boolean namedArgs) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
final Expression initExpr = fNode.getInitialValueExpression();
@@ -365,16 +381,19 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return assignWithDefault(namedArgs, assignInit, param, assignStmt);
}
- private static Statement createConstructorStatementArrayOrCloneable(FieldNode fNode, Parameter namedArgsMap) {
+ private static Statement createConstructorStatementArrayOrCloneable(FieldNode fNode, Parameter namedArgsMap, boolean shouldNullCheck) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
final ClassNode fieldType = fNode.getType();
final Expression param = getParam(fNode, namedArgsMap != null);
Statement assignStmt = assignS(fieldExpr, cloneArrayOrCloneableExpr(param, fieldType));
- assignStmt = ifElseS(equalsNullX(param), assignNullS(fieldExpr), assignStmt);
+ assignStmt = ifElseS(
+ equalsNullX(param),
+ shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr),
+ assignStmt);
final Statement assignInit;
final Expression initExpr = fNode.getInitialValueExpression();
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) {
- assignInit = assignNullS(fieldExpr);
+ assignInit = shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr);
} else {
assignInit = assignS(fieldExpr, cloneArrayOrCloneableExpr(initExpr, fieldType));
}
@@ -385,6 +404,7 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return namedArgs ? findArg(fNode.getName()) : varX(fNode.getName(), fNode.getType());
}
+ // only used by deprecated method
private static Statement createConstructorStatementDate(FieldNode fNode, boolean namedArgs) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
Expression initExpr = fNode.getInitialValueExpression();
@@ -399,15 +419,18 @@ public class ImmutablePropertyHandler extends PropertyHandler {
return assignWithDefault(namedArgs, assignInit, param, assignStmt);
}
- private static Statement createConstructorStatementDate(FieldNode fNode, Parameter namedArgsMap) {
+ private static Statement createConstructorStatementDate(FieldNode fNode, Parameter namedArgsMap, boolean shouldNullCheck) {
final Expression fieldExpr = propX(varX("this"), fNode.getName());
final Expression param = getParam(fNode, namedArgsMap != null);
Statement assignStmt = assignS(fieldExpr, cloneDateExpr(param));
- assignStmt = ifElseS(equalsNullX(param), assignNullS(fieldExpr), assignStmt);
+ assignStmt = ifElseS(
+ equalsNullX(param),
+ shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr),
+ assignStmt);
final Statement assignInit;
Expression initExpr = fNode.getInitialValueExpression();
if (initExpr == null || (initExpr instanceof ConstantExpression && ((ConstantExpression) initExpr).isNullExpression())) {
- assignInit = assignNullS(fieldExpr);
+ assignInit = shouldNullCheck ? NullCheckASTTransformation.makeThrowStmt(fNode.getName()) : assignNullS(fieldExpr);
} else {
assignInit = assignS(fieldExpr, cloneDateExpr(initExpr));
}
diff --git a/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
index 07a090f..a955f00 100644
--- a/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/EqualsAndHashCodeASTTransformation.java
@@ -25,6 +25,7 @@ import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
import org.codehaus.groovy.ast.expr.BinaryExpression;
@@ -206,13 +207,15 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
final BlockStatement body = new BlockStatement();
VariableExpression other = varX("other");
body.addStatement(returnS(isInstanceOfX(other, GenericsUtils.nonGeneric(cNode))));
- addGeneratedMethod(cNode,
+ MethodNode canEqual = addGeneratedMethod(cNode,
hasExistingCanEqual ? "_canEqual" : "canEqual",
hasExistingCanEqual ? ACC_PRIVATE : ACC_PUBLIC,
ClassHelper.boolean_TYPE,
params(param(OBJECT_TYPE, other.getName())),
ClassNode.EMPTY_ARRAY,
body);
+ // don't null check this: prefer false to IllegalArgumentException
+ NullCheckASTTransformation.markAsProcessed(canEqual);
}
public static void createEquals(ClassNode cNode, boolean includeFields, boolean callSuper, boolean useCanEqual, List<String> excludes, List<String> includes) {
@@ -296,13 +299,15 @@ public class EqualsAndHashCodeASTTransformation extends AbstractASTTransformatio
// default
body.addStatement(returnS(constX(Boolean.TRUE,true)));
- addGeneratedMethod(cNode,
+ MethodNode equal = addGeneratedMethod(cNode,
hasExistingEquals ? "_equals" : "equals",
hasExistingEquals ? ACC_PRIVATE : ACC_PUBLIC,
ClassHelper.boolean_TYPE,
params(param(OBJECT_TYPE, other.getName())),
ClassNode.EMPTY_ARRAY,
body);
+ // don't null check this: prefer false to IllegalArgumentException
+ NullCheckASTTransformation.markAsProcessed(equal);
}
private static BinaryExpression differentSelfRecursivePropertyX(PropertyNode pNode, Expression other) {
diff --git a/src/main/java/org/codehaus/groovy/transform/NullCheckASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NullCheckASTTransformation.java
index 0308a0f..051998c 100644
--- a/src/main/java/org/codehaus/groovy/transform/NullCheckASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/NullCheckASTTransformation.java
@@ -28,12 +28,17 @@ import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
+import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.stmt.BlockStatement;
+import org.codehaus.groovy.ast.stmt.ThrowStatement;
import org.codehaus.groovy.classgen.BytecodeSequence;
import org.codehaus.groovy.control.CompilePhase;
import org.codehaus.groovy.control.SourceUnit;
+import java.util.List;
+
import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.isGenerated;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
import static org.codehaus.groovy.ast.ClassHelper.make;
@@ -47,24 +52,24 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
/**
* Handles generation of code for the @NullCheck annotation.
*/
-@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+@GroovyASTTransformation(phase = CompilePhase.INSTRUCTION_SELECTION)
public class NullCheckASTTransformation extends AbstractASTTransformation {
- private static final Class<NullCheck> MY_CLASS = NullCheck.class;
- private static final ClassNode MY_TYPE = make(MY_CLASS);
- private static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage();
+ public static final ClassNode NULL_CHECK_TYPE = make(NullCheck.class);
+ private static final String NULL_CHECK_NAME = "@" + NULL_CHECK_TYPE.getNameWithoutPackage();
private static final ClassNode EXCEPTION = ClassHelper.make(IllegalArgumentException.class);
+ private static final String NULL_CHECK_IS_PROCESSED = "NullCheck.isProcessed";
@Override
public void visit(ASTNode[] nodes, SourceUnit source) {
init(nodes, source);
AnnotatedNode parent = (AnnotatedNode) nodes[1];
AnnotationNode anno = (AnnotationNode) nodes[0];
- if (!MY_TYPE.equals(anno.getClassNode())) return;
- boolean includeGenerated = memberHasValue(anno, "includeGenerated", true);
+ if (!NULL_CHECK_TYPE.equals(anno.getClassNode())) return;
+ boolean includeGenerated = isIncludeGenerated(anno);
if (parent instanceof ClassNode) {
ClassNode cNode = (ClassNode) parent;
- if (!checkNotInterface(cNode, MY_TYPE_NAME)) return;
+ if (!checkNotInterface(cNode, NULL_CHECK_NAME)) return;
for (ConstructorNode cn : cNode.getDeclaredConstructors()) {
adjustMethod(cn, includeGenerated);
}
@@ -73,16 +78,32 @@ public class NullCheckASTTransformation extends AbstractASTTransformation {
}
} else if (parent instanceof MethodNode) {
// handles constructor case too
- adjustMethod((MethodNode) parent, includeGenerated);
+ adjustMethod((MethodNode) parent, false);
}
}
+ private boolean isIncludeGenerated(AnnotationNode anno) {
+ return memberHasValue(anno, "includeGenerated", true);
+ }
+
+ public static boolean hasIncludeGenerated(ClassNode cNode) {
+ List<AnnotationNode> annotations = cNode.getAnnotations(NULL_CHECK_TYPE);
+ if (annotations.isEmpty()) return false;
+ return hasIncludeGenerated(annotations.get(0));
+ }
+
+ private static boolean hasIncludeGenerated(AnnotationNode node) {
+ final Expression member = node.getMember("includeGenerated");
+ return member instanceof ConstantExpression && ((ConstantExpression) member).getValue().equals(true);
+ }
+
private void adjustMethod(MethodNode mn, boolean includeGenerated) {
BlockStatement newCode = getCodeAsBlock(mn);
if (mn.getParameters().length == 0) return;
boolean generated = isGenerated(mn);
int startingIndex = 0;
if (!includeGenerated && generated) return;
+ if (isMarkedAsProcessed(mn)) return;
if (mn instanceof ConstructorNode) {
// some transform has been here already and we assume it knows what it is doing
if (mn.getFirstStatement() instanceof BytecodeSequence) return;
@@ -97,9 +118,27 @@ public class NullCheckASTTransformation extends AbstractASTTransformation {
}
}
for (Parameter p : mn.getParameters()) {
- newCode.getStatements().add(startingIndex, ifS(isNullX(varX(p)),
- throwS(ctorX(EXCEPTION, constX(p.getName() + " cannot be null")))));
+ if (ClassHelper.isPrimitiveType(p.getType())) continue;
+ newCode.getStatements().add(startingIndex, ifS(isNullX(varX(p)), makeThrowStmt(p.getName())));
}
mn.setCode(newCode);
}
+
+ public static ThrowStatement makeThrowStmt(String name) {
+ return throwS(ctorX(EXCEPTION, constX(name + " cannot be null")));
+ }
+
+ /**
+ * Mark a method as already processed.
+ *
+ * @param mn the method node to be considered already processed
+ */
+ public static void markAsProcessed(MethodNode mn) {
+ mn.setNodeMetaData(NULL_CHECK_IS_PROCESSED, Boolean.TRUE);
+ }
+
+ private static boolean isMarkedAsProcessed(MethodNode mn) {
+ Boolean r = mn.getNodeMetaData(NULL_CHECK_IS_PROCESSED);
+ return null != r && r;
+ }
}
diff --git a/src/test/org/codehaus/groovy/transform/NullCheckTransformTest.groovy b/src/test/org/codehaus/groovy/transform/NullCheckTransformTest.groovy
index 0e849bf..75c4133 100644
--- a/src/test/org/codehaus/groovy/transform/NullCheckTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/NullCheckTransformTest.groovy
@@ -63,6 +63,35 @@ class NullCheckTransformTest extends GroovyTestCase {
import groovy.transform.*
import static groovy.test.GroovyAssert.shouldFail
+ @AutoExternalize
+ @NullCheck
+ class OneHolder {
+ String one
+ }
+
+ @AutoExternalize
+ @NullCheck(includeGenerated=true)
+ class TwoHolder {
+ String two
+ }
+
+ def one = new OneHolder(one: 'One')
+ // fail late without null checking via subsequent NPE
+ def ex = shouldFail(NullPointerException) { one.writeExternal(null) }
+ assert ex.message == 'Cannot invoke method writeObject() on null object'
+
+ def two = new TwoHolder(two: 'Two')
+ // fail early with null checking in place via IAE
+ ex = shouldFail(IllegalArgumentException) { two.writeExternal(null) }
+ assert ex.message == 'out cannot be null'
+ '''
+ }
+
+ void testNullCheckWithEquals() {
+ assertScript '''
+ import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
+
@EqualsAndHashCode
@NullCheck
class OneHolder {
@@ -78,8 +107,8 @@ class NullCheckTransformTest extends GroovyTestCase {
def one = new OneHolder(one: 'One')
assert !one.equals(null)
def two = new TwoHolder(two: 'Two')
- def ex = shouldFail(IllegalArgumentException) { two.equals(null) }
- assert ex.message == 'other cannot be null'
+ // method equals flags itself as already null-check processed so as to prefer false over IllegalArgumentException here
+ assert !two.equals(null)
'''
}
@@ -129,4 +158,135 @@ class NullCheckTransformTest extends GroovyTestCase {
assert ex.message == 'second cannot be null'
'''
}
+
+ void testNullCheckWithImmutable1() {
+ assertScript '''
+ import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
+
+ @Immutable
+ @NullCheck(includeGenerated=true)
+ class Foo {
+ List<Integer> numbers
+ String sep
+ String bar(final String prefix) { numbers.collect{ prefix + it }.join(sep) }
+ }
+
+ assert new Foo(numbers: 0..1, sep: '-').bar('test') == 'test0-test1'
+ def ex = shouldFail(IllegalArgumentException) { new Foo().bar('test') }
+ assert ex.message.matches(/(sep|numbers) cannot be null/)
+ ex = shouldFail(IllegalArgumentException) { new Foo(null).bar('test') }
+ assert ex.message.contains('args cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: null, sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1, sep: null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(null, '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(0..1, null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ '''
+ }
+
+ void testNullCheckWithImmutable2() {
+ assertScript '''
+ import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
+
+ @Immutable
+ @NullCheck(includeGenerated=true)
+ class Foo {
+ List<Integer> numbers = null
+ String sep = null
+ String bar(final String prefix) { numbers.collect{ prefix + it }.join(sep) }
+ }
+
+ assert new Foo(numbers: 0..1, sep: '-').bar('test') == 'test0-test1'
+ def ex = shouldFail(IllegalArgumentException) { new Foo().bar('test') }
+ assert ex.message.matches(/(sep|numbers) cannot be null/)
+ ex = shouldFail(IllegalArgumentException) { new Foo(null).bar('test') }
+ assert ex.message.contains('args cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: null, sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1, sep: null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(null, '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(0..1, null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ '''
+ }
+
+ void testNullCheckWithImmutable3() {
+ assertScript '''
+ import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
+
+ @Immutable
+ @NullCheck(includeGenerated=true)
+ class Foo {
+ List<Integer> numbers = 0..2
+ String sep
+ String bar(final String prefix) { numbers.collect{ prefix + it }.join(sep) }
+ }
+
+ assert new Foo(numbers: 0..1, sep: '-').bar('test') == 'test0-test1'
+ def ex = shouldFail(IllegalArgumentException) { new Foo().bar('test') }
+ assert ex.message.matches(/(sep|numbers) cannot be null/)
+ ex = shouldFail(IllegalArgumentException) { new Foo(null).bar('test') }
+ assert ex.message.contains('args cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: null, sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ assert new Foo(sep: '-').bar('test') == 'test0-test1-test2\'
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1, sep: null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(null, '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(0..1, null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ '''
+ }
+
+ void testNullCheckWithImmutable4() {
+ assertScript '''
+ import groovy.transform.*
+ import static groovy.test.GroovyAssert.shouldFail
+
+ @Immutable
+ @NullCheck(includeGenerated=true)
+ class Foo {
+ List<Integer> numbers
+ String sep = '*'
+ String bar(final String prefix) { numbers.collect{ prefix + it }.join(sep) }
+ }
+
+ assert new Foo(numbers: 0..1, sep: '-').bar('test') == 'test0-test1'
+ def ex = shouldFail(IllegalArgumentException) { new Foo().bar('test') }
+ assert ex.message.matches(/(sep|numbers) cannot be null/)
+ ex = shouldFail(IllegalArgumentException) { new Foo(null).bar('test') }
+ assert ex.message.contains('args cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: null, sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(sep: '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ assert new Foo(numbers: 0..1).bar('test') == 'test0*test1'
+ ex = shouldFail(IllegalArgumentException) { new Foo(numbers: 0..1, sep: null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(null, '-').bar('test') }
+ assert ex.message.contains('numbers cannot be null')
+ ex = shouldFail(IllegalArgumentException) { new Foo(0..1, null).bar('test') }
+ assert ex.message.contains('sep cannot be null')
+ '''
+ }
}