Repository: groovy Updated Branches: refs/heads/master de9c8803c -> 923fd338e
GROOVY-7600: @Immutable support for Optional (closes #367) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/923fd338 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/923fd338 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/923fd338 Branch: refs/heads/master Commit: 923fd338eb4be18292cd99b7f1618ba752cb7d1b Parents: de9c880 Author: paulk <pa...@asert.com.au> Authored: Tue Jul 19 17:17:50 2016 +1000 Committer: paulk <pa...@asert.com.au> Committed: Mon Jul 25 20:03:46 2016 +1000 ---------------------------------------------------------------------- src/main/groovy/transform/Immutable.java | 5 +- .../transform/ImmutableASTTransformation.java | 9 ++ .../transform/ImmutableTransformTest.groovy | 118 +++++++++++++++++-- 3 files changed, 118 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/923fd338/src/main/groovy/transform/Immutable.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/transform/Immutable.java b/src/main/groovy/transform/Immutable.java index d0567e6..89876a4 100644 --- a/src/main/groovy/transform/Immutable.java +++ b/src/main/groovy/transform/Immutable.java @@ -52,8 +52,9 @@ import java.lang.annotation.Target; * <li>Properties must be of an immutable type or a type with a strategy for handling non-immutable * characteristics. Specifically, the type must be one of the primitive or wrapper types, Strings, enums, * other {@code @Immutable} classes or known immutables (e.g. java.awt.Color, java.net.URI, java.util.UUID). - * Also handled are Cloneable classes, collections, maps and arrays, and other "effectively immutable" - * classes with special handling (e.g. java.util.Date). + * Also handled are Cloneable classes, collections, maps and arrays, other "effectively immutable" + * classes with special handling (e.g. java.util.Date), and usages of java.util.Optional where the + * contained type is immutable (e.g. Optional<String>). * <li>Properties automatically have private, final backing fields with getters. * Attempts to update the property will result in a {@code ReadOnlyPropertyException}. * <li>A map-based constructor is provided which allows you to set properties by name. http://git-wip-us.apache.org/repos/asf/groovy/blob/923fd338/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java b/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java index f0b00cf..4378f0b 100644 --- a/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java +++ b/src/main/org/codehaus/groovy/transform/ImmutableASTTransformation.java @@ -30,6 +30,7 @@ import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.GenericsType; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; import org.codehaus.groovy.ast.VariableScope; @@ -527,6 +528,14 @@ public class ImmutableASTTransformation extends AbstractASTTransformation { return true; if (!fieldType.isResolved()) return false; + if ("java.util.Optional".equals(fieldType.getName()) && fieldType.getGenericsTypes() != null && fieldType.getGenericsTypes().length == 1) { + GenericsType optionalType = fieldType.getGenericsTypes()[0]; + if (optionalType.isResolved() && !optionalType.isPlaceholder() && !optionalType.isWildcard()) { + String name = optionalType.getType().getName(); + if (inImmutableList(name) || knownImmutableClasses.contains(name)) return true; + if (optionalType.getType().isEnum() || !optionalType.getType().getAnnotations(MY_TYPE).isEmpty()) return true; + } + } return fieldType.isEnum() || ClassHelper.isPrimitiveType(fieldType) || !fieldType.getAnnotations(MY_TYPE).isEmpty(); http://git-wip-us.apache.org/repos/asf/groovy/blob/923fd338/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy index 3c19842..f867c5b 100644 --- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy +++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy @@ -18,12 +18,40 @@ */ package org.codehaus.groovy.transform +import org.codehaus.groovy.control.MultipleCompilationErrorsException +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TestName +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +import static org.junit.Assume.assumeTrue + /** - * @author Paul King - * @author Tim Yates + * Tests for the @Immutable transform. */ +@RunWith(JUnit4) class ImmutableTransformTest extends GroovyShellTestCase { + @Rule public TestName nameRule = new TestName() + + @Before + void setUp() { + super.setUp() + // check java version requirements + def v = System.getProperty("java.specification.version") + assert v + assumeTrue('Test requires jre8+', nameRule.methodName.endsWith('_vm8').implies(new BigDecimal(v) >= 1.8)) + } + + @After + void tearDown() { + super.tearDown() + } + + @Test void testImmutable() { def objects = evaluate(''' import groovy.transform.Immutable @@ -42,6 +70,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert objects[0].nums.class.name.contains("Unmodifiable") } + @Test void testImmutableClonesListAndCollectionFields() { def objects = evaluate(""" import groovy.transform.Immutable @@ -65,6 +94,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assertTrue objects[1].otherNums.class.name.contains("Unmodifiable") } + @Test void testImmutableField() { def person = evaluate(""" import groovy.transform.Immutable @@ -78,6 +108,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } } + @Test void testCloneableField() { def (originalDolly, lab) = evaluate(""" import groovy.transform.Immutable @@ -105,6 +136,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert clonedDolly2.name == clonedDolly.name } + @Test void testCloneableFieldNotCloneableObject() { def cls = shouldFail(CloneNotSupportedException) { def objects = evaluate(""" @@ -127,6 +159,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert cls == 'Dolly' } + @Test void testImmutableListProp() { def objects = evaluate(""" import groovy.transform.Immutable @@ -146,6 +179,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert objects[0].nums.size() == 2 } + @Test void testImmutableAsMapKey() { assertScript """ import groovy.transform.Immutable @@ -159,6 +193,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testImmutableWithOnlyMap() { assertScript """ import groovy.transform.Immutable @@ -169,6 +204,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testImmutableWithPrivateStaticFinalField() { assertScript """ @groovy.transform.Immutable class Foo { @@ -178,6 +214,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testImmutableWithInvalidPropertyName() { def msg = shouldFail(MissingPropertyException) { assertScript """ @@ -189,6 +226,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert msg.contains('No such property: missing for class: Simple') } + @Test void testImmutableWithHashMap() { assertScript """ import groovy.transform.Immutable @@ -207,6 +245,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testDefaultValuesAreImmutable_groovy6293() { assertScript """ import groovy.transform.Immutable @@ -218,6 +257,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testNoArgConstructor_groovy6473() { assertScript """ import groovy.transform.Immutable @@ -229,6 +269,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testImmutableEquals() { assertScript """ import groovy.transform.Immutable @@ -246,6 +287,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testExistingToString() { assertScript """ import groovy.transform.Immutable @@ -269,6 +311,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testExistingEquals() { assertScript """ import groovy.transform.Immutable @@ -311,6 +354,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testExistingHashCode() { assertScript """ import groovy.transform.Immutable @@ -345,6 +389,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { """ } + @Test void testBuiltinImmutables() { assertScript ''' import java.awt.Color @@ -364,6 +409,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testPrivateFieldAssignedViaConstructor() { assertScript ''' import groovy.transform.Immutable @@ -387,6 +433,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testPrivateFinalFieldAssignedViaConstructorShouldCauseError() { shouldFail(ReadOnlyPropertyException) { evaluate ''' @@ -399,6 +446,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } } + @Test void testImmutableWithImmutableFields() { assertScript ''' import groovy.transform.Immutable @@ -409,6 +457,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testImmutableWithConstant() { assertScript ''' import groovy.transform.Immutable @@ -424,6 +473,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testStaticsAllowed_ThoughUsuallyBadDesign() { // design here is questionable as getDescription() method is not idempotent assertScript ''' @@ -454,6 +504,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testImmutableToStringVariants() { assertScript ''' import groovy.transform.* @@ -475,6 +526,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testImmutableUsageOnInnerClasses() { assertScript ''' import groovy.transform.Immutable @@ -492,6 +544,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testKnownImmutableClassesWithNamedParameters() { assertScript ''' import groovy.transform.* @@ -507,6 +560,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testKnownImmutableClassesWithExplicitConstructor() { assertScript ''' @groovy.transform.Immutable(knownImmutableClasses = [Address]) @@ -522,6 +576,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testKnownImmutableClassesWithCoercedConstruction() { assertScript ''' @groovy.transform.Immutable(knownImmutableClasses = [Address]) @@ -537,6 +592,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { ''' } + @Test void testKnownImmutableClassesMissing() { def msg = shouldFail(RuntimeException) { evaluate ''' @@ -556,6 +612,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-5828 + @Test void testKnownImmutableCollectionClass() { assertScript ''' @groovy.transform.Immutable @@ -572,6 +629,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-5828 + @Test void testKnownImmutables() { assertScript ''' // ok, Items not really immutable but pretend so for the purpose of this test @@ -587,6 +645,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-5449 + @Test void testShouldNotThrowNPE() { def msg = shouldFail(RuntimeException) { evaluate ''' @@ -600,6 +659,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-6192 + @Test void testWithEqualsAndHashCodeASTOverride() { assertScript ''' import groovy.transform.* @@ -616,7 +676,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-6354 - public void testCopyWith() { + @Test + void testCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |class Person { @@ -651,7 +712,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert !alice.is( tim ) } - public void testGenericsCopyWith() { + @Test + void testGenericsCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |class Person { @@ -674,7 +736,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert !alice.is( tim ) } - public void testWithPrivatesCopyWith() { + @Test + void testWithPrivatesCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith=true) |class Foo { @@ -704,7 +767,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert alice.full() == 'Alice Yates (ali)' } - public void testStaticWithPrivatesCopyWith() { + @Test + void testStaticWithPrivatesCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith=true) |@groovy.transform.CompileStatic @@ -735,7 +799,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert alice.full() == 'Alice Yates (ali)' } - public void testTypedWithPrivatesCopyWith() { + @Test + void testTypedWithPrivatesCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith=true) |@groovy.transform.TypeChecked @@ -766,7 +831,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert alice.full() == 'Alice Yates (ali)' } - public void testStaticCopyWith() { + @Test + void testStaticCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |@groovy.transform.CompileStatic @@ -802,7 +868,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert !alice.is( tim ) } - public void testTypedCopyWith() { + @Test + void testTypedCopyWith() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |@groovy.transform.TypeChecked @@ -838,7 +905,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert !alice.is( tim ) } - public void testCopyWithSkipping() { + @Test + void testCopyWithSkipping() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |class Person { @@ -859,7 +927,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert result.first == [ 'tim', 'tim' ] } - public void testStaticCopyWithSkipping() { + @Test + void testStaticCopyWithSkipping() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |@groovy.transform.CompileStatic @@ -881,7 +950,8 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert result.first == [ 'tim', 'tim' ] } - public void testTypedCopyWithSkipping() { + @Test + void testTypedCopyWithSkipping() { def tester = new GroovyClassLoader().parseClass( '''@groovy.transform.Immutable(copyWith = true) |@groovy.transform.TypeChecked @@ -904,6 +974,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-7227 + @Test void testKnownImmutablesWithInvalidPropertyNameResultsInError() { def message = shouldFail { evaluate """ @@ -919,6 +990,7 @@ class ImmutableTransformTest extends GroovyShellTestCase { } // GROOVY-7162 + @Test void testImmutableWithSuperClass() { assertScript ''' import groovy.transform.* @@ -943,4 +1015,26 @@ class ImmutableTransformTest extends GroovyShellTestCase { assert d2.toString() == 'Athlete(sport:Tennis, name:Roger Federer)' ''' } + + // GROOVY-7600 + @Test + void testImmutableWithOptional_vm8() { + assertScript ''' + @groovy.transform.Immutable class Person { + String name + Optional<String> address + } + def p = new Person('Joe', Optional.of('Home')) + assert p.toString() == 'Person(Joe, Optional[Home])' + assert p.address.get() == 'Home' + ''' + shouldFail(MultipleCompilationErrorsException) { + evaluate ''' + @groovy.transform.Immutable class Person { + String name + Optional<Date> address + } + ''' + } + } }
