From 81134b51642e2001cd5c193698aab809b7fd3f98 Mon Sep 17 00:00:00 2001 From: emcmanus Date: Tue, 2 Jul 2019 10:22:39 -0700 Subject: [PATCH] Fix a template bug concerning @AutoOneOf arrays. Like AutoValue, AutoOneOf allows values to be primitive arrays, but the code template referenced an undefined variable in that case. The implementation of foo.getMyArray() copies the array, so the implementation of equals(Object) checks whether the Object is of the generated subclass, and if so accesses the private myArray field directly. For @AutoValue Foo, this subclass is AutoValue_Foo, and the $subclass template variable is set to that. But for @AutoOneOf Foo, there is a subclass per property, and we actually want AutoOneOf_Foo.Impl_myArray. So we need to make sure that $subclass is set to that in the #equalsThatExpression macro. This requires updating to EscapeVelocity 0.9.1, which allows variable references within strings, like "Impl_$p". RELNOTES=Primitive arrays now work in @AutoOneOf classes. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=256190191 --- value/processor/pom.xml | 2 +- .../com/google/auto/value/AutoOneOfTest.java | 34 +++++++++++++++++++ .../google/auto/value/processor/autooneof.vm | 2 +- .../google/auto/value/processor/autovalue.vm | 2 +- .../auto/value/processor/equalshashcode.vm | 2 +- 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/value/processor/pom.xml b/value/processor/pom.xml index 98ab6d61c9..edcbc8f918 100644 --- a/value/processor/pom.xml +++ b/value/processor/pom.xml @@ -61,7 +61,7 @@ com.google.escapevelocity escapevelocity - 0.9 + 0.9.1 net.ltgt.gradle.incap diff --git a/value/src/it/functional/src/test/java/com/google/auto/value/AutoOneOfTest.java b/value/src/it/functional/src/test/java/com/google/auto/value/AutoOneOfTest.java index fae96f4c64..a7888f2241 100644 --- a/value/src/it/functional/src/test/java/com/google/auto/value/AutoOneOfTest.java +++ b/value/src/it/functional/src/test/java/com/google/auto/value/AutoOneOfTest.java @@ -470,6 +470,40 @@ public void reservedWordProperty() { assertThat(pkg.toString()).isEqualTo("LetterOrPackage{package=pacquet}"); } + @AutoOneOf(ArrayValue.Kind.class) + public abstract static class ArrayValue { + public enum Kind { + STRING, + INTS + } + + public abstract Kind getKind(); + + public abstract String string(); + + @SuppressWarnings("mutable") + public abstract int[] ints(); + + public static ArrayValue ofString(String string) { + return AutoOneOf_AutoOneOfTest_ArrayValue.string(string); + } + + public static ArrayValue ofInts(int[] ints) { + return AutoOneOf_AutoOneOfTest_ArrayValue.ints(ints); + } + } + + @Test + public void arrayValues() { + ArrayValue string = ArrayValue.ofString("foo"); + ArrayValue ints1 = ArrayValue.ofInts(new int[] {17, 23}); + ArrayValue ints2 = ArrayValue.ofInts(new int[] {17, 23}); + new EqualsTester() + .addEqualityGroup(string) + .addEqualityGroup(ints1, ints2) + .testEquals(); + } + @Retention(RetentionPolicy.RUNTIME) public @interface CopyTest { int value(); diff --git a/value/src/main/java/com/google/auto/value/processor/autooneof.vm b/value/src/main/java/com/google/auto/value/processor/autooneof.vm index 1bf46e1a38..155d99f920 100644 --- a/value/src/main/java/com/google/auto/value/processor/autooneof.vm +++ b/value/src/main/java/com/google/auto/value/processor/autooneof.vm @@ -189,7 +189,7 @@ final class $generatedClass { if (x instanceof $origClass) { $origClass$wildcardTypes that = ($origClass$wildcardTypes) x; return this.${kindGetter}() == that.${kindGetter}() - && #equalsThatExpression($p); + && #equalsThatExpression($p "Impl_$p"); } else { return false; } diff --git a/value/src/main/java/com/google/auto/value/processor/autovalue.vm b/value/src/main/java/com/google/auto/value/processor/autovalue.vm index 3a3fb037cf..f74e8b3937 100644 --- a/value/src/main/java/com/google/auto/value/processor/autovalue.vm +++ b/value/src/main/java/com/google/auto/value/processor/autovalue.vm @@ -145,7 +145,7 @@ ${modifiers}class $subclass$formalTypes extends $origClass$actualTypes { $origClass$wildcardTypes that = ($origClass$wildcardTypes) o; return ## #foreach ($p in $props) - #equalsThatExpression ($p)## + #equalsThatExpression ($p $subclass)## #if ($foreach.hasNext) && ## diff --git a/value/src/main/java/com/google/auto/value/processor/equalshashcode.vm b/value/src/main/java/com/google/auto/value/processor/equalshashcode.vm index 10775ee46a..094f6c3c54 100644 --- a/value/src/main/java/com/google/auto/value/processor/equalshashcode.vm +++ b/value/src/main/java/com/google/auto/value/processor/equalshashcode.vm @@ -40,7 +40,7 @@ ## The expression should be surrounded by parentheses if otherwise there might be precedence ## problems when it is followed by &&. ## A reminder that trailing ## here serves to delete the newline, which we don't want in the output. -#macro (equalsThatExpression $p) +#macro (equalsThatExpression $p $subclass) #if ($p.kind == "FLOAT") Float.floatToIntBits(this.$p) == Float.floatToIntBits(that.${p.getter}()) ## #elseif ($p.kind == "DOUBLE")