Skip to content
This repository was archived by the owner on Oct 16, 2024. It is now read-only.

Commit 183d824

Browse files
alicederynalicederyn
authored
Merge pull request #228 from google/issues/227.collection.generates.invalid.code
Cast Collection to List when necessary
2 parents 7866cee + 1c8f31a commit 183d824

File tree

8 files changed

+182
-14
lines changed

8 files changed

+182
-14
lines changed

‎src/it/vanilla/src/main/java/org/inferred/freebuilder/WildcardListType.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/*
2+
* Copyright 2016 Google Inc. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.inferred.freebuilder;
17+
18+
import java.util.List;
19+
20+
@FreeBuilder
21+
public interface WildcardListType {
22+
List<? extends Number> numbers();
23+
24+
class Builder extends WildcardListType_Builder { }
25+
}

‎src/it/vanilla/src/test/java/org/inferred/freebuilder/WildcardListTypeTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2016 Google Inc. All rights reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.inferred.freebuilder;
17+
18+
import static org.junit.Assert.assertEquals;
19+
20+
import java.util.Arrays;
21+
22+
import org.junit.Test;
23+
import org.junit.runner.RunWith;
24+
import org.junit.runners.JUnit4;
25+
26+
/** FreeBuilder test using {@link WildcardListType}. */
27+
@RunWith(JUnit4.class)
28+
public class WildcardListTypeTest {
29+
30+
private static final String NAME_1 = "name1";
31+
private static final String NAME_2 = "name2";
32+
private static final String NAME_3 = "name3";
33+
34+
@Test
35+
public void testBuild() {
36+
WildcardListType value = new WildcardListType.Builder()
37+
.addNumbers(1)
38+
.addNumbers(2.8)
39+
.build();
40+
assertEquals(Arrays.asList(1, 2.8), value.numbers());
41+
}
42+
}

‎src/main/java/org/inferred/freebuilder/processor/ListPropertyFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,8 @@ public void addMergeFromValue(Block code, String value) {
410410
property.getName(),
411411
ImmutableList.class,
412412
elementType)
413-
.addLine(" %s = %s.%s();", property.getName(), value, property.getGetterName())
413+
.addLine(" %s = %s.copyOf(%s.%s());",
414+
property.getName(), ImmutableList.class, value, property.getGetterName())
414415
.addLine("} else {");
415416
}
416417
code.addLine("%s(%s.%s());", addAllMethod(property), value, property.getGetterName());

‎src/main/java/org/inferred/freebuilder/processor/SetPropertyFactory.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,8 @@ public void addMergeFromValue(Block code, String value) {
381381
if (code.feature(GUAVA).isAvailable()) {
382382
code.addLine("if (%s instanceof %s && %s == %s.<%s>of()) {",
383383
value, metadata.getValueType(), property.getName(), ImmutableSet.class, elementType)
384-
.addLine(" %s = %s.%s();", property.getName(), value, property.getGetterName())
384+
.addLine(" %s = %s.copyOf(%s.%s());",
385+
property.getName(), ImmutableSet.class, value, property.getGetterName())
385386
.addLine("} else {");
386387
}
387388
code.addLine("%s(%s.%s());", addAllMethod(property), value, property.getGetterName());

‎src/test/java/org/inferred/freebuilder/processor/ListPrefixlessPropertyTest.java

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.junit.runners.Parameterized.Parameters;
4343
import org.junit.runners.Parameterized.UseParametersRunnerFactory;
4444

45+
import java.util.Collection;
4546
import java.util.Iterator;
4647
import java.util.List;
4748
import java.util.stream.IntStream;
@@ -587,6 +588,53 @@ public void testBuilderClearAfterMergeFromValue() {
587588
.runTest();
588589
}
589590

591+
@Test
592+
public void testCollectionProperty() {
593+
// See also https://github.com/google/FreeBuilder/issues/227
594+
behaviorTester
595+
.with(new Processor(features))
596+
.with(new SourceBuilder()
597+
.addLine("package com.example;")
598+
.addLine("@%s", FreeBuilder.class)
599+
.addLine("public abstract class DataType {")
600+
.addLine(" public abstract %s<%s> items();", Collection.class, String.class)
601+
.addLine("")
602+
.addLine(" public static class Builder extends DataType_Builder {}")
603+
.addLine("}")
604+
.build())
605+
.with(testBuilder()
606+
.addLine("DataType value = new DataType.Builder()")
607+
.addLine(" .addItems(\"one\")")
608+
.addLine(" .addItems(\"two\")")
609+
.addLine(" .build();")
610+
.addLine("assertThat(value.items()).containsExactly(\"one\", \"two\").inOrder();")
611+
.build())
612+
.runTest();
613+
}
614+
615+
@Test
616+
public void testCollectionProperty_withWildcard() {
617+
behaviorTester
618+
.with(new Processor(features))
619+
.with(new SourceBuilder()
620+
.addLine("package com.example;")
621+
.addLine("@%s", FreeBuilder.class)
622+
.addLine("public abstract class DataType {")
623+
.addLine(" public abstract %s<? extends %s> items();", Collection.class, Number.class)
624+
.addLine("")
625+
.addLine(" public static class Builder extends DataType_Builder {}")
626+
.addLine("}")
627+
.build())
628+
.with(testBuilder()
629+
.addLine("DataType value = new DataType.Builder()")
630+
.addLine(" .addItems(1)")
631+
.addLine(" .addItems(2.7)")
632+
.addLine(" .build();")
633+
.addLine("assertThat(value.items()).containsExactly(1, 2.7).inOrder();")
634+
.build())
635+
.runTest();
636+
}
637+
590638
@Test
591639
public void testImmutableListProperty() {
592640
assumeGuavaAvailable();
@@ -611,6 +659,31 @@ public void testImmutableListProperty() {
611659
.runTest();
612660
}
613661

662+
@Test
663+
public void testImmutableListProperty_withWildcard() {
664+
assumeGuavaAvailable();
665+
behaviorTester
666+
.with(new Processor(features))
667+
.with(new SourceBuilder()
668+
.addLine("package com.example;")
669+
.addLine("@%s", FreeBuilder.class)
670+
.addLine("public abstract class DataType {")
671+
.addLine(" public abstract %s<? extends %s> items();",
672+
ImmutableList.class, Number.class)
673+
.addLine("")
674+
.addLine(" public static class Builder extends DataType_Builder {}")
675+
.addLine("}")
676+
.build())
677+
.with(testBuilder()
678+
.addLine("DataType value = new DataType.Builder()")
679+
.addLine(" .addItems(1)")
680+
.addLine(" .addItems(2.7)")
681+
.addLine(" .build();")
682+
.addLine("assertThat(value.items()).containsExactly(1, 2.7).inOrder();")
683+
.build())
684+
.runTest();
685+
}
686+
614687
@Test
615688
public void testValidation_varargsAdd() {
616689
thrown.expectMessage(STRING_VALIDATION_ERROR_MESSAGE);

‎src/test/java/org/inferred/freebuilder/processor/ListSourceTest.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,12 @@ public void test_guava_j6() {
221221
" */",
222222
" public Person.Builder mergeFrom(Person value) {",
223223
" if (value instanceof Person_Builder.Value && name == ImmutableList.<String>of()) {",
224-
" name = value.getName();",
224+
" name = ImmutableList.copyOf(value.getName());",
225225
" } else {",
226226
" addAllName(value.getName());",
227227
" }",
228228
" if (value instanceof Person_Builder.Value && age == ImmutableList.<Integer>of()) {",
229-
" age = value.getAge();",
229+
" age = ImmutableList.copyOf(value.getAge());",
230230
" } else {",
231231
" addAllAge(value.getAge());",
232232
" }",
@@ -545,12 +545,12 @@ public void test_guava_j7() {
545545
" */",
546546
" public Person.Builder mergeFrom(Person value) {",
547547
" if (value instanceof Person_Builder.Value && name == ImmutableList.<String>of()) {",
548-
" name = value.getName();",
548+
" name = ImmutableList.copyOf(value.getName());",
549549
" } else {",
550550
" addAllName(value.getName());",
551551
" }",
552552
" if (value instanceof Person_Builder.Value && age == ImmutableList.<Integer>of()) {",
553-
" age = value.getAge();",
553+
" age = ImmutableList.copyOf(value.getAge());",
554554
" } else {",
555555
" addAllAge(value.getAge());",
556556
" }",
@@ -938,12 +938,12 @@ public void test_guava_j8() {
938938
" */",
939939
" public Person.Builder mergeFrom(Person value) {",
940940
" if (value instanceof Person_Builder.Value && name == ImmutableList.<String>of()) {",
941-
" name = value.getName();",
941+
" name = ImmutableList.copyOf(value.getName());",
942942
" } else {",
943943
" addAllName(value.getName());",
944944
" }",
945945
" if (value instanceof Person_Builder.Value && age == ImmutableList.<Integer>of()) {",
946-
" age = value.getAge();",
946+
" age = ImmutableList.copyOf(value.getAge());",
947947
" } else {",
948948
" addAllAge(value.getAge());",
949949
" }",
@@ -1848,12 +1848,12 @@ public void test_prefixless() {
18481848
" */",
18491849
" public Person.Builder mergeFrom(Person value) {",
18501850
" if (value instanceof Person_Builder.Value && name == ImmutableList.<String>of()) {",
1851-
" name = value.name();",
1851+
" name = ImmutableList.copyOf(value.name());",
18521852
" } else {",
18531853
" addAllName(value.name());",
18541854
" }",
18551855
" if (value instanceof Person_Builder.Value && age == ImmutableList.<Integer>of()) {",
1856-
" age = value.age();",
1856+
" age = ImmutableList.copyOf(value.age());",
18571857
" } else {",
18581858
" addAllAge(value.age());",
18591859
" }",

‎src/test/java/org/inferred/freebuilder/processor/SetPropertyTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,32 @@ public void testImmutableSetProperty() {
591591
.runTest();
592592
}
593593

594+
@Test
595+
public void testImmutableSetProperty_withWildcard() {
596+
assumeGuavaAvailable();
597+
behaviorTester
598+
.with(new Processor(features))
599+
.with(new SourceBuilder()
600+
.addLine("package com.example;")
601+
.addLine("@%s", FreeBuilder.class)
602+
.addLine("public interface DataType {")
603+
.addLine(" %s<? extends %s> %s;",
604+
ImmutableSet.class, elements.type(), convention.getter())
605+
.addLine("")
606+
.addLine(" class Builder extends DataType_Builder {}")
607+
.addLine("}")
608+
.build())
609+
.with(testBuilder()
610+
.addLine("DataType value = new DataType.Builder()")
611+
.addLine(" .addItems(%s)", elements.example(0))
612+
.addLine(" .addItems(%s)", elements.example(1))
613+
.addLine(" .build();")
614+
.addLine("assertThat(value.%s).containsExactly(%s).inOrder();",
615+
convention.getter(), elements.examples(0, 1))
616+
.build())
617+
.runTest();
618+
}
619+
594620
@Test
595621
public void testValidation_varargsAdd() {
596622
thrown.expect(IllegalArgumentException.class);

‎src/test/java/org/inferred/freebuilder/processor/SetSourceTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public void test_guava_j6() {
154154
" */",
155155
" public Person.Builder mergeFrom(Person value) {",
156156
" if (value instanceof Person_Builder.Value && name == ImmutableSet.<String>of()) {",
157-
" name = value.getName();",
157+
" name = ImmutableSet.copyOf(value.getName());",
158158
" } else {",
159159
" addAllName(value.getName());",
160160
" }",
@@ -382,7 +382,7 @@ public void test_guava_j7() {
382382
" */",
383383
" public Person.Builder mergeFrom(Person value) {",
384384
" if (value instanceof Person_Builder.Value && name == ImmutableSet.<String>of()) {",
385-
" name = value.getName();",
385+
" name = ImmutableSet.copyOf(value.getName());",
386386
" } else {",
387387
" addAllName(value.getName());",
388388
" }",
@@ -647,7 +647,7 @@ public void test_guava_j8() {
647647
" */",
648648
" public Person.Builder mergeFrom(Person value) {",
649649
" if (value instanceof Person_Builder.Value && name == ImmutableSet.<String>of()) {",
650-
" name = value.getName();",
650+
" name = ImmutableSet.copyOf(value.getName());",
651651
" } else {",
652652
" addAllName(value.getName());",
653653
" }",
@@ -1308,7 +1308,7 @@ public void test_prefixless() {
13081308
" */",
13091309
" public Person.Builder mergeFrom(Person value) {",
13101310
" if (value instanceof Person_Builder.Value && name == ImmutableSet.<String>of()) {",
1311-
" name = value.name();",
1311+
" name = ImmutableSet.copyOf(value.name());",
13121312
" } else {",
13131313
" addAllName(value.name());",
13141314
" }",

0 commit comments

Comments
 (0)